diff --git a/.gitignore b/.gitignore index bb8a633540..8eb39b4aeb 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 0000000000..c5b4df3446 --- /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 0000000000..0e1c2e4cdb --- /dev/null +++ b/app/services/time_entries/delete_service.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. +#++ + +## +# 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 + result, errors = validate_and_yield(time_entry, user) do + time_entry.destroy + end + + ServiceResult.new(success: result, errors: errors, result: time_entry) + end +end diff --git a/docs/api/apiv3/endpoints/time_entries.apib b/docs/api/apiv3/endpoints/time_entries.apib index 878684ea67..b548cf2ddf 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 authorized to access this resource." + } + ++ 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 requested resource could not be found." + } + + ## Time entries [/api/v3/time_entries{?offset,pageSize,filters,sortBy}] + Model diff --git a/frontend/npm-shrinkwrap.json b/frontend/npm-shrinkwrap.json index efca4a7e13..3695a29ff6 100644 --- a/frontend/npm-shrinkwrap.json +++ b/frontend/npm-shrinkwrap.json @@ -7445,7 +7445,7 @@ "jquery-mockjax": { "version": "2.2.2", "resolved": "https://registry.npmjs.org/jquery-mockjax/-/jquery-mockjax-2.2.2.tgz", - "integrity": "sha512-stVgUln5bo7g14KjeU45c31nAYlBHAUVBo4JbdoDYIJ3i6Nzlu8mkjKcAviwad9AU1dBEHHpuzA7R6mKcWgvZA==", + "integrity": "sha1-nH0rEDpEz2HJnMeKnueHhQS2CQc=", "dev": true, "requires": { "jquery": ">=1.5.2" diff --git a/lib/api/v3/time_entries/time_entries_api.rb b/lib/api/v3/time_entries/time_entries_api.rb index c19296eb02..4001cc249f 100644 --- a/lib/api/v3/time_entries/time_entries_api.rb +++ b/lib/api/v3/time_entries/time_entries_api.rb @@ -105,6 +105,16 @@ module API fail ::API::Errors::ErrorBase.create_and_merge_errors(result.errors) end end + + delete do + call = ::TimeEntries::DeleteService.new(time_entry: @time_entry, user: current_user).call + + if call.success? + status 202 + else + fail ::API::Errors::ErrorBase.create_and_merge_errors(call.errors) + 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 0000000000..de99b44ee7 --- /dev/null +++ b/spec/contracts/time_entries/delete_contract_spec.rb @@ -0,0 +1,117 @@ +#-- 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_user) { current_user } + let(:time_entry_activity) { FactoryBot.build_stubbed(:time_entry_activity) } + let(:time_entry_spent_on) { Date.today } + let(:time_entry_hours) { 5 } + let(:time_entry_comments) { "A comment" } + let(:work_package_visible) { true } + let(:permissions) { %i[edit_time_entries] } + + let(:time_entry) do + FactoryBot.build_stubbed(:time_entry, + 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 + + before do + allow(time_entry_work_package) + .to receive(:visible?) + .with(current_user) + .and_return(work_package_visible) + end + + 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 188836308e..98fc1047af 100644 --- a/spec/requests/api/v3/time_entry_resource_spec.rb +++ b/spec/requests/api/v3/time_entry_resource_spec.rb @@ -493,4 +493,71 @@ 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) } + let(:permissions) { %i(edit_own_time_entries view_time_entries view_work_packages) } + let(:params) {} + + before do + time_entry + other_time_entry + custom_value + + delete path, params.to_json, 'CONTENT_TYPE' => 'application/json' + end + + it 'deleted the time entry' do + expect(subject.status).to eq(202) + end + + context 'when lacking permissions' do + let(:permissions) { %i(view_time_entries) } + + it 'returns 403' do + expect(subject.status) + .to eql(403) + end + 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 202 + 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 the user being the author' do + it_behaves_like 'deletes the time_entry' + end + + context 'with the user not being the author' do + let(:time_entry) { other_time_entry } + + context 'but permission to edit all time entries' do + let(:permissions) { %i(edit_time_entries view_time_entries view_work_packages) } + + it_behaves_like 'deletes the time_entry' + end + + context 'with permission to delete own time entries' do + it_behaves_like 'does not delete the time_entry', 403 + end + end + end end