[#42204] add file links to work package resource

- https://community.openproject.org/work_packages/42204
- added file links include module for other resources
- fixed some rubocop issues in work package representer
- added representer tests

docs/api/apiv3/components/schemas/work_package_model.yml

@@ -307,6 +307,24 @@ properties:
                 
                 **Permission** view work packages
               readOnly: true
+      addFileLink:
+        allOf:
+          - $ref: './link.yml'
+          - description: |-
+              Add a file link to the work package
+
+              # Conditions
+
+              **Permission**: manage file links
+      fileLinks:
+        allOf:
+          - $ref: './link.yml'
+          - description: |-
+              Gets the file link collection of this work package
+
+              # Conditions
+
+              **Permission**: view file links
       parent:
         allOf:
           - "$ref": "./link.yml"

lib/api/v3/work_packages/work_package_representer.rb

@@ -35,6 +35,7 @@ module API
         include API::Decorators::FormattableProperty
         include API::Caching::CachedRepresenter
         include ::API::V3::Attachments::AttachableRepresenterMixin
+        include ::API::V3::FileLinks::FileLinkRelationRepresenter
         extend ::API::V3::Utilities::CustomFieldInjector::RepresenterClass
 
         cached_representer key_parts: %i(project),
@@ -351,10 +352,10 @@ module API
                         next unless doc.key?('date')
 
                         date = decorator
-                               .datetime_formatter
+                                 .datetime_formatter
-                               .parse_date(doc['date'],
+                                 .parse_date(doc['date'],
-                                           name.to_s.camelize(:lower),
+                                             name.to_s.camelize(:lower),
-                                           allow_nil: true)
+                                             allow_nil: true)
 
                         self.due_date = self.start_date = date
                       },
@@ -473,10 +474,10 @@ module API
 
                               new_parent = if href
                                              id = ::API::Utilities::ResourceLinkParser
-                                                  .parse_id href,
+                                                    .parse_id href,
-                                                            property: 'parent',
+                                                              property: 'parent',
-                                                            expected_version: '3',
+                                                              expected_version: '3',
-                                                            expected_namespace: 'work_packages'
+                                                              expected_namespace: 'work_packages'
 
                                              WorkPackage.find_by(id: id) ||
                                                ::WorkPackage::InexistentWorkPackage.new(id: id)
@@ -536,10 +537,10 @@ module API
         def relations
           self_path = api_v3_paths.work_package_relations(represented.id)
           visible_relations = represented
