chore(prlint): apply needs-maintainer-review label when PR is assigned

PRs can only be assigned by members with write access. If a PR has been assigned to someone, remove the `needs-community-review` label (since it implies a maintainer is already working on that PR) and apply the `needs-maintainer-review` label. In order to facilitate this, the `pull_request_target` event should also be run when a PR is assigned/unassigned.
aws · Oct 31, 2023 · 15415b8 · 15415b8
1 parent b545448
commit 15415b8
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 7 deletions.
diff --git a/.github/workflows/pr-linter.yml b/.github/workflows/pr-linter.yml
@@ -11,6 +11,8 @@ on:
       - opened
       - synchronize
       - reopened
+      - assigned
+      - unassigned
   workflow_run:
     workflows: [PR Linter Trigger]
     types:

diff --git a/tools/@aws-cdk/prlint/lint.ts b/tools/@aws-cdk/prlint/lint.ts
@@ -365,7 +365,7 @@ export class PullRequestLinter {
    *   7. It links to a p2 issue and has an approved community review
    */
   private async assessNeedsReview(
-    pr: Pick<GitHubPr, 'mergeable_state' | 'draft' | 'labels' | 'number'>,
+    pr: Pick<GitHubPr, 'mergeable_state' | 'draft' | 'labels' | 'number' | 'assignee' | 'assignees'>,
   ): Promise<void> {
     const reviews = await this.client.pulls.listReviews(this.prParams);
     console.log(JSON.stringify(reviews.data));
@@ -397,6 +397,10 @@ export class PullRequestLinter {
         && review.state === 'COMMENTED',
     );
 
+    // NOTE: assignment can only be performed by users with write access to the
+    // repository.
+    const isAssigned = (pr.assignee || pr.assignees?.length) ? true : false;
+
     const prLinterFailed = reviews.data.find((review) => review.user?.login === 'aws-cdk-automation' && review.state !== 'DISMISSED') as Review;
     const userRequestsExemption = pr.labels.some(label => (label.name === Exemption.REQUEST_EXEMPTION || label.name === Exemption.REQUEST_CLARIFICATION));
     console.log('evaluation: ', JSON.stringify({
@@ -408,6 +412,7 @@ export class PullRequestLinter {
       communityRequestedChanges,
       communityApproved,
       userRequestsExemption,
+      isAssigned,
     }, undefined, 2));
 
     const fixesP1 = pr.labels.some(label => label.name === 'p1');
@@ -430,10 +435,11 @@ export class PullRequestLinter {
     }
 
     // needs-maintainer-review means one of the following
-    // 1) fixes a p1 bug
-    // 2) is already community approved
-    // 3) is authored by a core team member
-    if (readyForReview && (fixesP1 || communityApproved || pr.labels.some(label => label.name === 'contribution/core'))) {
+    // 1) is assigned
+    // 2) fixes a p1 bug
+    // 3) is already community approved
+    // 4) is authored by a core team member
+    if (readyForReview && (isAssigned || fixesP1 || communityApproved || pr.labels.some(label => label.name === 'contribution/core'))) {
       this.addLabel('pr/needs-maintainer-review', pr);
       this.removeLabel('pr/needs-community-review', pr);
     } else if (readyForReview && !fixesP1) {

diff --git a/tools/@aws-cdk/prlint/test/lint.test.ts b/tools/@aws-cdk/prlint/test/lint.test.ts
@@ -514,8 +514,7 @@ describe('integration tests required on features', () => {
     test('with label no error', async () => {
       labels.push({ name: 'pr-linter/cli-integ-tested' });
       const prLinter = configureMock(issue, files);
-      await prLinter.validatePullRequestTarget(SHA);
-      // THEN: no exception
+      expect(prLinter.validatePullRequestTarget(SHA)).resolves;
     });
 
     test('with aws-cdk-automation author', async () => {
@@ -889,6 +888,65 @@ describe('integration tests required on features', () => {
       });
       expect(mockAddLabel.mock.calls).toEqual([]);
     });
+
+  });
+});
+
+describe('handling assignees', () => {
+  test('needs maintainer review when assigned', async () => {
+    // GIVEN
+    mockListReviews.mockImplementation(() => ({ data: [] }));
+    const assignee = { login: 'testassignee' }
+    const testPr: Subset<linter.GitHubPr>  = {
+      number: 1234,
+      assignee,
+      title: 'chore(prlint): sample message',
+      labels: [],
+    };
+
+    // WHEN
+    const prLinter = configureMock(testPr);
+    await prLinter.validatePullRequestTarget(SHA);
+
+    // THEN
+    expect(mockAddLabel.mock.calls[0][0]).toEqual({
+      issue_number: 1234,
+      labels: ['pr/needs-maintainer-review'],
+      owner: 'aws',
+      repo: 'aws-cdk',
+    });
+  });
+
+  test('assigning removes community label to apply maintainer', async () => {
+    // GIVEN
+    mockListReviews.mockImplementation(() => ({ data: [] }));
+    const assignee = { login: 'testassignee' }
+    const testPr: Subset<linter.GitHubPr> = {
+      number: 1234,
+      assignees: [assignee],
+      title: 'chore(prlint): sample message',
+      labels: [
+        { name: 'pr/needs-community-review' }
+      ],
+    };
+
+    // WHEN
+    const prLinter = configureMock(testPr);
+    await prLinter.validatePullRequestTarget(SHA);
+
+    // THEN
+    expect(mockRemoveLabel.mock.calls[0][0]).toEqual({
+      issue_number: 1234,
+      name: 'pr/needs-community-review',
+      owner: 'aws',
+      repo: 'aws-cdk',
+    });
+    expect(mockAddLabel.mock.calls[0][0]).toEqual({
+      issue_number: 1234,
+      labels: ['pr/needs-maintainer-review'],
+      owner: 'aws',
+      repo: 'aws-cdk'
+    });
   });
 });