Merge branch 'ajk-add-mr-interaction-for-assignees' into 'master'

Change type of merge request assignees in GraphQL

See merge request gitlab-org/gitlab!59770

app/graphql/types/merge_request_type.rb

@@ -130,7 +130,10 @@ module Types
 
     field :milestone, Types::MilestoneType, null: true,
           description: 'The milestone of the merge request.'
-    field :assignees, Types::UserType.connection_type, null: true, complexity: 5,
+    field :assignees,
+          type: Types::MergeRequests::AssigneeType.connection_type,
+          null: true,
+          complexity: 5,
           description: 'Assignees of the merge request.'
     field :reviewers,
           type: Types::MergeRequests::ReviewerType.connection_type,

app/graphql/types/merge_requests/assignee_type.rb

+# frozen_string_literal: true
+
+module Types
+  module MergeRequests
+    class AssigneeType < ::Types::UserType
+      include FindClosest
+      include ::Types::MergeRequests::InteractsWithMergeRequest
+
+      graphql_name 'MergeRequestAssignee'
+      description 'A user assigned to a merge request.'
+      authorize :read_user
+    end
+  end
+end

app/graphql/types/merge_requests/interacts_with_merge_request.rb

+# frozen_string_literal: true
+
+module Types
+  module MergeRequests
+    module InteractsWithMergeRequest
+      extend ActiveSupport::Concern
+
+      included do
+        field :merge_request_interaction,
+              type: ::Types::UserMergeRequestInteractionType,
+              null: true,
+              extras: [:parent],
+              description: "Details of this user's interactions with the merge request."
+      end
+
+      def merge_request_interaction(parent:)
+        merge_request = closest_parent(::Types::MergeRequestType, parent)
+        return unless merge_request
+
+        Users::MergeRequestInteraction.new(user: object, merge_request: merge_request)
+      end
+    end
+  end
+end

app/graphql/types/merge_requests/reviewer_type.rb

@@ -4,23 +4,11 @@ module Types
   module MergeRequests
     class ReviewerType < ::Types::UserType
       include FindClosest
+      include ::Types::MergeRequests::InteractsWithMergeRequest
 
       graphql_name 'MergeRequestReviewer'
-      description 'A user from whom a merge request review has been requested.'
+      description 'A user assigned to a merge request as a reviewer.'
       authorize :read_user
-
-      field :merge_request_interaction,
-            type: ::Types::UserMergeRequestInteractionType,
-            null: true,
-            extras: [:parent],
-            description: "Details of this user's interactions with the merge request."
-
-      def merge_request_interaction(parent:)
-        merge_request = closest_parent(::Types::MergeRequestType, parent)
-        return unless merge_request
-
-        Users::MergeRequestInteraction.new(user: object, merge_request: merge_request)
-      end
     end
   end
 end

app/graphql/types/user_interface.rb

