Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: gitlab add MR approvals related information #1098

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions source/dsl/GitLabDSL.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ export interface GitLabJSONDSL {
mr: GitLabMR
/** All of the individual commits in the merge request */
commits: GitLabMRCommit[]
/** Merge Request-level MR approvals Configuration */
approvals: GitLabApproval
}

// danger.gitlab
Expand Down Expand Up @@ -269,3 +271,22 @@ export interface GitLabRepositoryCompare {
compare_timeout: boolean
compare_same_ref: boolean
}

export interface GitLabApproval {
id: number
iid: number
project_id: number
title: string
description: string
state: "closed" | "open" | "locked" | "merged"
created_at: string
updated_at: string
merge_status: "can_be_merged"
approvals_required: number
approvals_left: number
approved_by?:
| {
user: GitLabUser
}[]
| GitLabUser[]
}
2 changes: 2 additions & 0 deletions source/platforms/GitLab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@ class GitLab implements Platform {
getPlatformReviewDSLRepresentation = async (): Promise<GitLabJSONDSL> => {
const mr = await this.getReviewInfo()
const commits = await this.api.getMergeRequestCommits()
const approvals = await this.api.getMergeRequestApprovals()
return {
metadata: this.api.repoMetadata,
mr,
commits,
approvals,
}
}

Expand Down
10 changes: 10 additions & 0 deletions source/platforms/gitlab/GitLabAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
GitLabUserProfile,
GitLabRepositoryFile,
GitLabRepositoryCompare,
GitLabApproval,
} from "../../dsl/GitLabDSL"

import { Gitlab } from "gitlab"
Expand Down Expand Up @@ -89,6 +90,15 @@ class GitLabAPI {
return mr
}

getMergeRequestApprovals = async (): Promise<GitLabApproval> => {
this.d(`getMergeRequestApprovals for repo: ${this.repoMetadata.repoSlug} pr: ${this.repoMetadata.pullRequestID}`)
const approvals = (await this.api.MergeRequests.approvals(this.repoMetadata.repoSlug, {
mergerequestIId: Number(this.repoMetadata.pullRequestID),
})) as GitLabApproval
this.d("getMergeRequestApprovals", approvals)
return approvals
}

