Fix time tracking endpoints for API v4

- Use issue/merge_request IID instead of ID
- Duplicate the original `TimeTrackingEndpoints` concern (+ specs) for V3, since
  this is a breaking change.

5 files changed, 249 insertions, 5 deletions

diff --git a/lib/api/time_tracking_endpoints.rb b/lib/api/time_tracking_endpoints.rb
index 85b5f7d98b8..05b4b490e27 100644
--- a/lib/api/time_tracking_endpoints.rb
+++ b/lib/api/time_tracking_endpoints.rb
@@ -5,11 +5,11 @@ module API
     included do
       helpers do
         def issuable_name
-          declared_params.has_key?(:issue_id) ? 'issue' : 'merge_request'
+          declared_params.has_key?(:issue_iid) ? 'issue' : 'merge_request'
         end
 
         def issuable_key
-          "#{issuable_name}_id".to_sym
+          "#{issuable_name}_iid".to_sym
         end
 
         def update_issuable_key
@@ -50,7 +50,7 @@ module API
 
       issuable_name            = name.end_with?('Issues') ? 'issue' : 'merge_request'
       issuable_collection_name = issuable_name.pluralize
-      issuable_key             = "#{issuable_name}_id".to_sym
+      issuable_key             = "#{issuable_name}_iid".to_sym
 
       desc "Set a time estimate for a project #{issuable_name}"
       params do
diff --git a/lib/api/v3/time_tracking_endpoints.rb b/lib/api/v3/time_tracking_endpoints.rb
new file mode 100644
index 00000000000..81ae4e8137d
--- /dev/null
+++ b/lib/api/v3/time_tracking_endpoints.rb
@@ -0,0 +1,116 @@
+module API
+  module V3
+    module TimeTrackingEndpoints
+      extend ActiveSupport::Concern
+
+      included do
+        helpers do
+          def issuable_name
+            declared_params.has_key?(:issue_id) ? 'issue' : 'merge_request'
+          end
+
+          def issuable_key
+            "#{issuable_name}_id".to_sym
+          end
+
+          def update_issuable_key
+            "update_#{issuable_name}".to_sym
+          end
+
+          def read_issuable_key
+            "read_#{issuable_name}".to_sym
+          end
+
+          def load_issuable
+            @issuable ||= begin
+                            case issuable_name
+                            when 'issue'
+                              find_project_issue(params.delete(issuable_key))
+                            when 'merge_request'
+                              find_project_merge_request(params.delete(issuable_key))
+                            end
+                          end
+          end
+
+          def update_issuable(attrs)
+            custom_params = declared_params(include_missing: false)
+            custom_params.merge!(attrs)
+
+            issuable = update_service.new(user_project, current_user, custom_params).execute(load_issuable)
+            if issuable.valid?
+              present issuable, with: ::API::Entities::IssuableTimeStats
+            else
+              render_validation_error!(issuable)
+            end
+          end
+
+          def update_service
+            issuable_name == 'issue' ? ::Issues::UpdateService : ::MergeRequests::UpdateService
+          end
+        end
+
+        issuable_name            = name.end_with?('Issues') ? 'issue' : 'merge_request'
+        issuable_collection_name = issuable_name.pluralize
+        issuable_key             = "#{issuable_name}_id".to_sym
+
+        desc "Set a time estimate for a project #{issuable_name}"
+        params do
+          requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}"
+          requires :duration, type: String, desc: 'The duration to be parsed'
+        end
+        post ":id/#{issuable_collection_name}/:#{issuable_key}/time_estimate" do
+          authorize! update_issuable_key, load_issuable
+
+          status :ok
+          update_issuable(time_estimate: Gitlab::TimeTrackingFormatter.parse(params.delete(:duration)))
+        end
+
+        desc "Reset the time estimate for a project #{issuable_name}"
+        params do
+          requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}"
+        end
+        post ":id/#{issuable_collection_name}/:#{issuable_key}/reset_time_estimate" do
+          authorize! update_issuable_key, load_issuable
+
+          status :ok
+          update_issuable(time_estimate: 0)
+        end
+
+        desc "Add spent time for a project #{issuable_name}"
+        params do
+          requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}"
+          requires :duration, type: String, desc: 'The duration to be parsed'
+        end
+        post ":id/#{issuable_collection_name}/:#{issuable_key}/add_spent_time" do
+          authorize! update_issuable_key, load_issuable
+
+          update_issuable(spend_time: {
+                            duration: Gitlab::TimeTrackingFormatter.parse(params.delete(:duration)),
+                            user: current_user
+                          })
+        end
+
+        desc "Reset spent time for a project #{issuable_name}"
+        params do
+          requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}"
+        end
+        post ":id/#{issuable_collection_name}/:#{issuable_key}/reset_spent_time" do
+          authorize! update_issuable_key, load_issuable
+
+          status :ok
+          update_issuable(spend_time: { duration: :reset, user: current_user })
+        end
+
+        desc "Show time stats for a project #{issuable_name}"
+        params do
+          requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}"
+        end
+        get ":id/#{issuable_collection_name}/:#{issuable_key}/time_stats" do
+          authorize! read_issuable_key, load_issuable
+
+          present load_issuable, with: ::API::Entities::IssuableTimeStats
+        end
+      end
+    end
+  end
+end
diff --git a/spec/requests/api/v3/issues_spec.rb b/spec/requests/api/v3/issues_spec.rb
index 2a8105d5a2b..1941ca0d7d8 100644
--- a/spec/requests/api/v3/issues_spec.rb
+++ b/spec/requests/api/v3/issues_spec.rb
@@ -1288,6 +1288,6 @@ describe API::V3::Issues, api: true  do
   describe 'time tracking endpoints' do
     let(:issuable) { issue }
 