+# frozen_string_literal: true
+
+module Types
+  module UserInterface
+    include Types::BaseInterface
+
+    graphql_name 'User'
+    description 'Representation of a GitLab user.'
+
+    field :user_permissions,
+          type: Types::PermissionTypes::User,
+          description: 'Permissions for the current user on the resource.',
+          null: false,
+          method: :itself
+
+    field :id,
+          type: GraphQL::ID_TYPE,
+          null: false,
+          description: 'ID of the user.'
+    field :bot,
+          type: GraphQL::BOOLEAN_TYPE,
+          null: false,
+          description: 'Indicates if the user is a bot.',
+          method: :bot?
+    field :username,
+          type: GraphQL::STRING_TYPE,
+          null: false,
+          description: 'Username of the user. Unique within this instance of GitLab.'
+    field :name,
+          type: GraphQL::STRING_TYPE,
+          null: false,
+          description: 'Human-readable name of the user.'
+    field :state,
+          type: Types::UserStateEnum,
+          null: false,
+          description: 'State of the user.'
+    field :email,
+          type: GraphQL::STRING_TYPE,
+          null: true,
+          description: 'User email.', method: :public_email,
+          deprecated: { reason: :renamed, replacement: 'User.publicEmail', milestone: '13.7' }
+    field :public_email,
+          type: GraphQL::STRING_TYPE,
+          null: true,
+          description: "User's public email."
+    field :avatar_url,
+          type: GraphQL::STRING_TYPE,
+          null: true,
+          description: "URL of the user's avatar."
+    field :web_url,
+          type: GraphQL::STRING_TYPE,
+          null: false,
+          description: 'Web URL of the user.'
+    field :web_path,
+          type: GraphQL::STRING_TYPE,
+          null: false,
+          description: 'Web path of the user.'
+    field :todos,
+          resolver: Resolvers::TodoResolver,
+          description: 'To-do items of the user.'
+    field :group_memberships,
+          type: Types::GroupMemberType.connection_type,
+          null: true,
+          description: 'Group memberships of the user.'
+    field :group_count,
+          resolver: Resolvers::Users::GroupCountResolver,
+          description: 'Group count for the user.',
+          feature_flag: :user_group_counts
+    field :status,
+          type: Types::UserStatusType,
+          null: true,
+          description: 'User status.'
+    field :location,
+          type: ::GraphQL::STRING_TYPE,
+          null: true,
+          description: 'The location of the user.'
+    field :project_memberships,
+          type: Types::ProjectMemberType.connection_type,
+          null: true,
+          description: 'Project memberships of the user.'
+    field :starred_projects,
+          description: 'Projects starred by the user.',
+          resolver: Resolvers::UserStarredProjectsResolver
+
+    # Merge request field: MRs can be authored, assigned, or assigned-for-review:
+    field :authored_merge_requests,
+          resolver: Resolvers::AuthoredMergeRequestsResolver,
+          description: 'Merge requests authored by the user.'
+    field :assigned_merge_requests,
+          resolver: Resolvers::AssignedMergeRequestsResolver,
+          description: 'Merge requests assigned to the user.'
+    field :review_requested_merge_requests,
+          resolver: Resolvers::ReviewRequestedMergeRequestsResolver,
+          description: 'Merge requests assigned to the user for review.'
+
+    field :snippets,
+          description: 'Snippets authored by the user.',
+          resolver: Resolvers::Users::SnippetsResolver
+    field :callouts,
+          Types::UserCalloutType.connection_type,
+          null: true,
+          description: 'User callouts that belong to the user.'
+
+    definition_methods do
+      def resolve_type(object, context)
+        # in the absense of other information, we cannot tell - just default to
+        # the core user type.
+        ::Types::UserType
+      end
+    end
+  end
+end

app/graphql/types/user_type.rb

 # frozen_string_literal: true
 
 module Types
-  class UserType < BaseObject
-    graphql_name 'User'
-    description 'Representation of a GitLab user.'
+  class UserType < ::Types::BaseObject
+    graphql_name 'UserCore'
+    description 'Core represention of a GitLab user.'
+    implements ::Types::UserInterface
 
     authorize :read_user
 
     present_using UserPresenter
