From 9eed759cd00fbc3d6a84e17061e07e18a40d3bdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20H=C3=B6ffken?= Date: Fri, 16 Nov 2018 17:48:35 +0100 Subject: [PATCH] [29029] DELETE option for TimeEntries --- .gitignore | 3 + app/contracts/time_entries/delete_contract.rb | 58 ++++++++++ app/services/time_entries/delete_service.rb | 52 +++++++++ docs/api/apiv3/endpoints/time_entries.apib | 42 +++++++ lib/api/v3/time_entries/time_entries_api.rb | 8 ++ .../time_entries/delete_contract_spec.rb | 107 ++++++++++++++++++ .../api/v3/time_entry_resource_spec.rb | 45 ++++++++ 7 files changed, 315 insertions(+) create mode 100644 app/contracts/time_entries/delete_contract.rb create mode 100644 app/services/time_entries/delete_service.rb create mode 100644 spec/contracts/time_entries/delete_contract_spec.rb diff --git a/.gitignore b/.gitignore index bb8a633540b..8eb39b4aeb5 100644 --- a/.gitignore +++ b/.gitignore @@ -41,6 +41,9 @@ npm-debug.log* /tmp *.swp +# Ignore Visual Studio Code files +/.vscode + # Ignore RubyMine files /.idea /frontend/.idea diff --git a/app/contracts/time_entries/delete_contract.rb b/app/contracts/time_entries/delete_contract.rb new file mode 100644 index 00000000000..c5b4df3446f --- /dev/null +++ b/app/contracts/time_entries/delete_contract.rb @@ -0,0 +1,58 @@ +#-- encoding: UTF-8 + +#-- 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 TimeEntries + class DeleteContract < BaseContract + def validate + unless user_allowed_to_delete? + errors.add :base, :error_unauthorized + end + + super + end + + private + + ## + # Users may delete time entries IF + # they have the :edit_time_entries or + # user == deleting user and :edit_own_time_entries + def user_allowed_to_delete? + edit_all = user.allowed_to?(:edit_time_entries, model.project) + edit_own = user.allowed_to?(:edit_own_time_entries, model.project) + + if model.user == user + edit_own || edit_all + else + edit_all + end + end + end +end diff --git a/app/services/time_entries/delete_service.rb b/app/services/time_entries/delete_service.rb new file mode 100644 index 00000000000..a67f1bc0493 --- /dev/null +++ b/app/services/time_entries/delete_service.rb @@ -0,0 +1,52 @@ +#-- 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. +#++ + +## +# Implements the deletion of a time entry. +class TimeEntries::DeleteService + include Concerns::Contracted + attr_accessor :user, :time_entry + + def initialize(user:, time_entry:) + self.user = user + self.time_entry = time_entry + self.contract_class = TimeEntries::DeleteContract + end + + ## + # Deletes the given time entry if allowed. + # + # @return True if the deletion has been initiated, false otherwise. + def call + validate_and_yield(time_entry, user) do + time_entry.destroy + end + end +end diff --git a/docs/api/apiv3/endpoints/time_entries.apib b/docs/api/apiv3/endpoints/time_entries.apib index 878684ea670..8d7a9550487 100644 --- a/docs/api/apiv3/endpoints/time_entries.apib +++ b/docs/api/apiv3/endpoints/time_entries.apib @@ -107,6 +107,48 @@ Depending on custom fields defined for time entries, additional properties might "message": "The requested resource could not be found." } + +## Delete time entry [DELETE] + +Permanently deletes the specified time entry. + ++ Parameters + + id (required, integer, `1`) ... Time entry id + ++ Response 202 + + Returned if the time entry was deleted successfully. + + Note that the response body is empty as of now. In future versions of the API a body + *might* be returned, indicating the progress of deletion. + + + Body + ++ Response 403 (application/hal+json) + + Returned if the client does not have sufficient permissions + + + Body + + { + "_type": "Error", + "errorIdentifier": "urn:openproject-org:api:v3:errors:MissingPermission", + "message": "You are not allowed to delete this time entry." + } + ++ Response 404 (application/hal+json) + + Returned if the time entry does not exist. + + + Body + + { + "_type": "Error", + "errorIdentifier": "urn:openproject-org:api:v3:errors:NotFound", + "message": "The specified user does not exist." + } + + ## Time entries [/api/v3/time_entries{?offset,pageSize,filters,sortBy}] + Model diff --git a/lib/api/v3/time_entries/time_entries_api.rb b/lib/api/v3/time_entries/time_entries_api.rb index c19296eb024..add067d2509 100644 --- a/lib/api/v3/time_entries/time_entries_api.rb +++ b/lib/api/v3/time_entries/time_entries_api.rb @@ -105,6 +105,14 @@ module API fail ::API::Errors::ErrorBase.create_and_merge_errors(result.errors) end end + + delete do + if ::TimeEntries::DeleteService.new(time_entry: @time_entry, user: current_user).call + status 202 + else + fail ::API::Errors::Unauthorized + end + end end mount ::API::V3::TimeEntries::TimeEntriesActivityAPI diff --git a/spec/contracts/time_entries/delete_contract_spec.rb b/spec/contracts/time_entries/delete_contract_spec.rb new file mode 100644 index 00000000000..882c7dbfb6c --- /dev/null +++ b/spec/contracts/time_entries/delete_contract_spec.rb @@ -0,0 +1,107 @@ +#-- 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' + +describe TimeEntries::DeleteContract do + let(:current_user) do + FactoryBot.build_stubbed(:user) do |user| + allow(user) + .to receive(:allowed_to?) do |permission, permission_project| + permissions.include?(permission) && time_entry_project == permission_project + end + end + end + let(:other_user) { FactoryBot.build_stubbed(:user) } + let(:time_entry_work_package) do + FactoryBot.build_stubbed(:work_package, + project: time_entry_project) + end + let(:time_entry_project) { FactoryBot.build_stubbed(:project) } + let(:time_entry_activity) { FactoryBot.build_stubbed(:time_entry_activity) } + let(:time_entry_user) { current_user } + let(:time_entry_spent_on) { Date.today } + let(:time_entry_hours) { 5 } + let(:time_entry_comments) { "A comment" } + let(:time_entry) do + TimeEntry.create(project: time_entry_project, + work_package: time_entry_work_package, + user: time_entry_user, + activity: time_entry_activity, + spent_on: time_entry_spent_on, + hours: time_entry_hours, + comments: time_entry_comments) + end + let(:permissions) { %i(edit_time_entries) } + + subject(:contract) { described_class.new(time_entry, current_user) } + + def expect_valid(valid, symbols = {}) + expect(contract.validate).to eq(valid) + + symbols.each do |key, arr| + expect(contract.errors.symbols_for(key)).to match_array arr + end + end + + shared_examples 'is valid' do + it 'is valid' do + expect_valid(true) + end + end + + it_behaves_like 'is valid' + + context 'when user is not allowed to delete time entries' do + let(:permissions) { [] } + + it 'is invalid' do + expect_valid(false, base: %i(error_unauthorized)) + end + end + + context 'when time_entry user is not contract user' do + let(:time_entry_user) { other_user } + + context 'when has permission' do + let(:permissions) { %i[edit_time_entries] } + + it 'is valid' do + expect_valid(true) + end + end + + context 'when has no permission' do + let(:permissions) { %i[edit_own_time_entries] } + it 'is invalid' do + expect_valid(false, base: %i(error_unauthorized)) + end + end + end +end diff --git a/spec/requests/api/v3/time_entry_resource_spec.rb b/spec/requests/api/v3/time_entry_resource_spec.rb index 188836308e9..5c1c0feff28 100644 --- a/spec/requests/api/v3/time_entry_resource_spec.rb +++ b/spec/requests/api/v3/time_entry_resource_spec.rb @@ -493,4 +493,49 @@ describe 'API v3 time_entry resource', type: :request do end end end + + describe 'DELETE /api/v3/time_entries/:id' do + let(:path) { api_v3_paths.time_entry time_entry.id } + + before do + delete path + end + + subject(:response) { last_response } + + shared_examples_for 'deletes the time_entry' do + it 'responds with HTTP No Content' do + expect(subject.status).to eq 204 + end + + it 'removes the time_entry from the DB' do + expect(TimeEntry.exists?(time_entry.id)).to be_falsey + end + end + + shared_examples_for 'does not delete the time_entry' do |status = 403| + it "responds with #{status}" do + expect(subject.status).to eq status + end + + it 'does not delete the time_entry' do + expect(TimeEntry.exists?(time_entry.id)).to be_truthy + end + end + + context "with an uncontainered time_entry" do + let(:container) { nil } + + context 'with the user being the author' do + it_behaves_like 'deletes the time_entry' + end + + context 'with the user not being the author' do + let(:author) { FactoryBot.create(:user) } + + it_behaves_like 'does not delete the time_entry', 404 + end + end + end + end