Sort API endpoints and implement feedback

5 files changed, 41 insertions, 35 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 374bb153c56..2dd9f148f05 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -48,6 +48,7 @@ v 8.9.0 (unreleased)
   - Upgrade to jQuery 2
   - Adds selected branch name to the dropdown toggle
   - Add API endpoint for Sidekiq Metrics !4653
+  - Refactoring Award Emoji with API support for Issues and MergeRequests
   - Use Knapsack to evenly distribute tests across multiple nodes
   - Add `sha` parameter to MR merge API, to ensure only reviewed changes are merged
   - Don't allow MRs to be merged when commits were added since the last review / page load
diff --git a/lib/api/api.rb b/lib/api/api.rb
index 7944c80cf7a..ef23c4d5de0 100644
--- a/lib/api/api.rb
+++ b/lib/api/api.rb
@@ -26,40 +26,41 @@ module API
     # Ensure the namespace is right, otherwise we might load Grape::API::Helpers
     helpers ::API::Helpers
 
-    mount ::API::Groups
+    # Sort these alphabetically
+    mount ::API::AwardEmoji
+    mount ::API::Branches
+    mount ::API::Builds
+    mount ::API::CommitStatuses
+    mount ::API::Commits
+    mount ::API::DeployKeys
+    mount ::API::Files
+    mount ::API::Gitignores
     mount ::API::GroupMembers
-    mount ::API::Users
-    mount ::API::Projects
-    mount ::API::Repositories
+    mount ::API::Groups
+    mount ::API::Internal
     mount ::API::Issues
-    mount ::API::Milestones
-    mount ::API::Session
+    mount ::API::Keys
+    mount ::API::Labels
+    mount ::API::Licenses
     mount ::API::MergeRequests
+    mount ::API::Milestones
+    mount ::API::Namespaces
     mount ::API::Notes
-    mount ::API::AwardEmoji
-    mount ::API::Internal
-    mount ::API::SystemHooks
-    mount ::API::ProjectSnippets
-    mount ::API::ProjectMembers
-    mount ::API::DeployKeys
     mount ::API::ProjectHooks
+    mount ::API::ProjectMembers
+    mount ::API::ProjectSnippets
+    mount ::API::Projects
+    mount ::API::Repositories
+    mount ::API::Runners
     mount ::API::Services
-    mount ::API::Files
-    mount ::API::Commits
-    mount ::API::CommitStatuses
-    mount ::API::Namespaces
-    mount ::API::Branches
-    mount ::API::Labels
+    mount ::API::Session
     mount ::API::Settings
-    mount ::API::Keys
+    mount ::API::SidekiqMetrics
+    mount ::API::Subscriptions
+    mount ::API::SystemHooks
     mount ::API::Tags
     mount ::API::Triggers
-    mount ::API::Builds
+    mount ::API::Users
     mount ::API::Variables
-    mount ::API::Runners
-    mount ::API::Licenses
-    mount ::API::Subscriptions
-    mount ::API::Gitignores
-    mount ::API::SidekiqMetrics
   end
 end
diff --git a/lib/api/award_emoji.rb b/lib/api/award_emoji.rb
index 26b30d30163..a7949b9e11d 100644
--- a/lib/api/award_emoji.rb
+++ b/lib/api/award_emoji.rb
@@ -17,9 +17,9 @@ module API
         # Example Request:
         #   GET /projects/:id/issues/:awardable_id/award_emoji
         get ":id/#{awardable_string}/:#{awardable_id_string}/award_emoji" do
-          awardable = user_project.send(awardable_string.to_sym).find(params[awardable_id_string.to_sym])
+          awardable = user_project.send(awardable_string.to_sym).find(params[awardable_id_string])
 
-          if can?(current_user, awardable_read_ability_name(awardable), awardable)
+          if can_read_awardable?(awardable)
             awards = paginate(awardable.award_emoji)
             present awards, with: Entities::AwardEmoji
           else
@@ -38,7 +38,7 @@ module API
         get ":id/#{awardable_string}/:#{awardable_id_string}/award_emoji/:award_id" do
           awardable = user_project.send(awardable_string.to_sym).find(params[awardable_id_string.to_sym])
 
-          if can?(current_user, awardable_read_ability_name(awardable), awardable)
+          if can_read_awardable?(awardable)
             present awardable.award_emoji.find(params[:award_id]), with: Entities::AwardEmoji
           else
             not_found!("Award Emoji")
@@ -49,16 +49,15 @@ module API
         #
         # Parameters:
         #   id (required) - The ID of a project
-        #   noteable_id (required) - The ID of an issue or snippet
+        #   awardable_id (required) - The ID of an issue or mr
         #   name (required) - The name of a award_emoji (without colons)
         # Example Request:
-        #   POST /projects/:id/issues/:noteable_id/notes
-        #   POST /projects/:id/snippets/:noteable_id/notes
+        #   POST /projects/:id/issues/:awardable_id/notes
         post ":id/#{awardable_string}/:#{awardable_id_string}/award_emoji" do
           required_attributes! [:name]
 
           awardable = user_project.send(awardable_string.to_sym).find(params[awardable_id_string.to_sym])
-          not_found!('Award Emoji') unless can?(current_user, awardable_read_ability_name(awardable), awardable)
+          not_found!('Award Emoji') unless can_read_awardable?(awardable)
 
           award = awardable.award_emoji.new(name: params[:name], user: current_user)
 
@@ -90,7 +89,12 @@ module API
     end
     helpers do
       def awardable_read_ability_name(awardable)
-        "read_#{awardable.class.to_s.underscore.downcase}".to_sym
+      end
+
+      def can_read_awardable?(awardable)
+        ability = "read_#{awardable.class.to_s.underscore}".to_sym
+
+        can?(current_user, ability, awardable)
       end
     end
   end
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index aa0b9ca3957..4c43257c48a 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -1,6 +1,6 @@
 module API
   # Issues API
-  class Issues < Grape::API    
+  class Issues < Grape::API
     before { authenticate! }
 
     helpers ::Gitlab::AkismetHelper
diff --git a/lib/api/notes.rb b/lib/api/notes.rb
index d4fcfd3d4d3..8bfa998dc53 100644
--- a/lib/api/notes.rb
+++ b/lib/api/notes.rb
@@ -144,7 +144,7 @@ module API
 
     helpers do
       def noteable_read_ability_name(noteable)
-        "read_#{noteable.class.to_s.underscore.downcase}".to_sym
+        "read_#{noteable.class.to_s.underscore}".to_sym
       end
     end
   end