getMergeRequestChanges = async (): Promise<GitLabMRChange[]> => {
this.d(`getMergeRequestChanges for repo: ${this.repoMetadata.repoSlug} pr: ${this.repoMetadata.pullRequestID}`)
const mr = (await this.api.MergeRequests.changes(
Expand Down
8 changes: 8 additions & 0 deletions source/platforms/gitlab/_tests/_gitlab_api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ describe("GitLab API", () => {
expect(result).toEqual(response)
})

it("getMergeRequestApprovals", async () => {
const { nockDone } = await nockBack("getMergeRequestApprovals.json")
const result = await api.getMergeRequestApprovals()
nockDone()
const { response } = loadFixture("getMergeRequestApprovals")
expect(result).toEqual(response)
})

it("getMergeRequestChanges", async () => {
const { nockDone } = await nockBack("getMergeRequestChanges.json")
const result = await api.getMergeRequestChanges()
Expand Down
1 change: 1 addition & 0 deletions source/platforms/gitlab/_tests/_gitlab_git.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ describe("GitLabGit DSL", () => {
const defaultField = "0.response"

api.getMergeRequestInfo = requestWithFixturedJSON(MRInfoFixture, defaultField)
api.getMergeRequestApprovals = requestWithFixturedJSON("getMergeRequestApprovals.json", defaultField)
api.getMergeRequestCommits = requestWithFixturedJSON("getMergeRequestCommits.json", defaultField)
api.getMergeRequestChanges = requestWithFixturedJSON("getMergeRequestChanges.json", `${defaultField}.changes`)
api.getCompareChanges = requestWithFixturedJSON("getCompareChanges.json", `${defaultField}.diffs`)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
[
{
"scope": "https://gitlab.com:443",
"method": "GET",
"path": "/api/v4/projects/gitlab-org%2Fgitlab-foss/merge_requests/27117/approvals",
"body": "",
"status": 200,
"response": {
"id": 27253868,
"iid": 27117,
"project_id": 13083,
"title": "Stable reviewer roulette",
"description": "Change reviewer roulette to always pick the same reviewers for the same\nbranch name. We do this by:\n\n1. Making the branch name 'canonical' across CE and EE by stripping a\n leading 'ce-' or 'ee-' and a trailing '-ce' or '-ee'. If people are\n following our branch naming guidelines, this should give the same\n branch name in both repos.\n2. Converting the branch name to a stable integer by taking the integer\n form of its MD5.\n3. Passing that integer as a seed to Ruby's `Random` class, which 'may\n be used to ensure repeatable sequences of pseudo-random numbers\n between different runs of the program' (from the Ruby documentation).\n\nThe upshot is that the same branch name (in CE and EE) should always\npick the same reviewers, and those should be evenly distributed across\nthe set of possible reviewers due to the use of MD5.\n\nAgain, I have a test script:\n\n```ruby\nrequire 'ffaker'\n\nclass Foo\n include Gitlab::Danger::Helper\nend\n\ndef spin(team, project, category, branch_name)\n # Strip leading and trailing CE/EE markers\n canonical_branch_name = branch_name.gsub(/^[ce]e-/, '').gsub(/-[ce]e$/, '')\n rng = Random.new(Digest::MD5.hexdigest(canonical_branch_name).to_i(16))\n\n reviewers = team.select { |member| member.reviewer?(project, category) }\n traintainers = team.select { |member| member.traintainer?(project, category) }\n maintainers = team.select { |member| member.maintainer?(project, category) }\n\n # TODO: filter out people who are currently not in the office\n # https://gitlab.com/gitlab-org/gitlab-ce/issues/57652\n #\n # TODO: take CODEOWNERS into account?\n # https://gitlab.com/gitlab-org/gitlab-ce/issues/57653\n\n # Make traintainers have triple the chance to be picked as a reviewer\n reviewer = (reviewers + traintainers + traintainers).sample(random: rng)\n maintainer = maintainers.sample(random: rng)\n\n [reviewer.username, maintainer.username]\nend\n\ndef random_branch_name\n FFaker::Filesystem.file_name\nend\n\nFFaker::Random.seed = 123\nteam = Foo.new.project_team\nresults = Hash.new(0)\n\n10_000.times do\n reviewer, maintainer = spin(team, 'gitlab-ce', 'backend', random_branch_name)\n\n results[reviewer] += 1\n results[maintainer] += 1\nend\n\nresults.sort_by(\u0026:last).reverse.each do |username, picked|\n puts \"#{username}: #{picked}\"\nend; nil\n```\n\nThis should output the same for you as it does for me, because we seed the branch names too!\n\n```\ndzaporozhets: 799\nmkozono: 797\ngrzesiek: 794\ngodfat: 793\nDouweM: 788\nnick.thomas: 773\ntkuah: 764\nstanhu: 761\nayufan: 759\njprovaznik: 758\njameslopez: 757\ndbalexandre: 754\nsmcgivern: 751\nsplattael: 744\nashmckenzie: 741\nrspeicher: 739\njarka: 738\nrymai: 735\nreprazent: 729\nmayra-cabrera: 713\nmdelaossa: 273\nrdavila: 269\nvsizov: 260\ntheoretick: 257\noswaldo: 255\nifarkas: 255\nbrytannia: 248\nengwan: 248\nfelipe_artur: 247\nrpereira2: 243\ntoon: 239\nrossfuhrman: 236\njamedjo: 235\nfjsanpedro: 229\nmatteeyah: 226\nbrodock: 226\nvzagorodny: 222\nzj: 220\nmarkglenfletcher: 219\ndosuken123: 206\n```\n\nCloses https://gitlab.com/gitlab-org/gitlab-ce/issues/57766.",
"state": "merged",
"created_at": "2019-04-08T10:59:38.140Z",
"updated_at": "2019-07-25T01:02:18.281Z",
"merge_status": "can_be_merged",
"approved": true,
"approvals_required": 1,
"approvals_left": 0,
"require_password_to_approve": null,
"approved_by": [
{
"user": {
"id": 171554,
"name": "Bob Van Landuyt",
"username": "reprazent",
"state": "active",
"avatar_url": "https://assets.gitlab-static.net/uploads/-/system/user/avatar/171554/avatar.png",
"web_url": "https://gitlab.com/reprazent"
}
},
{
"user": {
"id": 283999,
"name": "Douglas Barbosa Alexandre",
"username": "dbalexandre",
"state": "active",
"avatar_url": "https://assets.gitlab-static.net/uploads/-/system/user/avatar/283999/avatar.png",
"web_url": "https://gitlab.com/dbalexandre"
}
}
],
"suggested_approvers": [],
"approvers": [],
"approver_groups": [],
"user_has_approved": false,
"user_can_approve": false,
"approval_rules_left": [],
"has_approval_rules": true,
"merge_request_approvers_available": true,
"multiple_approval_rules_available": true
}
}
]