From 69debf8c0c6a8bbcd5e2e61a0b0b0e04a8ee9167 Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Thu, 21 Nov 2019 15:27:57 +0100 Subject: [PATCH 1/2] Add Bcf Topic DeleteService and DeleteEndpoint --- app/services/base_services/base_contracted.rb | 4 ++ app/services/base_services/delete.rb | 2 +- app/services/base_services/write.rb | 4 -- lib/api/utilities/endpoints/delete.rb | 20 +++++-- .../contracts/bcf/issues/delete_contract.rb | 34 +++++++++++ .../bcf/api/v2_1/endpoints/delete.rb | 54 +++++++++++++++++ .../controllers/bcf/api/v2_1/topics_api.rb | 5 ++ .../app/services/bcf/issues/delete_service.rb | 49 +++++++++++++++ modules/bcf/lib/open_project/bcf/engine.rb | 2 +- .../requests/api/bcf/v2_1/topics_api_spec.rb | 60 ++++++++++++++++++- 10 files changed, 223 insertions(+), 11 deletions(-) create mode 100644 modules/bcf/app/contracts/bcf/issues/delete_contract.rb create mode 100644 modules/bcf/app/controllers/bcf/api/v2_1/endpoints/delete.rb create mode 100644 modules/bcf/app/services/bcf/issues/delete_service.rb diff --git a/app/services/base_services/base_contracted.rb b/app/services/base_services/base_contracted.rb index b32e451bb4..13f421a2f1 100644 --- a/app/services/base_services/base_contracted.rb +++ b/app/services/base_services/base_contracted.rb @@ -88,5 +88,9 @@ module BaseServices def default_contract_class raise NotImplementedError end + + def namespace + self.class.name.deconstantize.pluralize + end end end diff --git a/app/services/base_services/delete.rb b/app/services/base_services/delete.rb index a3ea551132..e6b481acee 100644 --- a/app/services/base_services/delete.rb +++ b/app/services/base_services/delete.rb @@ -49,7 +49,7 @@ module BaseServices protected def default_contract_class - "#{model.class.name.demodulize.pluralize}::DeleteContract".constantize + "#{namespace}::DeleteContract".constantize end end end diff --git a/app/services/base_services/write.rb b/app/services/base_services/write.rb index 03be3b55ec..cc01e4e7a1 100644 --- a/app/services/base_services/write.rb +++ b/app/services/base_services/write.rb @@ -73,10 +73,6 @@ module BaseServices raise NotImplementedError end - def namespace - self.class.name.deconstantize - end - def instance_class namespace.singularize.constantize end diff --git a/lib/api/utilities/endpoints/delete.rb b/lib/api/utilities/endpoints/delete.rb index 4fc3243b42..baf0e0ecc6 100644 --- a/lib/api/utilities/endpoints/delete.rb +++ b/lib/api/utilities/endpoints/delete.rb @@ -38,10 +38,12 @@ module API def initialize(model:, instance_generator: default_instance_generator(model), - process_service: nil) + process_service: nil, + api_name: model.name.demodulize) self.model = model self.instance_generator = instance_generator self.process_service = process_service || deduce_process_service + self.api_name = api_name end def mount @@ -67,6 +69,7 @@ module API def render(call) if success?(call) yield + present_success(call) else present_error(call) end @@ -78,7 +81,8 @@ module API attr_accessor :model, :instance_generator, - :process_service + :process_service, + :api_name private @@ -86,21 +90,29 @@ module API fail ::API::Errors::ErrorBase.create_and_merge_errors(call.errors) end + def present_success(call) + # Handle success cases by subclasses + end + def success?(call) call.success? end def deduce_process_service - "::#{deduce_namespace}::DeleteService".constantize + "::#{deduce_backend_namespace}::DeleteService".constantize end - def deduce_namespace + def deduce_backend_namespace demodulized_name.pluralize end def demodulized_name model.name.demodulize end + + def deduce_api_namespace + api_name.pluralize + end end end end diff --git a/modules/bcf/app/contracts/bcf/issues/delete_contract.rb b/modules/bcf/app/contracts/bcf/issues/delete_contract.rb new file mode 100644 index 0000000000..30f19e64c5 --- /dev/null +++ b/modules/bcf/app/contracts/bcf/issues/delete_contract.rb @@ -0,0 +1,34 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2017 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 doc/COPYRIGHT.rdoc for more details. +#++ + +module Bcf::Issues + class DeleteContract < ::DeleteContract + delete_permission :manage_bcf + + end +end diff --git a/modules/bcf/app/controllers/bcf/api/v2_1/endpoints/delete.rb b/modules/bcf/app/controllers/bcf/api/v2_1/endpoints/delete.rb new file mode 100644 index 0000000000..e1ced89ac5 --- /dev/null +++ b/modules/bcf/app/controllers/bcf/api/v2_1/endpoints/delete.rb @@ -0,0 +1,54 @@ +#-- 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. +#++ + +module Bcf::API::V2_1::Endpoints + class Delete < API::Utilities::Endpoints::Delete + include ModifyMixin + + def render_success(call) + render_representer + .new(call.result) + end + + def success_status + 200 + end + + private + + def render_representer + "::Bcf::API::V2_1::#{deduce_api_namespace}::SingleRepresenter".constantize + end + + def deduce_process_service + "::Bcf::#{deduce_backend_namespace}::DeleteService".constantize + end + end +end diff --git a/modules/bcf/app/controllers/bcf/api/v2_1/topics_api.rb b/modules/bcf/app/controllers/bcf/api/v2_1/topics_api.rb index e9f4744601..323c32e3df 100644 --- a/modules/bcf/app/controllers/bcf/api/v2_1/topics_api.rb +++ b/modules/bcf/app/controllers/bcf/api/v2_1/topics_api.rb @@ -75,6 +75,11 @@ module Bcf::API::V2_1 .new(model: Bcf::Issue, api_name: 'Topics') .mount + + delete &::Bcf::API::V2_1::Endpoints::Delete + .new(model: Bcf::Issue, + api_name: 'Topics') + .mount end end end diff --git a/modules/bcf/app/services/bcf/issues/delete_service.rb b/modules/bcf/app/services/bcf/issues/delete_service.rb new file mode 100644 index 0000000000..ccfece7acf --- /dev/null +++ b/modules/bcf/app/services/bcf/issues/delete_service.rb @@ -0,0 +1,49 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2019 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 doc/COPYRIGHT.rdoc for more details. +#++ + +module Bcf::Issues + class DeleteService < ::BaseServices::Delete + private + + def before_perform(params) + associated_wp = WorkPackage.find(model.work_package_id) + wp_call = WorkPackages::DeleteService + .new(user: user, + model: associated_wp.reload) + .call(params) + + if wp_call.success? + super(params) + else + wp_call + end + end + end +end diff --git a/modules/bcf/lib/open_project/bcf/engine.rb b/modules/bcf/lib/open_project/bcf/engine.rb index cf10abb9d9..8d556ba82a 100644 --- a/modules/bcf/lib/open_project/bcf/engine.rb +++ b/modules/bcf/lib/open_project/bcf/engine.rb @@ -48,7 +48,7 @@ module OpenProject::Bcf dependencies: %i[view_work_packages] permission :manage_bcf, { 'bcf/issues': %i[index upload prepare_import configure_import perform_import] }, - dependencies: %i[view_linked_issues view_work_packages add_work_packages edit_work_packages] + dependencies: %i[view_linked_issues view_work_packages add_work_packages edit_work_packages delete_work_packages] end OpenProject::AccessControl.permission(:view_work_packages).actions << 'bcf/issues/redirect_to_bcf_issues_list' diff --git a/modules/bcf/spec/requests/api/bcf/v2_1/topics_api_spec.rb b/modules/bcf/spec/requests/api/bcf/v2_1/topics_api_spec.rb index f949708d8d..afcae07567 100644 --- a/modules/bcf/spec/requests/api/bcf/v2_1/topics_api_spec.rb +++ b/modules/bcf/spec/requests/api/bcf/v2_1/topics_api_spec.rb @@ -48,7 +48,12 @@ describe 'BCF 2.1 topics resource', type: :request, content_type: :json, with_ma let(:edit_member_user) do FactoryBot.create(:user, member_in_project: project, - member_with_permissions: %i[manage_bcf add_work_packages view_linked_issues]) + member_with_permissions: %i[manage_bcf add_work_packages view_linked_issues delete_work_packages]) + end + let(:edit_work_package_member_user) do + FactoryBot.create(:user, + member_in_project: project, + member_with_permissions: %i[add_work_packages delete_work_packages]) end let(:non_member_user) do FactoryBot.create(:user) @@ -172,6 +177,59 @@ describe 'BCF 2.1 topics resource', type: :request, content_type: :json, with_ma end end + describe 'DELETE /api/bcf/2.1/projects/:project_id/topics/:uuid' do + let(:path) { "/api/bcf/2.1/projects/#{project.id}/topics/#{bcf_issue.uuid}" } + let(:current_user) { edit_member_user } + + before do + login_as(current_user) + bcf_issue + work_package + delete path + end + + it_behaves_like 'bcf api successful response' do + let(:expected_body) do + { + "assigned_to": assignee.mail, + "creation_author": work_package.author.mail, + "creation_date": work_package.created_at.iso8601, + "description": work_package.description, + "due_date": nil, + "guid": bcf_issue.uuid, + "index": bcf_issue.index, + "labels": bcf_issue.labels, + "priority": work_package.priority.name, + "modified_author": current_user.mail, + "modified_date": work_package.updated_at.iso8601, + "reference_links": [ + api_v3_paths.work_package(work_package.id) + ], + "stage": bcf_issue.stage, + "title": work_package.subject, + "topic_status": work_package.status.name, + "topic_type": work_package.type.name + } + end + end + + it 'deletes the Bcf Issue as well as the belonging Work Package' do + expect(WorkPackage.where(id: work_package.id)).to match_array [] + expect(Bcf::Issue.where(id: bcf_issue.id)).to match_array [] + end + + context 'lacking permission to manage bcf' do + let(:current_user) { edit_work_package_member_user } + + it_behaves_like 'bcf api not allowed response' + + it 'deletes neither the Work Package nor the Bcf Issue' do + expect(WorkPackage.where(id: work_package.id)).to match_array [work_package] + expect(Bcf::Issue.where(id: bcf_issue.id)).to match_array [bcf_issue] + end + end + end + describe 'POST /api/bcf/2.1/projects/:project_id/topics' do let(:path) { "/api/bcf/2.1/projects/#{project.id}/topics" } let(:current_user) { edit_member_user } From b4f7a940c523b470188164a90493eb4d02e8aaac Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Mon, 25 Nov 2019 09:34:27 +0100 Subject: [PATCH 2/2] Let delete endpoint return 204 --- .../contracts/bcf/issues/delete_contract.rb | 1 - .../bcf/api/v2_1/endpoints/delete.rb | 4 --- modules/bcf/lib/open_project/bcf/engine.rb | 6 ++++- .../requests/api/bcf/v2_1/shared_responses.rb | 2 +- .../requests/api/bcf/v2_1/topics_api_spec.rb | 25 +++---------------- 5 files changed, 9 insertions(+), 29 deletions(-) diff --git a/modules/bcf/app/contracts/bcf/issues/delete_contract.rb b/modules/bcf/app/contracts/bcf/issues/delete_contract.rb index 30f19e64c5..98b4a53d6e 100644 --- a/modules/bcf/app/contracts/bcf/issues/delete_contract.rb +++ b/modules/bcf/app/contracts/bcf/issues/delete_contract.rb @@ -29,6 +29,5 @@ module Bcf::Issues class DeleteContract < ::DeleteContract delete_permission :manage_bcf - end end diff --git a/modules/bcf/app/controllers/bcf/api/v2_1/endpoints/delete.rb b/modules/bcf/app/controllers/bcf/api/v2_1/endpoints/delete.rb index e1ced89ac5..1247c18b7e 100644 --- a/modules/bcf/app/controllers/bcf/api/v2_1/endpoints/delete.rb +++ b/modules/bcf/app/controllers/bcf/api/v2_1/endpoints/delete.rb @@ -37,10 +37,6 @@ module Bcf::API::V2_1::Endpoints .new(call.result) end - def success_status - 200 - end - private def render_representer diff --git a/modules/bcf/lib/open_project/bcf/engine.rb b/modules/bcf/lib/open_project/bcf/engine.rb index 8d556ba82a..60e3a2006b 100644 --- a/modules/bcf/lib/open_project/bcf/engine.rb +++ b/modules/bcf/lib/open_project/bcf/engine.rb @@ -48,7 +48,11 @@ module OpenProject::Bcf dependencies: %i[view_work_packages] permission :manage_bcf, { 'bcf/issues': %i[index upload prepare_import configure_import perform_import] }, - dependencies: %i[view_linked_issues view_work_packages add_work_packages edit_work_packages delete_work_packages] + dependencies: %i[view_linked_issues + view_work_packages + add_work_packages + edit_work_packages + delete_work_packages] end OpenProject::AccessControl.permission(:view_work_packages).actions << 'bcf/issues/redirect_to_bcf_issues_list' diff --git a/modules/bcf/spec/requests/api/bcf/v2_1/shared_responses.rb b/modules/bcf/spec/requests/api/bcf/v2_1/shared_responses.rb index 5db1743e6e..2b437651d2 100644 --- a/modules/bcf/spec/requests/api/bcf/v2_1/shared_responses.rb +++ b/modules/bcf/spec/requests/api/bcf/v2_1/shared_responses.rb @@ -31,7 +31,7 @@ shared_examples_for 'bcf api successful response' do expect(subject.status) .to eql(defined?(expected_status) ? expected_status : 200) expect(subject.body).to be_json_eql(expected_body.to_json) - expect(subject.headers['Content-Type']).to eql 'application/json; charset=utf-8' + expect(subject.headers['Content-Type']).to eql 'application/json; charset=utf-8' unless defined?(no_content) end end diff --git a/modules/bcf/spec/requests/api/bcf/v2_1/topics_api_spec.rb b/modules/bcf/spec/requests/api/bcf/v2_1/topics_api_spec.rb index afcae07567..24443a4a54 100644 --- a/modules/bcf/spec/requests/api/bcf/v2_1/topics_api_spec.rb +++ b/modules/bcf/spec/requests/api/bcf/v2_1/topics_api_spec.rb @@ -189,28 +189,9 @@ describe 'BCF 2.1 topics resource', type: :request, content_type: :json, with_ma end it_behaves_like 'bcf api successful response' do - let(:expected_body) do - { - "assigned_to": assignee.mail, - "creation_author": work_package.author.mail, - "creation_date": work_package.created_at.iso8601, - "description": work_package.description, - "due_date": nil, - "guid": bcf_issue.uuid, - "index": bcf_issue.index, - "labels": bcf_issue.labels, - "priority": work_package.priority.name, - "modified_author": current_user.mail, - "modified_date": work_package.updated_at.iso8601, - "reference_links": [ - api_v3_paths.work_package(work_package.id) - ], - "stage": bcf_issue.stage, - "title": work_package.subject, - "topic_status": work_package.status.name, - "topic_type": work_package.type.name - } - end + let(:expected_status) { 204 } + let(:expected_body) { nil } + let(:no_content) { true } end it 'deletes the Bcf Issue as well as the belonging Work Package' do