Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-action] Fix bug: request status being overwritten #362

Merged
merged 1 commit into from
Nov 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 78 additions & 4 deletions libs/recnet-release-action/src/lib/github.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,19 +348,93 @@ describe("GitHubAPI", () => {
});

describe("requestReviewers", () => {
it("should request reviewers for a PR", async () => {
it("should request only new reviewers for a PR", async () => {
const prNumber = 1;
const reviewers = ["reviewer1", "reviewer2"];
const newReviewers = ["reviewer1", "reviewer2", "reviewer3"];
const existingReviewers = {
users: [{ login: "reviewer1" }],
};

await github.requestReviewers(prNumber, reviewers);
// Mock the GET request for current reviewers
mockOctokit.request.mockResolvedValueOnce({ data: existingReviewers });

await github.requestReviewers(prNumber, newReviewers);

// Verify GET request was called
expect(mockOctokit.request).toHaveBeenCalledWith(
"GET /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers",
{
owner: env.inputs.owner,
repo: env.inputs.repo,
pull_number: prNumber,
}
);

// Verify POST request was called with only new reviewers
expect(mockOctokit.request).toHaveBeenCalledWith(
"POST /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers",
{
owner: env.inputs.owner,
repo: env.inputs.repo,
pull_number: prNumber,
reviewers: ["reviewer2", "reviewer3"], // only new reviewers
}
);
});

it("should not make POST request if all reviewers are already requested", async () => {
const prNumber = 1;
const newReviewers = ["reviewer1", "reviewer2"];
const existingReviewers = {
users: [{ login: "reviewer1" }, { login: "reviewer2" }],
};

// Mock the GET request for current reviewers
mockOctokit.request.mockResolvedValueOnce({ data: existingReviewers });

await github.requestReviewers(prNumber, newReviewers);

// Verify only GET request was called
expect(mockOctokit.request).toHaveBeenCalledTimes(1);
expect(mockOctokit.request).toHaveBeenCalledWith(
"GET /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers",
{
owner: env.inputs.owner,
repo: env.inputs.repo,
pull_number: prNumber,
}
);
});

it("should request all reviewers if none are currently requested", async () => {
const prNumber = 1;
const newReviewers = ["reviewer1", "reviewer2"];
const existingReviewers = {
users: [],
};

// Mock the GET request for current reviewers
mockOctokit.request.mockResolvedValueOnce({ data: existingReviewers });

await github.requestReviewers(prNumber, newReviewers);

// Verify both requests were made
expect(mockOctokit.request).toHaveBeenCalledWith(
"GET /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers",
{
owner: env.inputs.owner,
repo: env.inputs.repo,
pull_number: prNumber,
}
);

expect(mockOctokit.request).toHaveBeenCalledWith(
"POST /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers",
{
owner: env.inputs.owner,
repo: env.inputs.repo,
pull_number: prNumber,
reviewers,
reviewers: newReviewers,
}
);
});
Expand Down
24 changes: 23 additions & 1 deletion libs/recnet-release-action/src/lib/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,35 @@ export class GitHubAPI {
}

async requestReviewers(prNumber: number, reviewers: string[]) {
// Get current reviewers
const { data: currentReviewers } = await this.octokit.request(
"GET /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers",
{
owner: this.owner,
repo: this.repo,
pull_number: prNumber,
}
);
// Get usernames of users who are currently requested
const currentReviewerLogins = new Set(
currentReviewers.users.map((user) => user.login)
);
// Filter out reviewers who have already reviewed or are already requested
const reviewersToAdd = reviewers.filter(
(reviewer) => !currentReviewerLogins.has(reviewer)
);

if (reviewersToAdd.length === 0) {
return;
}

await this.octokit.request(
"POST /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers",
{
owner: this.owner,
repo: this.repo,
pull_number: prNumber,
reviewers,
reviewers: reviewersToAdd,
}
);
}
Expand Down
Loading