Commit f243a31c authored by Stan Hu's avatar Stan Hu

Merge branch 'ajk-gql-mr-diff-stats' into 'master'

Add diff stats fields to merge request type

See merge request gitlab-org/gitlab!34966
parents 4671a3b1 0033e821
# frozen_string_literal: true
module Types
# rubocop: disable Graphql/AuthorizeTypes
# Types that use DiffStatsType should have their own authorization
class DiffStatsSummaryType < BaseObject
graphql_name 'DiffStatsSummary'
description 'Aggregated summary of changes'
field :additions, GraphQL::INT_TYPE, null: false,
description: 'Number of lines added'
field :deletions, GraphQL::INT_TYPE, null: false,
description: 'Number of lines deleted'
field :changes, GraphQL::INT_TYPE, null: false,
description: 'Number of lines changed'
def changes
object[:additions] + object[:deletions]
end
end
# rubocop: enable Graphql/AuthorizeTypes
end
# frozen_string_literal: true
module Types
# rubocop: disable Graphql/AuthorizeTypes
# Types that use DiffStatsType should have their own authorization
class DiffStatsType < BaseObject
graphql_name 'DiffStats'
description 'Changes to a single file'
field :path, GraphQL::STRING_TYPE, null: false,
description: 'File path, relative to repository root'
field :additions, GraphQL::INT_TYPE, null: false,
description: 'Number of lines added to this file'
field :deletions, GraphQL::INT_TYPE, null: false,
description: 'Number of lines deleted from this file'
end
# rubocop: enable Graphql/AuthorizeTypes
end
...@@ -54,6 +54,13 @@ module Types ...@@ -54,6 +54,13 @@ module Types
description: 'Indicates if the merge has been set to be merged when its pipeline succeeds (MWPS)' description: 'Indicates if the merge has been set to be merged when its pipeline succeeds (MWPS)'
field :diff_head_sha, GraphQL::STRING_TYPE, null: true, field :diff_head_sha, GraphQL::STRING_TYPE, null: true,
description: 'Diff head SHA of the merge request' description: 'Diff head SHA of the merge request'
field :diff_stats, [Types::DiffStatsType], null: true, calls_gitaly: true,
description: 'Details about which files were changed in this merge request' do
argument :path, GraphQL::STRING_TYPE, required: false, description: 'A specific file-path'
end
field :diff_stats_summary, Types::DiffStatsSummaryType, null: true, calls_gitaly: true,
description: 'Summary of which files were changed in this merge request'
field :merge_commit_sha, GraphQL::STRING_TYPE, null: true, field :merge_commit_sha, GraphQL::STRING_TYPE, null: true,
description: 'SHA of the merge request commit (set once merged)' description: 'SHA of the merge request commit (set once merged)'
field :user_notes_count, GraphQL::INT_TYPE, null: true, field :user_notes_count, GraphQL::INT_TYPE, null: true,
...@@ -134,5 +141,24 @@ module Types ...@@ -134,5 +141,24 @@ module Types
end end
field :task_completion_status, Types::TaskCompletionStatus, null: false, field :task_completion_status, Types::TaskCompletionStatus, null: false,
description: Types::TaskCompletionStatus.description description: Types::TaskCompletionStatus.description
def diff_stats(path: nil)
stats = Array.wrap(object.diff_stats&.to_a)
if path.present?
stats.select { |s| s.path == path }
else
stats
end
end
def diff_stats_summary
nil_stats = { additions: 0, deletions: 0 }
return nil_stats unless object.diff_stats.present?
object.diff_stats.each_with_object(nil_stats) do |status, hash|
hash.merge!(additions: status.additions, deletions: status.deletions) { |_, x, y| x + y }
end
end
end end
end end
---
title: Add diff stats fields to merge request type
merge_request: 34966
author:
type: added
...@@ -3076,6 +3076,46 @@ type DiffRefs { ...@@ -3076,6 +3076,46 @@ type DiffRefs {
startSha: String! startSha: String!
} }
"""
Changes to a single file
"""
type DiffStats {
"""
Number of lines added to this file
"""
additions: Int!
"""
Number of lines deleted from this file
"""
deletions: Int!
"""
File path, relative to repository root
"""
path: String!
}
"""
Aggregated summary of changes
"""
type DiffStatsSummary {
"""
Number of lines added
"""
additions: Int!
"""
Number of lines changed
"""
changes: Int!
"""
Number of lines deleted
"""
deletions: Int!
}
type Discussion implements ResolvableInterface { type Discussion implements ResolvableInterface {
""" """
Timestamp of the discussion's creation Timestamp of the discussion's creation
...@@ -6665,6 +6705,21 @@ type MergeRequest implements Noteable { ...@@ -6665,6 +6705,21 @@ type MergeRequest implements Noteable {
""" """
diffRefs: DiffRefs diffRefs: DiffRefs
"""
Details about which files were changed in this merge request
"""
diffStats(
"""
A specific file-path
"""
path: String
): [DiffStats!]
"""
Summary of which files were changed in this merge request
"""
diffStatsSummary: DiffStatsSummary
""" """
Indicates if comments on the merge request are locked to members only Indicates if comments on the merge request are locked to members only
""" """
......
...@@ -8508,6 +8508,140 @@ ...@@ -8508,6 +8508,140 @@
"enumValues": null, "enumValues": null,
"possibleTypes": null "possibleTypes": null
}, },
{
"kind": "OBJECT",
"name": "DiffStats",
"description": "Changes to a single file",
"fields": [
{
"name": "additions",
"description": "Number of lines added to this file",
"args": [
],
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "Int",
"ofType": null
}
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "deletions",
"description": "Number of lines deleted from this file",
"args": [
],
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "Int",
"ofType": null
}
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "path",
"description": "File path, relative to repository root",
"args": [
],
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "String",
"ofType": null
}
},
"isDeprecated": false,
"deprecationReason": null
}
],
"inputFields": null,
"interfaces": [
],
"enumValues": null,
"possibleTypes": null
},
{
"kind": "OBJECT",
"name": "DiffStatsSummary",
"description": "Aggregated summary of changes",
"fields": [
{
"name": "additions",
"description": "Number of lines added",
"args": [
],
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "Int",
"ofType": null
}
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "changes",
"description": "Number of lines changed",
"args": [
],
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "Int",
"ofType": null
}
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "deletions",
"description": "Number of lines deleted",
"args": [
],
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "Int",
"ofType": null
}
},
"isDeprecated": false,
"deprecationReason": null
}
],
"inputFields": null,
"interfaces": [
],
"enumValues": null,
"possibleTypes": null
},
{ {
"kind": "OBJECT", "kind": "OBJECT",
"name": "Discussion", "name": "Discussion",
...@@ -18512,6 +18646,51 @@ ...@@ -18512,6 +18646,51 @@
"isDeprecated": false, "isDeprecated": false,
"deprecationReason": null "deprecationReason": null
}, },
{
"name": "diffStats",
"description": "Details about which files were changed in this merge request",
"args": [
{
"name": "path",
"description": "A specific file-path",
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"defaultValue": null
}
],
"type": {
"kind": "LIST",
"name": null,
"ofType": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "OBJECT",
"name": "DiffStats",
"ofType": null
}
}
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "diffStatsSummary",
"description": "Summary of which files were changed in this merge request",
"args": [
],
"type": {
"kind": "OBJECT",
"name": "DiffStatsSummary",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{ {
"name": "discussionLocked", "name": "discussionLocked",
"description": "Indicates if comments on the merge request are locked to members only", "description": "Indicates if comments on the merge request are locked to members only",
...@@ -517,6 +517,26 @@ Autogenerated return type of DestroySnippet ...@@ -517,6 +517,26 @@ Autogenerated return type of DestroySnippet
| `headSha` | String! | SHA of the HEAD at the time the comment was made | | `headSha` | String! | SHA of the HEAD at the time the comment was made |
| `startSha` | String! | SHA of the branch being compared against | | `startSha` | String! | SHA of the branch being compared against |
## DiffStats
Changes to a single file
| Name | Type | Description |
| --- | ---- | ---------- |
| `additions` | Int! | Number of lines added to this file |
| `deletions` | Int! | Number of lines deleted from this file |
| `path` | String! | File path, relative to repository root |
## DiffStatsSummary
Aggregated summary of changes
| Name | Type | Description |
| --- | ---- | ---------- |
| `additions` | Int! | Number of lines added |
| `changes` | Int! | Number of lines changed |
| `deletions` | Int! | Number of lines deleted |
## Discussion ## Discussion
| Name | Type | Description | | Name | Type | Description |
...@@ -1002,6 +1022,8 @@ Autogenerated return type of MarkAsSpamSnippet ...@@ -1002,6 +1022,8 @@ Autogenerated return type of MarkAsSpamSnippet
| `descriptionHtml` | String | The GitLab Flavored Markdown rendering of `description` | | `descriptionHtml` | String | The GitLab Flavored Markdown rendering of `description` |
| `diffHeadSha` | String | Diff head SHA of the merge request | | `diffHeadSha` | String | Diff head SHA of the merge request |
| `diffRefs` | DiffRefs | References of the base SHA, the head SHA, and the start SHA for this merge request | | `diffRefs` | DiffRefs | References of the base SHA, the head SHA, and the start SHA for this merge request |
| `diffStats` | DiffStats! => Array | Details about which files were changed in this merge request |
| `diffStatsSummary` | DiffStatsSummary | Summary of which files were changed in this merge request |
| `discussionLocked` | Boolean! | Indicates if comments on the merge request are locked to members only | | `discussionLocked` | Boolean! | Indicates if comments on the merge request are locked to members only |
| `downvotes` | Int! | Number of downvotes for the merge request | | `downvotes` | Int! | Number of downvotes for the merge request |
| `forceRemoveSourceBranch` | Boolean | Indicates if the project settings will lead to source branch deletion after merge | | `forceRemoveSourceBranch` | Boolean | Indicates if the project settings will lead to source branch deletion after merge |
......
...@@ -15,7 +15,8 @@ RSpec.describe GitlabSchema.types['MergeRequest'] do ...@@ -15,7 +15,8 @@ RSpec.describe GitlabSchema.types['MergeRequest'] do
description_html state created_at updated_at source_project target_project description_html state created_at updated_at source_project target_project
project project_id source_project_id target_project_id source_branch project project_id source_project_id target_project_id source_branch
target_branch work_in_progress merge_when_pipeline_succeeds diff_head_sha target_branch work_in_progress merge_when_pipeline_succeeds diff_head_sha
merge_commit_sha user_notes_count should_remove_source_branch diff_refs merge_commit_sha user_notes_count should_remove_source_branch
diff_refs diff_stats diff_stats_summary
force_remove_source_branch merge_status in_progress_merge_commit_sha force_remove_source_branch merge_status in_progress_merge_commit_sha
merge_error allow_collaboration should_be_rebased rebase_commit_sha merge_error allow_collaboration should_be_rebased rebase_commit_sha
rebase_in_progress merge_commit_message default_merge_commit_message rebase_in_progress merge_commit_message default_merge_commit_message
......
...@@ -43,6 +43,54 @@ RSpec.describe 'getting merge request information nested in a project' do ...@@ -43,6 +43,54 @@ RSpec.describe 'getting merge request information nested in a project' do
expect(merge_request_graphql_data['author']['username']).to eq(merge_request.author.username) expect(merge_request_graphql_data['author']['username']).to eq(merge_request.author.username)
end end
it 'includes diff stats' do
be_natural = an_instance_of(Integer).and(be >= 0)
post_graphql(query, current_user: current_user)
sums = merge_request_graphql_data['diffStats'].reduce([0, 0, 0]) do |(a, d, c), node|
a_, d_ = node.values_at('additions', 'deletions')
[a + a_, d + d_, c + a_ + d_]
end
expect(merge_request_graphql_data).to include(
'diffStats' => all(a_hash_including('path' => String, 'additions' => be_natural, 'deletions' => be_natural)),
'diffStatsSummary' => a_hash_including('additions' => be_natural, 'deletions' => be_natural, 'changes' => be_natural)
)
# diff_stats is consistent with summary
expect(merge_request_graphql_data['diffStatsSummary']
.values_at('additions', 'deletions', 'changes')).to eq(sums)
# diff_stats_summary is internally consistent
expect(merge_request_graphql_data['diffStatsSummary']
.values_at('additions', 'deletions').sum)
.to eq(merge_request_graphql_data.dig('diffStatsSummary', 'changes'))
.and be_positive
end
context 'requesting a specific diff stat' do
let(:diff_stat) { merge_request.diff_stats.first }
let(:query) do
graphql_query_for(:project, { full_path: project.full_path },
query_graphql_field(:merge_request, { iid: merge_request.iid.to_s }, [
query_graphql_field(:diff_stats, { path: diff_stat.path }, all_graphql_fields_for('DiffStats'))
])
)
end
it 'includes only the requested stats' do
post_graphql(query, current_user: current_user)
expect(merge_request_graphql_data).to include(
'diffStats' => contain_exactly(
a_hash_including('path' => diff_stat.path, 'additions' => diff_stat.additions, 'deletions' => diff_stat.deletions)
)
)
end
end
it 'includes correct mergedAt value when merged' do it 'includes correct mergedAt value when merged' do
time = 1.week.ago time = 1.week.ago
merge_request.mark_as_merged merge_request.mark_as_merged
......
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