feat: review comments in threads

christophehurpeau · Feb 20, 2022 · 69b8cbe · 69b8cbe
1 parent 0780443
commit 69b8cbe
Show file tree

Hide file tree

Showing 10 changed files with 387 additions and 154 deletions.
diff --git a/.yarn/cache/delay-npm-5.0.0-1d1c758b46-62f151151e.zip b/.yarn/cache/delay-npm-5.0.0-1d1c758b46-62f151151e.zip
diff --git a/package.json b/package.json
@@ -97,6 +97,7 @@
     "body-parser": "1.19.1",
     "conventional-changelog-conventionalcommits": "4.6.3",
     "cookie-parser": "1.4.6",
+    "delay": "5.0.0",
     "dotenv": "8.6.0",
     "emoji-regex": "10.0.0",
     "issue-parser": "6.0.0",

diff --git a/src/context/repoContext.ts b/src/context/repoContext.ts
@@ -90,12 +90,14 @@ interface RepoContextWithoutTeamContext<GroupNames extends string> {
   hasApprovesReview: (labels: PullRequestLabels) => boolean;
   getNeedsReviewGroupNames: (labels: PullRequestLabels) => GroupNames[];
   lockPullRequest: (
+    eventName: string,
     pullRequest: PullRequestDataMinimumData,
     callback: () => Promise<void> | void,
   ) => Promise<void>;
 
   /** @deprecated */
   lockPR: (
+    eventName: string,
     prId: string,
     prNumber: number,
     callback: () => Promise<void> | void,
@@ -300,13 +302,15 @@ async function initRepoContext<
   };
 
   const lockPR = (
+    eventName: string,
     prOrPrIssueId: string,
     prNumber: number,
     callback: () => Promise<void> | void,
   ): Promise<void> =>
     new Promise((resolve, reject) => {
       const prNumberAsString = String(prNumber);
       const logInfos = {
+        eventName,
         repo: fullName,
         prOrPrIssueId,
         prNumber,
@@ -331,10 +335,16 @@ async function initRepoContext<
     });
 
   const lockPullRequest = (
+    eventName: string,
     pullRequest: PullRequestDataMinimumData,
     callback: () => Promise<void> | void,
   ): Promise<void> => {
-    return lockPR(String(pullRequest.id), pullRequest.number, callback);
+    return lockPR(
+      eventName,
+      String(pullRequest.id),
+      pullRequest.number,
+      callback,
+    );
   };
 
   const removePrFromAutomergeQueue = async (
@@ -387,8 +397,8 @@ async function initRepoContext<
     const timeout = setTimeout(
       () => {
         // eslint-disable-next-line @typescript-eslint/no-floating-promises
-        lockPR('reschedule', -1, () => {
-          return lockPR(String(pr.id), pr.number, async () => {
+        lockPR('reschedule', 'reschedule', -1, () => {
+          return lockPR('reschedule', String(pr.id), pr.number, async () => {
             try {
               const [pullRequest, reviewflowPrContext] = await Promise.all([
                 fetchPr(context, pr.number),

diff --git a/src/context/slack/SlackMessage.ts b/src/context/slack/SlackMessage.ts
@@ -4,5 +4,5 @@ export interface SlackMessage {
   text: string;
   blocks?: KnownBlock[];
   secondaryBlocks?: KnownBlock[];
-  ts?: string;
+  threadTs?: string;
 }
diff --git a/src/context/slack/initTeamSlack.ts b/src/context/slack/initTeamSlack.ts
@@ -174,7 +174,9 @@ export const initTeamSlack = async <GroupNames extends string>(
       }
       const user = membersMap.get(githubLogin);
       if (!user) return githubLogin;
-      return `<@${user.member.id}>`;
+      return process.env.REVIEWFLOW_DEBUG
+        ? `<@${user.member.id}> (${githubLogin})`
+        : `<@${user.member.id}>`;
     },
     postMessage: async (
       category: MessageCategory,
@@ -214,12 +216,16 @@ export const initTeamSlack = async <GroupNames extends string>(
       const result = await user.slackClient.chat.postMessage({
         username: process.env.REVIEWFLOW_NAME,
         channel: user.im.id,
-        text: message.text,
+        text: process.env.REVIEWFLOW_DEBUG
+          ? `${message.text} (${category})`
+          : message.text,
         blocks: message.blocks,
         attachments: message.secondaryBlocks
           ? [{ blocks: message.secondaryBlocks }]
           : undefined,
-        thread_ts: message.ts,
+        thread_ts: message.threadTs,
+        unfurl_links: false,
+        unfurl_media: false,
       });
       if (!result.ok) return null;
       return {