Merge branch '28251-mr-and-issue-iids-for-api-v4' into 'master'

API routes referencing a specific issue should use the issue `iid`

Closes #28251

See merge request !9530

11 files changed, 277 insertions, 65 deletions

diff --git a/lib/api/api.rb b/lib/api/api.rb
index 8dbe8875fe8..1bf20f76ad6 100644
--- a/lib/api/api.rb
+++ b/lib/api/api.rb
@@ -5,6 +5,8 @@ module API
     version %w(v3 v4), using: :path
 
     version 'v3', using: :path do
+      helpers ::API::V3::Helpers
+
       mount ::API::V3::AwardEmoji
       mount ::API::V3::Boards
       mount ::API::V3::Branches
diff --git a/lib/api/award_emoji.rb b/lib/api/award_emoji.rb
index 07a1bcdbe18..f9e0c2c4e16 100644
--- a/lib/api/award_emoji.rb
+++ b/lib/api/award_emoji.rb
@@ -3,12 +3,16 @@ module API
     include PaginationParams
 
     before { authenticate! }
-    AWARDABLES = %w[issue merge_request snippet].freeze
+    AWARDABLES = [
+      { type: 'issue', find_by: :iid },
+      { type: 'merge_request', find_by: :iid },
+      { type: 'snippet', find_by: :id }
+    ].freeze
 
     resource :projects do
-      AWARDABLES.each do |awardable_type|
-        awardable_string = awardable_type.pluralize
-        awardable_id_string = "#{awardable_type}_id"
+      AWARDABLES.each do |awardable_params|
+        awardable_string = awardable_params[:type].pluralize
+        awardable_id_string = "#{awardable_params[:type]}_#{awardable_params[:find_by]}"
 
         params do
           requires :id, type: String, desc: 'The ID of a project'
@@ -104,10 +108,10 @@ module API
               note_id = params.delete(:note_id)
 
               awardable.notes.find(note_id)
-            elsif params.include?(:issue_id)
-              user_project.issues.find(params[:issue_id])
-            elsif params.include?(:merge_request_id)
-              user_project.merge_requests.find(params[:merge_request_id])
+            elsif params.include?(:issue_iid)
+              user_project.issues.find_by!(iid: params[:issue_iid])
+            elsif params.include?(:merge_request_iid)
+              user_project.merge_requests.find_by!(iid: params[:merge_request_iid])
             else
               user_project.snippets.find(params[:snippet_id])
             end
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index f325f0a3050..a9b364da9e1 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -82,16 +82,16 @@ module API
       label || not_found!('Label')
     end
 
-    def find_project_issue(id)
-      IssuesFinder.new(current_user, project_id: user_project.id).find(id)
+    def find_project_issue(iid)
+      IssuesFinder.new(current_user, project_id: user_project.id).find_by!(iid: iid)
     end
 
-    def find_project_merge_request(id)
-      MergeRequestsFinder.new(current_user, project_id: user_project.id).find(id)
+    def find_project_merge_request(iid)
+      MergeRequestsFinder.new(current_user, project_id: user_project.id).find_by!(iid: iid)
     end
 
-    def find_merge_request_with_access(id, access_level = :read_merge_request)
-      merge_request = user_project.merge_requests.find(id)
+    def find_merge_request_with_access(iid, access_level = :read_merge_request)
+      merge_request = user_project.merge_requests.find_by!(iid: iid)
       authorize! access_level, merge_request
       merge_request
     end
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index bda74069ad5..4a9f2b26fb2 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -102,10 +102,10 @@ module API
         success Entities::Issue
       end
       params do
-        requires :issue_id, type: Integer, desc: 'The ID of a project issue'
+        requires :issue_iid, type: Integer, desc: 'The internal ID of a project issue'
       end
-      get ":id/issues/:issue_id" do
-        issue = find_project_issue(params[:issue_id])
+      get ":id/issues/:issue_iid" do
+        issue = find_project_issue(params[:issue_iid])
         present issue, with: Entities::Issue, current_user: current_user, project: user_project
       end
 
@@ -152,7 +152,7 @@ module API
         success Entities::Issue
       end
       params do
-        requires :issue_id, type: Integer, desc: 'The ID of a project issue'
+        requires :issue_iid, type: Integer, desc: 'The internal ID of a project issue'
         optional :title, type: String, desc: 'The title of an issue'
         optional :updated_at, type: DateTime,
                               desc: 'Date time when the issue was updated. Available only for admins and project owners.'
@@ -161,8 +161,8 @@ module API
         at_least_one_of :title, :description, :assignee_id, :milestone_id,
                         :labels, :created_at, :due_date, :confidential, :state_event
       end
