Skip to content
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
31 changes: 25 additions & 6 deletions src/features/pr-conflict-indicator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ describe("injectPRConflictIndicator", () => {

it("checks only visible PRs and renders a conflict status without creating a label", async () => {
vi.mocked(fetchPRConflictStatuses).mockResolvedValue([
{ number: 7, mergeable: "CONFLICTING" },
{ number: 8, mergeable: "MERGEABLE" },
{ number: 7, state: "OPEN", mergeable: "CONFLICTING" },
{ number: 8, state: "OPEN", mergeable: "MERGEABLE" },
]);

injectPRConflictIndicator();
Expand Down Expand Up @@ -102,7 +102,7 @@ describe("injectPRConflictIndicator", () => {
await vi.waitFor(() => expect(fetchPRConflictStatuses).toHaveBeenCalledTimes(1));

injectPRConflictIndicator();
resolveStatuses([{ number: 7, mergeable: "CONFLICTING" }]);
resolveStatuses([{ number: 7, state: "OPEN", mergeable: "CONFLICTING" }]);

await vi.waitFor(() =>
expect(row7.querySelector(".better-github-conflict-indicator")).not.toBeNull(),
Expand All @@ -111,10 +111,10 @@ describe("injectPRConflictIndicator", () => {

it("retries unknown and missing statuses on the next polling pass", async () => {
vi.mocked(fetchPRConflictStatuses)
.mockResolvedValueOnce([{ number: 7, mergeable: "UNKNOWN" }])
.mockResolvedValueOnce([{ number: 7, state: "OPEN", mergeable: "UNKNOWN" }])
.mockResolvedValueOnce([
{ number: 7, mergeable: "CONFLICTING" },
{ number: 8, mergeable: "MERGEABLE" },
{ number: 7, state: "OPEN", mergeable: "CONFLICTING" },
{ number: 8, state: "OPEN", mergeable: "MERGEABLE" },
]);

injectPRConflictIndicator();
Expand All @@ -137,4 +137,23 @@ describe("injectPRConflictIndicator", () => {
await vi.waitFor(() => expect(fetchPRConflictStatuses).toHaveBeenCalledTimes(2));
expect(row7.querySelector(".better-github-conflict-indicator")).not.toBeNull();
});

it("ignores merged stacked PR conflicts without retrying them", async () => {
vi.mocked(fetchPRConflictStatuses).mockResolvedValue([
{ number: 7, state: "MERGED", mergeable: "CONFLICTING" },
]);

injectPRConflictIndicator();

const row7 = document.getElementById("issue_7")!;
observerCallback(
[{ target: row7, isIntersecting: true }] as unknown as IntersectionObserverEntry[],
{} as IntersectionObserver,
);
await vi.waitFor(() => expect(fetchPRConflictStatuses).toHaveBeenCalledTimes(1));

injectPRConflictIndicator();
expect(observe.mock.calls.filter(([row]) => row === row7)).toHaveLength(1);
expect(row7.querySelector(".better-github-conflict-indicator")).toBeNull();
});
});
8 changes: 4 additions & 4 deletions src/features/pr-conflict-indicator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ async function checkRows(
const statuses = await fetchPRConflictStatuses(owner, repo, [...rowByNumber.keys()]);
if (currentGeneration !== generation) return;

const statusByNumber = new Map(statuses.map(({ number, mergeable }) => [number, mergeable]));
const statusByNumber = new Map(statuses.map((status) => [status.number, status]));
for (const [number, row] of rowByNumber) {
const mergeable = statusByNumber.get(number);
if (mergeable !== "MERGEABLE" && mergeable !== "CONFLICTING") {
const status = statusByNumber.get(number);
if (!status || (status.state === "OPEN" && status.mergeable === "UNKNOWN")) {
checkedRows.delete(row);
continue;
}
if (mergeable !== "CONFLICTING") continue;
if (status.state !== "OPEN" || status.mergeable !== "CONFLICTING") continue;

if (!row?.isConnected || row.querySelector(`.${INDICATOR_CLASS}`) || hasConflictLabel(row)) {
continue;
Expand Down
1 change: 1 addition & 0 deletions src/lib/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export interface PRBranchInfo {

export interface PRConflictStatus {
number: number;
state: "OPEN" | "CLOSED" | "MERGED";
mergeable: "CONFLICTING" | "MERGEABLE" | "UNKNOWN";
}

Expand Down
13 changes: 7 additions & 6 deletions src/service-worker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -382,9 +382,9 @@ describe("service worker", () => {
jsonResponse({
data: {
repository: {
pr_1: { mergeable: "CONFLICTING" },
pr_2: { mergeable: "MERGEABLE" },
pr_3: { mergeable: "UNKNOWN" },
pr_1: { state: "OPEN", mergeable: "CONFLICTING" },
pr_2: { state: "OPEN", mergeable: "MERGEABLE" },
pr_3: { state: "OPEN", mergeable: "UNKNOWN" },
},
},
}),
Expand All @@ -400,12 +400,13 @@ describe("service worker", () => {
expect(response).toEqual({
ok: true,
data: [
{ number: 1, mergeable: "CONFLICTING" },
{ number: 2, mergeable: "MERGEABLE" },
{ number: 3, mergeable: "UNKNOWN" },
{ number: 1, state: "OPEN", mergeable: "CONFLICTING" },
{ number: 2, state: "OPEN", mergeable: "MERGEABLE" },
{ number: 3, state: "OPEN", mergeable: "UNKNOWN" },
],
});
const query = JSON.parse(vi.mocked(fetch).mock.calls[0][1]?.body as string).query as string;
expect(query).toContain("state");
expect(query).toContain("mergeable");
expect(query).not.toContain("mergeStateStatus");
});
Expand Down
9 changes: 7 additions & 2 deletions src/service-worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,14 +273,19 @@ async function fetchPRConflictStatuses(
keys: [...prNumbers].sort((a, b) => a - b),
aliasFor: (n) => `pr_${n}`,
buildNodeQuery: (n) => `pullRequest(number: ${n}) {
state
mergeable
}`,
parseNode: (n, pr) => {
const state = pr.state;
const mergeable = pr.mergeable;
if (mergeable !== "CONFLICTING" && mergeable !== "MERGEABLE" && mergeable !== "UNKNOWN") {
if (
(state !== "OPEN" && state !== "CLOSED" && state !== "MERGED") ||
(mergeable !== "CONFLICTING" && mergeable !== "MERGEABLE" && mergeable !== "UNKNOWN")
) {
return null;
}
return { number: n, mergeable };
return { number: n, state, mergeable };
},
});
}
Expand Down