fix: comment username could be random (#156)

close #145

Author: JounQin

.changeset/early-badgers-love.md

@@ -0,0 +1,5 @@
+---
+"changesets-gitlab": patch
+---
+
+fix: use discussion or note API accordingly

.changeset/eight-tools-sing.md

@@ -0,0 +1,5 @@
+---
+"changesets-gitlab": patch
+---
+
+fix: comment username could be random - close #145

src/comment.ts

@@ -5,14 +5,21 @@ import type {
   VersionType,
 } from '@changesets/types'
 import type { Gitlab } from '@gitbeaker/core'
-import type { MergeRequestChangesSchema } from '@gitbeaker/rest'
+import type {
+  DiscussionNoteSchema,
+  DiscussionSchema,
+  MergeRequestChangesSchema,
+  MergeRequestNoteSchema,
+  NoteSchema,
+} from '@gitbeaker/rest'
 import { captureException } from '@sentry/node'
 import { humanId } from 'human-id'
 import { markdownTable } from 'markdown-table'
 
 import * as context from './context.js'
 import { env } from './env.js'
 import { getChangedPackages } from './get-changed-packages.js'
+import type { LooseString } from './types.js'
 import { getUsername } from './utils.js'
 
 import { createApi } from './index.js'
@@ -104,32 +111,89 @@ ${changedPackages.map(x => `"${x}": patch`).join('\n')}
 ${title}
 `)
 
-const getNoteInfo = (api: Gitlab, mrIid: number | string) =>
-  api.MergeRequestDiscussions.all(context.projectId, mrIid).then(
-    async discussions => {
-      for (const discussion of discussions) {
-        if (!discussion.notes) {
-          continue
+const isMrNote = (
+  discussionOrNote: DiscussionSchema | MergeRequestNoteSchema,
+): discussionOrNote is MergeRequestNoteSchema =>
+  'noteable_type' in discussionOrNote &&
+  discussionOrNote.noteable_type === 'MergeRequest'
+
+const RANDOM_BOT_NAME_PATTERN = /^((?:project|group)_\d+_bot\w*)_[\da-z]+$/i
+
+const isChangesetBotNote = (
+  note: DiscussionNoteSchema | NoteSchema,
+  username: string,
+  random?: boolean,
+) =>
+  (note.author.username === username ||
+    (random &&
+      note.author.username.match(RANDOM_BOT_NAME_PATTERN)?.[1] === username)) &&
+  // We need to ensure the note is generated by us, but we don't have an app bot like GitHub
+  // @see https://github.com/apps/changeset-bot
+  note.body.includes(generatedByBotNote)
+
+async function getNoteInfo(
+  api: Gitlab,
+  mrIid: number | string,
+  commentType: LooseString<'discussion'>,
+  random?: boolean,
+): Promise<{ discussionId: string; noteId: number } | null | undefined>
+async function getNoteInfo(
+  api: Gitlab,
+  mrIid: number | string,
+  commentType: LooseString<'note'>,
+  random?: boolean,
+): Promise<{ noteId: number } | null | undefined>
+async function getNoteInfo(
+  api: Gitlab,
+  mrIid: number | string,
+  commentType: LooseString<'discussion' | 'note'>,
+  random?: boolean,
+): Promise<
+  | { discussionId: string; noteId: number }
+  | { noteId: number }
+  | null
+  | undefined
+> {
+  const discussionOrNotes = await (commentType === 'discussion'
+    ? api.MergeRequestDiscussions.all(context.projectId, mrIid)
+    : api.MergeRequestNotes.all(context.projectId, +mrIid))
+
+  const username = await getUsername(api)
+
+  for (const discussionOrNote of discussionOrNotes) {
+    if (isMrNote(discussionOrNote)) {
+      if (isChangesetBotNote(discussionOrNote, username, random)) {
+        return {
+          noteId: discussionOrNote.id,
         }
+      }
+      continue
+    }
 
-        const username = await getUsername(api)
-        const changesetBotNote = discussion.notes.find(
-          note =>
-            note.author.username === username &&
-            // We need to ensure the note is generated by us, but we don't have an app bot like GitHub
-            // @see https://github.com/apps/changeset-bot
-            note.body.includes(generatedByBotNote),
-        )
+    if (!discussionOrNote.notes) {
+      continue
+    }
 
-        if (changesetBotNote) {
-          return {
-            discussionId: discussion.id,
-            noteId: changesetBotNote.id,
-          }
-        }
+    const changesetBotNote = discussionOrNote.notes.find(note =>
+      isChangesetBotNote(note, username),
+    )
+
+    if (changesetBotNote) {
+      return {
+        discussionId: discussionOrNote.id,
+        noteId: changesetBotNote.id,
       }
-    },
-  )
+    }
+  }
+
+  /**
+   * The `username` used for commenting could be random, if we haven't tested the random `username`, then test it
+   *
+   * @see https://docs.gitlab.com/ee/development/internal_users.html
+   * @see https://github.com/un-ts/changesets-gitlab/issues/145#issuecomment-1860610958
+   */
+  return random ? null : getNoteInfo(api, mrIid, commentType, true)
+}
 
 const hasChangesetBeenAdded = async (
   changedFilesPromise: Promise<MergeRequestChangesSchema>,
@@ -176,7 +240,7 @@ export const comment = async () => {
 
     const [noteInfo, hasChangeset, { changedPackages, releasePlan }] =
       await Promise.all([
-        getNoteInfo(api, mrIid),
+        getNoteInfo(api, mrIid, GITLAB_COMMENT_TYPE),
         hasChangesetBeenAdded(changedFilesPromise),
         getChangedPackages({
           changedFiles: changedFilesPromise.then(x =>
@@ -217,42 +281,44 @@ export const comment = async () => {
         : getAbsentMessage(latestCommitSha, addChangesetUrl, releasePlan)) +
       errFromFetchingChangedFiles
 
-    if (GITLAB_COMMENT_TYPE === 'discussion') {
-      if (noteInfo) {
-        return api.MergeRequestDiscussions.editNote(
+    switch (GITLAB_COMMENT_TYPE) {
+      case 'discussion': {
+        if (noteInfo) {
+          return api.MergeRequestDiscussions.editNote(
+            context.projectId,
+            mrIid,
+            noteInfo.discussionId,
+            noteInfo.noteId,
+            {
+              body: prComment,
+            },
+          )
+        }
+
+        return api.MergeRequestDiscussions.create(
           context.projectId,
           mrIid,
-          noteInfo.discussionId,
-          noteInfo.noteId,
-          {
-            body: prComment,
-          },
+          prComment,
         )
       }
+      case 'note': {
+        if (noteInfo) {
+          return api.MergeRequestNotes.edit(
+            context.projectId,
+            mrIid,
+            noteInfo.noteId,
+            { body: prComment },
+          )
+        }
 
-      return api.MergeRequestDiscussions.create(
-        context.projectId,
-        mrIid,
-        prComment,
-      )
-    }
-
-    if (GITLAB_COMMENT_TYPE === 'note') {
-      if (noteInfo) {
-        return api.MergeRequestNotes.edit(
-          context.projectId,
-          mrIid,
-          noteInfo.noteId,
-          { body: prComment },
+        return api.MergeRequestNotes.create(context.projectId, mrIid, prComment)
+      }
+      default: {
+        throw new Error(
+          `Invalid comment type "${GITLAB_COMMENT_TYPE}", should be "discussion" or "note"`,
         )
       }
-
-      return api.MergeRequestNotes.create(context.projectId, mrIid, prComment)
     }
-
-    throw new Error(
-      `Invalid comment type "${GITLAB_COMMENT_TYPE}", should be "discussion" or "note"`,
-    )
   } catch (err: unknown) {
     console.error(err)
     throw err

src/env.ts

@@ -29,6 +29,6 @@ export const env = {
         setFailed('Please add the `GITLAB_TOKEN` to the changesets action')
       }
     }
-    return process.env.GITLAB_TOKEN as string
+    return process.env.GITLAB_TOKEN!
   },
 } as Env