-    include_examples 'time tracking endpoints', 'issue'
+    include_examples 'V3 time tracking endpoints', 'issue'
   end
 end
diff --git a/spec/requests/api/v3/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb
index b7ed643bc21..d73e9635c9b 100644
--- a/spec/requests/api/v3/merge_requests_spec.rb
+++ b/spec/requests/api/v3/merge_requests_spec.rb
@@ -712,7 +712,7 @@ describe API::MergeRequests, api: true  do
   describe 'Time tracking' do
     let(:issuable) { merge_request }
 
-    include_examples 'time tracking endpoints', 'merge_request'
+    include_examples 'V3 time tracking endpoints', 'merge_request'
   end
 
   def mr_with_later_created_and_updated_at_time
diff --git a/spec/support/api/v3/time_tracking_shared_examples.rb b/spec/support/api/v3/time_tracking_shared_examples.rb
new file mode 100644
index 00000000000..f982b10d999
--- /dev/null
+++ b/spec/support/api/v3/time_tracking_shared_examples.rb
@@ -0,0 +1,128 @@
+shared_examples 'V3 time tracking endpoints' do |issuable_name|
+  issuable_collection_name = issuable_name.pluralize
+
+  describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/time_estimate" do
+    context 'with an unauthorized user' do
+      subject { post(v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", non_member), duration: '1w') }
+
+      it_behaves_like 'an unauthorized API user'
+    end
+
+    it "sets the time estimate for #{issuable_name}" do
+      post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: '1w'
+
+      expect(response).to have_http_status(200)
+      expect(json_response['human_time_estimate']).to eq('1w')
+    end
+
+    describe 'updating the current estimate' do
+      before do
+        post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: '1w'
+      end
+
+      context 'when duration has a bad format' do
+        it 'does not modify the original estimate' do
+          post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: 'foo'
+
+          expect(response).to have_http_status(400)
+          expect(issuable.reload.human_time_estimate).to eq('1w')
+        end
+      end
+
+      context 'with a valid duration' do
+        it 'updates the estimate' do
+          post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: '3w1h'
+
+          expect(response).to have_http_status(200)
+          expect(issuable.reload.human_time_estimate).to eq('3w 1h')
+        end
+      end
+    end
+  end
+
+  describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/reset_time_estimate" do
+    context 'with an unauthorized user' do
+      subject { post(v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_time_estimate", non_member)) }
+
+      it_behaves_like 'an unauthorized API user'
+    end
+
+    it "resets the time estimate for #{issuable_name}" do
+      post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_time_estimate", user)
+
+      expect(response).to have_http_status(200)
+      expect(json_response['time_estimate']).to eq(0)
+    end
+  end
+
+  describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/add_spent_time" do
+    context 'with an unauthorized user' do
+      subject do
+        post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", non_member),
+             duration: '2h'
+      end
+
+      it_behaves_like 'an unauthorized API user'
+    end
+
+    it "add spent time for #{issuable_name}" do
+      post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", user),
+           duration: '2h'
+
+      expect(response).to have_http_status(201)
+      expect(json_response['human_total_time_spent']).to eq('2h')
+    end
+
+    context 'when subtracting time' do
+      it 'subtracts time of the total spent time' do
+        issuable.update_attributes!(spend_time: { duration: 7200, user: user })
+
+        post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", user),
+             duration: '-1h'
+
+        expect(response).to have_http_status(201)
+        expect(json_response['total_time_spent']).to eq(3600)
+      end
+    end
+
+    context 'when time to subtract is greater than the total spent time' do
+      it 'does not modify the total time spent' do
+        issuable.update_attributes!(spend_time: { duration: 7200, user: user })
+
+        post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", user),
+             duration: '-1w'
+
+        expect(response).to have_http_status(400)
+        expect(json_response['message']['time_spent'].first).to match(/exceeds the total time spent/)
+      end
+    end
+  end
+
+  describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/reset_spent_time" do
+    context 'with an unauthorized user' do
+      subject { post(v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_spent_time", non_member)) }
+
+      it_behaves_like 'an unauthorized API user'
+    end
+
+    it "resets spent time for #{issuable_name}" do
+      post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_spent_time", user)
+
+      expect(response).to have_http_status(200)
+      expect(json_response['total_time_spent']).to eq(0)
+    end
+  end
+
+  describe "GET /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/time_stats" do
+    it "returns the time stats for #{issuable_name}" do
+      issuable.update_attributes!(spend_time: { duration: 1800, user: user },
+                                  time_estimate: 3600)
+
+      get v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_stats", user)
+
+      expect(response).to have_http_status(200)
+      expect(json_response['total_time_spent']).to eq(1800)
+      expect(json_response['time_estimate']).to eq(3600)
+    end
+  end
+end