enable uploading attachments for budgets

app/models/cost_object.rb

@@ -94,9 +94,9 @@ class CostObject < ActiveRecord::Base
     if [FixedCostObject.name, VariableCostObject.name].include?(to)
       self.type = to
       self.save!
-      return CostObject.find(id)
+      CostObject.find(id)
     else
-      return self
+      self
     end
   end
 

app/views/cost_objects/_form.html.erb

@@ -24,7 +24,10 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
   <%= f.text_field :subject, required: true, autofocus: true, container_class: '-wide' %>
 </div>
 <div class="form--field">
-  <%= f.text_area :description, rows: (@cost_object.description.blank? ? 10 : [[10, @cost_object.description.length / 50].max, 100].min), cols: 60, container_class: '-wide' %>
+  <%= f.text_area :description,
+                  container_class: '-xxwide',
+                  with_text_formatting: true,
+                  resource: ::API::V3::Budgets::BudgetRepresenter.new(f.object, current_user: current_user) %>
 </div>
 <div class="form--field">
   <%= f.text_field :fixed_date, container_class: '-xslim' %>
@@ -41,5 +44,3 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 <div style="clear: both;"> </div>
 
 <%= render :partial => 'attachments/form' %>
-
-<%= wikitoolbar_for 'cost_object_description' %>

frontend/module/hal/resources/budget-resource.ts

@@ -0,0 +1,41 @@
+// -- copyright
+// OpenProject is a project management system.
+// Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
+//
+// This program is free software; you can redistribute it and/or
+// modify it under the terms of the GNU General Public License version 3.
+//
+// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+// Copyright (C) 2006-2013 Jean-Philippe Lang
+// Copyright (C) 2010-2013 the ChiliProject Team
+//
+// This program is free software; you can redistribute it and/or
+// modify it under the terms of the GNU General Public License
+// as published by the Free Software Foundation; either version 2
+// of the License, or (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+//
+// See doc/COPYRIGHT.rdoc for more details.
+
+import {HalResource} from 'core-app/modules/hal/resources/hal-resource';
+import {Attachable} from 'core-app/modules/hal/resources/mixins/attachable-mixin';
+
+export interface BudgetResourceLinks {
+    addAttachment(attachment:HalResource):Promise<any>;
+}
+
+class BudgetBaseResource extends HalResource {
+    public $links:BudgetResourceLinks;
+}
+
+export const BudgetResource = Attachable(BudgetBaseResource);
+
+export interface BudgetResource extends BudgetResourceLinks {
+}

frontend/module/main.ts

@@ -28,6 +28,7 @@ import {APP_INITIALIZER, Injector, NgModule} from '@angular/core';
 import {OpenProjectPluginContext} from "core-app/modules/plugins/plugin-context";
 import {CostsByTypeDisplayField} from './wp-display/wp-display-costs-by-type-field.module';
 import {CurrencyDisplayField} from './wp-display/wp-display-currency-field.module';
