From cfbe63303aee9837be8ca0ed262910fdaf926b72 Mon Sep 17 00:00:00 2001 From: Soner Sayakci Date: Tue, 11 May 2021 19:26:04 +0000 Subject: [PATCH] Add thread support for GitLab --- source/commands/ci/runner.ts | 1 + source/commands/utils/sharedDangerfileArgs.ts | 3 ++ source/dsl/GitLabDSL.ts | 20 +++++++++ source/platforms/BitBucketCloud.ts | 8 ++++ source/platforms/BitBucketServer.ts | 8 ++++ source/platforms/FakePlatform.ts | 8 ++++ source/platforms/GitHub.ts | 8 ++++ source/platforms/GitLab.ts | 34 +++++++++++++- source/platforms/LocalGit.ts | 8 ++++ source/platforms/gitlab/GitLabAPI.ts | 44 ++++++++++++++++--- source/platforms/platform.ts | 6 ++- source/runner/Executor.ts | 10 ++++- 12 files changed, 147 insertions(+), 11 deletions(-) diff --git a/source/commands/ci/runner.ts b/source/commands/ci/runner.ts index 073e8c98e..970195080 100644 --- a/source/commands/ci/runner.ts +++ b/source/commands/ci/runner.ts @@ -71,6 +71,7 @@ export const runRunner = async (app: SharedCLI, config?: Partial) ignoreOutOfDiffComments: app.ignoreOutOfDiffComments, newComment: app.newComment || false, removePreviousComments: app.removePreviousComments || false, + useThread: app.useThread || false } const processName = (app.process && addSubprocessCallAguments(app.process.split(" "))) || undefined diff --git a/source/commands/utils/sharedDangerfileArgs.ts b/source/commands/utils/sharedDangerfileArgs.ts index da0351aa1..b1dda80f7 100644 --- a/source/commands/utils/sharedDangerfileArgs.ts +++ b/source/commands/utils/sharedDangerfileArgs.ts @@ -33,6 +33,8 @@ export interface SharedCLI extends program.CommanderStatic { newComment?: boolean /** Removes all previous comment and create a new one in the end of the list */ removePreviousComments?: boolean + /** Use thread instance of notes */ + useThread?: boolean } export default (command: any) => @@ -51,6 +53,7 @@ export default (command: any) => .option("--use-github-checks", "Use GitHub Checks", false) .option("--ignoreOutOfDiffComments", "Ignore inline-comments that are in lines which were not changed", false) .option("--newComment", "Makes Danger post a new comment instead of editing its previous one", false) + .option("--useThread", "Makes Danger post a thread instead of note", false) .option( "--removePreviousComments", "Removes all previous comment and create a new one in the end of the list", diff --git a/source/dsl/GitLabDSL.ts b/source/dsl/GitLabDSL.ts index c47705796..20d29ee34 100644 --- a/source/dsl/GitLabDSL.ts +++ b/source/dsl/GitLabDSL.ts @@ -292,3 +292,23 @@ export interface GitLabApproval { }[] | GitLabUser[] } + +export interface GitLabDiscussionAuthor { + id: number + name: string + username: string + state: boolean +} + +export interface GitLabDiscussionNote { + id: number + type: string | null + body: string + author: GitLabDiscussionAuthor +} + +export interface GitLabDiscussion { + id: number, + individual_note: boolean, + notes: GitLabDiscussionNote[] +} diff --git a/source/platforms/BitBucketCloud.ts b/source/platforms/BitBucketCloud.ts index c74c9cac7..0983713ef 100644 --- a/source/platforms/BitBucketCloud.ts +++ b/source/platforms/BitBucketCloud.ts @@ -67,6 +67,14 @@ export class BitBucketCloud implements Platform { return true } + supportsThreads() { + return false; + } + + updateOrCreateThread = async (): Promise => { + return undefined; + } + /** * Returns the response for the new comment * diff --git a/source/platforms/BitBucketServer.ts b/source/platforms/BitBucketServer.ts index 4084f9ff9..056def285 100644 --- a/source/platforms/BitBucketServer.ts +++ b/source/platforms/BitBucketServer.ts @@ -117,6 +117,14 @@ export class BitBucketServer implements Platform { return true } + supportsThreads() { + return false; + } + + updateOrCreateThread = async (): Promise => { + return undefined; + } + /** * Returns the response for the new comment * diff --git a/source/platforms/FakePlatform.ts b/source/platforms/FakePlatform.ts index 63d3f3425..376b62765 100644 --- a/source/platforms/FakePlatform.ts +++ b/source/platforms/FakePlatform.ts @@ -56,6 +56,14 @@ export class FakePlatform implements Platform { return true } + supportsThreads() { + return false; + } + + updateOrCreateThread = async (): Promise => { + return undefined; + } + async updateOrCreateComment(_dangerID: string, _newComment: string): Promise { return "https://github.com/orta/github-pages-with-jekyll/pull/5#issuecomment-383402256" } diff --git a/source/platforms/GitHub.ts b/source/platforms/GitHub.ts index 9fa51e8a8..3812b616a 100644 --- a/source/platforms/GitHub.ts +++ b/source/platforms/GitHub.ts @@ -75,6 +75,14 @@ export function GitHub(api: GitHubAPI) { getFileContents: api.fileContents, executeRuntimeEnvironment, + + supportsThreads() { + return false; + }, + + async updateOrCreateThread(): Promise { + return undefined; + } } as GitHubType } diff --git a/source/platforms/GitLab.ts b/source/platforms/GitLab.ts index fffcde0f8..343cbe55f 100644 --- a/source/platforms/GitLab.ts +++ b/source/platforms/GitLab.ts @@ -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 => { + 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}`; + } + } + updateOrCreateComment = async (dangerID: string, newComment: string): Promise => { 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 => { @@ -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(); + + return threads.filter( + (discussion: GitLabDiscussion) => { + return discussion.notes[0].author.id === dangerUserId && discussion.notes[0].body.includes(dangerIDToString(dangerID)); + } + ) + } + updateStatus = async (): Promise => { d("updateStatus", {}) return true diff --git a/source/platforms/LocalGit.ts b/source/platforms/LocalGit.ts index 265d10ad6..1824ea74a 100644 --- a/source/platforms/LocalGit.ts +++ b/source/platforms/LocalGit.ts @@ -71,6 +71,14 @@ export class LocalGit implements Platform { return true } + supportsThreads() { + return false; + } + + updateOrCreateThread = async (): Promise => { + return undefined; + } + async updateOrCreateComment(_dangerID: string, _newComment: string): Promise { return undefined } diff --git a/source/platforms/gitlab/GitLabAPI.ts b/source/platforms/gitlab/GitLabAPI.ts index 2e9637303..cf8cd5f4e 100644 --- a/source/platforms/gitlab/GitLabAPI.ts +++ b/source/platforms/gitlab/GitLabAPI.ts @@ -1,6 +1,8 @@ import { RepoMetaData } from "../../dsl/BitBucketServerDSL" import { api as fetch } from "../../api/fetch" import { + GitLabApproval, + GitLabDiscussion, GitLabDiscussionTextPosition, GitLabInlineNote, GitLabMR, @@ -8,10 +10,9 @@ import { 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 => { + createMergeRequestDiscussion = async (content: string, position?: GitLabDiscussionTextPosition): Promise => { 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 => { + 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 + body + ) as GitLabDiscussion; + } + createMergeRequestNote = async (body: string): Promise => { 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 { + this.d('getMergeRequestThreads'); + + return await this.api.MergeRequestDiscussions.all(this.repoMetadata.repoSlug, this.repoMetadata.pullRequestID) as GitLabDiscussion[]; + } } export default GitLabAPI diff --git a/source/platforms/platform.ts b/source/platforms/platform.ts index dd3f47652..e7fafa20e 100644 --- a/source/platforms/platform.ts +++ b/source/platforms/platform.ts @@ -69,6 +69,8 @@ export interface PlatformCommunicator { supportsCommenting: () => boolean /** Does the platform support inline comments? */ supportsInlineComments: () => boolean + /** Does the platform supports threads? */ + supportsThreads: () => boolean /** Allows the platform to do whatever it wants, instead of using the default commenting system */ handlePostingResults?: (results: DangerResults, options: ExecutorOptions) => void /** Gets inline comments for current PR */ @@ -85,6 +87,8 @@ export interface PlatformCommunicator { deleteMainComment: (dangerID: string) => Promise /** Replace the main Danger comment, returning the URL to the issue */ updateOrCreateComment: (dangerID: string, newComment: string) => Promise + /** Replaces the created thread */ + updateOrCreateThread: (dangerID: string, newComment: string) => Promise /** Sets the current PR's status */ updateStatus: ( passed: boolean | "pending", @@ -148,7 +152,7 @@ export function getPlatformForEnv(env: Env, source: CISource): Platform { // They need to set the token up for GitHub actions to work if (env["GITHUB_EVENT_NAME"] && !ghToken) { console.error(`You need to add GITHUB_TOKEN to your Danger action in the workflow: - + - name: Danger JS uses: danger/danger-js@X.Y.Z ${chalk.green(`env: diff --git a/source/runner/Executor.ts b/source/runner/Executor.ts index d541a6011..8eb7a6fb5 100644 --- a/source/runner/Executor.ts +++ b/source/runner/Executor.ts @@ -64,6 +64,8 @@ export interface ExecutorOptions { newComment?: boolean /** Removes all previous comment and create a new one in the end of the list */ removePreviousComments?: boolean + /** Uses threads instead of notes */ + useThread?: boolean } // This is still badly named, maybe it really should just be runner? @@ -314,7 +316,13 @@ export class Executor { comment = githubResultsTemplate(dangerID, mergedResults, commitID) } - if (this.options.newComment) { + if (this.options.useThread) { + if (!this.platform.supportsThreads) { + throw new Error(`Platform ${this.platform.name} does not support threads`); + } + + issueURL = await this.platform.updateOrCreateThread(dangerID, comment); + } else if (this.options.newComment) { issueURL = await this.platform.createComment(dangerID, comment) } else { issueURL = await this.platform.updateOrCreateComment(dangerID, comment)