-      put ':id/issues/:issue_id' do
-        issue = user_project.issues.find(params.delete(:issue_id))
+      put ':id/issues/:issue_iid' do
+        issue = user_project.issues.find_by!(iid: params.delete(:issue_iid))
         authorize! :update_issue, issue
 
         # Setting created_at time only allowed for admins and project owners
@@ -189,11 +189,11 @@ module API
         success Entities::Issue
       end
       params do
-        requires :issue_id, type: Integer, desc: 'The ID of a project issue'
+        requires :issue_iid, type: Integer, desc: 'The internal ID of a project issue'
         requires :to_project_id, type: Integer, desc: 'The ID of the new project'
       end
-      post ':id/issues/:issue_id/move' do
-        issue = user_project.issues.find_by(id: params[:issue_id])
+      post ':id/issues/:issue_iid/move' do
+        issue = user_project.issues.find_by(iid: params[:issue_iid])
         not_found!('Issue') unless issue
 
         new_project = Project.find_by(id: params[:to_project_id])
@@ -209,10 +209,10 @@ module API
 
       desc 'Delete a project issue'
       params do
-        requires :issue_id, type: Integer, desc: 'The ID of a project issue'
+        requires :issue_iid, type: Integer, desc: 'The internal ID of a project issue'
       end
-      delete ":id/issues/:issue_id" do
-        issue = user_project.issues.find_by(id: params[:issue_id])
+      delete ":id/issues/:issue_iid" do
+        issue = user_project.issues.find_by(iid: params[:issue_iid])
         not_found!('Issue') unless issue
 
         authorize!(:destroy_issue, issue)
diff --git a/lib/api/merge_request_diffs.rb b/lib/api/merge_request_diffs.rb
index 4901a7cfea6..a59e39cca26 100644
--- a/lib/api/merge_request_diffs.rb
+++ b/lib/api/merge_request_diffs.rb
@@ -13,11 +13,11 @@ module API
 
       params do
         requires :id, type: String, desc: 'The ID of a project'
-        requires :merge_request_id, type: Integer, desc: 'The ID of a merge request'
+        requires :merge_request_iid, type: Integer, desc: 'The IID of a merge request'
         use :pagination
       end
-      get ":id/merge_requests/:merge_request_id/versions" do
-        merge_request = find_merge_request_with_access(params[:merge_request_id])
+      get ":id/merge_requests/:merge_request_iid/versions" do
+        merge_request = find_merge_request_with_access(params[:merge_request_iid])
 
         present paginate(merge_request.merge_request_diffs), with: Entities::MergeRequestDiff
       end
@@ -29,12 +29,12 @@ module API
 
       params do
         requires :id, type: String, desc: 'The ID of a project'
-        requires :merge_request_id, type: Integer, desc: 'The ID of a merge request'
+        requires :merge_request_iid, type: Integer, desc: 'The IID of a merge request'
         requires :version_id, type: Integer, desc: 'The ID of a merge request diff version'
       end
 
-      get ":id/merge_requests/:merge_request_id/versions/:version_id" do
-        merge_request = find_merge_request_with_access(params[:merge_request_id])
+      get ":id/merge_requests/:merge_request_iid/versions/:version_id" do
+        merge_request = find_merge_request_with_access(params[:merge_request_iid])
 
         present merge_request.merge_request_diffs.find(params[:version_id]), with: Entities::MergeRequestDiffFull
       end
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 6fc33a7a54a..7a03955a045 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -101,23 +101,23 @@ module API
 
       desc 'Delete a merge request'
       params do
-        requires :merge_request_id, type: Integer, desc: 'The ID of a merge request'
+        requires :merge_request_iid, type: Integer, desc: 'The IID of a merge request'
       end
-      delete ":id/merge_requests/:merge_request_id" do
-        merge_request = find_project_merge_request(params[:merge_request_id])
+      delete ":id/merge_requests/:merge_request_iid" do
+        merge_request = find_project_merge_request(params[:merge_request_iid])
 
         authorize!(:destroy_merge_request, merge_request)
         merge_request.destroy
       end
 
       params do
-        requires :merge_request_id, type: Integer, desc: 'The ID of a merge request'
+        requires :merge_request_iid, type: Integer, desc: 'The IID of a merge request'
       end
       desc 'Get a single merge request' do
         success Entities::MergeRequest
       end
-      get ':id/merge_requests/:merge_request_id' do
-        merge_request = find_merge_request_with_access(params[:merge_request_id])
+      get ':id/merge_requests/:merge_request_iid' do
+        merge_request = find_merge_request_with_access(params[:merge_request_iid])
 
         present merge_request, with: Entities::MergeRequest, current_user: current_user, project: user_project
       end
@@ -125,8 +125,8 @@ module API
       desc 'Get the commits of a merge request' do
         success Entities::RepoCommit
       end
