Skip to content

Commit

Permalink
feat: replace PR "Request Changes" with a "API CHANGES REQUESTED" com…
Browse files Browse the repository at this point in the history
…ment (#163)
  • Loading branch information
itsananderson authored Aug 13, 2024
1 parent 50f8b15 commit 8482d7f
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 9 deletions.
49 changes: 45 additions & 4 deletions spec/api-review-state.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ describe('api review', () => {
data: [
{
user: { id: 1, login: 'ckerr' },
body: 'Fix this please!',
body: 'API CHANGES REQUESTED',
state: 'CHANGES_REQUESTED',
},
{
Expand All @@ -188,6 +188,11 @@ describe('api review', () => {
body: 'API DECLINED',
state: 'COMMENTED',
},
{
user: { id: 5, login: 'itsananderson' },
body: 'API CHANGES REQUESTED',
state: 'COMMENTED',
},
],
});

Expand All @@ -197,6 +202,7 @@ describe('api review', () => {
{ login: 'jkleinsc' },
{ login: 'nornagon' },
{ login: 'ckerr' },
{ login: 'itsananderson' },
],
});

Expand All @@ -212,6 +218,7 @@ describe('api review', () => {
#### Requested Changes
* @ckerr
* @itsananderson
#### Declined
* @jkleinsc
Expand All @@ -226,7 +233,7 @@ describe('api review', () => {
expect(users).toEqual({
approved: ['codebytere', 'nornagon'],
declined: ['jkleinsc'],
requestedChanges: ['ckerr'],
requestedChanges: ['ckerr', 'itsananderson'],
});
});

Expand All @@ -250,8 +257,8 @@ describe('api review', () => {
},
{
user: { id: 2, login: 'jkleinsc' },
body: 'This test is failing!!',
state: 'CHANGES_REQUESTED',
body: 'API CHANGES REQUESTED',
state: 'COMMENTED',
submitted_at: '2020-12-10T01:24:55Z',
},
],
Expand All @@ -269,6 +276,40 @@ describe('api review', () => {
});
});

it('should correctly parse user approvals when a reviewer requests changes and then approves', async () => {
const { pull_request } = loadFixture(
'api-review-state/pull_request.requested_review_label.json',
);

moctokit.pulls.listReviews.mockReturnValue({
data: [
{
user: { id: 2, login: 'marshallofsound' },
body: 'API CHANGES REQUESTED',
state: 'COMMENTED',
submitted_at: '2020-12-09T01:24:55Z',
},
{
user: { id: 2, login: 'marshallofsound' },
body: 'API LGTM',
state: 'COMMENTED',
submitted_at: '2020-12-10T01:24:55Z',
},
],
});

moctokit.teams.listMembersInOrg.mockReturnValue({
data: [{ login: 'marshallofsound' }],
});

const users = await addOrUpdateAPIReviewCheck(moctokit, pull_request);
expect(users).toEqual({
approved: ['marshallofsound'],
declined: [],
requestedChanges: [],
});
});

it(`should correctly update api review check for ${REVIEW_LABELS.APPROVED} label`, async () => {
const { pull_request } = loadFixture(
'api-review-state/pull_request.approved_review_label.json',
Expand Down
11 changes: 6 additions & 5 deletions src/api-review-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,14 +159,15 @@ export async function addOrUpdateAPIReviewCheck(octokit: Context['octokit'], pr:

const lgtm = /API LGTM/i;
const decline = /API DECLINED/i;
const changesRequested = /API CHANGES REQUESTED/i;

// Combine reviews/comments and filter by recency.
const filtered = ([...comments, ...reviews] as CommentOrReview[]).reduce((items, item) => {
if (!item?.body || !item.user) return items;

const changeRequest = item.state === REVIEW_STATUS.CHANGES_REQUESTED;
const reviewComment = lgtm.test(item.body) || decline.test(item.body);
if (!reviewComment && !changeRequest) return items;
const reviewComment =
lgtm.test(item.body) || decline.test(item.body) || changesRequested.test(item.body);
if (!reviewComment) return items;

const prev = items[item.user.id];
if (!prev) {
Expand Down Expand Up @@ -209,13 +210,13 @@ export async function addOrUpdateAPIReviewCheck(octokit: Context['octokit'], pr:
const approved = allReviews.filter((r) => r.body?.match(lgtm)).map((r) => r.user?.login);
const declined = allReviews.filter((r) => r.body?.match(decline)).map((r) => r.user?.login);
const requestedChanges = allReviews
.filter((r) => r.state === REVIEW_STATUS.CHANGES_REQUESTED)
.filter((r) => r.body?.match(changesRequested))
.map((r) => r.user?.login);

log(
'addOrUpdateAPIReviewCheck',
LogLevel.INFO,
`PR ${pr.number} has ${approved.length} API LGTMs, ${declined.length} API DECLINEDs, and ${requestedChanges.length} change requests`,
`PR ${pr.number} has ${approved.length} API LGTMs, ${declined.length} API DECLINEDs, and ${requestedChanges.length} API CHANGES REQUESTED`,
);

const users = { approved, declined, requestedChanges };
Expand Down

0 comments on commit 8482d7f

Please sign in to comment.