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
Add thread support for GitLab #1140
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -2,7 +2,7 @@ import GitLabAPI from "./gitlab/GitLabAPI" | |||||||
import { Platform, Comment } from "./platform" | ||||||||
import { GitDSL, GitJSONDSL } from "../dsl/GitDSL" | ||||||||
import { GitCommit } from "../dsl/Commit" | ||||||||
import { GitLabDSL, GitLabJSONDSL, GitLabNote } from "../dsl/GitLabDSL" | ||||||||
import { GitLabDiscussion, GitLabDSL, GitLabJSONDSL, GitLabNote } from "../dsl/GitLabDSL" | ||||||||
|
||||||||
import { debug } from "../debug" | ||||||||
import { dangerIDToString } from "../runner/templates/githubIssueTemplate" | ||||||||
|
@@ -92,6 +92,23 @@ class GitLab implements Platform { | |||||||
return true | ||||||||
} | ||||||||
|
||||||||
supportsThreads() { | ||||||||
return true; | ||||||||
} | ||||||||
|
||||||||
updateOrCreateThread = async (dangerID: string, newComment: string): Promise<string> => { | ||||||||
const threads = await this.getDangerThreads(dangerID); | ||||||||
|
||||||||
if (threads.length) { | ||||||||
await this.api.updateMergeRequestDiscussion(threads[0].id, threads[0].notes[0].id, newComment); | ||||||||
return `${this.api.mergeRequestURL}#note_${threads[0].notes[0].id}`; | ||||||||
} else { | ||||||||
const thread = await this.api.createMergeRequestDiscussion(newComment); | ||||||||
|
||||||||
return `${this.api.mergeRequestURL}#note_${thread.notes[0].id}`; | ||||||||
Comment on lines
+103
to
+108
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe the zero could be a const with a description. Looks like a magic number without any type of context of why the code should the zero index and not any other |
||||||||
} | ||||||||
} | ||||||||
|
||||||||
updateOrCreateComment = async (dangerID: string, newComment: string): Promise<string> => { | ||||||||
d("updateOrCreateComment", { dangerID, newComment }) | ||||||||
|
||||||||
|
@@ -131,7 +148,7 @@ class GitLab implements Platform { | |||||||
|
||||||||
const mr = await this.api.getMergeRequestInfo() | ||||||||
|
||||||||
return this.api.createMergeRequestDiscussion(comment, { | ||||||||
const thread = await this.api.createMergeRequestDiscussion(comment, { | ||||||||
position_type: "text", | ||||||||
base_sha: mr.diff_refs.base_sha, | ||||||||
start_sha: mr.diff_refs.start_sha, | ||||||||
|
@@ -141,6 +158,8 @@ class GitLab implements Platform { | |||||||
new_path: path, | ||||||||
new_line: line, | ||||||||
}) | ||||||||
|
||||||||
return `${this.api.mergeRequestURL}#note_${thread.notes[0].id}`; | ||||||||
} | ||||||||
|
||||||||
updateInlineComment = async (comment: string, id: string): Promise<GitLabNote> => { | ||||||||
|
@@ -178,6 +197,17 @@ class GitLab implements Platform { | |||||||
) | ||||||||
} | ||||||||
|
||||||||
getDangerThreads = async (dangerID: string) => { | ||||||||
const { id: dangerUserId } = await this.api.getUser() | ||||||||
const threads = await this.api.getMergeRequestThreads(); | ||||||||
Comment on lines
+201
to
+202
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be resolved with Promise.all
Suggested change
|
||||||||
|
||||||||
return threads.filter( | ||||||||
(discussion: GitLabDiscussion) => { | ||||||||
return discussion.notes[0].author.id === dangerUserId && discussion.notes[0].body.includes(dangerIDToString(dangerID)); | ||||||||
} | ||||||||
) | ||||||||
} | ||||||||
|
||||||||
updateStatus = async (): Promise<boolean> => { | ||||||||
d("updateStatus", {}) | ||||||||
return true | ||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,18 @@ | ||
import { RepoMetaData } from "../../dsl/BitBucketServerDSL" | ||
import { api as fetch } from "../../api/fetch" | ||
import { | ||
GitLabApproval, | ||
GitLabDiscussion, | ||
GitLabDiscussionTextPosition, | ||
GitLabInlineNote, | ||
GitLabMR, | ||
GitLabMRChange, | ||
GitLabMRChanges, | ||
GitLabMRCommit, | ||
GitLabNote, | ||
GitLabUserProfile, | ||
GitLabRepositoryFile, | ||
GitLabRepositoryCompare, | ||
GitLabApproval, | ||
GitLabRepositoryFile, | ||
GitLabUserProfile, | ||
} from "../../dsl/GitLabDSL" | ||
|
||
import { Gitlab } from "gitlab" | ||
|
@@ -136,7 +137,7 @@ class GitLabAPI { | |
return inlineNotes | ||
} | ||
|
||
createMergeRequestDiscussion = async (content: string, position: GitLabDiscussionTextPosition): Promise<string> => { | ||
createMergeRequestDiscussion = async (content: string, position?: GitLabDiscussionTextPosition): Promise<GitLabDiscussion> => { | ||
this.d( | ||
"createMergeRequestDiscussion", | ||
this.repoMetadata.repoSlug, | ||
|
@@ -146,11 +147,15 @@ class GitLabAPI { | |
) | ||
const api = this.api.MergeRequestDiscussions | ||
|
||
const options: any = {}; | ||
|
||
if (position) { | ||
options.position = position; | ||
} | ||
|
||
try { | ||
// @ts-ignore | ||
const result: string = await api.create(this.repoMetadata.repoSlug, this.repoMetadata.pullRequestID, content, { | ||
position: position, | ||
}) | ||
const result = await api.create(this.repoMetadata.repoSlug, this.repoMetadata.pullRequestID, content, options) as GitLabDiscussion; | ||
this.d("createMergeRequestDiscussion", result) | ||
return result | ||
} catch (e) { | ||
|
@@ -159,6 +164,26 @@ class GitLabAPI { | |
} | ||
} | ||
|
||
updateMergeRequestDiscussion = async (discussionId: number, noteId: number, body: string): Promise<GitLabDiscussion> => { | ||
this.d( | ||
"updateMergeRequestDiscussion", | ||
this.repoMetadata.repoSlug, | ||
this.repoMetadata.pullRequestID, | ||
discussionId, | ||
noteId, | ||
body, | ||
) | ||
|
||
return await this.api.MergeRequestDiscussions.editNote( | ||
this.repoMetadata.repoSlug, | ||
this.repoMetadata.pullRequestID, | ||
discussionId, | ||
noteId, | ||
// @ts-ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hello Shym I got a question, Why you need this ts-ignore here? There is any cast issue with body property? |
||
body | ||
) as GitLabDiscussion; | ||
} | ||
|
||
createMergeRequestNote = async (body: string): Promise<GitLabNote> => { | ||
this.d("createMergeRequestNote", this.repoMetadata.repoSlug, this.repoMetadata.pullRequestID, body) | ||
const api = this.api.MergeRequestNotes | ||
|
@@ -239,6 +264,11 @@ class GitLabAPI { | |
const compare = (await api.compare(projectId, base, head)) as GitLabRepositoryCompare | ||
return compare.diffs | ||
} | ||
async getMergeRequestThreads(): Promise<GitLabDiscussion[]> { | ||
this.d('getMergeRequestThreads'); | ||
|
||
return await this.api.MergeRequestDiscussions.all(this.repoMetadata.repoSlug, this.repoMetadata.pullRequestID) as GitLabDiscussion[]; | ||
} | ||
} | ||
|
||
export default GitLabAPI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the type is infered?