-
-    expose_permissions Types::PermissionTypes::User
-
-    field :id,
-          type: GraphQL::ID_TYPE,
-          null: false,
-          description: 'ID of the user.'
-    field :bot,
-          type: GraphQL::BOOLEAN_TYPE,
-          null: false,
-          description: 'Indicates if the user is a bot.',
-          method: :bot?
-    field :username,
-          type: GraphQL::STRING_TYPE,
-          null: false,
-          description: 'Username of the user. Unique within this instance of GitLab.'
-    field :name,
-          type: GraphQL::STRING_TYPE,
-          null: false,
-          description: 'Human-readable name of the user.'
-    field :state,
-          type: Types::UserStateEnum,
-          null: false,
-          description: 'State of the user.'
-    field :email,
-          type: GraphQL::STRING_TYPE,
-          null: true,
-          description: 'User email.', method: :public_email,
-          deprecated: { reason: :renamed, replacement: 'User.publicEmail', milestone: '13.7' }
-    field :public_email,
-          type: GraphQL::STRING_TYPE,
-          null: true,
-          description: "User's public email."
-    field :avatar_url,
-          type: GraphQL::STRING_TYPE,
-          null: true,
-          description: "URL of the user's avatar."
-    field :web_url,
-          type: GraphQL::STRING_TYPE,
-          null: false,
-          description: 'Web URL of the user.'
-    field :web_path,
-          type: GraphQL::STRING_TYPE,
-          null: false,
-          description: 'Web path of the user.'
-    field :todos,
-          resolver: Resolvers::TodoResolver,
-          description: 'To-do items of the user.'
-    field :group_memberships,
-          type: Types::GroupMemberType.connection_type,
-          null: true,
-          description: 'Group memberships of the user.'
-    field :group_count,
-          resolver: Resolvers::Users::GroupCountResolver,
-          description: 'Group count for the user.',
-          feature_flag: :user_group_counts
-    field :status,
-          type: Types::UserStatusType,
-          null: true,
-          description: 'User status.'
-    field :location,
-          type: ::GraphQL::STRING_TYPE,
-          null: true,
-          description: 'The location of the user.'
-    field :project_memberships,
-          type: Types::ProjectMemberType.connection_type,
-          null: true,
-          description: 'Project memberships of the user.'
-    field :starred_projects,
-          description: 'Projects starred by the user.',
-          resolver: Resolvers::UserStarredProjectsResolver
-
-    # Merge request field: MRs can be authored, assigned, or assigned-for-review:
-    field :authored_merge_requests,
-          resolver: Resolvers::AuthoredMergeRequestsResolver,
-          description: 'Merge requests authored by the user.'
-    field :assigned_merge_requests,
-          resolver: Resolvers::AssignedMergeRequestsResolver,
-          description: 'Merge requests assigned to the user.'
-    field :review_requested_merge_requests,
-          resolver: Resolvers::ReviewRequestedMergeRequestsResolver,
-          description: 'Merge requests assigned to the user for review.'
-
-    field :snippets,
-          description: 'Snippets authored by the user.',
-          resolver: Resolvers::Users::SnippetsResolver
-    field :callouts,
-          Types::UserCalloutType.connection_type,
-          null: true,
-          description: 'User callouts that belong to the user.'
   end
 end

changelogs/unreleased/ajk-add-mr-interaction-for-assignees.yml

+---
+title: Add merge request interaction details to MergeRequest.assignees
+merge_request: 59770
+author:
+type: changed

lib/gitlab/graphql/docs/templates/default.md.haml

@@ -86,8 +86,8 @@
   | `nodes` | `[item!]` | The items in the current page. |
 
   The precise type of `Edge` and `Item` depends on the kind of connection. A
