Do not approve multiple times (#35)

5monkeys · Mar 17, 2023 · 5f98557 · 5f98557
1 parent f4c1bc4
commit 5f98557
Show file tree

Hide file tree

Showing 4 changed files with 113 additions and 15 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -0,0 +1,7 @@
+[*]
+charset = utf-8
+end_of_line = lf
+insert_final_newline = true
+indent_style = space
+indent_size = 2
+trim_trailing_whitespace = true
diff --git a/dist/index.js b/dist/index.js
@@ -8336,12 +8336,20 @@ async function validate(pullRequest) {
     }
     throw error;
   }
-  // Approve Release
-  await review(
-    pullRequest,
-    getReviewApproveEvent(),
-    core.getInput("valid_release_message")
-  );
+
+  // Approve Release if our latest review wasn't an approval
+  const approveEvent = getReviewApproveEvent();
+  const shouldApproveAgain =
+    approveEvent === REVIEW_APPROVE
+      ? !(await hasPreviouslyApproved(pullRequest))
+      : approveEvent === REVIEW_COMMENT;
+  if (shouldApproveAgain) {
+    await review(
+      pullRequest,
+      approveEvent,
+      core.getInput("valid_release_message")
+    );
+  }
   return pullRequest;
 }
 
@@ -8553,6 +8561,18 @@ async function release(pullRequest) {
   });
 }
 
+async function hasPreviouslyApproved(pullRequest) {
+  const userResponse = await client.rest.users.getAuthenticated();
+  const response = await client.rest.pulls.listReviews({
+    pull_number: pullRequest.number,
+    ...github.context.repo,
+  });
+  const lastReview = response.data
+    .filter((review) => review.user.login === userResponse.data.login)
+    .pop();
+  return lastReview ? lastReview.state === "APPROVED" : false;
+}
+
 class ValidationError extends Error {
   constructor(message) {
     super(message);
@@ -8562,6 +8582,7 @@ class ValidationError extends Error {
 
 module.exports = {
   action,
+  hasPreviouslyApproved,
   validate,
   validateTitle,
   validateBody,

diff --git a/src/action.js b/src/action.js
@@ -58,12 +58,20 @@ async function validate(pullRequest) {
     }
     throw error;
   }
-  // Approve Release
-  await review(
-    pullRequest,
-    getReviewApproveEvent(),
-    core.getInput("valid_release_message")
-  );
+
+  // Approve Release if our latest review wasn't an approval
+  const approveEvent = getReviewApproveEvent();
+  const shouldApproveAgain =
+    approveEvent === REVIEW_APPROVE
+      ? !(await hasPreviouslyApproved(pullRequest))
+      : approveEvent === REVIEW_COMMENT;
+  if (shouldApproveAgain) {
+    await review(
+      pullRequest,
+      approveEvent,
+      core.getInput("valid_release_message")
+    );
+  }
   return pullRequest;
 }
 
@@ -275,6 +283,18 @@ async function release(pullRequest) {
   });
 }
 
+async function hasPreviouslyApproved(pullRequest) {
+  const userResponse = await client.rest.users.getAuthenticated();
+  const response = await client.rest.pulls.listReviews({
+    pull_number: pullRequest.number,
+    ...github.context.repo,
+  });
+  const lastReview = response.data
+    .filter((review) => review.user.login === userResponse.data.login)
+    .pop();
+  return lastReview ? lastReview.state === "APPROVED" : false;
+}
+
 class ValidationError extends Error {
   constructor(message) {
     super(message);
@@ -284,6 +304,7 @@ class ValidationError extends Error {
 
 module.exports = {
   action,
+  hasPreviouslyApproved,
   validate,
   validateTitle,
   validateBody,

diff --git a/src/action.test.js b/src/action.test.js
@@ -6,6 +6,7 @@ const owner = "someowner";
 const repo = "somerepo";
 
 beforeEach(() => {
+  nock.cleanAll();
   process.env["INPUT_REPO_TOKEN"] = "hunter2";
   process.env["GITHUB_REPOSITORY"] = `${owner}/${repo}`;
   process.env["INPUT_TAG_TRANSFORMER"] = "title";
@@ -26,6 +27,7 @@ test("validate", async () => {
   const prNumber = "1";
   const tag = `release/${currentDate}`;
   const headSha = "deadbeef";
+  const ciUser = "our-ci-user";
   const pullRequest = {
     number: prNumber,
     title: currentDate,
@@ -43,7 +45,14 @@ test("validate", async () => {
     .post(`/repos/${owner}/${repo}/statuses/${headSha}`)
     .reply(200)
     .get(`/repos/${owner}/${repo}/releases/tags/${encodeURIComponent(tag)}`)
-    .reply(404);
+    .reply(404)
+    .get(`/user`)
+    .reply(200, { login: ciUser })
+    .get(`/repos/${owner}/${repo}/pulls/${prNumber}/reviews`)
+    .reply(200, [
+      { user: { login: ciUser }, state: "CHANGES_REQUESTED" },
+      { user: { login: "someone-else" }, state: "APPROVED" },
+    ]);
 
   const validatedPR = await validate(pullRequest);
   expect(validatedPR).toBe(pullRequest);
@@ -67,6 +76,7 @@ test("validateSkipping", async () => {
   const prNumber = "1";
   const tag = `release/${currentDate}`;
   const headSha = "deadbeef";
+  const ciUser = "our-ci-user";
   const pullRequest = {
     number: prNumber,
     title: currentDate,
@@ -76,7 +86,7 @@ test("validateSkipping", async () => {
     head: { ref: "dev", sha: headSha },
     validate: false,
   };
-  nock("https://api.github.com")
+  const scope = nock("https://api.github.com")
     .persist()
     .post(`/repos/${owner}/${repo}/issues/${prNumber}/labels`)
     .reply(200)
@@ -85,13 +95,21 @@ test("validateSkipping", async () => {
     .post(`/repos/${owner}/${repo}/statuses/${headSha}`)
     .reply(200)
     .get(`/repos/${owner}/${repo}/releases/tags/${encodeURIComponent(tag)}`)
-    .reply(404);
+    .reply(404)
+    .get(`/user`)
+    .reply(200, { login: ciUser })
+    .get(`/repos/${owner}/${repo}/pulls/${prNumber}/reviews`)
+    .reply(200, [
+      { user: { login: ciUser }, state: "CHANGES_REQUESTED" },
+      { user: { login: "someone-else" }, state: "APPROVED" },
+    ]);
 
   const validatedPR = await validate(pullRequest);
   expect(validatedPR).toBe(pullRequest);
   await expect(validate({ ...pullRequest, title: "badtitle" })).rejects.toThrow(
     /Invalid release title/
   );
+  scope.done();
 });
 
 test("getReviewApproveEvent", () => {
@@ -112,6 +130,37 @@ test("getReviewFailEvent", () => {
   expect(getReviewFailEvent()).toBe("COMMENT");
 });
 
+test("hasPreviouslyApproved", async () => {
+  const { hasPreviouslyApproved } = require("./action");
+  const prNumber = "1";
+  const ciUser = "our-ci-user";
+  const pullRequest = { number: prNumber };
+  const scope = nock("https://api.github.com")
+    .get(`/user`)
+    .reply(200, { login: ciUser })
+    .get(`/repos/${owner}/${repo}/pulls/${prNumber}/reviews`)
+    .reply(200, [
+      { user: { login: ciUser }, state: "CHANGES_REQUESTED" },
+      { user: { login: "someone-else" }, state: "APPROVED" },
+    ]);
+
+  expect(await hasPreviouslyApproved(pullRequest)).toBe(false);
+  scope.done();
+
+  const nextScope = nock("https://api.github.com")
+    .get(`/user`)
+    .reply(200, { login: ciUser })
+    .get(`/repos/${owner}/${repo}/pulls/${prNumber}/reviews`)
+    .reply(200, [
+      { user: { login: ciUser }, state: "CHANGES_REQUESTED" },
+      { user: { login: "someone-else" }, state: "APPROVED" },
+      { user: { login: ciUser }, state: "APPROVED" },
+    ]);
+
+  expect(await hasPreviouslyApproved(pullRequest)).toBe(true);
+  nextScope.done();
+});
+
 test("getTagName", () => {
   const { getTagName } = require("./action");