+import {BudgetResource} from './hal/resources/budget-resource';
 
 export function initializeCostsPlugin() {
     return () => {
@@ -39,6 +40,9 @@ export function initializeCostsPlugin() {
             displayFieldService.addFieldType(CostsByTypeDisplayField, 'costs', ['costsByType']);
             displayFieldService.addFieldType(CurrencyDisplayField, 'currency', ['laborCosts', 'materialCosts', 'overallCosts']);
 
+            let halResourceService = pluginContext.services.halResource;
+            halResourceService.registerResource('Budget', { cls: BudgetResource });
+
             pluginContext.hooks.workPackageSingleContextMenu(function(params:any) {
                 return {
                     key: 'log_costs',

lib/api/v3/attachments/attachments_by_budget_api.rb

@@ -0,0 +1,52 @@
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+module API
+  module V3
+    module Attachments
+      class AttachmentsByBudgetAPI < ::API::OpenProjectAPI
+        resources :attachments do
+          helpers API::V3::Attachments::AttachmentsByContainerAPI::Helpers
+
+          helpers do
+            def container
+              @budget
+            end
+
+            def get_attachment_self_path
+              api_v3_paths.attachments_by_budget(container.id)
+            end
+          end
+
+          get &API::V3::Attachments::AttachmentsByContainerAPI.read
+          post &API::V3::Attachments::AttachmentsByContainerAPI.create
+        end
+      end
+    end
+  end
+end

lib/api/v3/budgets/budget_representer.rb

@@ -25,12 +25,31 @@ module API
     module Budgets
       class BudgetRepresenter < ::API::Decorators::Single
         self_link title_getter: ->(*) { represented.subject }
+
         link :staticPath do
+          next if represented.new_record?
           {
             href: cost_object_path(represented.id)
           }
         end
 
+        link :attachments do
+          next if represented.new_record?
+          {
+            href: api_v3_paths.attachments_by_budget(represented.id)
+          }
+        end
+
+        link :addAttachment do
+          next if represented.new_record?
+          next unless current_user_allowed_to(:edit_cost_objects, context: represented.project)
+
+          {
+            href: api_v3_paths.attachments_by_budget(represented.id),
+            method: :post
+          }
+        end
+
         property :id, render_nil: true
         property :subject, render_nil: true
 

lib/api/v3/budgets/budgets_api.rb

@@ -42,6 +42,8 @@ module API
             get do
               BudgetRepresenter.new(@budget, current_user: current_user)
             end
+
+            mount ::API::V3::Attachments::AttachmentsByBudgetAPI
           end
         end
       end

lib/open_project/costs/engine.rb

@@ -130,6 +130,10 @@ module OpenProject::Costs
       "#{project(project_id)}/budgets"
     end
 
+    add_api_path :attachments_by_budget do |id|
+      "#{budget(id)}/attachments"
+    end
+
     add_api_endpoint 'API::V3::Root' do
       mount ::API::V3::Budgets::BudgetsAPI
       mount ::API::V3::CostEntries::CostEntriesAPI

spec/features/budgets/attachment_upload_spec.rb

@@ -0,0 +1,91 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+require 'features/page_objects/notification'
+
+describe 'Upload attachment to budget', js: true do
+  let(:user) do
+    FactoryBot.create :user,
+                      member_in_project: project,
+                      member_with_permissions: %i[view_cost_objects
+                                                  edit_cost_objects]
+  end
+  let(:project) { FactoryBot.create(:project) }
+  let(:attachments) { ::Components::Attachments.new }
+  let(:image_fixture) { Rails.root.join('spec/fixtures/files/image.png') }
+  let(:editor) { ::Components::WysiwygEditor.new }
+
+  before do
+    login_as(user)
+  end
+
+  it 'can upload an image to new and existing budgets via drag & drop' do
+    visit projects_cost_objects_path(project)
+
+    within '.toolbar-items' do
+      click_on "Budget"
+    end
+
+    fill_in "Subject", with: 'New budget'
+
+    # adding an image
+    editor.in_editor do |container, editable|
+      attachments.drag_and_drop_file(editable, image_fixture)
+
+      # Besides testing caption functionality this also slows down clicking on the submit button
+      # so that the image is properly embedded
+      editable.find('figure.image figcaption').base.send_keys('Image uploaded on creation')
+    end
+
+    click_on 'Create'
+
+    expect(page).to have_selector('#content img', count: 1)
+    expect(page).to have_content('Image uploaded on creation')
+
+    within '.toolbar-items' do
+      click_on "Update"
+    end
+
+    editor.in_editor do |container, editable|
+      attachments.drag_and_drop_file(editable, image_fixture)
+
+      # Besides testing caption functionality this also slows down clicking on the submit button
+      # so that the image is properly embedded
+      editable.find('figure.image figcaption').base.send_keys('Image uploaded the second time')
+    end
+
+    click_on 'Submit'
+
+    expect(page).to have_selector('#content img', count: 2)
+    expect(page).to have_content('Image uploaded on creation')
+    expect(page).to have_content('Image uploaded the second time')
+  end
+end

spec/lib/api/v3/budgets/budget_representer_spec.rb

@@ -20,22 +20,24 @@
 require 'spec_helper'
 
 describe ::API::V3::Budgets::BudgetRepresenter do
+  include ::API::V3::Utilities::PathHelper
+
   let(:project) { FactoryBot.build(:project, id: 999) }
-  let(:user) {
+  let(:user) do
     FactoryBot.build(:user,
-                      member_in_project: project,
+                     member_in_project: project,
+                     created_on: 1.day.ago,
+                     updated_on: Date.today)
+  end
+  let(:budget) do
+    FactoryBot.create(:cost_object,
+                      author: user,
+                      project: project,
                       created_on: 1.day.ago,
                       updated_on: Date.today)
-  }
-  let(:budget) {
-    FactoryBot.create(:cost_object,
-                       author: user,
-                       project: project,
-                       created_on: 1.day.ago,
-                       updated_on: Date.today)
-  }
+  end
 
-  let(:representer)  { described_class.new(budget, current_user: user) }
+  let(:representer) { described_class.new(budget, current_user: user) }
 
   context 'generation' do
     subject(:generated) { representer.to_json }
@@ -43,11 +45,23 @@ describe ::API::V3::Budgets::BudgetRepresenter do
     describe 'self link' do
       it_behaves_like 'has a titled link' do
         let(:link) { 'self' }
-        let(:href) { "/api/v3/budgets/#{budget.id}" }
+        let(:href) { api_v3_paths.budget(budget.id) }
         let(:title) { budget.subject }
       end
     end
 
+    it_behaves_like 'has an untitled link' do
+      let(:link) { :attachments }
+      let(:href) { api_v3_paths.attachments_by_budget budget.id }
+    end
+
+    it_behaves_like 'has an untitled action link' do
+      let(:link) { :addAttachment }
+      let(:href) { api_v3_paths.attachments_by_budget budget.id }
+      let(:method) { :post }
+      let(:permission) { :edit_cost_objects }
+    end
+
     it 'indicates its type' do
       is_expected.to be_json_eql('Budget'.to_json).at_path('_type')
     end

spec/lib/api/v3/path_helper_spec.rb

@@ -72,4 +72,10 @@ describe ::API::V3::Utilities::PathHelper do
 
     it { is_expected.to eql('/api/v3/projects/42/budgets') }
   end
+
+  describe '#attachments_by_budget' do
+    subject { helper.attachments_by_budget 42 }
+
+    it { is_expected.to eql('/api/v3/budgets/42/attachments') }
+  end
 end

spec/lib/api/v3/work_packages/work_package_representer_spec.rb

@@ -23,48 +23,48 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
   include API::V3::Utilities::PathHelper
 
   let(:project) { FactoryBot.create(:project) }
-  let(:role) {
+  let(:role) do
     FactoryBot.create(:role, permissions: [:view_time_entries,
                                             :view_cost_entries,
                                             :view_cost_rates,
                                             :view_work_packages])
-  }
-  let(:user) {
+  end
+  let(:user) do
     FactoryBot.create(:user,
-                       member_in_project: project,
-                       member_through_role: role)
-  }
+                      member_in_project: project,
+                      member_through_role: role)
+  end
 
   let(:cost_object) { FactoryBot.create(:cost_object, project: project) }
-  let(:cost_entry_1) {
+  let(:cost_entry_1) do
     FactoryBot.create(:cost_entry,
-                       work_package: work_package,
-                       project: project,
-                       units: 3,
-                       spent_on: Date.today,
-                       user: user,
-                       comments: 'Entry 1')
-  }
-  let(:cost_entry_2) {
+                      work_package: work_package,
+                      project: project,
+                      units: 3,
+                      spent_on: Date.today,
+                      user: user,
+                      comments: 'Entry 1')
+  end
+  let(:cost_entry_2) do
     FactoryBot.create(:cost_entry,
-                       work_package: work_package,
-                       project: project,
-                       units: 3,
-                       spent_on: Date.today,
-                       user: user,
-                       comments: 'Entry 2')
-  }
-
-  let(:work_package) {
+                      work_package: work_package,
+                      project: project,
+                      units: 3,
+                      spent_on: Date.today,
+                      user: user,
+                      comments: 'Entry 2')
+  end
+
+  let(:work_package) do
     FactoryBot.create(:work_package,
-                       project_id: project.id,
-                       cost_object: cost_object)
-  }
-  let(:representer) {
+                      project_id: project.id,
+                      cost_object: cost_object)
+  end
+  let(:representer) do
     described_class.new(work_package,
                         current_user: user,
                         embed_links: true)
-  }
+  end
 
   before(:each) do
     allow(User).to receive(:current).and_return user
@@ -116,27 +116,27 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
 
       describe 'spentTime' do
         context 'time entry with single hour' do
-          let(:time_entry) {
+          let(:time_entry) do
             FactoryBot.create(:time_entry,
-                               project: work_package.project,
-                               work_package: work_package,
-                               hours: 1.0)
-          }
+                              project: work_package.project,
+                              work_package: work_package,
+                              hours: 1.0)
+          end
 
-          before do time_entry end
+          before { time_entry }
 
           it { is_expected.to be_json_eql('PT1H'.to_json).at_path('spentTime') }
         end
 
         context 'time entry with multiple hours' do
-          let(:time_entry) {
+          let(:time_entry) do
             FactoryBot.create(:time_entry,
-                               project: work_package.project,
-                               work_package: work_package,
-                               hours: 42.5)
-          }
+                              project: work_package.project,
+                              work_package: work_package,
+                              hours: 42.5)
+          end
 
