From 71091b5f434f8db470fc03b3fa489f388de96d47 Mon Sep 17 00:00:00 2001
From: Julien Elbaz <elbywan@hotmail.com>
Date: Mon, 6 Jun 2022 10:33:09 +0200
Subject: [PATCH] Bot autocloses PRs when body is invalid (#250)

* ci: the bot now checks the body of prs

* chore: add pull request template description section
---
 .github/pull_request_template.md | 13 ++++----
 tools/github-bot/src/index.ts    | 56 +++++++++++++++++++-------------
 tools/github-bot/src/tools.ts    | 41 +++++++++++++++++++++++
 3 files changed, 81 insertions(+), 29 deletions(-)

diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md
index d8fbcf562140..ea7766fdeffe 100644
--- a/.github/pull_request_template.md
+++ b/.github/pull_request_template.md
@@ -4,19 +4,20 @@ Please make sure to read CONTRIBUTING.md if you have not already.
 Disclaimer: Pull Requests that do not comply with the rules will be arbitrarily closed.
 -->
 
+### 📝 Description
+
 _Replace this text by a clear and concise description of what this pull request is about and why it is needed._
 
 ### ❓ Context
 
-- **Impacted projects**: ` ` <!-- precise end user projects impacted -->
-- **Linked resource(s)**: ` ` <!-- attach any ticket number if relevant (JIRA / Github issue number) -->
-
+- **Impacted projects**: `` <!-- The list of end user projects impacted by the change. -->
+- **Linked resource(s)**: `` <!-- Attach any ticket number if relevant. (JIRA / Github issue number) -->
 
 ### ✅ Checklist
 
-- [ ] **Test coverage**. <!-- Are your changes covered by tests? Features must be tested. bugfixes must bring the test that would have detected the bug. -->
-- [ ] **Atomic delivery**. <!-- Is this pull request standalone? In order words, does it depend on nothing else? Please explain if not checked. -->
-- [ ] **No breaking changes**. <!-- If there are breaking changes, please explain why. -->
+- [ ] **Test coverage** <!-- Are your changes covered by tests? Features must be tested, bugfixes must include a test that would have detected the issue. -->
+- [ ] **Atomic delivery** <!-- Is this pull request standalone? In order words, does it depend on nothing else? Please explain if not checked. -->
+- [ ] **No breaking changes** <!-- If there are breaking changes, please explain why. -->
 
 ### 📸 Demo
 
diff --git a/tools/github-bot/src/index.ts b/tools/github-bot/src/index.ts
index c649a335dc7b..4818d792d190 100644
--- a/tools/github-bot/src/index.ts
+++ b/tools/github-bot/src/index.ts
@@ -1,5 +1,5 @@
 import { Probot } from "probot";
-import { commands, isValidBranchName, isValidUser } from "./tools";
+import { commands, isValidBody, isValidBranchName, isValidUser } from "./tools";
 
 export default (app: Probot) => {
   commands(app, "generate-screenshots", async (context, data) => {
@@ -28,34 +28,44 @@ export default (app: Probot) => {
   app.on(["pull_request.opened", "pull_request.reopened"], async (context) => {
     const { payload, octokit } = context;
     const repository = context.repo();
+
     const branch = payload.pull_request.head.ref;
     const login = payload.pull_request.user.login;
+
+    if (!isValidUser(login)) return;
+
     const isBranchValid = isValidBranchName(branch);
-    const isUserValid = isValidUser(login);
-    let body = "";
-    let comment;
+    const isBodyValid = isValidBody(payload.pull_request.body);
+
+    let body =
+      `❌ @${login}\n\n` +
+      "#### Unfortunately this PR does not comply with the [Contributing Conventions](https://github.com/LedgerHQ/ledger-live/blob/develop/CONTRIBUTING.md) and will be closed automatically.\n" +
+      "\n" +
+      "Feel free to reopen this PR once you have browsed through the guidelines.\n" +
+      "\n" +
+      "-------\n" +
+      "\n" +
+      "Found Issues:\n";
 
-    if (!isUserValid) return;
+    let comment;
 
     if (!isBranchValid) {
-      body = `@${login}
-      Unfortunately this branch name (**${branch}**) does not follow the [CONTRIBUTING.MD](https://github.com/LedgerHQ/ledger-live/blob/develop/CONTRIBUTING.md) conventions and will be closed automatically.
-      Feel free to reopen this PR once you have browsed through the guidelines.
-      `;
-
-      comment = context.issue({
-        body,
-      });
-
-      await octokit.issues.createComment(comment);
-      await octokit.pulls.update({
-        owner: repository.owner,
-        repo: repository.repo,
-        pull_number: payload.number,
-        state: "closed",
-      });
-
-      return;
+      body += `- _the branch name \`${branch}\` is invalid_\n`;
     }
+
+    if (!isBodyValid) {
+      body += `- _you overrode or did not fill in the [pull request template](https://github.com/LedgerHQ/ledger-live/blob/develop/.github/pull_request_template.md) properly_\n`;
+    }
+    comment = context.issue({
+      body,
+    });
+
+    await octokit.issues.createComment(comment);
+    await octokit.pulls.update({
+      owner: repository.owner,
+      repo: repository.repo,
+      pull_number: payload.number,
+      state: "closed",
+    });
   });
 };
diff --git a/tools/github-bot/src/tools.ts b/tools/github-bot/src/tools.ts
index a4fa11253485..8592e07ae293 100644
--- a/tools/github-bot/src/tools.ts
+++ b/tools/github-bot/src/tools.ts
@@ -7,6 +7,47 @@ export const isValidBranchName = (branch: string): boolean =>
 export const isValidUser = (user: string): boolean =>
   !["ledgerlive", "live-github-bot[bot]"].includes(user);
 
+export const isValidBody = (body: string | null): boolean => {
+  if (!body) return false;
+
+  const description =
+    "_Replace this text by a clear and concise description of what this pull request is about and why it is needed._";
+
+  const requiredHeadings = [
+    "### 📝 Description",
+    "### ❓ Context",
+    "### ✅ Checklist",
+    "### 🚀 Expectations to reach",
+  ];
+
+  const results = body.split("\r\n").reduce(
+    (acc, line) => {
+      // Dummy description line has not been replaced.
+      if (line === description) {
+        return {
+          ...acc,
+          dummyDescription: true,
+        };
+      }
+
+      const headingIndex = requiredHeadings.indexOf(line);
+      // Template required heading is still in the body.
+      if (headingIndex > -1) {
+        acc.matchHeadings[headingIndex] = true;
+        return acc;
+      }
+
+      return acc;
+    },
+    {
+      dummyDescription: false,
+      matchHeadings: requiredHeadings.map(() => false),
+    }
+  );
+
+  return !results.dummyDescription && results.matchHeadings.every(Boolean);
+};
+
 /**
  *  commands is a helper to wrap the validation of some text from issues commented on PR and PR messages
  *  so we can trigger specific actions