Merge branch 'ajk-globalid-award-emojis' into 'master'

[GraphQL] Award Emojis: simplify authorization check

See merge request gitlab-org/gitlab!36109

app/graphql/mutations/award_emojis/add.rb

@@ -8,8 +8,6 @@ module Mutations
       def resolve(args)
         awardable = authorized_find!(id: args[:awardable_id])
 
-        check_object_is_awardable!(awardable)
-
         service = ::AwardEmojis::AddService.new(awardable, args[:name], current_user).execute
 
         {

app/graphql/mutations/award_emojis/base.rb

@@ -3,6 +3,10 @@
 module Mutations
   module AwardEmojis
     class Base < BaseMutation
+      include ::Mutations::FindsByGid
+
+      NOT_EMOJI_AWARDABLE = 'You cannot award emoji to this resource.'
+
       authorize :award_emoji
 
       argument :awardable_id,
@@ -22,20 +26,15 @@ module Mutations
 
       private
 
-      def find_object(id:)
-        # TODO: remove this line when the compatibility layer is removed
+      # TODO: remove this method when the compatibility layer is removed
       # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
-        id = ::Types::GlobalIDType[::Awardable].coerce_isolated_input(id)
-        GitlabSchema.find_by_gid(id)
+      def find_object(id:)
+        super(id: ::Types::GlobalIDType[::Awardable].coerce_isolated_input(id))
       end
 
-      # Called by mutations methods after performing an authorization check
-      # of an awardable object.
-      def check_object_is_awardable!(object)
-        unless object.is_a?(Awardable) && object.emoji_awardable?
-          raise Gitlab::Graphql::Errors::ResourceNotAvailable,
-                'Cannot award emoji to this resource'
-        end
+      def authorize!(object)
+        super
+        raise_resource_not_available_error!(NOT_EMOJI_AWARDABLE) unless object.emoji_awardable?
       end
     end
   end

app/graphql/mutations/award_emojis/remove.rb

@@ -8,8 +8,6 @@ module Mutations
       def resolve(args)
         awardable = authorized_find!(id: args[:awardable_id])
 
-        check_object_is_awardable!(awardable)
-
         service = ::AwardEmojis::DestroyService.new(awardable, args[:name], current_user).execute
 
         {

app/graphql/mutations/award_emojis/toggle.rb

@@ -12,8 +12,6 @@ module Mutations
       def resolve(args)
         awardable = authorized_find!(id: args[:awardable_id])
 
-        check_object_is_awardable!(awardable)
-
         service = ::AwardEmojis::ToggleService.new(awardable, args[:name], current_user).execute
 
         toggled_on = awardable.awarded_emoji?(args[:name], current_user)

app/graphql/mutations/concerns/mutations/finds_by_gid.rb

+# frozen_string_literal: true
+
+module Mutations
+  module FindsByGid
+    def find_object(id:)
+      GitlabSchema.find_by_gid(id)
+    end
+  end
+end

lib/gitlab/graphql/authorize/authorize_resource.rb

@@ -62,8 +62,8 @@ module Gitlab
           end
         end
 
-        def raise_resource_not_available_error!
-          raise Gitlab::Graphql::Errors::ResourceNotAvailable, RESOURCE_ACCESS_ERROR
+        def raise_resource_not_available_error!(msg = RESOURCE_ACCESS_ERROR)
+          raise Gitlab::Graphql::Errors::ResourceNotAvailable, msg
         end
       end
     end

spec/graphql/mutations/concerns/mutations/finds_by_gid_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Mutations::FindsByGid do
+  include GraphqlHelpers
+
+  let(:mutation_class) do
+    Class.new(Mutations::BaseMutation) do
+      authorize :read_user
+
+      include Mutations::FindsByGid
+    end
+  end
+
+  let(:query) { double('Query', schema: GitlabSchema) }
+  let(:context) { GraphQL::Query::Context.new(query: query, object: nil, values: { current_user: user }) }
+  let(:user) { create(:user) }
+  let(:gid) { user.to_global_id }
+
+  subject(:mutation) { mutation_class.new(object: nil, context: context, field: nil) }
+
+  it 'calls GitlabSchema.find_by_gid to find objects during authorized_find!' do
+    expect(mutation.authorized_find!(id: gid)).to eq(user)
+  end
+end

spec/requests/api/graphql/mutations/award_emojis/add_spec.rb

@@ -56,10 +56,10 @@ RSpec.describe 'Adding an AwardEmoji' do
       it_behaves_like 'a mutation that does not create an AwardEmoji'
 
       it_behaves_like 'a mutation that returns top-level errors',
-                      errors: ['Cannot award emoji to this resource']
+        errors: ['You cannot award emoji to this resource.']
     end
 
-    context 'when the given awardable an Awardable' do
+    context 'when the given awardable is an Awardable' do
       it 'creates an emoji' do
         expect do
           post_graphql_mutation(mutation, current_user: current_user)

spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb

@@ -55,7 +55,7 @@ RSpec.describe 'Toggling an AwardEmoji' do
       it_behaves_like 'a mutation that does not create or destroy an AwardEmoji'
 
       it_behaves_like 'a mutation that returns top-level errors',
-                      errors: ['Cannot award emoji to this resource']
+        errors: ['You cannot award emoji to this resource.']
     end
 
     context 'when the given awardable is an Awardable' do

spec/support/shared_examples/requests/graphql_shared_examples.rb

@@ -9,3 +9,8 @@ RSpec.shared_examples 'a working graphql query' do
     expect(json_response.keys).to include('data')
   end
 end
+
+RSpec.shared_examples 'a mutation on an unauthorized resource' do
+  it_behaves_like 'a mutation that returns top-level errors',
+                      errors: [::Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR]
+end