-          before do time_entry end
+          before { time_entry }
 
           it { is_expected.to be_json_eql('P1DT18H30M'.to_json).at_path('spentTime') }
         end
@@ -150,32 +150,32 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
         end
 
         context 'only view_own_time_entries permission' do
-          let(:own_time_entries_role) {
+          let(:own_time_entries_role) do
             FactoryBot.create(:role, permissions: [:view_own_time_entries,
                                                     :view_work_packages])
-          }
+          end
 
-          let(:user2) {
+          let(:user2) do
             FactoryBot.create(:user,
-                               member_in_project: project,
-                               member_through_role: own_time_entries_role)
-          }
+                              member_in_project: project,
+                              member_through_role: own_time_entries_role)
+          end
 
-          let!(:own_time_entry) {
+          let!(:own_time_entry) do
             FactoryBot.create(:time_entry,
-                               project: work_package.project,
-                               work_package: work_package,
-                               hours: 2,
-                               user: user2)
-          }
+                              project: work_package.project,
+                              work_package: work_package,
+                              hours: 2,
+                              user: user2)
+          end
 
-          let!(:other_time_entry) {
+          let!(:other_time_entry) do
             FactoryBot.create(:time_entry,
-                               project: work_package.project,
-                               work_package: work_package,
-                               hours: 1,
-                               user: user)
-          }
+                              project: work_package.project,
+                              work_package: work_package,
+                              hours: 1,
+                              user: user)
+          end
 
           before do
             allow(User).to receive(:current).and_return(user2)