-      get ':id/merge_requests/:merge_request_id/commits' do
-        merge_request = find_merge_request_with_access(params[:merge_request_id])
+      get ':id/merge_requests/:merge_request_iid/commits' do
+        merge_request = find_merge_request_with_access(params[:merge_request_iid])
         commits = ::Kaminari.paginate_array(merge_request.commits)
 
         present paginate(commits), with: Entities::RepoCommit
@@ -135,8 +135,8 @@ module API
       desc 'Show the merge request changes' do
         success Entities::MergeRequestChanges
       end
-      get ':id/merge_requests/:merge_request_id/changes' do
-        merge_request = find_merge_request_with_access(params[:merge_request_id])
+      get ':id/merge_requests/:merge_request_iid/changes' do
+        merge_request = find_merge_request_with_access(params[:merge_request_iid])
 
         present merge_request, with: Entities::MergeRequestChanges, current_user: current_user
       end
@@ -154,8 +154,8 @@ module API
                         :milestone_id, :labels, :state_event,
                         :remove_source_branch
       end
-      put ':id/merge_requests/:merge_request_id' do
-        merge_request = find_merge_request_with_access(params.delete(:merge_request_id), :update_merge_request)
+      put ':id/merge_requests/:merge_request_iid' do
+        merge_request = find_merge_request_with_access(params.delete(:merge_request_iid), :update_merge_request)
 
         mr_params = declared_params(include_missing: false)
         mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch) if mr_params[:remove_source_branch].present?
@@ -180,8 +180,8 @@ module API
                                                 desc: 'When true, this merge request will be merged when the pipeline succeeds'
         optional :sha, type: String, desc: 'When present, must have the HEAD SHA of the source branch'
       end
-      put ':id/merge_requests/:merge_request_id/merge' do
-        merge_request = find_project_merge_request(params[:merge_request_id])
+      put ':id/merge_requests/:merge_request_iid/merge' do
+        merge_request = find_project_merge_request(params[:merge_request_iid])
 
         # Merge request can not be merged
         # because user dont have permissions to push into target branch
@@ -216,8 +216,8 @@ module API
       desc 'Cancel merge if "Merge When Pipeline Succeeds" is enabled' do
         success Entities::MergeRequest
       end
-      post ':id/merge_requests/:merge_request_id/cancel_merge_when_pipeline_succeeds' do
-        merge_request = find_project_merge_request(params[:merge_request_id])
+      post ':id/merge_requests/:merge_request_iid/cancel_merge_when_pipeline_succeeds' do
+        merge_request = find_project_merge_request(params[:merge_request_iid])
 
         unauthorized! unless merge_request.can_cancel_merge_when_pipeline_succeeds?(current_user)
 
@@ -232,8 +232,8 @@ module API
       params do
         use :pagination
       end
-      get ':id/merge_requests/:merge_request_id/comments' do
-        merge_request = find_merge_request_with_access(params[:merge_request_id])
+      get ':id/merge_requests/:merge_request_iid/comments' do
+        merge_request = find_merge_request_with_access(params[:merge_request_iid])
         present paginate(merge_request.notes.fresh), with: Entities::MRNote
       end
 
@@ -243,8 +243,8 @@ module API
       params do
         requires :note, type: String, desc: 'The text of the comment'
       end