-  [`UserConnection`](#userconnection) will have nodes that have the type
-  [`[User!]`](#user), and edges that have the type [`UserEdge`](#useredge).
+  [`ProjectConnection`](#projectconnection) will have nodes that have the type
+  [`[Project!]`](#project), and edges that have the type [`ProjectEdge`](#projectedge).
 
   ### Connection types
 

lib/gitlab/graphql/present/field_extension.rb

@@ -13,7 +13,8 @@ module Gitlab
           # inner Schema::Object#object. This depends on whether the field
           # has a @resolver_proc or not.
           if object.is_a?(::Types::BaseObject)
-            object.present(field.owner, attrs)
+            type = field.owner.kind.abstract? ? object.class : field.owner
+            object.present(type, attrs)
             yield(object, arguments)
           else
             # This is the legacy code-path, hit if the field has a @resolver_proc

spec/graphql/types/user_type_spec.rb

@@ -5,7 +5,11 @@ require 'spec_helper'
 RSpec.describe GitlabSchema.types['User'] do
   specify { expect(described_class.graphql_name).to eq('User') }
 
-  specify { expect(described_class).to require_graphql_authorizations(:read_user) }
+  specify do
+    runtime_type = described_class.resolve_type(build(:user), {})
+
+    expect(runtime_type).to require_graphql_authorizations(:read_user)
+  end
 
   it 'has the expected fields' do
     expected_fields = %w[

spec/lib/gitlab/graphql/present/field_extension_spec.rb

@@ -33,6 +33,38 @@ RSpec.describe Gitlab::Graphql::Present::FieldExtension do
     end
   end
 
+  context 'when the field is declared on an interface, and implemented by a presenter' do
+    let(:interface) do
+      Module.new do
+        include ::Types::BaseInterface
+
+        field :interface_field, GraphQL::STRING_TYPE, null: true
+      end
+    end
+
+    let(:implementation) do
+      type = fresh_object_type('Concrete')
+      type.present_using(concrete_impl)
+      type.implements(interface)
+      type
+    end
+
+    def concrete_impl
+      Class.new(base_presenter) do
+        def interface_field
+          'made of concrete'
+        end
+      end
+    end
+
+    it 'resolves the interface field using the implementation from the presenter' do
+      field = ::Types::BaseField.new(name: :interface_field, type: GraphQL::STRING_TYPE, null: true, owner: interface)
+      value = resolve_field(field, object, object_type: implementation)
+
+      expect(value).to eq 'made of concrete'
+    end
+  end
+
   describe 'interactions with inheritance' do
     def parent
       type = fresh_object_type('Parent')

spec/requests/api/graphql/project/merge_request_spec.rb

@@ -311,23 +311,23 @@ RSpec.describe 'getting merge request information nested in a project' do
     end
   end
 
-  context 'when requesting information about MR interactions' do
+  shared_examples 'when requesting information about MR interactions' do
     let_it_be(:user) { create(:user) }
 
     let(:selected_fields) { all_graphql_fields_for('UserMergeRequestInteraction') }
 
     let(:mr_fields) do
       query_nodes(
-        :reviewers,
+        field,
         query_graphql_field(:merge_request_interaction, nil, selected_fields)
       )
     end
 
     def interaction_data
-      graphql_data_at(:project, :merge_request, :reviewers, :nodes, :merge_request_interaction)
+      graphql_data_at(:project, :merge_request, field, :nodes, :merge_request_interaction)
     end
 
-    context 'when the user does not have interactions' do
+    context 'when the user is not assigned' do
       it 'returns null data' do
         post_graphql(query)
 
@@ -338,7 +338,7 @@ RSpec.describe 'getting merge request information nested in a project' do
     context 'when the user is a reviewer, but has not reviewed' do
       before do
         project.add_guest(user)
-        merge_request.merge_request_reviewers.create!(reviewer: user)
+        assign_user(user)
       end
 
       it 'returns falsey values' do
@@ -346,8 +346,8 @@ RSpec.describe 'getting merge request information nested in a project' do
 
         expect(interaction_data).to contain_exactly a_hash_including(
           'canMerge' => false,
-          'canUpdate' => false,
-          'reviewState' => 'UNREVIEWED',
+          'canUpdate' => can_update,
+          'reviewState' => unreviewed,
           'reviewed' => false,
           'approved' => false
         )
@@ -357,7 +357,9 @@ RSpec.describe 'getting merge request information nested in a project' do
     context 'when the user has interacted' do
       before do
         project.add_maintainer(user)
-        merge_request.merge_request_reviewers.create!(reviewer: user, state: 'reviewed')
+        assign_user(user)
+        r = merge_request.merge_request_reviewers.find_or_create_by!(reviewer: user)
+        r.update!(state: 'reviewed')
         merge_request.approved_by_users << user
       end
 
@@ -392,7 +394,10 @@ RSpec.describe 'getting merge request information nested in a project' do
         end
 
         it 'does not suffer from N+1' do
-          merge_request.merge_request_reviewers.create!(reviewer: user, state: 'reviewed')
+          assign_user(user)
+          merge_request.merge_request_reviewers
+            .find_or_create_by!(reviewer: user)
+            .update!(state: 'reviewed')
 
           baseline = ActiveRecord::QueryRecorder.new do
             post_graphql(query)
@@ -401,7 +406,8 @@ RSpec.describe 'getting merge request information nested in a project' do
           expect(interaction_data).to contain_exactly(include(reviewed))
 
           other_users.each do |user|
-            merge_request.merge_request_reviewers.create!(reviewer: user)
+            assign_user(user)
+            merge_request.merge_request_reviewers.find_or_create_by!(reviewer: user)
           end
 
           expect { post_graphql(query) }.not_to exceed_query_limit(baseline)
@@ -435,4 +441,24 @@ RSpec.describe 'getting merge request information nested in a project' do
       end
     end
   end
+
+  it_behaves_like 'when requesting information about MR interactions' do
+    let(:field) { :reviewers }
+    let(:unreviewed) { 'UNREVIEWED' }
+    let(:can_update) { false }
+
+    def assign_user(user)
+      merge_request.merge_request_reviewers.create!(reviewer: user)
+    end
+  end
+
+  it_behaves_like 'when requesting information about MR interactions' do
+    let(:field) { :assignees }
+    let(:unreviewed) { nil }
+    let(:can_update) { true } # assignees can update MRs
+
+    def assign_user(user)
+      merge_request.assignees << user
+    end
+  end
 end