diff --git a/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestCli.test.ts b/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestCli.test.ts index 9bbe1468f8f54..1a2e323b04f1d 100644 --- a/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestCli.test.ts +++ b/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestCli.test.ts @@ -257,6 +257,120 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => { }), ); + it.effect("leaves out of Reviewing what the viewer has voted on and still steps over it", () => + Effect.gen(function* () { + const [voted, waiting, groupRequested] = pullRequestRows(3, 1); + const viewer = { displayName: "Bilal Hassan", uniqueName: "Bilal@acme.dev" }; + mockedExecute.mockReturnValueOnce( + Effect.succeed( + output( + JSON.stringify([ + { ...voted, reviewers: [{ ...viewer, vote: 10 }] }, + { ...waiting, reviewers: [{ ...viewer, vote: 0 }] }, + { + ...groupRequested, + reviewers: [{ displayName: "[web]\\Reviewers", uniqueName: "web-reviewers" }], + }, + ]), + ), + ), + ); + 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, 3]); + assert.isTrue(batch.truncated); + assert.strictEqual(batch.cursorAdvance, 3); + }), + ); + + it.effect("stops reading Reviewing pages of voted pull requests once it has one to show", () => + Effect.gen(function* () { + const viewer = { displayName: "Bilal Hassan", uniqueName: "bilal@acme.dev" }; + const page = (count: number, firstNumber: number, pendingNumbers: ReadonlyArray) => + Effect.succeed( + output( + JSON.stringify( + pullRequestRows(count, firstNumber).map((row) => ({ + ...row, + reviewers: [ + { ...viewer, vote: pendingNumbers.includes(Number(row.pullRequestId)) ? 0 : 10 }, + ], + })), + ), + ), + ); + // Once one row is kept, `--top` asks for one fewer. + mockedExecute + .mockReturnValueOnce(page(3, 1, [1])) + .mockReturnValueOnce(page(2, 4, [])) + .mockReturnValueOnce(page(2, 6, [])) + .mockReturnValueOnce(page(2, 8, [8])); + const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli; + + const batch = yield* cli.listPullRequests({ + cwd: "/w", + repository: "web", + state: "all", + involvement: "reviewing", + viewer: "bilal@acme.dev", + limit: 2, + }); + + assert.strictEqual(mockedExecute.mock.calls.length, 3); + expect(batch.items.map((item) => item.number)).toEqual([1]); + assert.isTrue(batch.truncated); + assert.strictEqual(batch.cursorAdvance, 7); + }), + ); + + it.effect("keeps reading past voted Reviewing pages until it finds one to show", () => + Effect.gen(function* () { + const viewer = { displayName: "Bilal Hassan", uniqueName: "bilal@acme.dev" }; + const page = (count: number, firstNumber: number, pendingNumbers: ReadonlyArray) => + Effect.succeed( + output( + JSON.stringify( + pullRequestRows(count, firstNumber).map((row) => ({ + ...row, + reviewers: [ + { ...viewer, vote: pendingNumbers.includes(Number(row.pullRequestId)) ? 0 : 10 }, + ], + })), + ), + ), + ); + mockedExecute + .mockReturnValueOnce(page(3, 1, [])) + .mockReturnValueOnce(page(3, 4, [])) + .mockReturnValueOnce(page(3, 7, [])) + .mockReturnValueOnce(page(3, 10, [10])); + const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli; + + const batch = yield* cli.listPullRequests({ + cwd: "/w", + repository: "web", + state: "all", + involvement: "reviewing", + viewer: "bilal@acme.dev", + limit: 2, + }); + + assert.strictEqual(mockedExecute.mock.calls.length, 4); + expect(batch.items.map((item) => item.number)).toEqual([10]); + assert.isTrue(batch.truncated); + assert.strictEqual(batch.cursorAdvance, 12); + }), + ); + it.effect("reads the signed-in account, which az reports as a bare value", () => Effect.gen(function* () { mockedExecute.mockReturnValueOnce( diff --git a/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestCli.ts b/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestCli.ts index 81c9b2e68fb6e..d47cecaa95e90 100644 --- a/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestCli.ts +++ b/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestCli.ts @@ -24,6 +24,7 @@ import { type AzureDevOpsItemContent, type AzureDevOpsIteration, type AzureDevOpsPullRequest, + type AzureDevOpsPullRequestBatch, type AzureDevOpsRepositoryLocation, } from "./azureDevOpsPullRequestJson.ts"; import type { ProviderListCursor } from "@t3tools/source-control-core/server/PullRequestProvider"; @@ -104,6 +105,11 @@ export type AzureDevOpsPullRequestCliError = const REST_API_VERSION = "7.1"; const PULL_REQUEST_LIST_MAX_OUTPUT_BYTES = 16 * 1024 * 1024; +/** + * Reviewing drops rows the viewer voted on; bound the reads one listing spends skipping them. The + * bound waits for one row, since an empty page gives the list service no cursor to resume from. + */ +const PULL_REQUEST_LIST_MAX_READS = 3; /** A 2,000-file page exceeds the default 1 MiB output budget. */ const CHANGE_ENTRIES_MAX_OUTPUT_BYTES = 8 * 1024 * 1024; /** JSON escaping and base64 can exceed the decoded file's size. */ @@ -161,6 +167,7 @@ export class AzureDevOpsPullRequestCli extends Context.Service< readonly cwd: string; readonly location: AzureDevOpsRepositoryLocation; readonly number: number; + readonly reviewersById: AzureDevOpsPullRequest["reviewersById"]; }) => Effect.Effect, AzureDevOpsPullRequestCliError>; readonly listIterations: (input: { @@ -234,6 +241,26 @@ function involvementArgs(input: { } } +/** `--reviewer` keeps a pull request after the viewer votes on it; only an unvoted one waits on them. */ +function withoutVotedByViewer( + batch: AzureDevOpsPullRequestBatch, + viewer: string, +): AzureDevOpsPullRequestBatch { + const login = viewer.toLowerCase(); + const isViewer = (candidate: string) => candidate.toLowerCase() === login; + const kept = batch.items.flatMap((item, index) => + item.reviewers.some((reviewer) => isViewer(reviewer.login)) && + !item.reviewRequestLogins.some(isViewer) + ? [] + : [{ item, rawIndex: batch.rawIndexes[index] ?? 0 }], + ); + return { + items: kept.map((entry) => entry.item), + rawIndexes: kept.map((entry) => entry.rawIndex), + rawCount: batch.rawCount, + }; +} + function actionArgs( action: PullRequestAction, mergeMethod: PullRequestMergeMethod | undefined, @@ -363,6 +390,7 @@ export const make = Effect.gen(function* () { readonly limit: number; readonly skip: number; readonly cursorAdvance: number; + readonly reads: number; readonly items: ReadonlyArray; }): Effect.Effect< { @@ -415,29 +443,37 @@ export const make = Effect.gen(function* () { }), ); } + const batch = + input.involvement === "reviewing" + ? withoutVotedByViewer(decoded.success, input.viewer) + : decoded.success; - const lastItemIndex = decoded.success.rawIndexes[remaining - 1]; + const lastItemIndex = batch.rawIndexes[remaining - 1]; if (lastItemIndex !== undefined) { const consumed = lastItemIndex + 1; return Effect.succeed({ - items: [...input.items, ...decoded.success.items.slice(0, remaining)], - truncated: consumed < decoded.success.rawCount || decoded.success.rawCount === top, + items: [...input.items, ...batch.items.slice(0, remaining)], + truncated: consumed < batch.rawCount || batch.rawCount === top, cursorAdvance: input.cursorAdvance + consumed, }); } - const items = [...input.items, ...decoded.success.items]; - if (decoded.success.rawCount < top) { + const items = [...input.items, ...batch.items]; + if ( + batch.rawCount < top || + (items.length > 0 && input.reads + 1 >= PULL_REQUEST_LIST_MAX_READS) + ) { return Effect.succeed({ items, - truncated: false, - cursorAdvance: input.cursorAdvance + decoded.success.rawCount, + truncated: batch.rawCount === top, + cursorAdvance: input.cursorAdvance + batch.rawCount, }); } return listPullRequestPage({ ...input, - skip: input.skip + decoded.success.rawCount, - cursorAdvance: input.cursorAdvance + decoded.success.rawCount, + skip: input.skip + batch.rawCount, + cursorAdvance: input.cursorAdvance + batch.rawCount, + reads: input.reads + 1, items, }); }), @@ -509,6 +545,7 @@ export const make = Effect.gen(function* () { limit: input.limit, skip: input.cursor?.delivered ?? 0, cursorAdvance: 0, + reads: 0, items: [], }); }), @@ -552,7 +589,7 @@ export const make = Effect.gen(function* () { resource: "pullRequestThreads", routeParameters: pullRequestRoute(input), maxOutputBytes: REVIEW_HISTORY_MAX_OUTPUT_BYTES, - decode: decodeThreadsJson, + decode: (raw) => decodeThreadsJson(raw, input.reviewersById), }); }), diff --git a/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.test.ts b/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.test.ts index 345d74de84ba1..4e609d60fc54b 100644 --- a/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.test.ts +++ b/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.test.ts @@ -35,6 +35,8 @@ const PULL_REQUEST = { body: "", reviewRequestLogins: [], reviewers: [], + reviewersById: new Map(), + reviewDecision: null, location: { project: "acme", repository: "web" }, autoMergeEnabled: false, }; diff --git a/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.ts b/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.ts index dbd9ae94875bc..b8c31b536f3f9 100644 --- a/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.ts +++ b/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.ts @@ -110,6 +110,7 @@ function toChangeRequest(pullRequest: AzureDevOpsPullRequest): ProviderChangeReq mergedAt: pullRequest.state === "merged" ? pullRequest.closedAt : null, updatedAt: pullRequest.updatedAt, reviewRequestLogins: pullRequest.reviewRequestLogins, + reviewDecision: pullRequest.reviewDecision, // Azure keeps labels on work items rather than on the pull request. labels: [], }; @@ -129,6 +130,7 @@ function toSummary(pullRequest: AzureDevOpsPullRequest): ProviderChangeRequestSu closedAt: pullRequest.state === "closed" ? pullRequest.closedAt : null, mergedAt: pullRequest.state === "merged" ? pullRequest.closedAt : null, updatedAt: pullRequest.updatedAt, + reviewDecision: pullRequest.reviewDecision, }; } @@ -368,6 +370,7 @@ export const make = Effect.gen(function* () { cwd: input.cwd, location: pullRequest.location, number: input.number, + reviewersById: pullRequest.reviewersById, }) .pipe( Effect.map((comments) => ({ comments, truncated: false })), diff --git a/packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.test.ts b/packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.test.ts index b9b1e36a94349..d715c37bb4938 100644 --- a/packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.test.ts +++ b/packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.test.ts @@ -39,6 +39,59 @@ function expectSuccess(result: Result.Result): A { const asJson = (value: unknown) => JSON.stringify(value); +const JANE_ID = "d6245f20-2af8-44f4-9451-8107cb2767db"; +const BILAL_ID = "8c8c7d32-6b1b-47f4-b2e9-30b477b5ab3d"; + +function reviewersById() { + const detail = expectSuccess( + decodePullRequestJson( + asJson( + pullRequest({ + reviewers: [ + { id: JANE_ID, displayName: "Jane Doe", uniqueName: "jane@acme.dev", vote: 10 }, + { id: BILAL_ID, displayName: "Bilal Hassan", uniqueName: "bilal@acme.dev", vote: -10 }, + ], + }), + ), + ), + ); + if (detail === null) throw new Error("expected a pull request"); + return detail.reviewersById; +} + +/** Azure's documented shape: a service account authors the comment; properties name the voter. */ +function voteThread(input: { + readonly id: number; + readonly vote: string | number | null; + readonly voterId: string; + readonly voterName: string; +}) { + return { + id: input.id, + properties: { + CodeReviewThreadType: { $type: "System.String", $value: "VoteUpdate" }, + ...(input.vote === null + ? {} + : { CodeReviewVoteResult: { $type: "System.String", $value: input.vote } }), + CodeReviewVotedByTfId: { $type: "System.String", $value: input.voterId }, + CodeReviewVotedByDisplayName: { $type: "System.String", $value: input.voterName }, + }, + comments: [ + { + id: 1, + content: `${input.voterName} voted ${input.vote}`, + commentType: "system", + author: { + id: "41113706-0000-0000-0000-000000000000", + displayName: "[DefaultCollection]\\Project Collection Service Accounts", + isContainer: true, + }, + publishedDate: `2026-07-0${input.id}T00:00:00Z`, + }, + ], + }; +} + describe("decodePullRequestListJson", () => { it("reads a pull request as a change request", () => { const batch = expectSuccess(decodePullRequestListJson(asJson([pullRequest()]))); @@ -158,6 +211,51 @@ describe("decodePullRequestJson", () => { expect(detail?.location).toBeNull(); }); + it.each([ + ["approved by a required reviewer", [{ vote: 10, isRequired: true }], [], "approved"], + ["rejected", [{ vote: -10 }], [], "changes-requested"], + ["not yet voted on", [{ vote: 0, isRequired: true }], ["me@example.com"], "review-required"], + [ + "approved but waiting on a required reviewer", + [{ vote: 10 }, { uniqueName: "jane@example.com", vote: 0, isRequired: true }], + ["jane@example.com"], + "review-required", + ], + [ + "approved with suggestions, with an optional reviewer still to vote", + [{ vote: 5 }, { uniqueName: "jane@example.com", vote: 0 }], + ["jane@example.com"], + "approved", + ], + [ + "sent back to its author despite an approval", + [{ vote: -5 }, { uniqueName: "jane@example.com", vote: 10, isRequired: true }], + [], + "changes-requested", + ], + ["without reviewers", [], [], null], + ["approved only by its author", [{ uniqueName: "bilal@acme.dev", vote: 10 }], [], null], + ])( + "reads the reviewers' votes on a pull request %s", + (_case, votes, reviewRequestLogins, reviewDecision) => { + const detail = expectSuccess( + decodePullRequestJson( + asJson( + pullRequest({ + reviewers: votes.map((vote) => ({ + displayName: "Reviewer", + uniqueName: "me@example.com", + ...vote, + })), + }), + ), + ), + ); + + expect(detail).toMatchObject({ reviewRequestLogins, reviewDecision }); + }, + ); + it("returns nothing when Azure gave no way to place the pull request at all", () => { const detail = expectSuccess( decodePullRequestJson(asJson(pullRequest({ url: null, repository: null }))), @@ -248,6 +346,86 @@ describe("decodeThreadsJson", () => { expect(comments.map((comment) => comment.id)).toEqual(["4:1", "4:2", "4:3"]); }); + it("reads a reviewer's vote, which Azure records as a system thread, as their review", () => { + const comments = expectSuccess( + decodeThreadsJson( + asJson({ + value: [ + voteThread({ id: 2, vote: "10", voterId: JANE_ID, voterName: "Jane Doe" }), + // Azure writes the same identity id without dashes and in either case. + voteThread({ + id: 3, + vote: "-10", + voterId: BILAL_ID.replaceAll("-", "").toUpperCase(), + voterName: "Bilal Hassan", + }), + ], + }), + reviewersById(), + ), + ); + + expect(comments).toMatchObject([ + { kind: "review", reviewState: "APPROVED", author: { login: "jane@acme.dev" } }, + { kind: "review", reviewState: "CHANGES_REQUESTED", author: { login: "bilal@acme.dev" } }, + ]); + }); + + it("keeps voters who are no longer reviewers apart even when they share a display name", () => { + const comments = expectSuccess( + decodeThreadsJson( + asJson({ + value: [ + voteThread({ + id: 2, + vote: "10", + voterId: "0f0f0f0f-0000-0000-0000-000000000000", + voterName: "Sam Lee", + }), + voteThread({ + id: 3, + vote: "-10", + voterId: "0e0e0e0e-0000-0000-0000-000000000000", + voterName: "Sam Lee", + }), + ], + }), + reviewersById(), + ), + ); + + expect(comments).toMatchObject([ + { + reviewState: "APPROVED", + author: { login: "0f0f0f0f000000000000000000000000", name: "Sam Lee" }, + }, + { + reviewState: "CHANGES_REQUESTED", + author: { login: "0e0e0e0e000000000000000000000000", name: "Sam Lee" }, + }, + ]); + }); + + it("reads a reset vote as withdrawing the verdict, in either value type, and skips a missing one", () => { + const comments = expectSuccess( + decodeThreadsJson( + asJson({ + value: [ + voteThread({ id: 2, vote: "10", voterId: JANE_ID, voterName: "Jane Doe" }), + voteThread({ id: 3, vote: 0, voterId: JANE_ID, voterName: "Jane Doe" }), + voteThread({ id: 4, vote: null, voterId: JANE_ID, voterName: "Jane Doe" }), + ], + }), + reviewersById(), + ), + ); + + expect(comments.map((comment) => [comment.author?.login, comment.reviewState])).toEqual([ + ["jane@acme.dev", "APPROVED"], + ["jane@acme.dev", "DISMISSED"], + ]); + }); + it("drops deleted threads and threads with nothing to show", () => { const comments = expectSuccess( decodeThreadsJson( diff --git a/packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.ts b/packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.ts index b3632ef1af98f..bfd551141b52b 100644 --- a/packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.ts +++ b/packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.ts @@ -11,6 +11,7 @@ import type { PullRequestComment, PullRequestMergeMethod, PullRequestMergeability, + PullRequestReviewDecision, PullRequestState, } from "@t3tools/contracts"; import { TrimmedNonEmptyString } from "@t3tools/contracts"; @@ -25,6 +26,22 @@ const RawIdentitySchema = Schema.Struct({ imageUrl: Schema.optional(Schema.NullOr(Schema.String)), }); +const RawReviewerSchema = Schema.Struct({ + ...RawIdentitySchema.fields, + id: Schema.optional(Schema.NullOr(Schema.String)), + /** 10 approved, 5 approved with suggestions, 0 none, -5 waiting for author, -10 rejected. */ + vote: Schema.optional(Schema.NullOr(Schema.Number)), + isRequired: Schema.optional(Schema.NullOr(Schema.Boolean)), +}); + +const RawThreadPropertySchema = Schema.optional( + Schema.NullOr( + Schema.Struct({ + $value: Schema.optional(Schema.NullOr(Schema.Union([Schema.String, Schema.Number]))), + }), + ), +); + const RawPullRequestSchema = Schema.Struct({ pullRequestId: Schema.Int, title: Schema.String, @@ -43,7 +60,7 @@ const RawPullRequestSchema = Schema.Struct({ ), mergeStatus: Schema.optional(Schema.NullOr(Schema.String)), createdBy: Schema.optional(Schema.NullOr(RawIdentitySchema)), - reviewers: Schema.optional(Schema.NullOr(Schema.Array(RawIdentitySchema))), + reviewers: Schema.optional(Schema.NullOr(Schema.Array(RawReviewerSchema))), sourceRefName: TrimmedNonEmptyString, targetRefName: TrimmedNonEmptyString, creationDate: TrimmedNonEmptyString, @@ -74,6 +91,16 @@ const RawPullRequestSchema = Schema.Struct({ const RawThreadSchema = Schema.Struct({ id: Schema.Int, isDeleted: Schema.optional(Schema.NullOr(Schema.Boolean)), + properties: Schema.optional( + Schema.NullOr( + Schema.Struct({ + CodeReviewThreadType: RawThreadPropertySchema, + CodeReviewVoteResult: RawThreadPropertySchema, + CodeReviewVotedByTfId: RawThreadPropertySchema, + CodeReviewVotedByDisplayName: RawThreadPropertySchema, + }), + ), + ), threadContext: Schema.optional( Schema.NullOr(Schema.Struct({ filePath: Schema.optional(Schema.NullOr(Schema.String)) })), ), @@ -124,6 +151,9 @@ export interface AzureDevOpsPullRequest { readonly body: string; readonly reviewRequestLogins: ReadonlyArray; readonly reviewers: ReadonlyArray; + /** Keyed by `identityKey`; vote threads name the voter only by identity id. */ + readonly reviewersById: ReadonlyMap; + readonly reviewDecision: PullRequestReviewDecision | null; readonly location: AzureDevOpsRepositoryLocation | null; readonly autoMergeEnabled: boolean; readonly autoMergeMethod?: PullRequestMergeMethod; @@ -137,6 +167,11 @@ function toActor(raw: Schema.Schema.Type | null | unde : { login, name: trimmed(raw?.displayName), avatarUrl: trimmed(raw?.imageUrl) }; } +/** Azure writes identity ids both with and without dashes. */ +function identityKey(id: string | null | undefined): string | null { + return trimmed(id)?.replaceAll("-", "").toLowerCase() || null; +} + function toState(raw: Schema.Schema.Type): PullRequestState { switch (raw.status?.trim().toLowerCase()) { case "completed": @@ -187,13 +222,69 @@ function toAutoMergeMethod( } } +function toReviewDecision( + allReviewers: ReadonlyArray>, + author: PullRequestActor | null, +): PullRequestReviewDecision | null { + // Azure adds an author who votes as a reviewer, but their vote does not review the change. + const reviewers = allReviewers.filter((reviewer) => toActor(reviewer)?.login !== author?.login); + if (reviewers.length === 0) return null; + const votes = reviewers.map((reviewer) => reviewer.vote ?? 0); + if (votes.some((vote) => vote < 0)) return "changes-requested"; + if (reviewers.some((reviewer, index) => reviewer.isRequired === true && votes[index] === 0)) { + return "review-required"; + } + return votes.some((vote) => vote > 0) ? "approved" : "review-required"; +} + +type RawThreadProperties = NonNullable["properties"]>; + +function propertyText(property: RawThreadProperties[keyof RawThreadProperties]): string | null { + const value = property?.$value; + return trimmed(value === null || value === undefined ? value : String(value)); +} + +/** Azure records a vote as a system thread; GitHub's review states are what consumers read. */ +function toVoteReviewState( + properties: RawThreadProperties | null | undefined, +): "APPROVED" | "CHANGES_REQUESTED" | "DISMISSED" | null { + if (propertyText(properties?.CodeReviewThreadType) !== "VoteUpdate") return null; + const vote = Number(propertyText(properties?.CodeReviewVoteResult) ?? NaN); + if (!Number.isFinite(vote)) return null; + return vote > 0 ? "APPROVED" : vote < 0 ? "CHANGES_REQUESTED" : "DISMISSED"; +} + +/** A vote's comment is authored by a service account; the voter is in the thread properties. */ +function toVoter( + properties: RawThreadProperties | null | undefined, + reviewersById: ReadonlyMap, +): PullRequestActor | null { + const key = identityKey(propertyText(properties?.CodeReviewVotedByTfId)); + const reviewer = key === null ? undefined : reviewersById.get(key); + if (reviewer !== undefined) return reviewer; + // Display names are not unique, so a voter no longer among the reviewers keeps their Azure id. + const name = propertyText(properties?.CodeReviewVotedByDisplayName); + const login = key ?? name; + return login === null ? null : { login, name, avatarUrl: null }; +} + function toPullRequest( raw: Schema.Schema.Type, ): AzureDevOpsPullRequest | null { const autoMergeMethod = toAutoMergeMethod(raw); - const reviewers = (raw.reviewers ?? []).flatMap((reviewer) => { + const rawReviewers = raw.reviewers ?? []; + const reviewers: PullRequestActor[] = []; + const reviewersById = new Map(); + for (const reviewer of rawReviewers) { const actor = toActor(reviewer); - return actor === null ? [] : [actor]; + if (actor === null) continue; + reviewers.push(actor); + const key = identityKey(reviewer.id); + if (key !== null) reviewersById.set(key, actor); + } + const reviewRequestLogins = rawReviewers.flatMap((reviewer) => { + const login = (reviewer.vote ?? 0) === 0 ? toActor(reviewer)?.login : undefined; + return login === undefined ? [] : [login]; }); const closedAt = trimmed(raw.closedDate); const url = trimmed( @@ -223,8 +314,10 @@ function toPullRequest( updatedAt: closedAt ?? raw.creationDate, closedAt, body: raw.description ?? "", - reviewRequestLogins: reviewers.map((reviewer) => reviewer.login), + reviewRequestLogins, reviewers, + reviewersById, + reviewDecision: toReviewDecision(rawReviewers, toActor(raw.createdBy)), location: toLocation(raw), autoMergeEnabled: (raw.autoCompleteSetBy ?? null) !== null, ...(autoMergeMethod === undefined ? {} : { autoMergeMethod }), @@ -287,6 +380,7 @@ export function decodeViewerJson(raw: string): Result.Result = new Map(), ): Result.Result, DecodeFailure> { const decoded = decodeThreadPage(raw); if (!Result.isSuccess(decoded)) { @@ -299,25 +393,29 @@ export function decodeThreadsJson( const thread = decodedThread.value; if (thread.isDeleted === true) continue; const path = toRepositoryPath(thread.threadContext?.filePath); + const voteReviewState = toVoteReviewState(thread.properties); + const voter = voteReviewState === null ? null : toVoter(thread.properties, reviewersById); for (const comment of thread.comments ?? []) { const publishedDate = trimmed(comment.publishedDate); + const isSystem = comment.commentType?.trim().toLowerCase() === "system"; if ( comment.isDeleted === true || - comment.commentType?.trim().toLowerCase() === "system" || + (isSystem && voteReviewState === null) || (comment.content ?? "").trim().length === 0 || publishedDate === null ) { continue; } + const reviewState = isSystem ? voteReviewState : null; comments.push({ id: `${thread.id}:${comment.id ?? 0}`, - kind: path === null ? "issue-comment" : "review-comment", - author: toActor(comment.author), + kind: reviewState !== null ? "review" : path === null ? "issue-comment" : "review-comment", + author: reviewState === null ? toActor(comment.author) : voter, body: comment.content ?? "", createdAt: publishedDate, url: null, - path, - reviewState: null, + path: reviewState === null ? path : null, + reviewState, }); } }