Skip to content

Use GitLab project ID instead of project name #994

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.

Sign up for GitHub

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

Open
farabi wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Use GitLab project ID instead of project name #994

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1813011
Add an environment variable for gitlab project ID.
SaadElOulladi123 Jan 21, 2020
7fa2aeb
Update gitlab api fixtures following gitlab project id use.
SaadElOulladi123 Jan 21, 2020
84ced7a
Update docs/usage/gitlab.html.md
farabi Feb 21, 2020
3981e72
Revert 84ced7a64c9b6d60fe3b4aeb4d6706293cb58989
SaadElOulladi123 Mar 12, 2020
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
4 changes: 4 additions & 0 deletions docs/usage/gitlab.html.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ variable on your CI system:

- `DANGER_GITLAB_API_TOKEN` = An access token for the account which will post comments

You will need also to add an environment variable for your gitlab project id:

- `DANGER_GITLAB_PROJECT_ID` = Gitlab project id can be found in (Settings -> General). if not defined, project name will be used instead

If you are using a GitLab version prior to 11.7 you will also need to define the following environment variable:

- `DANGER_GITLAB_HOST` = Defaults to `https://gitlab.com` but you can use it for your own url
Expand Down
2 changes: 1 addition & 1 deletion source/danger-incoming-process-schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1189,7 +1189,7 @@
"type": "string"
},
"repoSlug": {
"description": "A path like \"artsy/eigen\"",
"description": "Project ID \"624\" or a path like \"artsy/eigen\"",
"type": "string"
}
},
Expand Down
12 changes: 6 additions & 6 deletions source/platforms/gitlab/_tests/_gitlab_api.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { readFileSync } from "fs"
const nockBack = nock.back
nockBack.fixtures = __dirname + "/fixtures"

// We're testing https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/27117
Copy link
Member

@orta orta Feb 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change dories me a bit, because this link used to work - but now it doesn't?

Copy link
Member

@rohit-gohri rohit-gohri Mar 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The repo was moved to gitlab-foss. New MR link: https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/27117
I updated the tests and fixtures for all Gitlab tests in #1002

Copy link
Member

@rohit-gohri rohit-gohri Mar 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I think tests should run for both projectId and project path

Copy link
Contributor

@glensc glensc Feb 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the link change should be reverted because https://gitlab.com/456/merge_requests/27117 is 404
or updated to the redirected link.

this is just a comment, the code can continue use numeric id

// We're testing https://gitlab.com/456/merge_requests/27117
// This has been chosen because it is already merged and publicly available, it's unlikely to change

