Skip to content
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,36 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => {
}),
);

it.effect("pages past reviewers who already voted when listing requested reviews", () =>
Effect.gen(function* () {
const row = (number: number, vote: number) => ({
...pullRequestRow,
pullRequestId: number,
reviewers: [{ uniqueName: "BILAL@acme.dev", vote }],
});
mockedExecute
.mockReturnValueOnce(
Effect.succeed(output(JSON.stringify([row(1, 10), row(2, 0), row(3, -10)]))),
)
.mockReturnValueOnce(Effect.succeed(output(JSON.stringify([row(4, 0), row(5, 0)]))));
const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli;
const batch = yield* cli.listPullRequests({
cwd: "/w",
repository: "web",
state: "open",
involvement: "reviewing",
viewer: "bilal@acme.dev",
limit: 2,
});

expect(batch.items.map((item) => item.number)).toEqual([2, 4]);
assert.isTrue(batch.truncated);
assert.strictEqual(batch.cursorAdvance, 4);
const nextArgs = argsOfCall(1);
assert.strictEqual(nextArgs[nextArgs.indexOf("--skip") + 1], "3");
}),
);

it.effect("reads the signed-in account, which az reports as a bare value", () =>
Effect.gen(function* () {
mockedExecute.mockReturnValueOnce(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,17 +416,31 @@ export const make = Effect.gen(function* () {
);
}

const lastItemIndex = decoded.success.rawIndexes[remaining - 1];
// Azure's reviewer query includes reviewers who already voted.
const eligibleIndexes = decoded.success.items.flatMap((item, index) =>
input.involvement !== "reviewing" ||
item.reviewRequestLogins.some(
(login) => login.toLowerCase() === input.viewer.toLowerCase(),
)
? [index]
: [],
);
const eligible = eligibleIndexes.map((index) => decoded.success.items[index]!);
const lastEligibleIndex = eligibleIndexes[remaining - 1];
const lastItemIndex =
lastEligibleIndex === undefined
? undefined
: decoded.success.rawIndexes[lastEligibleIndex];
if (lastItemIndex !== undefined) {
const consumed = lastItemIndex + 1;
return Effect.succeed({
items: [...input.items, ...decoded.success.items.slice(0, remaining)],
items: [...input.items, ...eligible.slice(0, remaining)],
truncated: consumed < decoded.success.rawCount || decoded.success.rawCount === top,
cursorAdvance: input.cursorAdvance + consumed,
});
}

const items = [...input.items, ...decoded.success.items];
const items = [...input.items, ...eligible];
if (decoded.success.rawCount < top) {
return Effect.succeed({
items,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,40 @@ describe("getChangeRequest", () => {
);
});

describe("review decision", () => {
it.effect("carries the decision Azure's votes add up to into every read of a pull request", () =>
Effect.gen(function* () {
const voted = { ...PULL_REQUEST, reviewDecision: "changes-requested" as const };
const provider = yield* make.pipe(
Effect.provide(
Layer.mock(AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli)({
listPullRequests: () =>
Effect.succeed({ items: [voted], truncated: false, cursorAdvance: 1 }),
getPullRequest: () => Effect.succeed(voted),
listIterations: () => Effect.succeed([ITERATION]),
listIterationChanges: () => Effect.succeed({ changes: [], truncated: false }),
}),
),
);
const ref = { cwd: "/w", repository: "acme/web", host: "dev.azure.com" };

const listed = yield* provider.listChangeRequests({
...ref,
state: "open",
involvement: "all",
viewer: "bilal@acme.dev",
limit: 10,
});
const summary = yield* provider.getChangeRequestSummary!({ ...ref, number: 7 });
const detail = yield* provider.getChangeRequest({ ...ref, number: 7 });

expect(listed.items[0]?.reviewDecision).toBe("changes-requested");
expect(summary.reviewDecision).toBe("changes-requested");
expect(detail.reviewDecision).toBe("changes-requested");
}),
);
});

describe("getDiff reads", () => {
it.effect("holds every reader together to one request's worth of processes", () =>
Effect.gen(function* () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ function toChangeRequest(pullRequest: AzureDevOpsPullRequest): ProviderChangeReq
mergedAt: pullRequest.state === "merged" ? pullRequest.closedAt : null,
updatedAt: pullRequest.updatedAt,
reviewRequestLogins: pullRequest.reviewRequestLogins,
...(pullRequest.reviewDecision === undefined
? {}
: { reviewDecision: pullRequest.reviewDecision }),
// Azure keeps labels on work items rather than on the pull request.
labels: [],
};
Expand Down Expand Up @@ -304,6 +307,9 @@ export const make = Effect.gen(function* () {
closedAt: pullRequest.state === "closed" ? pullRequest.closedAt : null,
mergedAt: pullRequest.state === "merged" ? pullRequest.closedAt : null,
updatedAt: pullRequest.updatedAt,
...(pullRequest.reviewDecision === undefined
? {}
: { reviewDecision: pullRequest.reviewDecision }),
})),
),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,52 @@ describe("decodePullRequestListJson", () => {
});
});

const reviewer = (login: string, fields: Record<string, unknown> = {}) => ({
displayName: login,
uniqueName: `${login}@acme.dev`,
...fields,
});

it.each([
["no reviewers", [], null, []],
["an optional reviewer yet to vote", [reviewer("a", { vote: 0 })], null, ["a"]],
["an approval", [reviewer("a", { vote: 10 })], "approved", []],
["an approval with suggestions", [reviewer("a", { vote: 5 })], "approved", []],
["a wait for the author", [reviewer("a", { vote: -5 })], "changes-requested", []],
[
"a rejection beside an approval",
[reviewer("a", { vote: 10 }), reviewer("b", { vote: -10 })],
"changes-requested",
[],
],
[
"a required reviewer yet to vote beside an optional approval",
[reviewer("a", { vote: 10 }), reviewer("b", { vote: 0, isRequired: true })],
"review-required",
["b"],
],
[
"a required group yet to vote",
[reviewer("leads", { vote: 0, isRequired: true, isContainer: true })],
"review-required",
["leads"],
],
[
"a required vote Azure has no meaning for",
[reviewer("a", { vote: 7, isRequired: true }), reviewer("b", { vote: "soon" })],
"review-required",
["a", "b"],
],
])("reads %s as a review decision", (_case, reviewers, reviewDecision, requested) => {
const batch = expectSuccess(decodePullRequestListJson(asJson([pullRequest({ reviewers })])));

expect(batch.items[0]?.reviewDecision).toBe(reviewDecision);
expect(batch.items[0]?.reviewRequestLogins).toEqual(
requested.map((login) => `${login}@acme.dev`),
);
expect(batch.items[0]?.reviewers).toHaveLength(reviewers.length);
});

it("skips a malformed row but still counts it, so paging does not stop early", () => {
const batch = expectSuccess(
decodePullRequestListJson(asJson([{ pullRequestId: "nope" }, pullRequest()])),
Expand Down Expand Up @@ -248,6 +294,86 @@ describe("decodeThreadsJson", () => {
expect(comments.map((comment) => comment.id)).toEqual(["4:1", "4:2", "4:3"]);
});

it("reads a vote Azure records on a system thread as a review by the voter", () => {
const service = {
displayName: "[acme]\\Project Collection Service Accounts",
isContainer: true,
};
const property = (value: unknown) => ({ $type: "System.String", $value: value });
const voteThread = (
id: number,
properties: Record<string, unknown>,
extra: Record<string, unknown> = {},
) => ({
id,
properties: { CodeReviewThreadType: property("VoteUpdate"), ...properties },
comments: [
{
id: 1,
content: "Somebody voted",
commentType: "system",
author: service,
publishedDate: `2026-07-0${id}T00:00:00Z`,
},
],
...extra,
});
const comments = expectSuccess(
decodeThreadsJson(
asJson({
value: [
voteThread(
1,
{
CodeReviewVoteResult: property("10"),
CodeReviewVotedByIdentity: property("1"),
CodeReviewVotedByDisplayName: property("Julius"),
},
{ identities: { "1": { displayName: "Julius", uniqueName: "julius@acme.dev" } } },
),
voteThread(2, {
CodeReviewVoteResult: { $type: "System.Int32", $value: -5 },
CodeReviewVotedByDisplayName: property("Bilal"),
}),
voteThread(3, { CodeReviewVoteResult: property("0") }),
voteThread(4, { CodeReviewVoteResult: property("7") }),
{
id: 5,
properties: { CodeReviewThreadType: property("MergeAttempt") },
comments: [
{
id: 1,
content: "The pull request is mergeable.",
commentType: "system",
publishedDate: "2026-07-05T00:00:00Z",
},
],
},
],
}),
),
);

expect(comments).toEqual([
{
id: "1:1",
kind: "review",
author: { login: "julius@acme.dev", name: "Julius", avatarUrl: null },
body: "",
createdAt: "2026-07-01T00:00:00Z",
url: null,
path: null,
reviewState: "APPROVED",
},
expect.objectContaining({
id: "2:1",
author: { login: "Bilal", name: "Bilal", avatarUrl: null },
reviewState: "CHANGES_REQUESTED",
}),
expect.objectContaining({ id: "3:1", author: null, reviewState: "DISMISSED" }),
]);
});

it("drops deleted threads and threads with nothing to show", () => {
const comments = expectSuccess(
decodeThreadsJson(
Expand Down
Loading
Loading