@@ -436,9 +436,9 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
 
   describe '.to_eager_load' do
     it 'includes the cost objects' do
-      expect(described_class.to_eager_load.any? { |el|
+      expect(described_class.to_eager_load.any? do |el|
         el == :cost_object
-      }).to be_truthy
+      end).to be_truthy
     end
   end
 end

spec/requests/api/attachments/attachments_by_budget_resource_spec.rb

@@ -0,0 +1,144 @@
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+require 'rack/test'
+
+describe 'API v3 Attachments by budget resource', type: :request do
+  include Rack::Test::Methods
+  include API::V3::Utilities::PathHelper
+  include FileHelpers
+
+  let(:current_user) do
+    FactoryBot.create(:user,
+                      member_in_project: project,
+                      member_with_permissions: permissions)
+  end
+  let(:project) { FactoryBot.create(:project) }
+  let(:permissions) { [:view_cost_objects] }
+  let(:budget) { FactoryBot.create(:cost_object, project: project) }
+
+  subject(:response) { last_response }
+
+  before do
+    allow(User).to receive(:current).and_return current_user
+  end
+
+  describe '#get' do
+    let(:get_path) { api_v3_paths.attachments_by_budget budget.id }
+
+    before do
+      FactoryBot.create_list(:attachment, 2, container: budget)
+      get get_path
+    end
+
+    it 'should respond with 200' do
+      expect(subject.status).to eq(200)
+    end
+
+    it_behaves_like 'API V3 collection response', 2, 2, 'Attachment'
+  end
+
+  describe '#post' do
+    let(:permissions) { %i[view_cost_objects edit_cost_objects] }
+
+    let(:request_path) { api_v3_paths.attachments_by_budget budget.id }
+    let(:request_parts) { { metadata: metadata, file: file } }
+    let(:metadata) { { fileName: 'cat.png' }.to_json }
+    let(:file) { mock_uploaded_file(name: 'original-filename.txt') }
+    let(:max_file_size) { 1 } # given in kiB
+
+    before do
+      allow(Setting).to receive(:attachment_max_size).and_return max_file_size.to_s
+      post request_path, request_parts
+    end
+
+    it 'should respond with HTTP Created' do
+      expect(subject.status).to eq(201)
+    end
+
+    it 'should return the new attachment' do
+      expect(subject.body).to be_json_eql('Attachment'.to_json).at_path('_type')
+    end
+
+    it 'ignores the original file name' do
+      expect(subject.body).to be_json_eql('cat.png'.to_json).at_path('fileName')
+    end
+
+    context 'metadata section is missing' do
+      let(:request_parts) { { file: file } }
+
+      it_behaves_like 'invalid request body', I18n.t('api_v3.errors.multipart_body_error')
+    end
+
+    context 'file section is missing' do
+      # rack-test won't send a multipart request without a file being present
+      # however as long as we depend on correctly named sections this test should do just fine
+      let(:request_parts) { { metadata: metadata, wrongFileSection: file } }
+
+      it_behaves_like 'invalid request body', I18n.t('api_v3.errors.multipart_body_error')
+    end
+
+    context 'metadata section is no valid JSON' do
+      let(:metadata) { '"fileName": "cat.png"' }
+
+      it_behaves_like 'parse error'
+    end
+
+    context 'metadata is missing the fileName' do
+      let(:metadata) { Hash.new.to_json }
+
+      it_behaves_like 'constraint violation' do
+        let(:message) { "fileName #{I18n.t('activerecord.errors.messages.blank')}" }
+      end
+    end
+
+    context 'file is too large' do
+      let(:file) { mock_uploaded_file(content: 'a' * 2.kilobytes) }
+      let(:expanded_localization) do
+        I18n.t('activerecord.errors.messages.file_too_large', count: max_file_size.kilobytes)
+      end
+
+      it_behaves_like 'constraint violation' do
+        let(:message) { "File #{expanded_localization}" }
+      end
+    end
+
+    context 'only allowed to add messages, but no edit permission' do
+      let(:permissions) { %i[view_messages add_messages] }
+
+      it_behaves_like 'unauthorized access'
+    end
+
+    context 'only allowed to view messages' do
+      let(:permissions) { [:view_messages] }
+
+      it_behaves_like 'unauthorized access'
+    end
+  end
+end