-                              .visible_relations(current_user)
+                                .visible_relations(current_user)
-                              .direct
+                                .direct
-                              .non_hierarchy
+                                .non_hierarchy
-                              .includes(::API::V3::Relations::RelationCollectionRepresenter.to_eager_load)
+                                .includes(::API::V3::Relations::RelationCollectionRepresenter.to_eager_load)
 
           ::API::V3::Relations::RelationCollectionRepresenter.new(visible_relations,
                                                                   self_link: self_path,
@@ -562,7 +563,7 @@ module API
 
         def derived_estimated_time=(value)
           represented.derived_estimated_hours = datetime_formatter
-            .parse_duration_to_hours(value, 'derivedEstimatedTime', allow_nil: true)
+                                                  .parse_duration_to_hours(value, 'derivedEstimatedTime', allow_nil: true)
         end
 
         def spent_time=(value)

modules/storages/lib/api/v3/file_links/file_link_relation_representer.rb

@@ -0,0 +1,77 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2022 the OpenProject GmbH
+#
+# 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,
+# 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 COPYRIGHT and LICENSE files for more details.
+#++
+
+module API
+  module V3
+    module FileLinks
+      module FileLinkRelationRepresenter
+        extend ActiveSupport::Concern
+
+        included do
+          link :fileLinks, cache_if: -> { render_file_links? } do
+            {
+              href: api_v3_paths.file_links(represented.id)
+            }
+          end
+
+          link :addFileLink, cache_if: -> { render_action_link? } do
+            {
+              href: api_v3_paths.file_links(represented.id),
+              method: :post
+            }
+          end
+
+          property :file_links,
+                   embedded: true,
+                   exec_context: :decorator,
+                   if: ->(*) { embed_links && render_file_links? },
+                   uncacheable: true
+
+          def file_links
+            ::API::V3::FileLinks::FileLinkCollectionRepresenter.new(represented.file_links,
+                                                                    self_link: api_v3_paths.file_links(represented.id),
+                                                                    current_user: current_user)
+          end
+
+          private
+
+          def render_action_link?
+            render_file_links? &&
+              current_user.allowed_to?(:manage_file_links, represented.project)
+          end
+
+          def render_file_links?
+            OpenProject::FeatureDecisions.storages_module_active? &&
+              represented.project.module_enabled?('storages') &&
+              current_user.allowed_to?(:view_file_links, represented.project)
+          end
+        end
+      end
+    end
+  end
+end

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

@@ -39,18 +39,18 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
     described_class.create(work_package, current_user: current_user, embed_links: embed_links)
   end
   let(:parent) { nil }
-  let(:priority) { build_stubbed(:priority, updated_at: Time.now) }
+  let(:priority) { build_stubbed(:priority, updated_at: Time.zone.now) }
   let(:assignee) { nil }
   let(:responsible) { nil }
   let(:schedule_manually) { nil }
-  let(:start_date) { Date.today.to_datetime }
+  let(:start_date) { Time.zone.today.to_datetime }
-  let(:due_date) { Date.today.to_datetime }
+  let(:due_date) { Time.zone.today.to_datetime }
   let(:type_milestone) { false }
   let(:estimated_hours) { nil }
   let(:derived_estimated_hours) { nil }
   let(:spent_hours) { 0 }
-  let(:derived_start_date) { Date.today - 4.days }
+  let(:derived_start_date) { Time.zone.today - 4.days }
-  let(:derived_due_date) { Date.today - 5.days }
+  let(:derived_due_date) { Time.zone.today - 5.days }
   let(:budget) { build_stubbed(:budget, project: project) }
   let(:work_package) do
     build_stubbed(:stubbed_work_package,
@@ -97,6 +97,8 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
       add_work_packages
       view_time_entries
       view_changesets
+      view_file_links
+      manage_file_links
       delete_work_packages
     ]
   end
@@ -109,7 +111,7 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
 
     type
   end
-  let(:status) { build_stubbed(:status, updated_at: Time.now) }
+  let(:status) { build_stubbed(:status, updated_at: Time.zone.now) }
   let(:available_custom_fields) { [] }
 
   before(:each) do
@@ -141,7 +143,7 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
       it_behaves_like 'API V3 formattable', 'description' do
         let(:format) { 'markdown' }
         let(:raw) { work_package.description }
-        let(:html) { '<p class="op-uc-p">' + work_package.description + '</p>' }
+        let(:html) { "<p class=\"op-uc-p\">#{work_package.description}</p>" }
       end
 
       describe 'scheduleManually' do
@@ -715,6 +717,60 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
         end
       end
 
+      describe 'fileLinks' do
+        let(:storages_feature_enabled) { true }
+
+        before do
+          allow(OpenProject::FeatureDecisions).to receive(:storages_module_active?).and_return(storages_feature_enabled)
+        end
+
+        it_behaves_like 'has an untitled link' do
+          let(:link) { 'fileLinks' }
+          let(:href) { api_v3_paths.file_links(work_package.id) }
+        end
+
+        it_behaves_like 'has an untitled action link' do
+          let(:permission) { :manage_file_links }
+          let(:link) { 'addFileLink' }
+          let(:href) { api_v3_paths.file_links(work_package.id) }
+          let(:method) { 'post' }
+        end
+
+        context 'if user has no permission to view file links' do
+          let(:permissions) { all_permissions - %i[view_file_links] }
+
+          it_behaves_like 'has no link' do
+            let(:link) { 'fileLinks' }
+          end
+        end
+
+        context 'if file storages feature is disabled' do
+          let(:storages_feature_enabled) { false }
+
+          it_behaves_like 'has no link' do
+            let(:link) { 'fileLinks' }
+          end
+
+          it_behaves_like 'has no link' do
+            let(:link) { 'addFileLink' }
+          end
+        end
+
+        context 'if project has storages module disabled' do
+          before do
+            project.enabled_module_names = project.enabled_module_names - ['storages']
+          end
+
+          it_behaves_like 'has no link' do
+            let(:link) { 'fileLinks' }
+          end
+
+          it_behaves_like 'has no link' do
+            let(:link) { 'addFileLink' }
+          end
+        end
+      end
+
       context 'when the user is not watching the work package' do
         it 'should have a link to watch' do
           expect(subject)
@@ -1151,6 +1207,39 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
         end
       end
 
+      describe 'fileLinks' do
+        let(:storages_feature_enabled) { true }
+        let(:storage) { build_stubbed(:storage) }
+        let(:file_link) { build_stubbed(:file_link, storage: storage, container: work_package) }
+
+        before do
+          allow(OpenProject::FeatureDecisions).to receive(:storages_module_active?).and_return(storages_feature_enabled)
+          allow(work_package).to receive(:file_links).and_return([file_link])
+        end
+
+        it 'embeds a collection' do
+          is_expected
+            .to be_json_eql('Collection'.to_json)
+                  .at_path('_embedded/fileLinks/_type')
+        end
+
+        it 'embeds with an href containing the work_package' do
+          is_expected
+            .to be_json_eql(api_v3_paths.file_links(work_package.id).to_json)
+                  .at_path('_embedded/fileLinks/_links/self/href')
+        end
+
+        it 'embeds the visible file links' do
+          is_expected
+            .to be_json_eql(1.to_json)
+                  .at_path('_embedded/fileLinks/total')
+
+          is_expected
+            .to be_json_eql(api_v3_paths.file_link(file_link.id).to_json)
+                  .at_path('_embedded/fileLinks/_embedded/elements/0/_links/self/href')
+        end
+      end
+
       describe 'customActions' do
         it 'has an array of customActions' do
           unassign_action = build_stubbed(:custom_action,
@@ -1223,7 +1312,7 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
         end
 
         it 'changes when the work_package is updated' do
-          work_package.updated_at = Time.now + 20.seconds
+          work_package.updated_at = Time.zone.now + 20.seconds
 
           expect(representer.json_cache_key)
             .not_to eql former_cache_key