Commit b472f3c5 authored by Alex Kalderimis's avatar Alex Kalderimis

[GQL] Add author and merged_at to MergeRequest Type

This adds the author and merged_at fields to the graphql API.

The authorize field service is changed to support arrays of lazy values,
which is a bug that had gone unnoticed.
parent f7ccbfca
...@@ -28,6 +28,8 @@ module Types ...@@ -28,6 +28,8 @@ module Types
description: 'Timestamp of when the merge request was created' description: 'Timestamp of when the merge request was created'
field :updated_at, Types::TimeType, null: false, field :updated_at, Types::TimeType, null: false,
description: 'Timestamp of when the merge request was last updated' description: 'Timestamp of when the merge request was last updated'
field :merged_at, Types::TimeType, null: true, complexity: 5,
description: 'Timestamp of when the merge request was merged, null if not merged'
field :source_project, Types::ProjectType, null: true, field :source_project, Types::ProjectType, null: true,
description: 'Source project of the merge request' description: 'Source project of the merge request'
field :target_project, Types::ProjectType, null: false, field :target_project, Types::ProjectType, null: false,
...@@ -109,6 +111,8 @@ module Types ...@@ -109,6 +111,8 @@ module Types
resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(Milestone, obj.milestone_id).find } resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(Milestone, obj.milestone_id).find }
field :assignees, Types::UserType.connection_type, null: true, complexity: 5, field :assignees, Types::UserType.connection_type, null: true, complexity: 5,
description: 'Assignees of the merge request' description: 'Assignees of the merge request'
field :author, Types::UserType, null: true,
description: 'User who created this merge request'
field :participants, Types::UserType.connection_type, null: true, complexity: 5, field :participants, Types::UserType.connection_type, null: true, complexity: 5,
description: 'Participants in the merge request' description: 'Participants in the merge request'
field :subscribed, GraphQL::BOOLEAN_TYPE, method: :subscribed?, null: false, complexity: 5, field :subscribed, GraphQL::BOOLEAN_TYPE, method: :subscribed?, null: false, complexity: 5,
......
---
title: Add missing Merge Request fields
merge_request: 30935
author:
type: added
...@@ -5767,6 +5767,11 @@ type MergeRequest implements Noteable { ...@@ -5767,6 +5767,11 @@ type MergeRequest implements Noteable {
last: Int last: Int
): UserConnection ): UserConnection
"""
User who created this merge request
"""
author: User
""" """
Timestamp of when the merge request was created Timestamp of when the merge request was created
""" """
...@@ -5917,6 +5922,11 @@ type MergeRequest implements Noteable { ...@@ -5917,6 +5922,11 @@ type MergeRequest implements Noteable {
""" """
mergeableDiscussionsState: Boolean mergeableDiscussionsState: Boolean
"""
Timestamp of when the merge request was merged, null if not merged
"""
mergedAt: Time
""" """
The milestone of the merge request The milestone of the merge request
""" """
......
...@@ -16085,6 +16085,20 @@ ...@@ -16085,6 +16085,20 @@
"isDeprecated": false, "isDeprecated": false,
"deprecationReason": null "deprecationReason": null
}, },
{
"name": "author",
"description": "User who created this merge request",
"args": [
],
"type": {
"kind": "OBJECT",
"name": "User",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{ {
"name": "createdAt", "name": "createdAt",
"description": "Timestamp of when the merge request was created", "description": "Timestamp of when the merge request was created",
...@@ -16499,6 +16513,20 @@ ...@@ -16499,6 +16513,20 @@
"isDeprecated": false, "isDeprecated": false,
"deprecationReason": null "deprecationReason": null
}, },
{
"name": "mergedAt",
"description": "Timestamp of when the merge request was merged, null if not merged",
"args": [
],
"type": {
"kind": "SCALAR",
"name": "Time",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{ {
"name": "milestone", "name": "milestone",
"description": "The milestone of the merge request", "description": "The milestone of the merge request",
...@@ -860,6 +860,7 @@ Autogenerated return type of MarkAsSpamSnippet ...@@ -860,6 +860,7 @@ Autogenerated return type of MarkAsSpamSnippet
| Name | Type | Description | | Name | Type | Description |
| --- | ---- | ---------- | | --- | ---- | ---------- |
| `allowCollaboration` | Boolean | Indicates if members of the target project can push to the fork | | `allowCollaboration` | Boolean | Indicates if members of the target project can push to the fork |
| `author` | User | User who created this merge request |
| `createdAt` | Time! | Timestamp of when the merge request was created | | `createdAt` | Time! | Timestamp of when the merge request was created |
| `defaultMergeCommitMessage` | String | Default merge commit message of the merge request | | `defaultMergeCommitMessage` | String | Default merge commit message of the merge request |
| `description` | String | Description of the merge request (Markdown rendered as HTML for caching) | | `description` | String | Description of the merge request (Markdown rendered as HTML for caching) |
...@@ -880,6 +881,7 @@ Autogenerated return type of MarkAsSpamSnippet ...@@ -880,6 +881,7 @@ Autogenerated return type of MarkAsSpamSnippet
| `mergeStatus` | String | Status of the merge request | | `mergeStatus` | String | Status of the merge request |
| `mergeWhenPipelineSucceeds` | Boolean | Indicates if the merge has been set to be merged when its pipeline succeeds (MWPS) | | `mergeWhenPipelineSucceeds` | Boolean | Indicates if the merge has been set to be merged when its pipeline succeeds (MWPS) |
| `mergeableDiscussionsState` | Boolean | Indicates if all discussions in the merge request have been resolved, allowing the merge request to be merged | | `mergeableDiscussionsState` | Boolean | Indicates if all discussions in the merge request have been resolved, allowing the merge request to be merged |
| `mergedAt` | Time | Timestamp of when the merge request was merged, null if not merged |
| `milestone` | Milestone | The milestone of the merge request | | `milestone` | Milestone | The milestone of the merge request |
| `project` | Project! | Alias for target_project | | `project` | Project! | Alias for target_project |
| `projectId` | Int! | ID of the merge request project | | `projectId` | Int! | ID of the merge request project |
......
...@@ -84,13 +84,25 @@ module Gitlab ...@@ -84,13 +84,25 @@ module Gitlab
elsif resolved_type.is_a? Array elsif resolved_type.is_a? Array
# A simple list of rendered types each object being an object to authorize # A simple list of rendered types each object being an object to authorize
resolved_type.select do |single_object_type| resolved_type.select do |single_object_type|
allowed_access?(current_user, unpromise(single_object_type).object) allowed_access?(current_user, realized(single_object_type).object)
end end
else else
raise "Can't authorize #{@field}" raise "Can't authorize #{@field}"
end end
end end
# Ensure that we are dealing with realized objects, not delayed promises
def realized(thing)
case thing
when BatchLoader::GraphQL
thing.sync
when GraphQL::Execution::Lazy
thing.value # part of the private api, but we need to unwrap it here.
else
thing
end
end
def allowed_access?(current_user, object) def allowed_access?(current_user, object)
object = object.sync if object.respond_to?(:sync) object = object.sync if object.respond_to?(:sync)
...@@ -113,17 +125,6 @@ module Gitlab ...@@ -113,17 +125,6 @@ module Gitlab
def scalar_type? def scalar_type?
node_type_for_basic_connection(@field.type).kind.scalar? node_type_for_basic_connection(@field.type).kind.scalar?
end end
# Sometimes we get promises, and have to resolve them. The dedicated way
# of doing this (GitlabSchema.after_lazy) is a private framework method,
# and so we use duck-typing interface inference here instead.
def unpromise(maybe_promise)
if maybe_promise.respond_to?(:value) && !maybe_promise.respond_to?(:object)
maybe_promise.value
else
maybe_promise
end
end
end end
end end
end end
......
...@@ -23,7 +23,7 @@ describe GitlabSchema.types['MergeRequest'] do ...@@ -23,7 +23,7 @@ describe GitlabSchema.types['MergeRequest'] do
source_branch_exists target_branch_exists source_branch_exists target_branch_exists
upvotes downvotes head_pipeline pipelines task_completion_status upvotes downvotes head_pipeline pipelines task_completion_status
milestone assignees participants subscribed labels discussion_locked time_estimate milestone assignees participants subscribed labels discussion_locked time_estimate
total_time_spent reference total_time_spent reference author merged_at
] ]
expect(described_class).to have_graphql_fields(*expected_fields) expect(described_class).to have_graphql_fields(*expected_fields)
......
...@@ -37,6 +37,30 @@ describe 'getting merge request information nested in a project' do ...@@ -37,6 +37,30 @@ describe 'getting merge request information nested in a project' do
expect(merge_request_graphql_data['webUrl']).to be_present expect(merge_request_graphql_data['webUrl']).to be_present
end end
it 'includes author' do
post_graphql(query, current_user: current_user)
expect(merge_request_graphql_data['author']['username']).to eq(merge_request.author.username)
end
it 'includes correct mergedAt value when merged' do
time = 1.week.ago
merge_request.mark_as_merged
merge_request.metrics.update_columns(merged_at: time)
post_graphql(query, current_user: current_user)
retrieved = merge_request_graphql_data['mergedAt']
expect(Time.zone.parse(retrieved)).to be_within(1.second).of(time)
end
it 'includes nil mergedAt value when not merged' do
post_graphql(query, current_user: current_user)
retrieved = merge_request_graphql_data['mergedAt']
expect(retrieved).to be_nil
end
context 'permissions on the merge request' do context 'permissions on the merge request' do
it 'includes the permissions for the current user on a public project' do it 'includes the permissions for the current user on a public project' do
expected_permissions = { expected_permissions = {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment