Fix a bug where teams aren't added as reviewers when the author comme…

…nts and the author is on the reviewer team (#87)

## Summary:
Previously, when Gerald would run, it wouldn't add a team to the reviewer list if any member of the team had already reviewed the PR.

The problem is that PR authors can add comments to their PRs, and those are "review" objects. So, if an author makes a draft PR, adds some comments, clicks "Ready for review", _and_ the author is a member of a team that Gerald would add, Gerald wouldn't add that team.

Very sneaky bug!! This fixes that. It still adds the team if the author commented (or any team member added a non-approving, non-rejecting review, such as a comment, or a pending review).

Issue: https://khanacademy.atlassian.net/browse/FEI-5620

## Test plan:
Buggy case:
1. Make a draft PR (in a repo where a team is always added, and you're a member of that team)
2. Make a comment on your PR
3. Click "Ready for review"
4. See that you team IS NOT added

Fixed cases:
1. Make a draft PR (in a repo where a team is always added, and you're a member of that team)
2. Make a comment on your PR
3. Click "Ready for review"
4. See that you team IS added

Fixed cases:
1. Make a draft PR (in a repo where a team is always added, and you're a member of that team)
2. Have another team-member make a comment on your PR
3. Click "Ready for review"
4. See that you team IS added

Fixed cases:
1. Make a draft PR (in a repo where a team is always added, and you're a member of that team)
2. Have another team-member APPROVE or REJECT your PR
3. Click "Ready for review"
4. See that you team IS NOT added

Some test PRs:
Khan/our-lovely-cli#673
Khan/our-lovely-cli#674
Khan/our-lovely-cli#675

Author: lillialexis

Reviewers: jeresig

Required Reviewers:

Approved By: jeresig

Checks: ⏭️  gerald, ⏭️  gerald, ✅ gerald, ⏭️  gerald, ⏭️  gerald, ✅ gerald, ✅ lint_and_unit, ✅ autofix, ✅ build_index

Pull Request URL: #87

src/runOnPullRequest.js

@@ -74,16 +74,42 @@ const updatePullRequestComment = async (
     }
 };
 
-const makeReviewRequests = async (reviewers: Array<string>, teamReviewers: Array<string>) => {
+const makeReviewRequests = async (
+    reviewers: Array<string>,
+    teamReviewers: Array<string>,
+    issuer: string,
+) => {
     // figure out who has already reviewed
     const {data: reviews} = await extraPermGithub.pulls.listReviews({
         ...ownerAndRepo,
         pull_number: context.issue.number,
     });
+
+    // List of any folks who have already reviewed, in any capacity, to be filtered out of the unfulfilled reviewers list
     const alreadyReviewed: Array<string> = reviews.map(review => review.user.login);
 
     // unfulfilled reviewers = everyone who hasn't reviewed
     const unfulfilledReviewers = reviewers.filter(reviewer => !alreadyReviewed.includes(reviewer));
+
+    if (reviewers.length > unfulfilledReviewers.length) {
+        console.log(
+            `Not adding ` +
+                `${reviewers.filter(r => !unfulfilledReviewers.includes(r)).join(', ')} ` +
+                `to the review request list as they have already reviewed.`,
+        );
+    }
+
+    // List of any folks who have reviewed meaningfully (approved or requested changes), to determine if the
+    // team reviewing has been fulfilled. Also, don't count the issuer, as they can add comments to their
+    // own PR, and be in the team.
+    const meaningfullyReviewed = reviews
+        .filter(
+            review =>
+                (review.state === 'APPROVED' || review.state === 'CHANGES_REQUESTED') &&
+                review.user.login !== issuer,
+        )
+        .map(review => review.user.login);
+
     const unfulfilledTeams = teamReviewers;
 
     for (const team of teamReviewers) {
@@ -94,9 +120,12 @@ const makeReviewRequests = async (reviewers: Array<string>, teamReviewers: Array
         const members = membership.map(member => member.login);
 
         // if a requested team has a review from a member, consider it fulfilled
-        for (const reviewer of alreadyReviewed) {
+        for (const reviewer of meaningfullyReviewed) {
             if (members.includes(reviewer)) {
                 unfulfilledTeams.splice(unfulfilledTeams.indexOf(team), 1);
+                console.log(
+                    `Not adding team ${team} to the review-request list as team member ${reviewer} has already reviewed.`,
+                );
                 break;
             }
         }
@@ -161,7 +190,12 @@ export const runOnPullRequest = async () => {
         {...ownerAndRepo, pull_number: context.issue.number},
         extraPermGithub,
     );
-    await makeReviewRequests(actualReviewers, teamReviewers);
+
+    await makeReviewRequests(
+        actualReviewers,
+        teamReviewers,
+        context.payload.pull_request.user.login,
+    );
 
     await updatePullRequestComment(megaComment, notified, reviewers, requiredReviewers);
 };