/** Returns a fixture. */
Expand All @@ -27,7 +27,7 @@ describe("GitLab API", () => {

beforeEach(() => {
api = new GitLabAPI(
{ pullRequestID: "27117", repoSlug: "gitlab-org/gitlab-ce" },
{ pullRequestID: "27117", repoSlug: "456" },
getGitLabAPICredentialsFromEnv({
DANGER_GITLAB_HOST: "gitlab.com",
DANGER_GITLAB_API_TOKEN: "FAKE_DANGER_GITLAB_API_TOKEN",
Expand All @@ -37,22 +37,22 @@ describe("GitLab API", () => {

it("configures host from CI_API_V4_URL", () => {
api = new GitLabAPI(
{ pullRequestID: "27117", repoSlug: "gitlab-org/gitlab-ce" },
{ pullRequestID: "27117", repoSlug: "456" },
getGitLabAPICredentialsFromEnv({
CI_API_V4_URL: "https://testciapiv4url.com/api/v4",
DANGER_GITLAB_API_TOKEN: "FAKE_DANGER_GITLAB_API_TOKEN",
})
)

expect(api.projectURL).toBe("https://testciapiv4url.com/gitlab-org/gitlab-ce")
expect(api.projectURL).toBe("https://testciapiv4url.com/456")
})

it("projectURL is defined", () => {
expect(api.projectURL).toBe("https://gitlab.com/gitlab-org/gitlab-ce")
expect(api.projectURL).toBe("https://gitlab.com/456")
})

it("mergeRequestURL is defined", () => {
expect(api.mergeRequestURL).toBe("https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/27117")
expect(api.mergeRequestURL).toBe("https://gitlab.com/456/merge_requests/27117")
})

const sanitizeUserResponse = (nocks: NockDefinition[]): NockDefinition[] => {
Expand Down
9 changes: 7 additions & 2 deletions source/platforms/gitlab/_tests/fixtures/getMergeRequestChanges.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{
"scope": "https://gitlab.com:443",
"method": "GET",
"path": "/api/v4/projects/gitlab-org%2Fgitlab-ce/merge_requests/27117/changes",
"path": "/api/v4/projects/456/merge_requests/27117/changes",
"body": "",
"status": 200,
"response": {
Expand Down Expand Up @@ -58,7 +58,12 @@
],
"source_project_id": 13083,
"target_project_id": 13083,
"labels": ["Danger bot", "Plan", "backend", "backstage"],
"labels": [
"Danger bot",
"Plan",
"backend",
"backstage"
],
"work_in_progress": false,
"milestone": {
"id": 655280,
Expand Down
4 changes: 2 additions & 2 deletions source/platforms/gitlab/_tests/fixtures/getMergeRequestCommits.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{
"scope": "https://gitlab.com:443",
"method": "GET",
"path": "/api/v4/projects/gitlab-org%2Fgitlab-ce/merge_requests/27117/commits",
"path": "/api/v4/projects/456/merge_requests/27117/commits",
"body": "",
"status": 200,
"response": [
Expand Down Expand Up @@ -51,7 +51,7 @@
"Etag",
"W/\"478bcb3a3b56ea7180c04159ff3adea5\"",
"Link",
"<https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab-ce/merge_requests/27117/commits?id=gitlab-org%2Fgitlab-ce&merge_request_iid=27117&page=1&per_page=>; rel=\"first\", <https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab-ce/merge_requests/27117/commits?id=gitlab-org%2Fgitlab-ce&merge_request_iid=27117&page=1&per_page=>; rel=\"last\"",
"<https://gitlab.com/api/v4/projects/456/merge_requests/27117/commits?id=456&merge_request_iid=27117&page=1&per_page=>; rel=\"first\", <https://gitlab.com/api/v4/projects/456/merge_requests/27117/commits?id=456&merge_request_iid=27117&page=1&per_page=>; rel=\"last\"",
"Vary",
"Origin",
"X-Content-Type-Options",
Expand Down
2 changes: 1 addition & 1 deletion source/platforms/gitlab/_tests/fixtures/getMergeRequestInfo.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{
"scope": "https://gitlab.com:443",
"method": "GET",
"path": "/api/v4/projects/gitlab-org%2Fgitlab-ce/merge_requests/27117",
"path": "/api/v4/projects/456/merge_requests/27117",
"body": "",
"status": 200,
"response": {
Expand Down
4 changes: 2 additions & 2 deletions source/platforms/gitlab/_tests/fixtures/getMergeRequestInlineNotes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{
"scope": "https://gitlab.com:443",
"method": "GET",
"path": "/api/v4/projects/gitlab-org%2Fgitlab-ce/merge_requests/27117/notes",
"path": "/api/v4/projects/456/merge_requests/27117/notes",
"body": "",
"status": 200,
"response": [
Expand Down Expand Up @@ -356,7 +356,7 @@
"Etag",
"W/\"edab8aad8eea37dd376785c34c7c250f\"",
"Link",
"<https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab-ce/merge_requests/27117/notes?id=gitlab-org%2Fgitlab-ce&noteable_id=27117&order_by=created_at&page=1&per_page=20&sort=desc>; rel=\"first\", <https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab-ce/merge_requests/27117/notes?id=gitlab-org%2Fgitlab-ce&noteable_id=27117&order_by=created_at&page=1&per_page=20&sort=desc>; rel=\"last\"",
"<https://gitlab.com/api/v4/projects/456/merge_requests/27117/notes?id=456&noteable_id=27117&order_by=created_at&page=1&per_page=20&sort=desc>; rel=\"first\", <https://gitlab.com/api/v4/projects/456/merge_requests/27117/notes?id=456&noteable_id=27117&order_by=created_at&page=1&per_page=20&sort=desc>; rel=\"last\"",
"Vary",
"Origin",
"X-Content-Type-Options",
Expand Down
4 changes: 2 additions & 2 deletions source/platforms/gitlab/_tests/fixtures/getMergeRequestNotes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{
"scope": "https://gitlab.com:443",
"method": "GET",
"path": "/api/v4/projects/gitlab-org%2Fgitlab-ce/merge_requests/27117/notes",
"path": "/api/v4/projects/456/merge_requests/27117/notes",
"body": "",
"status": 200,
"response": [
Expand Down Expand Up @@ -356,7 +356,7 @@
"Etag",
"W/\"edab8aad8eea37dd376785c34c7c250f\"",
"Link",
"<https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab-ce/merge_requests/27117/notes?id=gitlab-org%2Fgitlab-ce&noteable_id=27117&order_by=created_at&page=1&per_page=20&sort=desc>; rel=\"first\", <https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab-ce/merge_requests/27117/notes?id=gitlab-org%2Fgitlab-ce&noteable_id=27117&order_by=created_at&page=1&per_page=20&sort=desc>; rel=\"last\"",
"<https://gitlab.com/api/v4/projects/456/merge_requests/27117/notes?id=456&noteable_id=27117&order_by=created_at&page=1&per_page=20&sort=desc>; rel=\"first\", <https://gitlab.com/api/v4/projects/456/merge_requests/27117/notes?id=456&noteable_id=27117&order_by=created_at&page=1&per_page=20&sort=desc>; rel=\"last\"",
"Vary",
"Origin",
"X-Content-Type-Options",
Expand Down
3 changes: 2 additions & 1 deletion source/platforms/platform.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,11 @@ export function getPlatformForEnv(env: Env, source: CISource): Platform {

// GitLab
if (env["DANGER_GITLAB_API_TOKEN"] || env["DANGER_PR_PLATFORM"] === GitLab.name) {
const repoSlug = env["DANGER_GITLAB_PROJECT_ID"] ? env["DANGER_GITLAB_PROJECT_ID"] : source.repoSlug
const api = new GitLabAPI(
{
pullRequestID: source.pullRequestID,
repoSlug: source.repoSlug,
repoSlug: repoSlug,
},
getGitLabAPICredentialsFromEnv(env)
)
Expand Down