From a64a561698365262f05b359d5e8d410028594ed1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tadeusz=20So=C5=9Bnierz?= Date: Mon, 14 Aug 2023 14:58:21 +0200 Subject: [PATCH] Bridge Gitlab comment replies as Matrix threads (#758) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Bridge Gitlab comment replies as Matrix threads * Persistently store Gitlab Discussion-Thread mapping * Remove leftover debug line * Denoise comment descriptions when they happen in Matrix threads * Make comment debouncing time configurable * Add some tests for Gitlab comments * De-only Gitlab comment tests * Linting * Changelog * Map multiple Gitlab discussions to a single Matrix thread We debounce Gitlab comments, so multiple discussions can end up in one thread. This ensures that replies to *any* of these discussions end up in the same thread. * Add tests for the many-to-one reply case * Move SerializedGitlabDiscussionThreads to Types * Update changelog.d/758.feature Co-authored-by: Will Hunt * Fix instructions for validating your config using Docker (#794) * Fix instructions for validating your config using Docker Fixes GH-787 * Changelog --------- Co-authored-by: Tadeusz Sośnierz * Add more icons to GitHub messages (#795) * Add more icons to GitHub messages * Add merged icon * Lint * Add changelog * Bump word-wrap from 1.2.3 to 1.2.4 (#799) * Bump word-wrap from 1.2.3 to 1.2.4 Bumps [word-wrap](https://github.com/jonschlinkert/word-wrap) from 1.2.3 to 1.2.4. - [Release notes](https://github.com/jonschlinkert/word-wrap/releases) - [Commits](https://github.com/jonschlinkert/word-wrap/compare/1.2.3...1.2.4) --- updated-dependencies: - dependency-name: word-wrap dependency-type: indirect ... Signed-off-by: dependabot[bot] * Add changelog --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Andrew Ferrazzutti * Update matrix-appservice-bridge to 9.0.1 (#800) * Bump semver from 5.7.1 to 5.7.2 (#797) Bumps [semver](https://github.com/npm/node-semver) from 5.7.1 to 5.7.2. - [Release notes](https://github.com/npm/node-semver/releases) - [Changelog](https://github.com/npm/node-semver/blob/v5.7.2/CHANGELOG.md) - [Commits](https://github.com/npm/node-semver/compare/v5.7.1...v5.7.2) --- updated-dependencies: - dependency-name: semver dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Will Hunt * 4.4.0 * 4.4.1 * Set the default commentDebouncMs for Gitlab in its Config * Rename `approvalState` to something more fitting * Update sample config --------- Signed-off-by: dependabot[bot] Co-authored-by: Tadeusz Sośnierz Co-authored-by: Will Hunt Co-authored-by: Connor Davis Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Andrew Ferrazzutti --- changelog.d/758.feature | 1 + config.sample.yml | 4 + src/Connections/GitlabRepo.ts | 126 +++++++++++++++-------- src/Gitlab/Types.ts | 3 + src/Stores/MemoryStorageProvider.ts | 10 ++ src/Stores/RedisStorageProvider.ts | 12 +++ src/Stores/StorageProvider.ts | 3 + src/config/Config.ts | 10 ++ tests/connections/GitlabRepoTest.ts | 150 +++++++++++++++++++++++++--- tests/utils/IntentMock.ts | 13 ++- 10 files changed, 277 insertions(+), 55 deletions(-) create mode 100644 changelog.d/758.feature diff --git a/changelog.d/758.feature b/changelog.d/758.feature new file mode 100644 index 00000000..9c82cd53 --- /dev/null +++ b/changelog.d/758.feature @@ -0,0 +1 @@ +Bridge Gitlab comment replies as Matrix threads. diff --git a/config.sample.yml b/config.sample.yml index 846494de..ae00b017 100644 --- a/config.sample.yml +++ b/config.sample.yml @@ -49,6 +49,10 @@ gitlab: # (Optional) Prefix used when creating ghost users for GitLab accounts. _gitlab_ + commentDebounceMs: + # (Optional) Aggregate comments by waiting this many miliseconds before posting them to Matrix. Defaults to 5000 (5 seconds) + + 5000 figma: # (Optional) Configure this to enable Figma support diff --git a/src/Connections/GitlabRepo.ts b/src/Connections/GitlabRepo.ts index 3142a585..fe1ca33e 100644 --- a/src/Connections/GitlabRepo.ts +++ b/src/Connections/GitlabRepo.ts @@ -12,7 +12,7 @@ import { CommandConnection } from "./CommandConnection"; import { Connection, IConnection, IConnectionState, InstantiateConnectionOpts, ProvisionConnectionOpts } from "./IConnection"; import { ConnectionWarning, GetConnectionsResponseItem } from "../provisioning/api"; import { ErrCode, ApiError, ValidatorApiError } from "../api" -import { AccessLevel } from "../Gitlab/Types"; +import { AccessLevel, SerializedGitlabDiscussionThreads } from "../Gitlab/Types"; import Ajv, { JSONSchemaType } from "ajv"; import { CommandError } from "../errors"; import QuickLRU from "@alloc/quick-lru"; @@ -59,8 +59,6 @@ const log = new Logger("GitLabRepoConnection"); const md = new markdown(); const PUSH_MAX_COMMITS = 5; -const MRRCOMMENT_DEBOUNCE_MS = 5000; - export type GitLabRepoResponseItem = GetConnectionsResponseItem; @@ -205,7 +203,7 @@ export class GitLabRepoConnection extends CommandConnection>, {as, intent, tokenStore, config}: InstantiateConnectionOpts) { + static async createConnectionForState(roomId: string, event: StateEvent>, {as, intent, storage, tokenStore, config}: InstantiateConnectionOpts) { if (!config.gitlab) { throw Error('GitLab is not configured'); } @@ -214,7 +212,13 @@ export class GitLabRepoConnection extends CommandConnection, { as, config, intent, tokenStore, getAllConnectionsOfType }: ProvisionConnectionOpts) { + public static async provisionConnection( + roomId: string, + requester: string, + data: Record, + { as, config, intent, storage, tokenStore, getAllConnectionsOfType }: ProvisionConnectionOpts + ) { if (!config.gitlab) { throw Error('GitLab is not configured'); } @@ -260,7 +269,8 @@ export class GitLabRepoConnection extends CommandConnection c.roomId === roomId && c.instance.url === connection.instance.url && c.path === connection.path); @@ -382,21 +392,19 @@ export class GitLabRepoConnection extends CommandConnection(); - /** - * GitLab provides NO threading information in its webhook response objects, - * so we need to determine if we've seen a comment for a line before, and - * skip it if we have (because it's probably a reply). - */ - private readonly mergeRequestSeenDiscussionIds = new QuickLRU({ maxSize: 100 }); + private readonly discussionThreads = new QuickLRU>({ maxSize: 100}); + private readonly hookFilter: HookFilter; private readonly grantChecker; + private readonly commentDebounceMs: number; constructor( roomId: string, @@ -407,6 +415,7 @@ export class GitLabRepoConnection extends CommandConnection this.discussionThreads.has(discussionId)); + if (discussionWithThread) { + const threadEventId = await this.discussionThreads.get(discussionWithThread)!.catch(_ => { /* already logged */ }); + if (threadEventId) { + relation = { + "m.relates_to": { + "event_id": threadEventId, + "rel_type": "m.thread" + }, + }; + } } - let content = `**${result.author}** ${approvalState} MR [${orgRepoName}#${mergeRequest.iid}](${mergeRequest.url}): "${mergeRequest.title}"${comments}`; + let action = relation ? 'replied' : 'commented on'; // this is the only place we need this, approve/unapprove don't appear in discussions + if (result.approved === true) { + action = '✅ approved' + } else if (result.approved === false) { + action = '🔴 unapproved'; + } + + const target = relation ? '' : ` MR [${orgRepoName}#${mergeRequest.iid}](${mergeRequest.url}): "${mergeRequest.title}"`; + let content = `**${result.author}** ${action}${target} ${comments}`; if (result.commentNotes) { content += "\n\n> " + result.commentNotes.join("\n\n> "); } - this.intent.sendEvent(this.roomId, { + const eventPromise = this.intent.sendEvent(this.roomId, { msgtype: "m.notice", body: content, formatted_body: md.renderInline(content), format: "org.matrix.custom.html", + ...relation, }).catch(ex => { log.error('Failed to send MR review message', ex); + return undefined; + }); + + for (const discussionId of result.discussions) { + if (!this.discussionThreads.has(discussionId)) { + this.discussionThreads.set(discussionId, eventPromise); + } + } + void this.persistDiscussionThreads().catch(ex => { + log.error(`Failed to persistently store Gitlab discussion threads for connection ${this.connectionId}:`, ex); }); } @@ -770,6 +806,7 @@ ${data.description}`; commentCount: number, commentNotes?: string[], approved?: boolean, + discussionId?: string, /** * If the MR contains only comments, skip it. */ @@ -789,16 +826,20 @@ ${data.description}`; if (!opts.skip) { existing.skip = false; } - existing.timeout = setTimeout(() => this.renderDebouncedMergeRequest(uniqueId, mergeRequest, project), MRRCOMMENT_DEBOUNCE_MS); + if (opts.discussionId) { + existing.discussions.push(opts.discussionId); + } + existing.timeout = setTimeout(() => this.renderDebouncedMergeRequest(uniqueId, mergeRequest, project), this.commentDebounceMs); return; } this.debounceMRComments.set(uniqueId, { commentCount: commentCount, commentNotes: commentNotes, + discussions: opts.discussionId ? [opts.discussionId] : [], skip: opts.skip, approved, author: user.name, - timeout: setTimeout(() => this.renderDebouncedMergeRequest(uniqueId, mergeRequest, project), MRRCOMMENT_DEBOUNCE_MS), + timeout: setTimeout(() => this.renderDebouncedMergeRequest(uniqueId, mergeRequest, project), this.commentDebounceMs), }); } @@ -840,19 +881,6 @@ ${data.description}`; ); } - private shouldHandleMRComment(event: IGitLabWebhookNoteEvent) { - // Check to see if this line has had a comment before - if (event.object_attributes.discussion_id) { - if (this.mergeRequestSeenDiscussionIds.has(event.object_attributes.discussion_id)) { - // If it has, this is probably a reply. Skip repeated replies. - return false; - } - // Otherwise, record that we have seen the line and continue (it's probably a genuine comment). - this.mergeRequestSeenDiscussionIds.set(event.object_attributes.discussion_id, undefined); - } - return true; - } - public async onCommentCreated(event: IGitLabWebhookNoteEvent) { if (this.hookFilter.shouldSkip('merge_request', 'merge_request.review')) { return; @@ -862,13 +890,11 @@ ${data.description}`; // Not a MR comment return; } - if (!this.shouldHandleMRComment(event)) { - // Skip it. - return; - } + this.debounceMergeRequestReview(event.user, event.merge_request, event.project, { commentCount: 1, commentNotes: this.state.includeCommentBody ? [event.object_attributes.note] : undefined, + discussionId: event.object_attributes.discussion_id, skip: this.hookFilter.shouldSkip('merge_request.review.comments'), }); } @@ -912,6 +938,24 @@ ${data.description}`; } // TODO: Clean up webhooks } + + private setDiscussionThreads(discussionThreads: SerializedGitlabDiscussionThreads): void { + for (const { discussionId, eventId } of discussionThreads) { + this.discussionThreads.set(discussionId, Promise.resolve(eventId)); + } + } + + private async persistDiscussionThreads(): Promise { + const serialized: SerializedGitlabDiscussionThreads = []; + for (const [discussionId, eventIdPromise] of this.discussionThreads.entriesAscending()) { + const eventId = await eventIdPromise.catch(_ => { /* logged elsewhere */ }); + if (eventId) { + serialized.push({ discussionId, eventId }); + } + + } + return this.storage.setGitlabDiscussionThreads(this.connectionId, serialized); + } } // Typescript doesn't understand Prototypes very well yet. diff --git a/src/Gitlab/Types.ts b/src/Gitlab/Types.ts index 0ef55965..b7a29198 100644 --- a/src/Gitlab/Types.ts +++ b/src/Gitlab/Types.ts @@ -220,6 +220,9 @@ export interface ProjectHook extends ProjectHookOpts { created_at?: string; } +/** newest last, to enable feeding it straight into an LRU cache */ +export type SerializedGitlabDiscussionThreads = { discussionId: string, eventId: string }[]; + export interface SimpleProject { avatar_url?: string; description?: string; diff --git a/src/Stores/MemoryStorageProvider.ts b/src/Stores/MemoryStorageProvider.ts index b81b6fe5..257c1da3 100644 --- a/src/Stores/MemoryStorageProvider.ts +++ b/src/Stores/MemoryStorageProvider.ts @@ -3,6 +3,7 @@ import { IBridgeStorageProvider, MAX_FEED_ITEMS } from "./StorageProvider"; import { IssuesGetResponseData } from "../github/Types"; import { ProvisionSession } from "matrix-appservice-bridge"; import QuickLRU from "@alloc/quick-lru"; +import { SerializedGitlabDiscussionThreads } from "../Gitlab/Types"; export class MemoryStorageProvider extends MSP implements IBridgeStorageProvider { private issues: Map = new Map(); @@ -11,6 +12,7 @@ export class MemoryStorageProvider extends MSP implements IBridgeStorageProvider private figmaCommentIds: Map = new Map(); private widgetSessions: Map = new Map(); private storedFiles = new QuickLRU({ maxSize: 128 }); + private gitlabDiscussionThreads = new Map(); private feedGuids = new Map>(); constructor() { @@ -97,4 +99,12 @@ export class MemoryStorageProvider extends MSP implements IBridgeStorageProvider public async setStoredTempFile(key: string, value: string) { this.storedFiles.set(key, value); } + + public async getGitlabDiscussionThreads(connectionId: string): Promise { + return this.gitlabDiscussionThreads.get(connectionId) ?? []; + } + + public async setGitlabDiscussionThreads(connectionId: string, value: SerializedGitlabDiscussionThreads): Promise { + this.gitlabDiscussionThreads.set(connectionId, value); + } } diff --git a/src/Stores/RedisStorageProvider.ts b/src/Stores/RedisStorageProvider.ts index 859ded1f..4f2343ce 100644 --- a/src/Stores/RedisStorageProvider.ts +++ b/src/Stores/RedisStorageProvider.ts @@ -5,6 +5,7 @@ import { Logger } from "matrix-appservice-bridge"; import { IBridgeStorageProvider, MAX_FEED_ITEMS } from "./StorageProvider"; import { IFilterInfo, IStorageProvider } from "matrix-bot-sdk"; import { ProvisionSession } from "matrix-appservice-bridge"; +import { SerializedGitlabDiscussionThreads } from "../Gitlab/Types"; const BOT_SYNC_TOKEN_KEY = "bot.sync_token."; const BOT_FILTER_KEY = "bot.filter."; @@ -16,6 +17,7 @@ const GH_ISSUES_LAST_COMMENT_KEY = "gh.issues.last_comment"; const GH_ISSUES_REVIEW_DATA_KEY = "gh.issues.review_data"; const FIGMA_EVENT_COMMENT_ID = "figma.comment_event_id"; const STORED_FILES_KEY = "storedfiles."; +const GL_DISCUSSIONTHREADS_KEY = "gl.discussion-threads"; const STORED_FILES_EXPIRE_AFTER = 24 * 60 * 60; // 24 hours const COMPLETED_TRANSACTIONS_EXPIRE_AFTER = 24 * 60 * 60; // 24 hours const ISSUES_EXPIRE_AFTER = 7 * 24 * 60 * 60; // 7 days @@ -204,6 +206,16 @@ export class RedisStorageProvider extends RedisStorageContextualProvider impleme await this.redis.set(STORED_FILES_KEY + key, value); } + public async getGitlabDiscussionThreads(connectionId: string): Promise { + const key = `${GL_DISCUSSIONTHREADS_KEY}:${connectionId}`; + return JSON.parse(await this.redis.get(key) ?? '[]'); + } + + public async setGitlabDiscussionThreads(connectionId: string, value: SerializedGitlabDiscussionThreads): Promise { + const key = `${GL_DISCUSSIONTHREADS_KEY}:${connectionId}`; + await this.redis.set(key, JSON.stringify(value)); + } + public async storeFeedGuids(url: string, ...guid: string[]): Promise { const feedKey = `${FEED_GUIDS}${url}`; await this.redis.lpush(feedKey, ...guid); diff --git a/src/Stores/StorageProvider.ts b/src/Stores/StorageProvider.ts index af3ee7b7..73790ff9 100644 --- a/src/Stores/StorageProvider.ts +++ b/src/Stores/StorageProvider.ts @@ -1,6 +1,7 @@ import { ProvisioningStore } from "matrix-appservice-bridge"; import { IAppserviceStorageProvider, IStorageProvider } from "matrix-bot-sdk"; import { IssuesGetResponseData } from "../github/Types"; +import { SerializedGitlabDiscussionThreads } from "../Gitlab/Types"; // Some RSS feeds can return a very small number of items then bounce // back to their "normal" size, so we cannot just clobber the recent GUID list per request or else we'll @@ -22,6 +23,8 @@ export interface IBridgeStorageProvider extends IAppserviceStorageProvider, ISto getFigmaCommentEventId(roomId: string, figmaCommentId: string): Promise; getStoredTempFile(key: string): Promise; setStoredTempFile(key: string, value: string): Promise; + getGitlabDiscussionThreads(connectionId: string): Promise; + setGitlabDiscussionThreads(connectionId: string, value: SerializedGitlabDiscussionThreads): Promise; storeFeedGuids(url: string, ...guid: string[]): Promise; hasSeenFeed(url: string, ...guid: string[]): Promise; hasSeenFeedGuid(url: string, guid: string): Promise; diff --git a/src/config/Config.ts b/src/config/Config.ts index bee5942d..6c29fea1 100644 --- a/src/config/Config.ts +++ b/src/config/Config.ts @@ -193,6 +193,7 @@ export interface BridgeConfigGitLabYAML { }, instances: {[name: string]: GitLabInstance}; userIdPrefix: string; + commentDebounceMs?: number; } export class BridgeConfigGitLab { @@ -205,6 +206,9 @@ export class BridgeConfigGitLab { @configKey("Prefix used when creating ghost users for GitLab accounts.", true) readonly userIdPrefix: string; + @configKey("Aggregate comments by waiting this many miliseconds before posting them to Matrix. Defaults to 5000 (5 seconds)", true) + readonly commentDebounceMs: number; + constructor(yaml: BridgeConfigGitLabYAML) { this.instances = yaml.instances; this.webhook = yaml.webhook; @@ -216,6 +220,12 @@ export class BridgeConfigGitLab { this.instances[name].url = url.slice(0, -1); } } + + if (yaml.commentDebounceMs === undefined) { + this.commentDebounceMs = 5000; + } else { + this.commentDebounceMs = yaml.commentDebounceMs; + } } @hideKey() diff --git a/tests/connections/GitlabRepoTest.ts b/tests/connections/GitlabRepoTest.ts index 39e17b58..ee87d765 100644 --- a/tests/connections/GitlabRepoTest.ts +++ b/tests/connections/GitlabRepoTest.ts @@ -5,6 +5,8 @@ import { ApiError, ErrCode, ValidatorApiError } from "../../src/api"; import { GitLabRepoConnection, GitLabRepoConnectionState } from "../../src/Connections"; import { expect } from "chai"; import { BridgeConfigGitLab } from "../../src/config/Config"; +import { IBridgeStorageProvider } from "../../src/Stores/StorageProvider"; +import { IntentMock } from "../utils/IntentMock"; const ROOM_ID = "!foo:bar"; @@ -20,20 +22,40 @@ const GITLAB_MR = { title: "My MR", }; -const GITLAB_ISSUE_CREATED_PAYLOAD = { - object_kind: "merge_request", - user: { - name: "Alice", - username: "alice", - }, - object_attributes: GITLAB_MR, - project: { - path_with_namespace: `${GITLAB_ORG_REPO.org}/${GITLAB_ORG_REPO.repo}`, - web_url: `https://gitlab.example.com/${GITLAB_ORG_REPO.org}/${GITLAB_ORG_REPO.repo}`, - } +const GITLAB_USER = { + name: "Alice", + username: "alice", }; -function createConnection(state: Record = {}, isExistingState=false) { +const GITLAB_PROJECT = { + path_with_namespace: `${GITLAB_ORG_REPO.org}/${GITLAB_ORG_REPO.repo}`, + web_url: `https://gitlab.example.com/${GITLAB_ORG_REPO.org}/${GITLAB_ORG_REPO.repo}`, +}; + +const GITLAB_ISSUE_CREATED_PAYLOAD = { + object_kind: "merge_request", + user: GITLAB_USER, + object_attributes: GITLAB_MR, + project: GITLAB_PROJECT, +}; + +const GITLAB_MR_COMMENT = { + 'object_kind': 'note', + 'event_type': 'note', + 'merge_request': GITLAB_MR, + 'object_attributes': { + 'discussion_id': '6babfc4ad3be2355db286ed50be111a5220d5751', + 'note': 'I am starting a new thread', + 'noteable_type': 'MergeRequest', + 'url': 'https://gitlab.com/tadeuszs/my-awesome-project/-/merge_requests/2#note_1455087141' + }, + 'project': GITLAB_PROJECT, + 'user': GITLAB_USER, +}; + +const COMMENT_DEBOUNCE_MS = 25; + +function createConnection(state: Record = {}, isExistingState=false): { connection: GitLabRepoConnection, intent: IntentMock } { const mq = createMessageQueue({ monolithic: true }); @@ -44,7 +66,9 @@ function createConnection(state: Record = {}, isExistingState=f ROOM_ID, "state_key", as, - {} as BridgeConfigGitLab, + { + commentDebounceMs: COMMENT_DEBOUNCE_MS, + } as BridgeConfigGitLab, intent, GitLabRepoConnection.validateState({ instance: "bar", @@ -55,10 +79,18 @@ function createConnection(state: Record = {}, isExistingState=f { url: "https://gitlab.example.com" }, + { + setGitlabDiscussionThreads: () => Promise.resolve(), + getGitlabDiscussionThreads: () => Promise.resolve([]), + } as unknown as IBridgeStorageProvider, ); return {connection, intent}; } +async function waitForDebouncing(): Promise { + return new Promise(resolve => setTimeout(resolve, COMMENT_DEBOUNCE_MS * 2)); +} + describe("GitLabRepoConnection", () => { describe("validateState", () => { it("can validate a completes state config", () => { @@ -128,6 +160,98 @@ describe("GitLabRepoConnection", () => { }, true); }); }); + describe("onCommentCreated", () => { + it("will handle an MR comment", async () => { + const { connection, intent } = createConnection(); + await connection.onCommentCreated(GITLAB_MR_COMMENT as never); + await waitForDebouncing(); + intent.expectEventMatches( + (ev: any) => ev.content.body.includes('**Alice** commented on MR'), + 'event body indicates MR comment' + ); + }); + it("will debounce MR comments", async () => { + const { connection, intent } = createConnection(); + await connection.onCommentCreated(GITLAB_MR_COMMENT as never); + await connection.onCommentCreated({ + ...GITLAB_MR_COMMENT, + 'object_attributes': { + ...GITLAB_MR_COMMENT.object_attributes, + 'discussion_id': 'fa5d', + 'note': 'different comment', + }, + } as never); + await waitForDebouncing(); + expect(intent.sentEvents.length).to.equal(1); + intent.expectEventMatches( + (ev: any) => ev.content.body.includes('with 2 comments'), + 'one event sent for both comments', + 0, + ); + }); + it("will add new comments in a Matrix thread", async () => { + const { connection, intent } = createConnection(); + await connection.onCommentCreated(GITLAB_MR_COMMENT as never); + await waitForDebouncing(); + await connection.onCommentCreated(GITLAB_MR_COMMENT as never); + await waitForDebouncing(); + expect(intent.sentEvents.length).to.equal(2); + intent.expectEventMatches( + (ev: any) => ev.content['m.relates_to'].event_id === 'event_0', + 'one event sent for both comments', + 1, + ); + }); + it("will correctly map new comments to aggregated discussions", async () => { + const { connection, intent } = createConnection(); + await connection.onCommentCreated({ + ...GITLAB_MR_COMMENT, + 'object_attributes': { + ...GITLAB_MR_COMMENT.object_attributes, + 'discussion_id': 'disc1', + }, + } as never); + await connection.onCommentCreated({ + ...GITLAB_MR_COMMENT, + 'object_attributes': { + ...GITLAB_MR_COMMENT.object_attributes, + 'discussion_id': 'disc2', + }, + } as never); + await waitForDebouncing(); + expect(intent.sentEvents.length).to.equal(1); + + await connection.onCommentCreated({ + ...GITLAB_MR_COMMENT, + 'object_attributes': { + ...GITLAB_MR_COMMENT.object_attributes, + 'discussion_id': 'disc1', + }, + } as never); + await waitForDebouncing(); + expect(intent.sentEvents.length).to.equal(2); + intent.expectEventMatches( + (ev: any) => ev.content['m.relates_to'].event_id === 'event_0', + 'disc1 reply goes to existing thread', + 1 + ); + + await connection.onCommentCreated({ + ...GITLAB_MR_COMMENT, + 'object_attributes': { + ...GITLAB_MR_COMMENT.object_attributes, + 'discussion_id': 'disc2', + }, + } as never); + await waitForDebouncing(); + expect(intent.sentEvents.length).to.equal(3); + intent.expectEventMatches( + (ev: any) => ev.content['m.relates_to'].event_id === 'event_0', + 'disc2 reply also goes to existing thread', + 2 + ); + }); + }); describe("onIssueCreated", () => { it("will handle a simple issue", async () => { const { connection, intent } = createConnection(); diff --git a/tests/utils/IntentMock.ts b/tests/utils/IntentMock.ts index 2a976060..5877055b 100644 --- a/tests/utils/IntentMock.ts +++ b/tests/utils/IntentMock.ts @@ -68,11 +68,12 @@ export class IntentMock { }); } - sendEvent(roomId: string, content: any) { + sendEvent(roomId: string, content: any): Promise { this.sentEvents.push({ roomId, content, }); + return Promise.resolve(`event_${this.sentEvents.length - 1}`); } expectNoEvent() { @@ -87,10 +88,20 @@ export class IntentMock { body.includes(matcher), `Expected event body ${eventIndex} to match '${matcher}'.\nMessage was: '${body}'` ).to.be.true; + return; } expect(!!this.sentEvents.find(ev => ev.content.body.includes(matcher)), `Expected any event body to match '${matcher}'`).to.be.true; } + expectEventMatches(matcher: (content: any) => boolean, description: string, eventIndex?: number) { + if (eventIndex !== undefined) { + expect(this.sentEvents[eventIndex], `Expected event ${eventIndex} to exist`).to.not.be.undefined; + expect(matcher(this.sentEvents[eventIndex]), description).to.be.true; + return; + } + expect(this.sentEvents.some(ev => matcher(ev)), description).to.be.true; + } + async ensureJoined() { return true; }