| author | |
| committer | |
| log | c12ba7581ac00ba2a623ad23283da5d03d34c2d8 |
| tree | aad3421300c8acdd2fe3859d0877c1aa19d87298 |
| parent | c1738f131d6f0d5c6dfabb3aad46755a25319663 |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com>13 files changed, 659 insertions(+), 254 deletions(-)
apps/server/src/pullRequest/GitHubPullRequestCli.ts+3| ... | @@ -2071,6 +2071,7 @@ export const make = Effect.gen(function* () { | ... | @@ -2071,6 +2071,7 @@ export const make = Effect.gen(function* () { |
| 2071 | }); | 2071 | }); |
| 2072 | const entries: GitHubReviewThreadEntry[] = []; | 2072 | const entries: GitHubReviewThreadEntry[] = []; |
| 2073 | const avatarsByLogin = new Map<string, string>(); | 2073 | const avatarsByLogin = new Map<string, string>(); |
| 2074 | const botLogins = new Set<string>(); | ||
| 2074 | const commitStats = new Map< | 2075 | const commitStats = new Map< |
| 2075 | string, | 2076 | string, |
| 2076 | { readonly additions: number; readonly deletions: number } | 2077 | { readonly additions: number; readonly deletions: number } |
| ... | @@ -2087,6 +2088,7 @@ export const make = Effect.gen(function* () { | ... | @@ -2087,6 +2088,7 @@ export const make = Effect.gen(function* () { |
| 2087 | do { | 2088 | do { |
| 2088 | const read: GitHubReviewThreadPage = yield* threadPage(cursor); | 2089 | const read: GitHubReviewThreadPage = yield* threadPage(cursor); |
| 2089 | entries.push(...read.threads); | 2090 | entries.push(...read.threads); |
| 2091 | for (const login of read.botLogins) botLogins.add(login); | ||
| 2090 | for (const [login, avatarUrl] of read.avatarsByLogin) | 2092 | for (const [login, avatarUrl] of read.avatarsByLogin) |
| 2091 | avatarsByLogin.set(login, avatarUrl); | 2093 | avatarsByLogin.set(login, avatarUrl); |
| 2092 | // The roster, the commits and the viewer's standing travel with every page, and the | 2094 | // The roster, the commits and the viewer's standing travel with every page, and the |
| ... | @@ -2152,6 +2154,7 @@ export const make = Effect.gen(function* () { | ... | @@ -2152,6 +2154,7 @@ export const make = Effect.gen(function* () { |
| 2152 | reactionsById, | 2154 | reactionsById, |
| 2153 | reviewers, | 2155 | reviewers, |
| 2154 | avatarsByLogin, | 2156 | avatarsByLogin, |
| 2157 | botLogins, | ||
| 2155 | commitStats, | 2158 | commitStats, |
| 2156 | commits, | 2159 | commits, |
| 2157 | viewer, | 2160 | viewer, |
apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts+4-1| ... | @@ -741,6 +741,7 @@ describe("getChangeRequest commits", () => { | ... | @@ -741,6 +741,7 @@ describe("getChangeRequest commits", () => { |
| 741 | reactionsById: new Map<string, ReadonlyArray<PullRequestReaction>>(), | 741 | reactionsById: new Map<string, ReadonlyArray<PullRequestReaction>>(), |
| 742 | reviewers: [], | 742 | reviewers: [], |
| 743 | avatarsByLogin: new Map<string, string>(), | 743 | avatarsByLogin: new Map<string, string>(), |
| 744 | botLogins: new Set<string>(), | ||
| 744 | commitStats: new Map<string, { readonly additions: number; readonly deletions: number }>(), | 745 | commitStats: new Map<string, { readonly additions: number; readonly deletions: number }>(), |
| 745 | viewer: { canUpdate: true, didAuthor: false }, | 746 | viewer: { canUpdate: true, didAuthor: false }, |
| 746 | }; | 747 | }; |
| ... | @@ -807,7 +808,7 @@ describe("getChangeRequestActivity dismissed reviews", () => { | ... | @@ -807,7 +808,7 @@ describe("getChangeRequestActivity dismissed reviews", () => { |
| 807 | const dismissedReview = (body: string) => ({ | 808 | const dismissedReview = (body: string) => ({ |
| 808 | id: "PRR_1", | 809 | id: "PRR_1", |
| 809 | kind: "review" as const, | 810 | kind: "review" as const, |
| 810 | author: null, | 811 | author: { login: "macroscopeapp", name: null, avatarUrl: null }, |
| 811 | body, | 812 | body, |
| 812 | createdAt: "2026-07-03T00:00:00Z", | 813 | createdAt: "2026-07-03T00:00:00Z", |
| 813 | url: null, | 814 | url: null, |
| ... | @@ -824,6 +825,7 @@ describe("getChangeRequestActivity dismissed reviews", () => { | ... | @@ -824,6 +825,7 @@ describe("getChangeRequestActivity dismissed reviews", () => { |
| 824 | reactionsById: new Map(), | 825 | reactionsById: new Map(), |
| 825 | reviewers: [], | 826 | reviewers: [], |
| 826 | avatarsByLogin: new Map(), | 827 | avatarsByLogin: new Map(), |
| 828 | botLogins: new Set(["macroscopeapp"]), | ||
| 827 | commitStats: new Map(), | 829 | commitStats: new Map(), |
| 828 | commits: [], | 830 | commits: [], |
| 829 | viewer: { canUpdate: true, didAuthor: false }, | 831 | viewer: { canUpdate: true, didAuthor: false }, |
| ... | @@ -850,6 +852,7 @@ describe("getChangeRequestActivity dismissed reviews", () => { | ... | @@ -850,6 +852,7 @@ describe("getChangeRequestActivity dismissed reviews", () => { |
| 850 | readActivity.pipe( | 852 | readActivity.pipe( |
| 851 | Effect.map((activity) => { | 853 | Effect.map((activity) => { |
| 852 | expect(activity.comments[0]?.body).toBe("Dismissing prior approval to re-evaluate 9b66581"); | 854 | expect(activity.comments[0]?.body).toBe("Dismissing prior approval to re-evaluate 9b66581"); |
| 855 | expect(activity.comments[0]?.author?.isBot).toBe(true); | ||
| 853 | }), | 856 | }), |
| 854 | Effect.provide(layerFor("<!-- Macroscope (Approvability) review body marker -->")), | 857 | Effect.provide(layerFor("<!-- Macroscope (Approvability) review body marker -->")), |
| 855 | ), | 858 | ), |
apps/server/src/pullRequest/GitHubPullRequestProvider.ts+30-5| ... | @@ -133,8 +133,11 @@ function withAvatar( | ... | @@ -133,8 +133,11 @@ function withAvatar( |
| 133 | actor: PullRequestActor | null, | 133 | actor: PullRequestActor | null, |
| 134 | avatarsByLogin: ReadonlyMap<string, string>, | 134 | avatarsByLogin: ReadonlyMap<string, string>, |
| 135 | host: string, | 135 | host: string, |
| 136 | botLogins?: ReadonlySet<string>, | ||
| 136 | ): PullRequestActor | null { | 137 | ): PullRequestActor | null { |
| 137 | if (actor === null || actor.avatarUrl !== null) return actor; | 138 | if (actor === null) return actor; |
| 139 | if (botLogins?.has(actor.login)) actor = { ...actor, isBot: true }; | ||
| 140 | if (actor.avatarUrl !== null) return actor; | ||
| 138 | const avatarUrl = avatarsByLogin.get(actor.login) ?? loginAvatarUrl(actor.login, host); | 141 | const avatarUrl = avatarsByLogin.get(actor.login) ?? loginAvatarUrl(actor.login, host); |
| 139 | return avatarUrl === null ? actor : { ...actor, avatarUrl }; | 142 | return avatarUrl === null ? actor : { ...actor, avatarUrl }; |
| 140 | } | 143 | } |
| ... | @@ -450,6 +453,7 @@ export const make = Effect.gen(function* () { | ... | @@ -450,6 +453,7 @@ export const make = Effect.gen(function* () { |
| 450 | truncated: true, | 453 | truncated: true, |
| 451 | reviewers: [], | 454 | reviewers: [], |
| 452 | avatarsByLogin: new Map<string, string>(), | 455 | avatarsByLogin: new Map<string, string>(), |
| 456 | botLogins: new Set<string>(), | ||
| 453 | commitStats: new Map< | 457 | commitStats: new Map< |
| 454 | string, | 458 | string, |
| 455 | { readonly additions: number; readonly deletions: number } | 459 | { readonly additions: number; readonly deletions: number } |
| ... | @@ -463,7 +467,12 @@ export const make = Effect.gen(function* () { | ... | @@ -463,7 +467,12 @@ export const make = Effect.gen(function* () { |
| 463 | ).pipe( | 467 | ).pipe( |
| 464 | Effect.mapError(fail("getChangeRequestActivity")), | 468 | Effect.mapError(fail("getChangeRequestActivity")), |
| 465 | Effect.map(([pullRequest, reviewThreads]): ProviderChangeRequestActivity => ({ | 469 | Effect.map(([pullRequest, reviewThreads]): ProviderChangeRequestActivity => ({ |
| 466 | author: withAvatar(pullRequest.author, reviewThreads.avatarsByLogin, input.host), | 470 | author: withAvatar( |
| 471 | pullRequest.author, | ||
| 472 | reviewThreads.avatarsByLogin, | ||
| 473 | input.host, | ||
| 474 | reviewThreads.botLogins, | ||
| 475 | ), | ||
| 467 | reviewers: reviewThreads.reviewers, | 476 | reviewers: reviewThreads.reviewers, |
| 468 | reactions: reviewThreads.reactions, | 477 | reactions: reviewThreads.reactions, |
| 469 | commits: (reviewThreads.commits.length > 0 | 478 | commits: (reviewThreads.commits.length > 0 |
| ... | @@ -473,7 +482,13 @@ export const make = Effect.gen(function* () { | ... | @@ -473,7 +482,13 @@ export const make = Effect.gen(function* () { |
| 473 | ...commit, | 482 | ...commit, |
| 474 | ...reviewThreads.commitStats.get(commit.oid), | 483 | ...reviewThreads.commitStats.get(commit.oid), |
| 475 | authors: commit.authors?.map( | 484 | authors: commit.authors?.map( |
| 476 | (author) => withAvatar(author, reviewThreads.avatarsByLogin, input.host) ?? author, | 485 | (author) => |
| 486 | withAvatar( | ||
| 487 | author, | ||
| 488 | reviewThreads.avatarsByLogin, | ||
| 489 | input.host, | ||
| 490 | reviewThreads.botLogins, | ||
| 491 | ) ?? author, | ||
| 477 | ), | 492 | ), |
| 478 | })), | 493 | })), |
| 479 | comments: [...pullRequest.comments, ...reviewThreads.comments] | 494 | comments: [...pullRequest.comments, ...reviewThreads.comments] |
| ... | @@ -489,7 +504,12 @@ export const make = Effect.gen(function* () { | ... | @@ -489,7 +504,12 @@ export const make = Effect.gen(function* () { |
| 489 | rendersEmpty(comment.body) | 504 | rendersEmpty(comment.body) |
| 490 | ? (reviewThreads.dismissalsByReviewId.get(comment.id) ?? comment.body) | 505 | ? (reviewThreads.dismissalsByReviewId.get(comment.id) ?? comment.body) |
| 491 | : comment.body, | 506 | : comment.body, |
| 492 | author: withAvatar(comment.author, reviewThreads.avatarsByLogin, input.host), | 507 | author: withAvatar( |
| 508 | comment.author, | ||
| 509 | reviewThreads.avatarsByLogin, | ||
| 510 | input.host, | ||
| 511 | reviewThreads.botLogins, | ||
| 512 | ), | ||
| 493 | // A comment out of `gh pr view --json` carries none of its own: that read | 513 | // A comment out of `gh pr view --json` carries none of its own: that read |
| 494 | // reports no reaction at all, so they arrive from the GraphQL page by node id. | 514 | // reports no reaction at all, so they arrive from the GraphQL page by node id. |
| 495 | reactions: comment.reactions ?? reviewThreads.reactionsById.get(comment.id) ?? [], | 515 | reactions: comment.reactions ?? reviewThreads.reactionsById.get(comment.id) ?? [], |
| ... | @@ -503,7 +523,12 @@ export const make = Effect.gen(function* () { | ... | @@ -503,7 +523,12 @@ export const make = Effect.gen(function* () { |
| 503 | ...thread, | 523 | ...thread, |
| 504 | comments: thread.comments.map((comment) => ({ | 524 | comments: thread.comments.map((comment) => ({ |
| 505 | ...comment, | 525 | ...comment, |
| 506 | author: withAvatar(comment.author, reviewThreads.avatarsByLogin, input.host), | 526 | author: withAvatar( |
| 527 | comment.author, | ||
| 528 | reviewThreads.avatarsByLogin, | ||
| 529 | input.host, | ||
| 530 | reviewThreads.botLogins, | ||
| 531 | ), | ||
| 507 | })), | 532 | })), |
| 508 | })), | 533 | })), |
| 509 | })), | 534 | })), |
apps/server/src/pullRequest/gitHubPullRequestJson.test.ts+14-2| ... | @@ -436,14 +436,26 @@ describe("review thread decoding", () => { | ... | @@ -436,14 +436,26 @@ describe("review thread decoding", () => { |
| 436 | requested: [{ login: "julius", name: "Julius", avatarUrl: "https://avatars/j.png" }], | 436 | requested: [{ login: "julius", name: "Julius", avatarUrl: "https://avatars/j.png" }], |
| 437 | // An app that has reviewed is no longer an outstanding request, which is why asking | 437 | // An app that has reviewed is no longer an outstanding request, which is why asking |
| 438 | // only for requests reported nobody on a pull request a bot had reviewed. | 438 | // only for requests reported nobody on a pull request a bot had reviewed. |
| 439 | reviewed: [{ login: "macroscopeapp", avatarUrl: "https://avatars/in/900172.png" }], | 439 | reviewed: [ |
| 440 | { | ||
| 441 | __typename: "Bot", | ||
| 442 | login: "macroscopeapp", | ||
| 443 | avatarUrl: "https://avatars/in/900172.png", | ||
| 444 | }, | ||
| 445 | ], | ||
| 440 | }), | 446 | }), |
| 441 | ), | 447 | ), |
| 442 | ); | 448 | ); |
| 443 | 449 | ||
| 450 | expect(result.botLogins).toEqual(new Set(["macroscopeapp"])); | ||
| 444 | expect(result.reviewers).toEqual([ | 451 | expect(result.reviewers).toEqual([ |
| 445 | { login: "julius", name: "Julius", avatarUrl: "https://avatars/j.png" }, | 452 | { login: "julius", name: "Julius", avatarUrl: "https://avatars/j.png" }, |
| 446 | { login: "macroscopeapp", name: null, avatarUrl: "https://avatars/in/900172.png" }, | 453 | { |
| 454 | login: "macroscopeapp", | ||
| 455 | name: null, | ||
| 456 | avatarUrl: "https://avatars/in/900172.png", | ||
| 457 | isBot: true, | ||
| 458 | }, | ||
| 447 | ]); | 459 | ]); |
| 448 | }); | 460 | }); |
| 449 | 461 |
apps/server/src/pullRequest/gitHubPullRequestJson.ts+24-10| ... | @@ -39,6 +39,8 @@ import { dedupeChecks } from "./pullRequestChecks.ts"; | ... | @@ -39,6 +39,8 @@ import { dedupeChecks } from "./pullRequestChecks.ts"; |
| 39 | * release that adds a conclusion or a review state must not fail the whole payload. | 39 | * release that adds a conclusion or a review state must not fail the whole payload. |
| 40 | */ | 40 | */ |
| 41 | const RawActorSchema = Schema.Struct({ | 41 | const RawActorSchema = Schema.Struct({ |
| 42 | __typename: Schema.optional(Schema.String), | ||
| 43 | is_bot: Schema.optional(Schema.Boolean), | ||
| 42 | /** | 44 | /** |
| 43 | * Optional because a review can be requested from a team or a mannequin, which the query has | 45 | * Optional because a review can be requested from a team or a mannequin, which the query has |
| 44 | * no fragment for and GraphQL answers with an empty object. A reviewer with no login names | 46 | * no fragment for and GraphQL answers with an empty object. A reviewer with no login names |
| ... | @@ -480,7 +482,7 @@ const RawReviewThreadsSchema = Schema.Struct({ | ... | @@ -480,7 +482,7 @@ const RawReviewThreadsSchema = Schema.Struct({ |
| 480 | ), | 482 | ), |
| 481 | ), | 483 | ), |
| 482 | /** | 484 | /** |
| 483 | * Reviews for their reactions alone: the words and the verdict arrive with | 485 | * Reviews for their reactions and actor identity: the words and the verdict arrive with |
| 484 | * `gh pr view --json reviews`, which reports no reaction of any kind. | 486 | * `gh pr view --json reviews`, which reports no reaction of any kind. |
| 485 | */ | 487 | */ |
| 486 | reviews: Schema.optional( | 488 | reviews: Schema.optional( |
| ... | @@ -489,6 +491,7 @@ const RawReviewThreadsSchema = Schema.Struct({ | ... | @@ -489,6 +491,7 @@ const RawReviewThreadsSchema = Schema.Struct({ |
| 489 | nodes: Schema.Array( | 491 | nodes: Schema.Array( |
| 490 | Schema.Struct({ | 492 | Schema.Struct({ |
| 491 | id: Schema.optional(Schema.NullOr(Schema.String)), | 493 | id: Schema.optional(Schema.NullOr(Schema.String)), |
| 494 | author: Schema.optional(Schema.NullOr(RawActorSchema)), | ||
| 492 | reactionGroups: RawReactionGroupsSchema, | 495 | reactionGroups: RawReactionGroupsSchema, |
| 493 | }), | 496 | }), |
| 494 | ), | 497 | ), |
| ... | @@ -676,7 +679,7 @@ export function pullRequestSearchGraphQlQuery(rows: number, includeStacks = fals | ... | @@ -676,7 +679,7 @@ export function pullRequestSearchGraphQlQuery(rows: number, includeStacks = fals |
| 676 | number | 679 | number |
| 677 | title | 680 | title |
| 678 | url | 681 | url |
| 679 | author { login avatarUrl ... on User { name } } | 682 | author { __typename login avatarUrl ... on User { name } } |
| 680 | headRefName | 683 | headRefName |
| 681 | baseRefName | 684 | baseRefName |
| 682 | state | 685 | state |
| ... | @@ -736,28 +739,28 @@ export const REVIEW_THREADS_GRAPHQL_QUERY = `query($owner: String!, $name: Strin | ... | @@ -736,28 +739,28 @@ export const REVIEW_THREADS_GRAPHQL_QUERY = `query($owner: String!, $name: Strin |
| 736 | comments(first: 10) { | 739 | comments(first: 10) { |
| 737 | totalCount | 740 | totalCount |
| 738 | pageInfo { hasNextPage endCursor } | 741 | pageInfo { hasNextPage endCursor } |
| 739 | nodes { id author { login avatarUrl } body createdAt url ${REACTION_GROUPS_FIELDS} } | 742 | nodes { id author { __typename login avatarUrl } body createdAt url ${REACTION_GROUPS_FIELDS} } |
| 740 | } | 743 | } |
| 741 | } | 744 | } |
| 742 | } | 745 | } |
| 743 | viewerCanUpdate | 746 | viewerCanUpdate |
| 744 | viewerDidAuthor | 747 | viewerDidAuthor |
| 745 | author { login avatarUrl } | 748 | author { __typename login avatarUrl } |
| 746 | ${REACTION_GROUPS_FIELDS} | 749 | ${REACTION_GROUPS_FIELDS} |
| 747 | comments(first: ${GRAPHQL_PAGE_SIZE}) { | 750 | comments(first: ${GRAPHQL_PAGE_SIZE}) { |
| 748 | nodes { id author { login avatarUrl } ${REACTION_GROUPS_FIELDS} } | 751 | nodes { id author { __typename login avatarUrl } ${REACTION_GROUPS_FIELDS} } |
| 749 | } | 752 | } |
| 750 | reviews(first: ${GRAPHQL_PAGE_SIZE}) { nodes { id ${REACTION_GROUPS_FIELDS} } } | 753 | reviews(first: ${GRAPHQL_PAGE_SIZE}) { nodes { id author { __typename login avatarUrl } ${REACTION_GROUPS_FIELDS} } } |
| 751 | reviewRequests(first: 50) { | 754 | reviewRequests(first: 50) { |
| 752 | nodes { | 755 | nodes { |
| 753 | requestedReviewer { | 756 | requestedReviewer { |
| 754 | ... on User { login name avatarUrl } | 757 | ... on User { login name avatarUrl } |
| 755 | ... on Bot { login avatarUrl } | 758 | ... on Bot { __typename login avatarUrl } |
| 756 | } | 759 | } |
| 757 | } | 760 | } |
| 758 | } | 761 | } |
| 759 | latestReviews(first: 50) { | 762 | latestReviews(first: 50) { |
| 760 | nodes { author { login avatarUrl } } | 763 | nodes { author { __typename login avatarUrl } } |
| 761 | } | 764 | } |
| 762 | reviewDismissals: timelineItems(itemTypes: [REVIEW_DISMISSED_EVENT], first: ${GRAPHQL_PAGE_SIZE}) { | 765 | reviewDismissals: timelineItems(itemTypes: [REVIEW_DISMISSED_EVENT], first: ${GRAPHQL_PAGE_SIZE}) { |
| 763 | pageInfo { hasNextPage endCursor } | 766 | pageInfo { hasNextPage endCursor } |
| ... | @@ -793,7 +796,7 @@ export const REVIEW_THREAD_COMMENTS_GRAPHQL_QUERY = `query($owner: String!, $nam | ... | @@ -793,7 +796,7 @@ export const REVIEW_THREAD_COMMENTS_GRAPHQL_QUERY = `query($owner: String!, $nam |
| 793 | pullRequest { id } | 796 | pullRequest { id } |
| 794 | comments(first: ${GRAPHQL_PAGE_SIZE}, after: $cursor) { | 797 | comments(first: ${GRAPHQL_PAGE_SIZE}, after: $cursor) { |
| 795 | pageInfo { hasNextPage endCursor } | 798 | pageInfo { hasNextPage endCursor } |
| 796 | nodes { id author { login avatarUrl } body createdAt url ${REACTION_GROUPS_FIELDS} } | 799 | nodes { id author { __typename login avatarUrl } body createdAt url ${REACTION_GROUPS_FIELDS} } |
| 797 | } | 800 | } |
| 798 | } | 801 | } |
| 799 | } | 802 | } |
| ... | @@ -1134,7 +1137,12 @@ function toActor(raw: Schema.Schema.Type<typeof RawActorSchema> | null | undefin | ... | @@ -1134,7 +1137,12 @@ function toActor(raw: Schema.Schema.Type<typeof RawActorSchema> | null | undefin |
| 1134 | const login = trimmed(raw?.login); | 1137 | const login = trimmed(raw?.login); |
| 1135 | return login === null | 1138 | return login === null |
| 1136 | ? null | 1139 | ? null |
| 1137 | : { login, name: trimmed(raw?.name), avatarUrl: trimmed(raw?.avatarUrl) }; | 1140 | : { |
| 1141 | login, | ||
| 1142 | name: trimmed(raw?.name), | ||
| 1143 | avatarUrl: trimmed(raw?.avatarUrl), | ||
| 1144 | ...(raw?.__typename === "Bot" || raw?.is_bot === true ? { isBot: true } : {}), | ||
| 1145 | }; | ||
| 1138 | } | 1146 | } |
| 1139 | 1147 | ||
| 1140 | function toCommitActor( | 1148 | function toCommitActor( |
| ... | @@ -1753,6 +1761,7 @@ export interface GitHubReviewThreadComments { | ... | @@ -1753,6 +1761,7 @@ export interface GitHubReviewThreadComments { |
| 1753 | * so an app's avatar arrives the same way a person's does. | 1761 | * so an app's avatar arrives the same way a person's does. |
| 1754 | */ | 1762 | */ |
| 1755 | readonly avatarsByLogin: ReadonlyMap<string, string>; | 1763 | readonly avatarsByLogin: ReadonlyMap<string, string>; |
| 1764 | readonly botLogins: ReadonlySet<string>; | ||
| 1756 | /** Per-commit line counts carried by the same bounded pull-request query. */ | 1765 | /** Per-commit line counts carried by the same bounded pull-request query. */ |
| 1757 | readonly commitStats: ReadonlyMap< | 1766 | readonly commitStats: ReadonlyMap< |
| 1758 | string, | 1767 | string, |
| ... | @@ -1791,6 +1800,7 @@ export interface GitHubReviewThreadPage { | ... | @@ -1791,6 +1800,7 @@ export interface GitHubReviewThreadPage { |
| 1791 | readonly reactionsById: ReadonlyMap<string, ReadonlyArray<PullRequestReaction>>; | 1800 | readonly reactionsById: ReadonlyMap<string, ReadonlyArray<PullRequestReaction>>; |
| 1792 | readonly reviewers: ReadonlyArray<PullRequestActor>; | 1801 | readonly reviewers: ReadonlyArray<PullRequestActor>; |
| 1793 | readonly avatarsByLogin: ReadonlyMap<string, string>; | 1802 | readonly avatarsByLogin: ReadonlyMap<string, string>; |
| 1803 | readonly botLogins: ReadonlySet<string>; | ||
| 1794 | readonly commitStats: ReadonlyMap< | 1804 | readonly commitStats: ReadonlyMap< |
| 1795 | string, | 1805 | string, |
| 1796 | { readonly additions: number; readonly deletions: number } | 1806 | { readonly additions: number; readonly deletions: number } |
| ... | @@ -1928,9 +1938,11 @@ export function decodeReviewThreadsJson( | ... | @@ -1928,9 +1938,11 @@ export function decodeReviewThreadsJson( |
| 1928 | }); | 1938 | }); |
| 1929 | const pullRequest = decoded.success.data.repository.pullRequest; | 1939 | const pullRequest = decoded.success.data.repository.pullRequest; |
| 1930 | const avatarsByLogin = new Map<string, string>(); | 1940 | const avatarsByLogin = new Map<string, string>(); |
| 1941 | const botLogins = new Set<string>(); | ||
| 1931 | for (const raw of [ | 1942 | for (const raw of [ |
| 1932 | pullRequest.author, | 1943 | pullRequest.author, |
| 1933 | ...(pullRequest.comments?.nodes ?? []).map((node) => node.author), | 1944 | ...(pullRequest.comments?.nodes ?? []).map((node) => node.author), |
| 1945 | ...(pullRequest.reviews?.nodes ?? []).map((node) => node.author), | ||
| 1934 | ...(pullRequest.reviewRequests?.nodes ?? []).map((node) => node.requestedReviewer), | 1946 | ...(pullRequest.reviewRequests?.nodes ?? []).map((node) => node.requestedReviewer), |
| 1935 | ...(pullRequest.latestReviews?.nodes ?? []).map((node) => node.author), | 1947 | ...(pullRequest.latestReviews?.nodes ?? []).map((node) => node.author), |
| 1936 | ...threads.nodes.flatMap((thread) => thread.comments.nodes.map((comment) => comment.author)), | 1948 | ...threads.nodes.flatMap((thread) => thread.comments.nodes.map((comment) => comment.author)), |
| ... | @@ -1938,6 +1950,7 @@ export function decodeReviewThreadsJson( | ... | @@ -1938,6 +1950,7 @@ export function decodeReviewThreadsJson( |
| 1938 | const login = trimmed(raw?.login); | 1950 | const login = trimmed(raw?.login); |
| 1939 | const avatarUrl = trimmed(raw?.avatarUrl); | 1951 | const avatarUrl = trimmed(raw?.avatarUrl); |
| 1940 | if (login !== null && avatarUrl !== null) avatarsByLogin.set(login, avatarUrl); | 1952 | if (login !== null && avatarUrl !== null) avatarsByLogin.set(login, avatarUrl); |
| 1953 | if (login !== null && toActor(raw)?.isBot) botLogins.add(login); | ||
| 1941 | } | 1954 | } |
| 1942 | const reviewers = new Map<string, PullRequestActor>(); | 1955 | const reviewers = new Map<string, PullRequestActor>(); |
| 1943 | for (const raw of [ | 1956 | for (const raw of [ |
| ... | @@ -1996,6 +2009,7 @@ export function decodeReviewThreadsJson( | ... | @@ -1996,6 +2009,7 @@ export function decodeReviewThreadsJson( |
| 1996 | reactionsById, | 2009 | reactionsById, |
| 1997 | reviewers: [...reviewers.values()], | 2010 | reviewers: [...reviewers.values()], |
| 1998 | avatarsByLogin, | 2011 | avatarsByLogin, |
| 2012 | botLogins, | ||
| 1999 | commitStats, | 2013 | commitStats, |
| 2000 | commits, | 2014 | commits, |
| 2001 | viewer: toPullRequestViewerFields(pullRequest), | 2015 | viewer: toPullRequestViewerFields(pullRequest), |
apps/web/src/components/pullRequest/PullRequestCommentBody.tsx created+61| ... | @@ -0,0 +1,61 @@ | ||
| 1 | import { useEffect, useId, useRef, useState, type ComponentProps } from "react"; | ||
| 2 | |||
| 3 | import { cn } from "~/lib/utils"; | ||
| 4 | import { Button } from "../ui/button"; | ||
| 5 | import { PullRequestMarkdown } from "./PullRequestMarkdown"; | ||
| 6 | |||
| 7 | /** Keep the complete markdown intact while limiting long reports to a readable preview. */ | ||
| 8 | export function PullRequestCommentBody({ | ||
| 9 | className, | ||
| 10 | ...props | ||
| 11 | }: ComponentProps<typeof PullRequestMarkdown>) { | ||
| 12 | const [expanded, setExpanded] = useState(false); | ||
| 13 | const [overflowing, setOverflowing] = useState(false); | ||
| 14 | const content = useRef<HTMLDivElement>(null); | ||
| 15 | const id = useId(); | ||
| 16 | |||
| 17 | useEffect(() => { | ||
| 18 | const element = content.current; | ||
| 19 | if (!element) return; | ||
| 20 | const measure = () => setOverflowing(element.getBoundingClientRect().height > 240); | ||
| 21 | measure(); | ||
| 22 | const observer = new ResizeObserver(measure); | ||
| 23 | observer.observe(element); | ||
| 24 | return () => observer.disconnect(); | ||
| 25 | }, []); | ||
| 26 | |||
| 27 | return ( | ||
| 28 | <div className={cn("min-w-0", className)}> | ||
| 29 | <div | ||
| 30 | id={id} | ||
| 31 | className={cn("relative", !expanded && "max-h-[240px] overflow-hidden")} | ||
| 32 | onFocusCapture={() => setExpanded(true)} | ||
| 33 | > | ||
| 34 | <div ref={content}> | ||
| 35 | <PullRequestMarkdown {...props} /> | ||
| 36 | </div> | ||
| 37 | {overflowing && !expanded ? ( | ||
| 38 | <div | ||
| 39 | aria-hidden | ||
| 40 | className="pointer-events-none absolute inset-x-0 bottom-0 h-10 bg-gradient-to-t from-background to-transparent" | ||
| 41 | /> | ||
| 42 | ) : null} | ||
| 43 | </div> | ||
| 44 | {overflowing ? ( | ||
| 45 | <Button | ||
| 46 | size="xs" | ||
| 47 | variant="ghost" | ||
| 48 | className="mt-2 text-muted-foreground" | ||
| 49 | aria-expanded={expanded} | ||
| 50 | aria-controls={id} | ||
| 51 | onClick={() => { | ||
| 52 | if (expanded) content.current?.parentElement?.scrollIntoView({ block: "nearest" }); | ||
| 53 | setExpanded(!expanded); | ||
| 54 | }} | ||
| 55 | > | ||
| 56 | {expanded ? "Show less" : "Show full comment"} | ||
| 57 | </Button> | ||
| 58 | ) : null} | ||
| 59 | </div> | ||
| 60 | ); | ||
| 61 | } | ||
apps/web/src/components/pullRequest/PullRequestReactions.tsx+2-13| ... | @@ -34,15 +34,7 @@ function reactionsSignature(reactions: ReadonlyArray<PullRequestReaction>): stri | ... | @@ -34,15 +34,7 @@ function reactionsSignature(reactions: ReadonlyArray<PullRequestReaction>): stri |
| 34 | .join(" "); | 34 | .join(" "); |
| 35 | } | 35 | } |
| 36 | 36 | ||
| 37 | /** | 37 | /** Reaction counts and an always-visible picker, routed to the supplied host subject. */ |
| 38 | * The reaction pills under a remark, and the picker that adds one. The same bar serves the | ||
| 39 | * description, a conversation comment and a review thread's comments: what differs between them | ||
| 40 | * is only which subject the host is told about. | ||
| 41 | * | ||
| 42 | * The add button is revealed by hovering the remark it belongs to, the way GitHub's is, so the | ||
| 43 | * parent must carry `group`. It stays put once there is something to press it beside, while the | ||
| 44 | * picker is open, and whenever it is focused — a control only a mouse can find is no control. | ||
| 45 | */ | ||
| 46 | export function PullRequestReactionBar({ | 38 | export function PullRequestReactionBar({ |
| 47 | reactions, | 39 | reactions, |
| 48 | canReact, | 40 | canReact, |
| ... | @@ -98,7 +90,7 @@ export function PullRequestReactionBar({ | ... | @@ -98,7 +90,7 @@ export function PullRequestReactionBar({ |
| 98 | if (shown.length === 0 && !canReact) return null; | 90 | if (shown.length === 0 && !canReact) return null; |
| 99 | 91 | ||
| 100 | return ( | 92 | return ( |
| 101 | <div className={cn("flex flex-wrap items-center gap-1", className)}> | 93 | <div className={cn("flex min-w-0 max-w-full flex-wrap items-center gap-1", className)}> |
| 102 | {shown.map((reaction) => ( | 94 | {shown.map((reaction) => ( |
| 103 | <Tooltip key={reaction.content}> | 95 | <Tooltip key={reaction.content}> |
| 104 | <TooltipTrigger | 96 | <TooltipTrigger |
| ... | @@ -136,9 +128,6 @@ export function PullRequestReactionBar({ | ... | @@ -136,9 +128,6 @@ export function PullRequestReactionBar({ |
| 136 | className={cn( | 128 | className={cn( |
| 137 | PILL_CLASS, | 129 | PILL_CLASS, |
| 138 | "border-border/70 px-1.5 text-muted-foreground hover:border-primary/60 hover:text-foreground", | 130 | "border-border/70 px-1.5 text-muted-foreground hover:border-primary/60 hover:text-foreground", |
| 139 | shown.length === 0 && | ||
| 140 | !pickerOpen && | ||
| 141 | "opacity-0 group-focus-within:opacity-100 group-hover:opacity-100 focus-visible:opacity-100", | ||
| 142 | )} | 131 | )} |
| 143 | /> | 132 | /> |
| 144 | } | 133 | } |
apps/web/src/components/pullRequest/PullRequestReviewAnnotation.tsx+10-10| ... | @@ -264,9 +264,18 @@ export function ReviewThreadCard({ | ... | @@ -264,9 +264,18 @@ export function ReviewThreadCard({ |
| 264 | <div className="mt-2 space-y-3"> | 264 | <div className="mt-2 space-y-3"> |
| 265 | {comments.map((comment) => ( | 265 | {comments.map((comment) => ( |
| 266 | <article key={comment.id} className="group min-w-0"> | 266 | <article key={comment.id} className="group min-w-0"> |
| 267 | <div className="flex items-center gap-2 text-xs text-muted-foreground"> | 267 | <div className="flex flex-wrap items-center gap-2 text-xs text-muted-foreground"> |
| 268 | <PullRequestActorLabel actor={comment.author} className="text-foreground" /> | 268 | <PullRequestActorLabel actor={comment.author} className="text-foreground" /> |
| 269 | <span>{formatRelativeTimeLabel(comment.createdAt)}</span> | 269 | <span>{formatRelativeTimeLabel(comment.createdAt)}</span> |
| 270 | <PullRequestReactionBar | ||
| 271 | className="ml-auto justify-end" | ||
| 272 | reactions={comment.reactions ?? []} | ||
| 273 | canReact={canReact} | ||
| 274 | subjectId={comment.id} | ||
| 275 | environmentId={environmentId} | ||
| 276 | reference={reference} | ||
| 277 | onRefresh={onReacted} | ||
| 278 | /> | ||
| 270 | </div> | 279 | </div> |
| 271 | {editingId === comment.id ? ( | 280 | {editingId === comment.id ? ( |
| 272 | <PullRequestMarkdownEditor | 281 | <PullRequestMarkdownEditor |
| ... | @@ -300,15 +309,6 @@ export function ReviewThreadCard({ | ... | @@ -300,15 +309,6 @@ export function ReviewThreadCard({ |
| 300 | ) : null} | 309 | ) : null} |
| 301 | </div> | 310 | </div> |
| 302 | )} | 311 | )} |
| 303 | <PullRequestReactionBar | ||
| 304 | className="mt-1.5" | ||
| 305 | reactions={comment.reactions ?? []} | ||
| 306 | canReact={canReact} | ||
| 307 | subjectId={comment.id} | ||
| 308 | environmentId={environmentId} | ||
| 309 | reference={reference} | ||
| 310 | onRefresh={onReacted} | ||
| 311 | /> | ||
| 312 | </article> | 312 | </article> |
| 313 | ))} | 313 | ))} |
| 314 | </div> | 314 | </div> |
apps/web/src/components/pullRequest/PullRequestSummaryTab.test.tsx+74-1| ... | @@ -1,5 +1,5 @@ | ... | @@ -1,5 +1,5 @@ |
| 1 | import { EnvironmentId, ProjectId, type PullRequestDetailView } from "@t3tools/contracts"; | 1 | import { EnvironmentId, ProjectId, type PullRequestDetailView } from "@t3tools/contracts"; |
| 2 | import { act } from "react"; | 2 | import { act, type ReactNode } from "react"; |
| 3 | import { create, type ReactTestRenderer } from "react-test-renderer"; | 3 | import { create, type ReactTestRenderer } from "react-test-renderer"; |
| 4 | import { afterEach, beforeEach, expect, it, vi } from "vite-plus/test"; | 4 | import { afterEach, beforeEach, expect, it, vi } from "vite-plus/test"; |
| 5 | 5 | ||
| ... | @@ -9,6 +9,11 @@ vi.mock("~/browser/useOpenLink", () => ({ useOpenLink: () => vi.fn() })); | ... | @@ -9,6 +9,11 @@ vi.mock("~/browser/useOpenLink", () => ({ useOpenLink: () => vi.fn() })); |
| 9 | vi.mock("./PullRequestMarkdown", () => ({ | 9 | vi.mock("./PullRequestMarkdown", () => ({ |
| 10 | PullRequestMarkdown: ({ text }: { text: string }) => <p>{text}</p>, | 10 | PullRequestMarkdown: ({ text }: { text: string }) => <p>{text}</p>, |
| 11 | })); | 11 | })); |
| 12 | vi.mock("../ui/tooltip", () => ({ | ||
| 13 | Tooltip: ({ children }: { children: ReactNode }) => children, | ||
| 14 | TooltipTrigger: ({ children }: { children: ReactNode }) => children, | ||
| 15 | TooltipPopup: () => null, | ||
| 16 | })); | ||
| 12 | 17 | ||
| 13 | import { PullRequestSummaryTab } from "./PullRequestSummaryTab"; | 18 | import { PullRequestSummaryTab } from "./PullRequestSummaryTab"; |
| 14 | 19 | ||
| ... | @@ -139,3 +144,71 @@ it("keeps an unsaved description when collapsed and reopened", () => { | ... | @@ -139,3 +144,71 @@ it("keeps an unsaved description when collapsed and reopened", () => { |
| 139 | click("Description"); | 144 | click("Description"); |
| 140 | expect(renderer.root.findByType("textarea").props.value).toBe("Unsaved description"); | 145 | expect(renderer.root.findByType("textarea").props.value).toBe("Unsaved description"); |
| 141 | }); | 146 | }); |
| 147 | |||
| 148 | it("opens bot reports in pages without hiding human comments", () => { | ||
| 149 | const value: PullRequestDetailView = { | ||
| 150 | ...detail, | ||
| 151 | commentCount: 13, | ||
| 152 | comments: Array.from({ length: 13 }, (_, index) => ({ | ||
| 153 | id: `comment-${index}`, | ||
| 154 | kind: "issue-comment", | ||
| 155 | author: { | ||
| 156 | login: index === 12 ? "human" : "review-app", | ||
| 157 | name: null, | ||
| 158 | avatarUrl: null, | ||
| 159 | isBot: index !== 12, | ||
| 160 | }, | ||
| 161 | body: index === 12 ? "Human comment" : `Bot report ${index}`, | ||
| 162 | createdAt: `2026-09-01T00:00:${String(index).padStart(2, "0")}Z`, | ||
| 163 | url: null, | ||
| 164 | path: null, | ||
| 165 | reviewState: null, | ||
| 166 | })), | ||
| 167 | }; | ||
| 168 | act(() => { | ||
| 169 | renderer = create(render(value)); | ||
| 170 | }); | ||
| 171 | expect(renderer.root.findAllByType("p").map((p) => p.children.join(""))).toContain( | ||
| 172 | "Human comment", | ||
| 173 | ); | ||
| 174 | expect( | ||
| 175 | renderer.root.findAllByType("p").some((p) => p.children.join("").startsWith("Bot report")), | ||
| 176 | ).toBe(false); | ||
| 177 | const group = renderer.root | ||
| 178 | .findAllByType("button") | ||
| 179 | .find((button) => button.props["aria-label"] === "12 bot comments")!; | ||
| 180 | act(() => group.props.onClick({ nativeEvent: {}, preventDefault() {}, stopPropagation() {} })); | ||
| 181 | expect( | ||
| 182 | renderer.root.findAllByType("p").filter((p) => p.children.join("").startsWith("Bot report")), | ||
| 183 | ).toHaveLength(10); | ||
| 184 | expect(renderer.root.findAllByType("p").map((p) => p.children.join(""))).not.toContain( | ||
| 185 | "Bot report 0", | ||
| 186 | ); | ||
| 187 | act(() => | ||
| 188 | renderer.root | ||
| 189 | .findAllByType("button") | ||
| 190 | .find((button) => button.children.includes(" older bot comment"))! | ||
| 191 | .props.onClick(), | ||
| 192 | ); | ||
| 193 | expect( | ||
| 194 | renderer.root.findAllByType("p").filter((p) => p.children.join("").startsWith("Bot report")), | ||
| 195 | ).toHaveLength(12); | ||
| 196 | act(() => group.props.onClick({ nativeEvent: {}, preventDefault() {}, stopPropagation() {} })); | ||
| 197 | act(() => group.props.onClick({ nativeEvent: {}, preventDefault() {}, stopPropagation() {} })); | ||
| 198 | expect(renderer.root.findAllByType("p").map((p) => p.children.join(""))).toContain( | ||
| 199 | "Bot report 0", | ||
| 200 | ); | ||
| 201 | act(() => | ||
| 202 | renderer.root | ||
| 203 | .findAllByType("button") | ||
| 204 | .find((button) => button.children.includes(" recent bot comments"))! | ||
| 205 | .props.onClick(), | ||
| 206 | ); | ||
| 207 | expect( | ||
| 208 | renderer.root.findAllByType("p").filter((p) => p.children.join("").startsWith("Bot report")), | ||
| 209 | ).toHaveLength(10); | ||
| 210 | act(() => renderer.update(render({ ...value, url: `${value.url}0`, number: 10 }))); | ||
| 211 | expect( | ||
| 212 | renderer.root.findAllByType("p").some((p) => p.children.join("").startsWith("Bot report")), | ||
| 213 | ).toBe(false); | ||
| 214 | }); |
apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx+394-171| ... | @@ -1,9 +1,9 @@ | ... | @@ -1,9 +1,9 @@ |
| 1 | import type { | 1 | import type { |
| 2 | EnvironmentId, | 2 | EnvironmentId, |
| 3 | PullRequestActor, | ||
| 4 | PullRequestComment, | 3 | PullRequestComment, |
| 5 | PullRequestDetailView, | 4 | PullRequestDetailView, |
| 6 | PullRequestRef, | 5 | PullRequestRef, |
| 6 | PullRequestReviewThread, | ||
| 7 | ScopedThreadRef, | 7 | ScopedThreadRef, |
| 8 | } from "@t3tools/contracts"; | 8 | } from "@t3tools/contracts"; |
| 9 | import { | 9 | import { |
| ... | @@ -28,7 +28,6 @@ import { Collapsible, CollapsiblePanel, CollapsibleTrigger } from "../ui/collaps | ... | @@ -28,7 +28,6 @@ import { Collapsible, CollapsiblePanel, CollapsibleTrigger } from "../ui/collaps |
| 28 | import { toastManager } from "../ui/toast"; | 28 | import { toastManager } from "../ui/toast"; |
| 29 | import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; | 29 | import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; |
| 30 | import { | 30 | import { |
| 31 | PullRequestActorAvatar, | ||
| 32 | PullRequestActorLabel, | 31 | PullRequestActorLabel, |
| 33 | PullRequestCheckStatusIcon, | 32 | PullRequestCheckStatusIcon, |
| 34 | PullRequestReviewOutcomeBadge, | 33 | PullRequestReviewOutcomeBadge, |
| ... | @@ -53,6 +52,7 @@ import { | ... | @@ -53,6 +52,7 @@ import { |
| 53 | canEditPullRequestComment, | 52 | canEditPullRequestComment, |
| 54 | } from "./pullRequestEditing.logic"; | 53 | } from "./pullRequestEditing.logic"; |
| 55 | import { PullRequestMarkdown } from "./PullRequestMarkdown"; | 54 | import { PullRequestMarkdown } from "./PullRequestMarkdown"; |
| 55 | import { PullRequestCommentBody } from "./PullRequestCommentBody"; | ||
| 56 | import { PullRequestMarkdownEditor } from "./PullRequestMarkdownEditor"; | 56 | import { PullRequestMarkdownEditor } from "./PullRequestMarkdownEditor"; |
| 57 | import { PullRequestReactionBar } from "./PullRequestReactions"; | 57 | import { PullRequestReactionBar } from "./PullRequestReactions"; |
| 58 | import { PullRequestConversationGhost } from "./PullRequestGhosts"; | 58 | import { PullRequestConversationGhost } from "./PullRequestGhosts"; |
| ... | @@ -64,18 +64,69 @@ function reviewerKey(login: string): string { | ... | @@ -64,18 +64,69 @@ function reviewerKey(login: string): string { |
| 64 | return login.toLowerCase(); | 64 | return login.toLowerCase(); |
| 65 | } | 65 | } |
| 66 | 66 | ||
| 67 | /** The avatar carries the attribution alone; who it is arrives on hover, like the reviewer row. */ | 67 | function CommentIdentity({ |
| 68 | function CommentAuthor({ actor }: { actor: PullRequestActor | null }) { | 68 | comment, |
| 69 | const login = actor?.login ?? "ghost"; | 69 | detail, |
| 70 | }: { | ||
| 71 | comment: PullRequestComment; | ||
| 72 | detail: PullRequestDetailView; | ||
| 73 | }) { | ||
| 74 | const actor = comment.author; | ||
| 75 | const profileUrl = | ||
| 76 | detail.provider === "github" && actor && !actor.login.endsWith("[bot]") | ||
| 77 | ? new URL(`/${encodeURIComponent(actor.login)}`, detail.url).toString() | ||
| 78 | : null; | ||
| 79 | return ( | ||
| 80 | <div className="flex min-w-0 flex-1 flex-wrap items-center gap-x-2 gap-y-1 text-xs"> | ||
| 81 | <PullRequestActorLabel | ||
| 82 | actor={actor} | ||
| 83 | profileUrl={profileUrl} | ||
| 84 | className="max-w-full font-medium text-foreground [&>img]:size-6 [&>span:first-child]:size-6" | ||
| 85 | /> | ||
| 86 | <Tooltip> | ||
| 87 | <TooltipTrigger | ||
| 88 | render={ | ||
| 89 | comment.url ? ( | ||
| 90 | <a | ||
| 91 | href={comment.url} | ||
| 92 | target="_blank" | ||
| 93 | rel="noopener noreferrer" | ||
| 94 | className="text-muted-foreground hover:text-foreground hover:underline" | ||
| 95 | /> | ||
| 96 | ) : ( | ||
| 97 | <span className="text-muted-foreground" /> | ||
| 98 | ) | ||
| 99 | } | ||
| 100 | > | ||
| 101 | <time dateTime={comment.createdAt}>{formatRelativeTimeLabel(comment.createdAt)}</time> | ||
| 102 | </TooltipTrigger> | ||
| 103 | <TooltipPopup> | ||
| 104 | {new Date(comment.createdAt).toLocaleString()} | ||
| 105 | {comment.url ? " · Open comment on host" : ""} | ||
| 106 | </TooltipPopup> | ||
| 107 | </Tooltip> | ||
| 108 | </div> | ||
| 109 | ); | ||
| 110 | } | ||
| 111 | |||
| 112 | function CommentLocation({ | ||
| 113 | comment, | ||
| 114 | thread, | ||
| 115 | }: { | ||
| 116 | comment: PullRequestComment; | ||
| 117 | thread: PullRequestReviewThread | undefined; | ||
| 118 | }) { | ||
| 119 | const path = thread?.path ?? comment.path; | ||
| 120 | if (!path) return null; | ||
| 121 | const label = `${path}${thread?.line ? `:${thread.line}` : ""}`; | ||
| 70 | return ( | 122 | return ( |
| 71 | <Tooltip> | 123 | <div className="mt-2 flex min-w-0 items-center gap-2 text-xs text-muted-foreground"> |
| 72 | <TooltipTrigger render={<span className="shrink-0 rounded-full" aria-label={login} />}> | 124 | <Tooltip> |
| 73 | <PullRequestActorAvatar actor={actor} /> | 125 | <TooltipTrigger render={<span className="truncate font-mono" />}>{label}</TooltipTrigger> |
| 74 | </TooltipTrigger> | 126 | <TooltipPopup>{label}</TooltipPopup> |
| 75 | <TooltipPopup side="bottom"> | 127 | </Tooltip> |
| 76 | {actor?.name && actor.name !== login ? `${actor.name} (@${login})` : login} | 128 | {thread?.isOutdated ? <span className="shrink-0">Outdated</span> : null} |
| 77 | </TooltipPopup> | 129 | </div> |
| 78 | </Tooltip> | ||
| 79 | ); | 130 | ); |
| 80 | } | 131 | } |
| 81 | 132 | ||
| ... | @@ -127,7 +178,8 @@ function CommentBody({ | ... | @@ -127,7 +178,8 @@ function CommentBody({ |
| 127 | } | 178 | } |
| 128 | return ( | 179 | return ( |
| 129 | <div className={cn("flex items-start gap-1", className)}> | 180 | <div className={cn("flex items-start gap-1", className)}> |
| 130 | <PullRequestMarkdown | 181 | <PullRequestCommentBody |
| 182 | key={comment.id} | ||
| 131 | className="min-w-0 flex-1" | 183 | className="min-w-0 flex-1" |
| 132 | text={comment.body} | 184 | text={comment.body} |
| 133 | cwd={editing.cwd} | 185 | cwd={editing.cwd} |
| ... | @@ -149,13 +201,15 @@ function CommentBody({ | ... | @@ -149,13 +201,15 @@ function CommentBody({ |
| 149 | ); | 201 | ); |
| 150 | } | 202 | } |
| 151 | 203 | ||
| 152 | /** Finished work — a resolved conversation or a dismissed approval — opens collapsed. */ | 204 | /** Finished work — a resolved conversation or a dismissed review — opens collapsed. */ |
| 153 | function CollapsedComment({ | 205 | function CollapsedComment({ |
| 154 | comment, | 206 | comment, |
| 155 | editing, | 207 | editing, |
| 156 | label, | 208 | label, |
| 157 | body, | 209 | body, |
| 158 | reactionBar, | 210 | reactionBar, |
| 211 | detail, | ||
| 212 | thread, | ||
| 159 | }: { | 213 | }: { |
| 160 | comment: PullRequestComment; | 214 | comment: PullRequestComment; |
| 161 | editing: CommentEditing; | 215 | editing: CommentEditing; |
| ... | @@ -163,49 +217,54 @@ function CollapsedComment({ | ... | @@ -163,49 +217,54 @@ function CollapsedComment({ |
| 163 | /** Null where the remark is nothing but its verdict, which a dismissal usually is. */ | 217 | /** Null where the remark is nothing but its verdict, which a dismissal usually is. */ |
| 164 | body: string | null; | 218 | body: string | null; |
| 165 | reactionBar: ReactNode; | 219 | reactionBar: ReactNode; |
| 220 | detail: PullRequestDetailView; | ||
| 221 | thread: PullRequestReviewThread | undefined; | ||
| 166 | }) { | 222 | }) { |
| 167 | const [open, setOpen] = useState(false); | 223 | const [open, setOpen] = useState(false); |
| 224 | const statusTriggerRef = useRef<HTMLButtonElement>(null); | ||
| 168 | return ( | 225 | return ( |
| 169 | <Collapsible open={open} onOpenChange={setOpen}> | 226 | <Collapsible open={open} onOpenChange={setOpen}> |
| 170 | <article className="group rounded-lg border border-border/60 [contain-intrinsic-block-size:44px] [content-visibility:auto]"> | 227 | <article className="group rounded-lg border border-border/60 [contain-intrinsic-block-size:44px] [content-visibility:auto]"> |
| 171 | <CollapsibleTrigger | 228 | <div className="p-3"> |
| 172 | className={cn( | 229 | <div className="flex flex-wrap items-start gap-2"> |
| 173 | "flex w-full items-center gap-2 p-3 text-left transition-opacity hover:opacity-100", | 230 | <CommentIdentity comment={comment} detail={detail} /> |
| 174 | open ? "opacity-100" : "opacity-65", | 231 | <CollapsibleTrigger |
| 175 | )} | 232 | ref={statusTriggerRef} |
| 176 | > | 233 | className="flex shrink-0 items-center gap-1.5 rounded-md px-2 py-1 text-xs text-muted-foreground hover:bg-muted hover:text-foreground" |
| 177 | <span className="flex min-w-0 flex-1 items-center gap-2 text-xs text-muted-foreground"> | 234 | > |
| 178 | <CommentAuthor actor={comment.author} /> | 235 | {label} |
| 179 | <span>{formatRelativeTimeLabel(comment.createdAt)}</span> | 236 | <ChevronDownIcon |
| 180 | <span>{label}</span> | 237 | aria-hidden |
| 181 | </span> | 238 | className={cn("size-3.5 transition-transform", open && "rotate-180")} |
| 182 | <ChevronDownIcon | 239 | /> |
| 183 | aria-hidden | 240 | </CollapsibleTrigger> |
| 184 | className={cn( | 241 | {reactionBar} |
| 185 | "size-3.5 shrink-0 text-muted-foreground/60 transition-transform", | 242 | </div> |
| 186 | open && "rotate-180", | 243 | <CommentLocation comment={comment} thread={thread} /> |
| 187 | )} | 244 | {!open && body ? ( |
| 188 | /> | 245 | <CollapsibleTrigger |
| 189 | </CollapsibleTrigger> | 246 | className="mt-2 block w-full truncate text-left text-xs text-muted-foreground hover:text-foreground" |
| 247 | onClick={() => statusTriggerRef.current?.focus({ preventScroll: true })} | ||
| 248 | > | ||
| 249 | {body | ||
| 250 | .replace(/<!--[\s\S]*?-->/gu, "") | ||
| 251 | .replace(/^\s*>?\s*\[!\w+\]\s*$/gmu, "") | ||
| 252 | .replace(/!?(\[([^\]]+)\])\([^)]*\)/gu, "$2") | ||
| 253 | .replace(/^[\s>#*-]+/gmu, "") | ||
| 254 | .replace(/[*`]/gu, "") | ||
| 255 | .replace(/\s+/g, " ") | ||
| 256 | .trim()} | ||
| 257 | </CollapsibleTrigger> | ||
| 258 | ) : null} | ||
| 259 | </div> | ||
| 190 | <CollapsiblePanel> | 260 | <CollapsiblePanel> |
| 191 | {open ? ( | 261 | {open ? ( |
| 192 | <div className="px-3 pb-3"> | 262 | <div className="px-3 pb-3"> |
| 193 | {comment.path ? ( | ||
| 194 | <Tooltip> | ||
| 195 | <TooltipTrigger | ||
| 196 | render={ | ||
| 197 | <p className="truncate text-xs text-muted-foreground">{comment.path}</p> | ||
| 198 | } | ||
| 199 | /> | ||
| 200 | <TooltipPopup side="top">{comment.path}</TooltipPopup> | ||
| 201 | </Tooltip> | ||
| 202 | ) : null} | ||
| 203 | {/* A dismissal carries no more words than an approval does, and an empty markdown | 263 | {/* A dismissal carries no more words than an approval does, and an empty markdown |
| 204 | block reads as a card somebody forgot to fill in. */} | 264 | block reads as a card somebody forgot to fill in. */} |
| 205 | {body === null && !editing.canEdit(comment) ? null : ( | 265 | {body === null && !editing.canEdit(comment) ? null : ( |
| 206 | <CommentBody className="mt-2" comment={comment} editing={editing} /> | 266 | <CommentBody className="mt-2" comment={comment} editing={editing} /> |
| 207 | )} | 267 | )} |
| 208 | {reactionBar} | ||
| 209 | </div> | 268 | </div> |
| 210 | ) : null} | 269 | ) : null} |
| 211 | </CollapsiblePanel> | 270 | </CollapsiblePanel> |
| ... | @@ -302,11 +361,107 @@ function Section({ | ... | @@ -302,11 +361,107 @@ function Section({ |
| 302 | ); | 361 | ); |
| 303 | } | 362 | } |
| 304 | 363 | ||
| 364 | function CommentGroup({ | ||
| 365 | label, | ||
| 366 | comments, | ||
| 367 | detail, | ||
| 368 | children, | ||
| 369 | onOpenChange, | ||
| 370 | }: { | ||
| 371 | label: string; | ||
| 372 | comments: readonly PullRequestComment[]; | ||
| 373 | detail: PullRequestDetailView; | ||
| 374 | children: ReactNode; | ||
| 375 | onOpenChange?: (open: boolean) => void; | ||
| 376 | }) { | ||
| 377 | const authors = [ | ||
| 378 | ...new Map( | ||
| 379 | comments.map((comment) => [reviewerKey(comment.author?.login ?? "ghost"), comment.author]), | ||
| 380 | ).values(), | ||
| 381 | ]; | ||
| 382 | const fileCount = new Set(comments.flatMap((comment) => (comment.path ? [comment.path] : []))) | ||
| 383 | .size; | ||
| 384 | const latest = comments.reduce<string | null>( | ||
| 385 | (date, comment) => (date === null || comment.createdAt > date ? comment.createdAt : date), | ||
| 386 | null, | ||
| 387 | ); | ||
| 388 | return ( | ||
| 389 | <Collapsible | ||
| 390 | className="overflow-hidden rounded-lg border border-border/70 bg-muted/20" | ||
| 391 | onOpenChange={onOpenChange} | ||
| 392 | > | ||
| 393 | <div className="flex items-center gap-3 pl-3"> | ||
| 394 | <div className="flex shrink-0 -space-x-1.5"> | ||
| 395 | {authors.slice(0, 3).map((actor) => ( | ||
| 396 | <PullRequestActorLabel | ||
| 397 | key={actor?.login ?? "ghost"} | ||
| 398 | actor={actor} | ||
| 399 | profileUrl={ | ||
| 400 | detail.provider === "github" && actor | ||
| 401 | ? new URL( | ||
| 402 | actor.isBot || actor.login.endsWith("[bot]") | ||
| 403 | ? `/apps/${encodeURIComponent(actor.login.replace(/\[bot\]$/, ""))}` | ||
| 404 | : `/${encodeURIComponent(actor.login)}`, | ||
| 405 | detail.url, | ||
| 406 | ).toString() | ||
| 407 | : null | ||
| 408 | } | ||
| 409 | labelClassName="sr-only" | ||
| 410 | className="relative rounded-full bg-background ring-2 ring-background hover:z-10 focus-visible:z-10 [&>img]:size-6 [&>span:first-child]:size-6" | ||
| 411 | /> | ||
| 412 | ))} | ||
| 413 | {authors.length > 3 ? ( | ||
| 414 | <span className="relative flex size-6 items-center justify-center rounded-full bg-muted text-[10px] text-muted-foreground ring-2 ring-background"> | ||
| 415 | +{authors.length - 3} | ||
| 416 | </span> | ||
| 417 | ) : null} | ||
| 418 | </div> | ||
| 419 | <CollapsibleTrigger | ||
| 420 | aria-label={label} | ||
| 421 | className="group flex min-w-0 flex-1 items-center gap-3 rounded-md py-3 pr-3 text-left hover:bg-muted/30" | ||
| 422 | > | ||
| 423 | <span className="min-w-0 flex-1 space-y-1"> | ||
| 424 | <span className="block text-xs font-medium text-foreground/90">{label}</span> | ||
| 425 | <span className="flex flex-wrap gap-x-1.5 text-[11px] text-muted-foreground"> | ||
| 426 | <span> | ||
| 427 | {authors.length} {authors.length === 1 ? "author" : "authors"} | ||
| 428 | </span> | ||
| 429 | {fileCount > 0 ? ( | ||
| 430 | <span> | ||
| 431 | · {fileCount} {fileCount === 1 ? "file" : "files"} | ||
| 432 | </span> | ||
| 433 | ) : null} | ||
| 434 | {latest ? ( | ||
| 435 | <span> | ||
| 436 | · Latest{" "} | ||
| 437 | <Tooltip> | ||
| 438 | <TooltipTrigger render={<time dateTime={latest} />}> | ||
| 439 | {formatRelativeTimeLabel(latest)} | ||
| 440 | </TooltipTrigger> | ||
| 441 | <TooltipPopup>{new Date(latest).toLocaleString()}</TooltipPopup> | ||
| 442 | </Tooltip> | ||
| 443 | </span> | ||
| 444 | ) : null} | ||
| 445 | </span> | ||
| 446 | </span> | ||
| 447 | <ChevronRightIcon | ||
| 448 | aria-hidden | ||
| 449 | className="size-3.5 shrink-0 text-muted-foreground transition-transform group-data-panel-open:rotate-90" | ||
| 450 | /> | ||
| 451 | </CollapsibleTrigger> | ||
| 452 | </div> | ||
| 453 | <CollapsiblePanel keepMounted> | ||
| 454 | <div className="border-t border-border/60 px-3 pb-3">{children}</div> | ||
| 455 | </CollapsiblePanel> | ||
| 456 | </Collapsible> | ||
| 457 | ); | ||
| 458 | } | ||
| 459 | |||
| 305 | /** | 460 | /** |
| 306 | * What a first render of the conversation carries. A pull request with two hundred comments is | 461 | * What a first render of the conversation carries. A pull request with two hundred comments is |
| 307 | * two hundred markdown documents, and the ones worth arriving for are the recent ones. | 462 | * two hundred markdown documents, and the ones worth arriving for are the recent ones. |
| 308 | */ | 463 | */ |
| 309 | const COMMENT_PAGE = 30; | 464 | const COMMENT_PAGE = 10; |
| 310 | 465 | ||
| 311 | export function PullRequestSummaryTab({ | 466 | export function PullRequestSummaryTab({ |
| 312 | environmentId, | 467 | environmentId, |
| ... | @@ -337,11 +492,35 @@ export function PullRequestSummaryTab({ | ... | @@ -337,11 +492,35 @@ export function PullRequestSummaryTab({ |
| 337 | // Keyed by the pull request, so opening another one starts at the end of its conversation | 492 | // Keyed by the pull request, so opening another one starts at the end of its conversation |
| 338 | // rather than wherever the last one had been read back to. | 493 | // rather than wherever the last one had been read back to. |
| 339 | const [shown, setShown] = useState({ url: detail.url, count: COMMENT_PAGE }); | 494 | const [shown, setShown] = useState({ url: detail.url, count: COMMENT_PAGE }); |
| 495 | const [openedBotGroup, setOpenedBotGroup] = useState<string | null>(null); | ||
| 496 | const [shownBots, setShownBots] = useState({ url: detail.url, count: COMMENT_PAGE }); | ||
| 497 | const shownBotComments = shownBots.url === detail.url ? shownBots.count : COMMENT_PAGE; | ||
| 340 | const shownComments = shown.url === detail.url ? shown.count : COMMENT_PAGE; | 498 | const shownComments = shown.url === detail.url ? shown.count : COMMENT_PAGE; |
| 499 | // A comment that already lives on a review thread is that thread: the thread carries the line | ||
| 500 | // and side the bare comment has lost, and a resolved one is finished work nobody should be | ||
| 501 | // invited to fix again — the same call the whole-review hand-off makes. | ||
| 502 | const threadByCommentId = new Map( | ||
| 503 | detail.reviewThreads.flatMap((thread) => | ||
| 504 | thread.comments.map((comment) => [comment.id, thread] as const), | ||
| 505 | ), | ||
| 506 | ); | ||
| 507 | |||
| 508 | const activeComments: PullRequestComment[] = []; | ||
| 509 | const finishedComments: PullRequestComment[] = []; | ||
| 510 | const botComments: PullRequestComment[] = []; | ||
| 511 | for (const comment of detail.comments) { | ||
| 512 | const finished = | ||
| 513 | threadByCommentId.get(comment.id)?.isResolved || | ||
| 514 | pullRequestReviewOutcome(comment.reviewState) === "dismissed"; | ||
| 515 | const bot = comment.author?.isBot === true || comment.author?.login.endsWith("[bot]"); | ||
| 516 | (finished ? finishedComments : bot ? botComments : activeComments).push(comment); | ||
| 517 | } | ||
| 341 | // Windowed by recency regardless of display order: expanding always reaches further back in | 518 | // Windowed by recency regardless of display order: expanding always reaches further back in |
| 342 | // time, whether the newest comment currently reads first or last. | 519 | // time, whether the newest comment currently reads first or last. |
| 343 | const recentComments = detail.comments.slice(Math.max(0, detail.comments.length - shownComments)); | 520 | const recentComments = activeComments.slice(Math.max(0, activeComments.length - shownComments)); |
| 344 | const hiddenCommentCount = detail.comments.length - recentComments.length; | 521 | const hiddenCommentCount = activeComments.length - recentComments.length; |
| 522 | const recentBotComments = botComments.slice(Math.max(0, botComments.length - shownBotComments)); | ||
| 523 | const hiddenBotCommentCount = botComments.length - recentBotComments.length; | ||
| 345 | const [commentOrder, setCommentOrder] = useState<"newest" | "oldest">("newest"); | 524 | const [commentOrder, setCommentOrder] = useState<"newest" | "oldest">("newest"); |
| 346 | const visibleComments = orderPullRequestComments(recentComments, commentOrder); | 525 | const visibleComments = orderPullRequestComments(recentComments, commentOrder); |
| 347 | const showOldestCommentsButton = | 526 | const showOldestCommentsButton = |
| ... | @@ -352,12 +531,12 @@ export function PullRequestSummaryTab({ | ... | @@ -352,12 +531,12 @@ export function PullRequestSummaryTab({ |
| 352 | className="w-full" | 531 | className="w-full" |
| 353 | onClick={() => setShown({ url: detail.url, count: shownComments + COMMENT_PAGE })} | 532 | onClick={() => setShown({ url: detail.url, count: shownComments + COMMENT_PAGE })} |
| 354 | > | 533 | > |
| 355 | Show {Math.min(hiddenCommentCount, COMMENT_PAGE)} oldest{" "} | 534 | Show {Math.min(hiddenCommentCount, COMMENT_PAGE)} older comment |
| 356 | {hiddenCommentCount === 1 ? "comment" : "comments"} | 535 | {hiddenCommentCount === 1 ? "" : "s"} ({hiddenCommentCount} hidden) |
| 357 | </Button> | 536 | </Button> |
| 358 | ) : null; | 537 | ) : null; |
| 359 | // Read from the whole conversation, not the window shown below it: a verdict older than the | 538 | // Read from the whole conversation, not the window shown below it: a verdict older than the |
| 360 | // last thirty comments still stands. | 539 | // visible comments still stands. |
| 361 | const reviewOutcomes = latestPullRequestReviewOutcomes(detail.comments, detail.commits); | 540 | const reviewOutcomes = latestPullRequestReviewOutcomes(detail.comments, detail.commits); |
| 362 | // Hosts do not promise one casing for a login across two fields of the same response, and | 541 | // Hosts do not promise one casing for a login across two fields of the same response, and |
| 363 | // none of them lets `Octocat` and `octocat` be two people — so matching on the literal string | 542 | // none of them lets `Octocat` and `octocat` be two people — so matching on the literal string |
| ... | @@ -393,15 +572,6 @@ export function PullRequestSummaryTab({ | ... | @@ -393,15 +572,6 @@ export function PullRequestSummaryTab({ |
| 393 | })), | 572 | })), |
| 394 | ]; | 573 | ]; |
| 395 | 574 | ||
| 396 | // A comment that already lives on a review thread is that thread: the thread carries the line | ||
| 397 | // and side the bare comment has lost, and a resolved one is finished work nobody should be | ||
| 398 | // invited to fix again — the same call the whole-review hand-off makes. | ||
| 399 | const threadByCommentId = new Map( | ||
| 400 | detail.reviewThreads.flatMap((thread) => | ||
| 401 | thread.comments.map((comment) => [comment.id, thread] as const), | ||
| 402 | ), | ||
| 403 | ); | ||
| 404 | |||
| 405 | const openLink = useOpenLink(threadRef); | 575 | const openLink = useOpenLink(threadRef); |
| 406 | const openCheck = (url: string) => { | 576 | const openCheck = (url: string) => { |
| 407 | void openLink(url).catch((error: unknown) => { | 577 | void openLink(url).catch((error: unknown) => { |
| ... | @@ -470,6 +640,78 @@ export function PullRequestSummaryTab({ | ... | @@ -470,6 +640,78 @@ export function PullRequestSummaryTab({ |
| 470 | }, | 640 | }, |
| 471 | }; | 641 | }; |
| 472 | 642 | ||
| 643 | const renderComment = (comment: PullRequestComment) => { | ||
| 644 | const thread = threadByCommentId.get(comment.id); | ||
| 645 | const body = visibleBody(comment.body); | ||
| 646 | const outcome = pullRequestReviewOutcome(comment.reviewState); | ||
| 647 | // An approval is a verdict, not a finding: there is nothing in it to fix. | ||
| 648 | const finding: PullRequestFinding | null = | ||
| 649 | (comment.kind !== "review" && comment.kind !== "review-comment") || outcome === "approved" | ||
| 650 | ? null | ||
| 651 | : thread === undefined | ||
| 652 | ? // Nor is a remark with nothing in it: offering to hand an empty review | ||
| 653 | // to a thread promises work it does not describe. | ||
| 654 | body === null | ||
| 655 | ? null | ||
| 656 | : { kind: "comment", comment } | ||
| 657 | : { kind: "thread", thread }; | ||
| 658 | const reactionBar = ( | ||
| 659 | <PullRequestReactionBar | ||
| 660 | reactions={comment.reactions ?? []} | ||
| 661 | canReact={detail.capabilities.reactions === true} | ||
| 662 | subjectId={comment.id} | ||
| 663 | environmentId={environmentId} | ||
| 664 | reference={reference} | ||
| 665 | onRefresh={onRefresh} | ||
| 666 | className="ml-auto justify-end" | ||
| 667 | /> | ||
| 668 | ); | ||
| 669 | return ( | ||
| 670 | <article | ||
| 671 | key={`${detail.url}:${comment.id}`} | ||
| 672 | // Offscreen comments skip style, layout and paint. Bot comments carry pages of | ||
| 673 | // highlighted code, and the conversation is below the description either way. | ||
| 674 | className="group rounded-lg border border-border/60 bg-background [contain-intrinsic-block-size:160px] [content-visibility:auto]" | ||
| 675 | > | ||
| 676 | <div className="flex flex-wrap items-start gap-2 rounded-t-lg bg-muted/25 px-3 py-2.5"> | ||
| 677 | <div className="flex min-w-0 flex-1 flex-wrap items-center gap-2 text-xs text-muted-foreground"> | ||
| 678 | <CommentIdentity comment={comment} detail={detail} /> | ||
| 679 | {outcome ? ( | ||
| 680 | <PullRequestReviewOutcomeBadge outcome={outcome} /> | ||
| 681 | ) : comment.reviewState ? ( | ||
| 682 | <span>{reviewStateLabel(comment.reviewState)}</span> | ||
| 683 | ) : null} | ||
| 684 | </div> | ||
| 685 | {/* Review remarks only. A plain conversation comment is talk, not a finding, | ||
| 686 | and offering to fix one would promise more than it says. */} | ||
| 687 | {onFixFinding && finding ? ( | ||
| 688 | <Button | ||
| 689 | size="xs" | ||
| 690 | variant="ghost" | ||
| 691 | className="-mt-1 shrink-0" | ||
| 692 | disabled={pendingFinding !== null && pendingFinding !== undefined} | ||
| 693 | onClick={() => onFixFinding(finding)} | ||
| 694 | > | ||
| 695 | <HammerIcon className="size-3" /> | ||
| 696 | {pendingFinding === pullRequestFindingKey(finding) ? "Preparing..." : fixFindingLabel} | ||
| 697 | </Button> | ||
| 698 | ) : null} | ||
| 699 | {reactionBar} | ||
| 700 | </div> | ||
| 701 | <div className="px-3"> | ||
| 702 | <CommentLocation comment={comment} thread={thread} /> | ||
| 703 | </div> | ||
| 704 | {/* A verdict usually carries no words, and an empty markdown block reads as | ||
| 705 | a card somebody forgot to fill in — the badge above already said it. | ||
| 706 | Kept where this reader may rewrite the remark: the pencil lives in here, | ||
| 707 | and hiding the block would take away the only way back to it. */} | ||
| 708 | {body === null && !commentEditing.canEdit(comment) ? null : ( | ||
| 709 | <CommentBody className="px-3 py-3" comment={comment} editing={commentEditing} /> | ||
| 710 | )} | ||
| 711 | </article> | ||
| 712 | ); | ||
| 713 | }; | ||
| 714 | |||
| 473 | return ( | 715 | return ( |
| 474 | <div className="h-full overflow-y-auto" data-pull-request-summary-scroll> | 716 | <div className="h-full overflow-y-auto" data-pull-request-summary-scroll> |
| 475 | <section className="px-4 pt-2.5 pb-1"> | 717 | <section className="px-4 pt-2.5 pb-1"> |
| ... | @@ -694,7 +936,7 @@ export function PullRequestSummaryTab({ | ... | @@ -694,7 +936,7 @@ export function PullRequestSummaryTab({ |
| 694 | </Section> | 936 | </Section> |
| 695 | 937 | ||
| 696 | <Section | 938 | <Section |
| 697 | title="Comments" | 939 | title={`Comments (${detail.commentCount})`} |
| 698 | actions={ | 940 | actions={ |
| 699 | <Button | 941 | <Button |
| 700 | size="xs" | 942 | size="xs" |
| ... | @@ -729,118 +971,99 @@ export function PullRequestSummaryTab({ | ... | @@ -729,118 +971,99 @@ export function PullRequestSummaryTab({ |
| 729 | ) : ( | 971 | ) : ( |
| 730 | <div className="space-y-3"> | 972 | <div className="space-y-3"> |
| 731 | {commentOrder === "oldest" ? showOldestCommentsButton : null} | 973 | {commentOrder === "oldest" ? showOldestCommentsButton : null} |
| 732 | {visibleComments.map((comment) => { | 974 | {visibleComments.map(renderComment)} |
| 733 | const thread = threadByCommentId.get(comment.id); | 975 | {commentOrder === "newest" ? showOldestCommentsButton : null} |
| 734 | const body = visibleBody(comment.body); | 976 | {shownComments > COMMENT_PAGE ? ( |
| 735 | const outcome = pullRequestReviewOutcome(comment.reviewState); | 977 | <Button |
| 736 | if (thread?.isResolved || outcome === "dismissed") { | 978 | size="sm" |
| 737 | return ( | 979 | variant="ghost" |
| 738 | <CollapsedComment | 980 | className="w-full" |
| 739 | key={comment.id} | 981 | onClick={() => setShown({ url: detail.url, count: COMMENT_PAGE })} |
| 740 | comment={comment} | 982 | > |
| 741 | editing={commentEditing} | 983 | Show only {COMMENT_PAGE} recent comments |
| 742 | label={thread?.isResolved ? "Resolved" : "Approval dismissed"} | 984 | </Button> |
| 743 | body={body} | 985 | ) : null} |
| 744 | reactionBar={ | 986 | {botComments.length > 0 ? ( |
| 745 | <PullRequestReactionBar | 987 | <CommentGroup |
| 746 | className="mt-2" | 988 | key={`bots:${detail.url}`} |
| 747 | reactions={comment.reactions ?? []} | 989 | label={`${botComments.length} bot comment${botComments.length === 1 ? "" : "s"}`} |
| 748 | canReact={detail.capabilities.reactions === true} | 990 | comments={botComments} |
| 749 | subjectId={comment.id} | 991 | detail={detail} |
| 750 | environmentId={environmentId} | 992 | onOpenChange={(open) => { |
| 751 | reference={reference} | 993 | if (open) setOpenedBotGroup(detail.url); |
| 752 | onRefresh={onRefresh} | 994 | }} |
| 753 | /> | 995 | > |
| 754 | } | 996 | <div className="space-y-3 pt-2"> |
| 755 | /> | 997 | {openedBotGroup === detail.url |
| 756 | ); | 998 | ? orderPullRequestComments(recentBotComments, commentOrder).map( |
| 757 | } | 999 | renderComment, |
| 758 | // An approval is a verdict, not a finding: there is nothing in it to fix. | 1000 | ) |
| 759 | const finding: PullRequestFinding | null = | 1001 | : null} |
| 760 | (comment.kind !== "review" && comment.kind !== "review-comment") || | 1002 | {hiddenBotCommentCount > 0 ? ( |
| 761 | outcome === "approved" | 1003 | <Button |
| 762 | ? null | 1004 | size="sm" |
| 763 | : thread === undefined | 1005 | variant="outline" |
| 764 | ? // Nor is a remark with nothing in it: offering to hand an empty review | 1006 | className="w-full" |
| 765 | // to a thread promises work it does not describe. | 1007 | onClick={() => |
| 766 | body === null | 1008 | setShownBots({ |
| 767 | ? null | 1009 | url: detail.url, |
| 768 | : { kind: "comment", comment } | 1010 | count: shownBotComments + COMMENT_PAGE, |
| 769 | : { kind: "thread", thread }; | 1011 | }) |
| 770 | // One bar, two homes. Under a card with words in it, it is the row beneath | 1012 | } |
| 771 | // them. A bodiless verdict has nothing above it, so a row reserved for an add | 1013 | > |
| 772 | // button nobody can see until they hover is a hole — there it rides the header | 1014 | Show {Math.min(hiddenBotCommentCount, COMMENT_PAGE)} older bot comment |
| 773 | // line instead, which keeps the affordance every sibling card offers. | 1015 | {hiddenBotCommentCount === 1 ? "" : "s"} ({hiddenBotCommentCount} hidden) |
| 774 | const reactionBar = ( | 1016 | </Button> |
| 775 | <PullRequestReactionBar | 1017 | ) : null} |
| 776 | reactions={comment.reactions ?? []} | 1018 | {shownBotComments > COMMENT_PAGE ? ( |
| 777 | canReact={detail.capabilities.reactions === true} | 1019 | <Button |
| 778 | subjectId={comment.id} | 1020 | size="sm" |
| 779 | environmentId={environmentId} | 1021 | variant="ghost" |
| 780 | reference={reference} | 1022 | className="w-full" |
| 781 | onRefresh={onRefresh} | 1023 | onClick={() => setShownBots({ url: detail.url, count: COMMENT_PAGE })} |
| 782 | {...(body === null ? {} : { className: "mt-2" })} | 1024 | > |
| 783 | /> | 1025 | Show only {COMMENT_PAGE} recent bot comments |
| 784 | ); | 1026 | </Button> |
| 785 | return ( | 1027 | ) : null} |
| 786 | <article | 1028 | </div> |
| 787 | key={comment.id} | 1029 | </CommentGroup> |
| 788 | // Offscreen comments skip style, layout and paint. Bot comments carry pages of | 1030 | ) : null} |
| 789 | // highlighted code, and the conversation is below the description either way. | 1031 | {finishedComments.length > 0 ? ( |
| 790 | className="group rounded-lg border border-border/60 p-3 [contain-intrinsic-block-size:120px] [content-visibility:auto]" | 1032 | <CommentGroup |
| 791 | > | 1033 | key={detail.url} |
| 792 | <div className="flex items-start gap-2"> | 1034 | label={`${finishedComments.length} resolved or dismissed comment${finishedComments.length === 1 ? "" : "s"}`} |
| 793 | <span className="flex min-w-0 flex-1 items-center gap-2 text-xs text-muted-foreground"> | 1035 | comments={finishedComments} |
| 794 | <CommentAuthor actor={comment.author} /> | 1036 | detail={detail} |
| 795 | <span>{formatRelativeTimeLabel(comment.createdAt)}</span> | 1037 | > |
| 796 | {outcome ? ( | 1038 | <div className="space-y-2 pt-2"> |
| 797 | <PullRequestReviewOutcomeBadge outcome={outcome} /> | 1039 | {orderPullRequestComments(finishedComments, commentOrder).map((comment) => { |
| 798 | ) : comment.reviewState ? ( | 1040 | const thread = threadByCommentId.get(comment.id); |
| 799 | <span>{reviewStateLabel(comment.reviewState)}</span> | 1041 | return ( |
| 800 | ) : null} | 1042 | <CollapsedComment |
| 801 | {body === null ? reactionBar : null} | 1043 | key={comment.id} |
| 802 | </span> | 1044 | comment={comment} |
| 803 | {/* Review remarks only. A plain conversation comment is talk, not a finding, | 1045 | editing={commentEditing} |
| 804 | and offering to fix one would promise more than it says. */} | 1046 | detail={detail} |
| 805 | {onFixFinding && finding ? ( | 1047 | thread={thread} |
| 806 | <Button | 1048 | label={thread?.isResolved ? "Resolved" : "Review dismissed"} |
| 807 | size="xs" | 1049 | body={visibleBody(comment.body)} |
| 808 | variant="ghost" | 1050 | reactionBar={ |
| 809 | className="-mt-1 shrink-0" | 1051 | <PullRequestReactionBar |
| 810 | disabled={pendingFinding !== null && pendingFinding !== undefined} | 1052 | className="ml-auto justify-end" |
| 811 | onClick={() => onFixFinding(finding)} | 1053 | reactions={comment.reactions ?? []} |
| 812 | > | 1054 | canReact={detail.capabilities.reactions === true} |
| 813 | <HammerIcon className="size-3" /> | 1055 | subjectId={comment.id} |
| 814 | {pendingFinding === pullRequestFindingKey(finding) | 1056 | environmentId={environmentId} |
| 815 | ? "Preparing..." | 1057 | reference={reference} |
| 816 | : fixFindingLabel} | 1058 | onRefresh={onRefresh} |
| 817 | </Button> | 1059 | /> |
| 818 | ) : null} | ||
| 819 | </div> | ||
| 820 | {comment.path ? ( | ||
| 821 | <Tooltip> | ||
| 822 | <TooltipTrigger | ||
| 823 | render={ | ||
| 824 | <p className="mt-1 truncate text-xs text-muted-foreground"> | ||
| 825 | {comment.path} | ||
| 826 | </p> | ||
| 827 | } | 1060 | } |
| 828 | /> | 1061 | /> |
| 829 | <TooltipPopup side="top">{comment.path}</TooltipPopup> | 1062 | ); |
| 830 | </Tooltip> | 1063 | })} |
| 831 | ) : null} | 1064 | </div> |
| 832 | {/* A verdict usually carries no words, and an empty markdown block reads as | 1065 | </CommentGroup> |
| 833 | a card somebody forgot to fill in — the badge above already said it. | 1066 | ) : null} |
| 834 | Kept where this reader may rewrite the remark: the pencil lives in here, | ||
| 835 | and hiding the block would take away the only way back to it. */} | ||
| 836 | {body === null && !commentEditing.canEdit(comment) ? null : ( | ||
| 837 | <CommentBody className="mt-2" comment={comment} editing={commentEditing} /> | ||
| 838 | )} | ||
| 839 | {body === null ? null : reactionBar} | ||
| 840 | </article> | ||
| 841 | ); | ||
| 842 | })} | ||
| 843 | {commentOrder === "newest" ? showOldestCommentsButton : null} | ||
| 844 | </div> | 1067 | </div> |
| 845 | )} | 1068 | )} |
| 846 | </> | 1069 | </> |
apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx+37-40| ... | @@ -217,7 +217,7 @@ function ConversationCard({ | ... | @@ -217,7 +217,7 @@ function ConversationCard({ |
| 217 | return ( | 217 | return ( |
| 218 | <article className="group py-2"> | 218 | <article className="group py-2"> |
| 219 | <div className="px-2"> | 219 | <div className="px-2"> |
| 220 | <div className="flex min-w-0 items-start gap-2"> | 220 | <div className="flex min-w-0 flex-wrap items-start gap-2"> |
| 221 | <div className="min-w-0 flex-1"> | 221 | <div className="min-w-0 flex-1"> |
| 222 | <div className="flex min-w-0 flex-wrap items-center gap-x-1.5 gap-y-1 text-xs"> | 222 | <div className="flex min-w-0 flex-wrap items-center gap-x-1.5 gap-y-1 text-xs"> |
| 223 | <ActorName actor={event.actor} /> | 223 | <ActorName actor={event.actor} /> |
| ... | @@ -245,6 +245,17 @@ function ConversationCard({ | ... | @@ -245,6 +245,17 @@ function ConversationCard({ |
| 245 | <PencilIcon className="size-3" /> | 245 | <PencilIcon className="size-3" /> |
| 246 | </Button> | 246 | </Button> |
| 247 | ) : null} | 247 | ) : null} |
| 248 | {reactions.canReact || event.reactions.length > 0 ? ( | ||
| 249 | <PullRequestReactionBar | ||
| 250 | className="ml-auto justify-end" | ||
| 251 | reactions={event.reactions} | ||
| 252 | canReact={reactions.canReact} | ||
| 253 | subjectId={event.id} | ||
| 254 | environmentId={reactions.environmentId} | ||
| 255 | reference={reactions.reference} | ||
| 256 | onRefresh={reactions.onRefresh} | ||
| 257 | /> | ||
| 258 | ) : null} | ||
| 248 | <OpenOnHostButton url={event.url} onOpen={onOpen} /> | 259 | <OpenOnHostButton url={event.url} onOpen={onOpen} /> |
| 249 | </div> | 260 | </div> |
| 250 | </div> | 261 | </div> |
| ... | @@ -272,18 +283,6 @@ function ConversationCard({ | ... | @@ -272,18 +283,6 @@ function ConversationCard({ |
| 272 | /> | 283 | /> |
| 273 | </div> | 284 | </div> |
| 274 | ) : null} | 285 | ) : null} |
| 275 | {reactions.canReact || event.reactions.length > 0 ? ( | ||
| 276 | <div className="px-2 pb-2"> | ||
| 277 | <PullRequestReactionBar | ||
| 278 | reactions={event.reactions} | ||
| 279 | canReact={reactions.canReact} | ||
| 280 | subjectId={event.id} | ||
| 281 | environmentId={reactions.environmentId} | ||
| 282 | reference={reactions.reference} | ||
| 283 | onRefresh={reactions.onRefresh} | ||
| 284 | /> | ||
| 285 | </div> | ||
| 286 | ) : null} | ||
| 287 | </article> | 286 | </article> |
| 288 | ); | 287 | ); |
| 289 | } | 288 | } |
| ... | @@ -469,14 +468,14 @@ function ReviewVerdictEvent({ | ... | @@ -469,14 +468,14 @@ function ReviewVerdictEvent({ |
| 469 | }) { | 468 | }) { |
| 470 | return ( | 469 | return ( |
| 471 | <div className="group relative mb-5 pl-12 [contain-intrinsic-block-size:48px] [content-visibility:auto]"> | 470 | <div className="group relative mb-5 pl-12 [contain-intrinsic-block-size:48px] [content-visibility:auto]"> |
| 472 | {/* Pinned rather than centred: this row grows with a body and a reaction bar, and a | 471 | {/* Pinned rather than centred: this row grows with a body, and a |
| 473 | centred avatar drifts down beside them instead of sitting by the name. */} | 472 | centred avatar drifts down beside it instead of sitting by the name. */} |
| 474 | <ActorTimelineMarker | 473 | <ActorTimelineMarker |
| 475 | actors={event.actor ? [event.actor] : []} | 474 | actors={event.actor ? [event.actor] : []} |
| 476 | className="top-6" | 475 | className="top-6" |
| 477 | fallback={<PullRequestReviewOutcomeIcon outcome={outcome} />} | 476 | fallback={<PullRequestReviewOutcomeIcon outcome={outcome} />} |
| 478 | /> | 477 | /> |
| 479 | <div className="flex min-w-0 items-start gap-2 py-1.5"> | 478 | <div className="flex min-w-0 flex-wrap items-start gap-2 py-1.5"> |
| 480 | <div className="min-w-0 flex-1"> | 479 | <div className="min-w-0 flex-1"> |
| 481 | <div className="flex min-w-0 flex-wrap items-center gap-x-1.5 gap-y-1 text-xs"> | 480 | <div className="flex min-w-0 flex-wrap items-center gap-x-1.5 gap-y-1 text-xs"> |
| 482 | <ActorName actor={event.actor} /> | 481 | <ActorName actor={event.actor} /> |
| ... | @@ -504,9 +503,6 @@ function ReviewVerdictEvent({ | ... | @@ -504,9 +503,6 @@ function ReviewVerdictEvent({ |
| 504 | <TooltipPopup>{pullRequestReviewOutcomeStaleLabel(outcome)}</TooltipPopup> | 503 | <TooltipPopup>{pullRequestReviewOutcomeStaleLabel(outcome)}</TooltipPopup> |
| 505 | </Tooltip> | 504 | </Tooltip> |
| 506 | </div> | 505 | </div> |
| 507 | {/* The reaction bar rides this line rather than taking one of its own. Its add button | ||
| 508 | is invisible until hovered but still occupies `h-6`, and under a verdict — usually a | ||
| 509 | single line with no body — a row of that reserved on its own reads as a hole. */} | ||
| 510 | <div className="mt-0.5 flex min-w-0 flex-wrap items-center gap-x-2 gap-y-1"> | 506 | <div className="mt-0.5 flex min-w-0 flex-wrap items-center gap-x-2 gap-y-1"> |
| 511 | <PullRequestMetaLine className="flex-wrap text-[11px] text-muted-foreground"> | 507 | <PullRequestMetaLine className="flex-wrap text-[11px] text-muted-foreground"> |
| 512 | <span>{formatRelativeTimeLabel(event.at)}</span> | 508 | <span>{formatRelativeTimeLabel(event.at)}</span> |
| ... | @@ -517,31 +513,32 @@ function ReviewVerdictEvent({ | ... | @@ -517,31 +513,32 @@ function ReviewVerdictEvent({ |
| 517 | </span> | 513 | </span> |
| 518 | ) : null} | 514 | ) : null} |
| 519 | </PullRequestMetaLine> | 515 | </PullRequestMetaLine> |
| 520 | {reactions.canReact || event.reactions.length > 0 ? ( | ||
| 521 | <PullRequestReactionBar | ||
| 522 | reactions={event.reactions} | ||
| 523 | canReact={reactions.canReact} | ||
| 524 | subjectId={event.id} | ||
| 525 | environmentId={reactions.environmentId} | ||
| 526 | reference={reactions.reference} | ||
| 527 | onRefresh={reactions.onRefresh} | ||
| 528 | /> | ||
| 529 | ) : null} | ||
| 530 | </div> | 516 | </div> |
| 531 | {/* An approval usually carries no words. When it does they are the review, so they stay | ||
| 532 | visible rather than being folded away with the ordinary conversation. */} | ||
| 533 | {event.body ? ( | ||
| 534 | <TimelineBody | ||
| 535 | body={event.body} | ||
| 536 | markdown={event.markdown} | ||
| 537 | cwd={cwd} | ||
| 538 | environmentId={reactions.environmentId} | ||
| 539 | threadRef={reactions.threadRef} | ||
| 540 | /> | ||
| 541 | ) : null} | ||
| 542 | </div> | 517 | </div> |
| 518 | {reactions.canReact || event.reactions.length > 0 ? ( | ||
| 519 | <PullRequestReactionBar | ||
| 520 | className="ml-auto justify-end" | ||
| 521 | reactions={event.reactions} | ||
| 522 | canReact={reactions.canReact} | ||
| 523 | subjectId={event.id} | ||
| 524 | environmentId={reactions.environmentId} | ||
| 525 | reference={reactions.reference} | ||
| 526 | onRefresh={reactions.onRefresh} | ||
| 527 | /> | ||
| 528 | ) : null} | ||
| 543 | <OpenOnHostButton url={event.url} onOpen={onOpen} /> | 529 | <OpenOnHostButton url={event.url} onOpen={onOpen} /> |
| 544 | </div> | 530 | </div> |
| 531 | {/* An approval usually carries no words. When it does they are the review, so they stay | ||
| 532 | visible rather than being folded away with the ordinary conversation. */} | ||
| 533 | {event.body ? ( | ||
| 534 | <TimelineBody | ||
| 535 | body={event.body} | ||
| 536 | markdown={event.markdown} | ||
| 537 | cwd={cwd} | ||
| 538 | environmentId={reactions.environmentId} | ||
| 539 | threadRef={reactions.threadRef} | ||
| 540 | /> | ||
| 541 | ) : null} | ||
| 545 | </div> | 542 | </div> |
| 546 | ); | 543 | ); |
| 547 | } | 544 | } |
apps/web/src/components/pullRequest/pullRequestPresentation.tsx+4-1| ... | @@ -422,7 +422,10 @@ export function PullRequestActorLabel({ | ... | @@ -422,7 +422,10 @@ export function PullRequestActorLabel({ |
| 422 | > | 422 | > |
| 423 | {label} | 423 | {label} |
| 424 | </TooltipTrigger> | 424 | </TooltipTrigger> |
| 425 | <TooltipPopup side="top">{profileUrl ? `Open ${login}'s profile` : login}</TooltipPopup> | 425 | <TooltipPopup side="top"> |
| 426 | {actor?.name && actor.name !== login ? `${actor.name} (@${login})` : login} | ||
| 427 | {profileUrl ? " · Open profile" : ""} | ||
| 428 | </TooltipPopup> | ||
| 426 | </Tooltip> | 429 | </Tooltip> |
| 427 | ); | 430 | ); |
| 428 | } | 431 | } |
packages/contracts/src/pullRequest.ts+2| ... | @@ -121,6 +121,8 @@ export const PullRequestBaseComparison = Schema.Literals(["up-to-date", "behind" | ... | @@ -121,6 +121,8 @@ export const PullRequestBaseComparison = Schema.Literals(["up-to-date", "behind" |
| 121 | export type PullRequestBaseComparison = typeof PullRequestBaseComparison.Type; | 121 | export type PullRequestBaseComparison = typeof PullRequestBaseComparison.Type; |
| 122 | 122 | ||
| 123 | export const PullRequestActor = Schema.Struct({ | 123 | export const PullRequestActor = Schema.Struct({ |
| 124 | /** Present when the host identifies an automated account. */ | ||
| 125 | isBot: Schema.optional(Schema.Boolean), | ||
| 124 | login: TrimmedNonEmptyString, | 126 | login: TrimmedNonEmptyString, |
| 125 | name: Schema.NullOr(Schema.String), | 127 | name: Schema.NullOr(Schema.String), |
| 126 | /** Null where a host does not report one, which is what the initials fall back to. */ | 128 | /** Null where a host does not report one, which is what the initials fall back to. */ |