diff --git a/docs/api/apiv3/endpoints/time_entries.apib b/docs/api/apiv3/endpoints/time_entries.apib index f71c5845c1..fe2fb4308f 100644 --- a/docs/api/apiv3/endpoints/time_entries.apib +++ b/docs/api/apiv3/endpoints/time_entries.apib @@ -1,8 +1,10 @@ # Group Time Entries ## Actions -| Link | Description | Condition | -|:-------------------:| -------------------------------------------------------------------- | ---------------------------------------------------------------- | +| Link | Description | Condition | +|:-------------------:| -------------------------------------------------------------------- | ---------------------------------------------------------------- | +| updateImmediately | Directly perform edits on this time entry | **Permission**: 'edit time entries' or 'edit own time entries if the time entry belongs to the user | +| delete | Delete this time entry | **Permission**: 'edit time entries' or 'edit own time entries if the time entry belongs to the user | None yet @@ -62,6 +64,14 @@ Depending on custom fields defined for time entries, additional properties might "self": { "href": "/api/v3/time_entries/1" }, + "updateImmediately": { + "href": "/api/v3/time_entries/1", + "method": "patch" + }, + "delete": { + "href": "/api/v3/time_entries/1", + "method": "delete" + }, "project": { "href": "/api/v3/projects/1", "title": "Some project" @@ -115,13 +125,10 @@ Permanently deletes the specified time entry. + Parameters + id (required, integer, `1`) ... Time entry id -+ Response 202 ++ Response 204 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) @@ -174,6 +181,14 @@ Permanently deletes the specified time entry. "self": { "href": "/api/v3/time_entries/1" }, + "updateImmediately": { + "href": "/api/v3/time_entries/1", + "method": "patch" + }, + "delete": { + "href": "/api/v3/time_entries/1", + "method": "delete" + }, "project": { "href": "/api/v3/projects/1", "title": "Some project" diff --git a/lib/api/v3/time_entries/time_entries_api.rb b/lib/api/v3/time_entries/time_entries_api.rb index 4001cc249f..4bfc7788a7 100644 --- a/lib/api/v3/time_entries/time_entries_api.rb +++ b/lib/api/v3/time_entries/time_entries_api.rb @@ -110,7 +110,7 @@ module API call = ::TimeEntries::DeleteService.new(time_entry: @time_entry, user: current_user).call if call.success? - status 202 + status 204 else fail ::API::Errors::ErrorBase.create_and_merge_errors(call.errors) end diff --git a/lib/api/v3/time_entries/time_entry_representer.rb b/lib/api/v3/time_entries/time_entry_representer.rb index bff785c220..35c6339f60 100644 --- a/lib/api/v3/time_entries/time_entry_representer.rb +++ b/lib/api/v3/time_entries/time_entry_representer.rb @@ -39,6 +39,24 @@ module API defaults render_nil: true + link :updateImmediately do + next unless update_allowed? + + { + href: api_v3_paths.time_entry(represented.id), + method: :patch + } + end + + link :delete do + next unless update_allowed? + + { + href: api_v3_paths.time_entry(represented.id), + method: :delete + } + end + property :id property :comments, @@ -109,6 +127,15 @@ module API def _type 'TimeEntry' end + + def update_allowed? + current_user_allowed_to(:edit_time_entries, context: represented.project) || + represented.user_id == current_user.id && current_user_allowed_to(:edit_own_time_entries, context: represented.project) + end + + def current_user_allowed_to(permission, context:) + current_user.allowed_to?(permission, context) + end end end end diff --git a/spec/lib/api/v3/time_entries/time_entry_representer_rendering_spec.rb b/spec/lib/api/v3/time_entries/time_entry_representer_rendering_spec.rb index 8095a6a027..849d2b1437 100644 --- a/spec/lib/api/v3/time_entries/time_entry_representer_rendering_spec.rb +++ b/spec/lib/api/v3/time_entries/time_entry_representer_rendering_spec.rb @@ -46,13 +46,21 @@ describe ::API::V3::TimeEntries::TimeEntryRepresenter, 'rendering' do let(:work_package) { time_entry.work_package } let(:activity) { FactoryBot.build_stubbed(:time_entry_activity) } let(:user) { FactoryBot.build_stubbed(:user) } + let(:current_user) { user } + let(:permissions) do + [:edit_time_entries] + end let(:representer) do - described_class.create(time_entry, current_user: user, embed_links: true) + described_class.create(time_entry, current_user: current_user, embed_links: true) end subject { representer.to_json } before do + allow(current_user) + .to receive(:allowed_to?) do |permission, context_project| + project == context_project && permissions.include?(permission) + end allow(time_entry) .to receive(:available_custom_fields) .and_return([]) @@ -146,6 +154,60 @@ describe ::API::V3::TimeEntries::TimeEntryRepresenter, 'rendering' do .at_path("_links/customField#{custom_field.id}/href") end end + + context 'when allowed to update' do + it_behaves_like 'has an untitled link' do + let(:link) { 'updateImmediately' } + let(:href) { api_v3_paths.time_entry(time_entry.id) } + let(:method) { :patch } + end + + it_behaves_like 'has an untitled link' do + let(:link) { 'delete' } + let(:href) { api_v3_paths.time_entry(time_entry.id) } + let(:method) { :delete } + end + end + + context 'when not allowed to update' do + let(:permissions) { [] } + it_behaves_like 'has no link' do + let(:link) { 'updateImmediately' } + end + + it_behaves_like 'has no link' do + let(:link) { 'delete' } + end + end + + context 'when allowed to edit own and it is own' do + let(:permissions) { [:edit_own_time_entries] } + + it_behaves_like 'has an untitled link' do + let(:link) { 'updateImmediately' } + let(:href) { api_v3_paths.time_entry(time_entry.id) } + let(:method) { :patch } + end + + it_behaves_like 'has an untitled link' do + let(:link) { 'delete' } + let(:href) { api_v3_paths.time_entry(time_entry.id) } + let(:method) { :delete } + end + end + + context 'when allowed to edit own and it is not own' do + let(:permissions) { [:edit_own_time_entries] } + let(:current_user) { FactoryBot.build_stubbed(:user) } + + it_behaves_like 'has no link' do + let(:link) { 'updateImmediately' } + end + + it_behaves_like 'has no link' do + let(:link) { 'delete' } + end + end end describe 'properties' do diff --git a/spec/requests/api/v3/time_entry_resource_spec.rb b/spec/requests/api/v3/time_entry_resource_spec.rb index 272cb2f033..cbff81cb20 100644 --- a/spec/requests/api/v3/time_entry_resource_spec.rb +++ b/spec/requests/api/v3/time_entry_resource_spec.rb @@ -625,7 +625,7 @@ describe 'API v3 time_entry resource', type: :request do end it 'deleted the time entry' do - expect(subject.status).to eq(202) + expect(subject.status).to eq(204) end context 'when lacking permissions' do @@ -641,7 +641,7 @@ describe 'API v3 time_entry resource', type: :request do shared_examples_for 'deletes the time_entry' do it 'responds with HTTP No Content' do - expect(subject.status).to eq 202 + expect(subject.status).to eq 204 end it 'removes the time_entry from the DB' do @@ -664,7 +664,7 @@ describe 'API v3 time_entry resource', type: :request do end context 'with the user not being the author' do - let(:time_entry) { other_time_entry } + 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) }