-      post ':id/merge_requests/:merge_request_id/comments' do
-        merge_request = find_merge_request_with_access(params[:merge_request_id], :create_note)
+      post ':id/merge_requests/:merge_request_iid/comments' do
+        merge_request = find_merge_request_with_access(params[:merge_request_iid], :create_note)
 
         opts = {
           note: params[:note],
@@ -267,8 +267,8 @@ module API
       params do
         use :pagination
       end
-      get ':id/merge_requests/:merge_request_id/closes_issues' do
-        merge_request = find_merge_request_with_access(params[:merge_request_id])
+      get ':id/merge_requests/:merge_request_iid/closes_issues' do
+        merge_request = find_merge_request_with_access(params[:merge_request_iid])
         issues = ::Kaminari.paginate_array(merge_request.closes_issues(current_user))
         present paginate(issues), with: issue_entity(user_project), current_user: current_user
       end
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/todos.rb b/lib/api/todos.rb
index e59030428da..d9b8837a5bb 100644
--- a/lib/api/todos.rb
+++ b/lib/api/todos.rb
@@ -5,8 +5,8 @@ module API
     before { authenticate! }
 
     ISSUABLE_TYPES = {
-      'merge_requests' => ->(id) { find_merge_request_with_access(id) },
-      'issues' => ->(id) { find_project_issue(id) }
+      'merge_requests' => ->(iid) { find_merge_request_with_access(iid) },
+      'issues' => ->(iid) { find_project_issue(iid) }
     }.freeze
 
     params do
@@ -14,13 +14,13 @@ module API
     end
     resource :projects do
       ISSUABLE_TYPES.each do |type, finder|
-        type_id_str = "#{type.singularize}_id".to_sym
+        type_id_str = "#{type.singularize}_iid".to_sym
 
         desc 'Create a todo on an issuable' do
           success Entities::Todo
         end
         params do
-          requires type_id_str, type: Integer, desc: 'The ID of an issuable'
+          requires type_id_str, type: Integer, desc: 'The IID of an issuable'
         end
         post ":id/#{type}/:#{type_id_str}/todo" do
           issuable = instance_exec(params[type_id_str], &finder)
diff --git a/lib/api/v3/award_emoji.rb b/lib/api/v3/award_emoji.rb
index 1e35283631f..cf9e1551f60 100644
--- a/lib/api/v3/award_emoji.rb
+++ b/lib/api/v3/award_emoji.rb
@@ -16,11 +16,64 @@ module API
             requires :"#{awardable_id_string}", type: Integer, desc: "The ID of an Issue, Merge Request or Snippet"
           end
 
-          [":id/#{awardable_string}/:#{awardable_id_string}/award_emoji",
-           ":id/#{awardable_string}/:#{awardable_id_string}/notes/:note_id/award_emoji"].each do |endpoint|
+          [
+            ":id/#{awardable_string}/:#{awardable_id_string}/award_emoji",
+            ":id/#{awardable_string}/:#{awardable_id_string}/notes/:note_id/award_emoji"
+          ].each do |endpoint|
+
+            desc 'Get a list of project +awardable+ award emoji' do
+              detail 'This feature was introduced in 8.9'
+              success Entities::AwardEmoji
+            end
+            params do
+              use :pagination
+            end
+            get endpoint do
+              if can_read_awardable?
+                awards = awardable.award_emoji
+                present paginate(awards), with: Entities::AwardEmoji
+              else
+                not_found!("Award Emoji")
+              end
+            end
+
+            desc 'Get a specific award emoji' do
+              detail 'This feature was introduced in 8.9'
+              success Entities::AwardEmoji
+            end
+            params do
+              requires :award_id, type: Integer, desc: 'The ID of the award'
+            end
+            get "#{endpoint}/:award_id" do
+              if can_read_awardable?
+                present awardable.award_emoji.find(params[:award_id]), with: Entities::AwardEmoji
+              else
+                not_found!("Award Emoji")
+              end
+            end
+
+            desc 'Award a new Emoji' do
+              detail 'This feature was introduced in 8.9'
+              success Entities::AwardEmoji
+            end
+            params do
+              requires :name, type: String, desc: 'The name of a award_emoji (without colons)'
+            end
+            post endpoint do
+              not_found!('Award Emoji') unless can_read_awardable? && can_award_awardable?
+
+              award = awardable.create_award_emoji(params[:name], current_user)
+
+              if award.persisted?
+                present award, with: Entities::AwardEmoji
+              else
+                not_found!("Award Emoji #{award.errors.messages}")
+              end
+            end
+
             desc 'Delete a +awardables+ award emoji' do
               detail 'This feature was introduced in 8.9'
-              success ::API::Entities::AwardEmoji
+              success Entities::AwardEmoji
             end
             params do
               requires :award_id, type: Integer, desc: 'The ID of an award emoji'
@@ -30,13 +83,22 @@ module API
 
               unauthorized! unless award.user == current_user || current_user.admin?
 
-              present award.destroy, with: ::API::Entities::AwardEmoji
+              award.destroy
+              present award, with: Entities::AwardEmoji
             end
           end
         end
       end
 
       helpers do
+        def can_read_awardable?
+          can?(current_user, read_ability(awardable), awardable)
+        end
+
+        def can_award_awardable?
+          awardable.user_can_award?(current_user, params[:name])
+        end
+
         def awardable
           @awardable ||=
             begin
@@ -53,6 +115,15 @@ module API
               end
             end
         end
+
+        def read_ability(awardable)
+          case awardable
+          when Note
+            read_ability(awardable.noteable)
+          else
+            :"read_#{awardable.class.to_s.underscore}"
+          end
+        end
       end
     end
   end
diff --git a/lib/api/v3/helpers.rb b/lib/api/v3/helpers.rb
new file mode 100644
index 00000000000..0f234d4cdad
--- /dev/null
+++ b/lib/api/v3/helpers.rb
@@ -0,0 +1,19 @@
+module API
+  module V3
+    module Helpers
+      def find_project_issue(id)
+        IssuesFinder.new(current_user, project_id: user_project.id).find(id)
+      end
+
+      def find_project_merge_request(id)
+        MergeRequestsFinder.new(current_user, project_id: user_project.id).find(id)
+      end
+
+      def find_merge_request_with_access(id, access_level = :read_merge_request)
+        merge_request = user_project.merge_requests.find(id)
+        authorize! access_level, merge_request
+        merge_request
+      end
+    end
+  end
+end
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