Merge pull request #6893 from opf/fix/time_entries_api_links

time entries api - link actions and use correct response on delete

[ci skip]

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"

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

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

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

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) }