| author | |
| committer | |
| log | 92d4a2e996b19239d95e32a3feb05208e0a52306 |
| tree | aaff2a754b619f463f9af0d32072be5ff99a06c5 |
| parent | 2fab18e289bdb2b1b767f05a307729d0a0d1c002 |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: Julius Marminge <julius0216@outlook.com>5 files changed, 70 insertions(+), 17 deletions(-)
apps/web/src/components/pullRequest/PullRequestListFilters.test.tsx+19-3| ... | @@ -3,7 +3,7 @@ import { CircleIcon } from "lucide-react"; | ... | @@ -3,7 +3,7 @@ import { CircleIcon } from "lucide-react"; |
| 3 | import { Children, isValidElement, type ReactElement, type ReactNode } from "react"; | 3 | import { Children, isValidElement, type ReactElement, type ReactNode } from "react"; |
| 4 | import { describe, expect, it, vi } from "vite-plus/test"; | 4 | import { describe, expect, it, vi } from "vite-plus/test"; |
| 5 | 5 | ||
| 6 | import { PullRequestFiltersMenu } from "./PullRequestListFilters"; | 6 | import { PullRequestFiltersMenu, pullRequestProjectKey } from "./PullRequestListFilters"; |
| 7 | 7 | ||
| 8 | function findValueChange( | 8 | function findValueChange( |
| 9 | node: ReactNode, | 9 | node: ReactNode, |
| ... | @@ -129,7 +129,7 @@ describe("pull request filters menu", () => { | ... | @@ -129,7 +129,7 @@ describe("pull request filters menu", () => { |
| 129 | const radioGroup = findValueChange(view); | 129 | const radioGroup = findValueChange(view); |
| 130 | expect(radioGroup).toBeDefined(); | 130 | expect(radioGroup).toBeDefined(); |
| 131 | 131 | ||
| 132 | radioGroup?.props.onValueChange(`${environmentId} ${projectId}`); | 132 | radioGroup?.props.onValueChange(pullRequestProjectKey({ id: projectId, environmentId })); |
| 133 | expect(onProject).not.toHaveBeenCalled(); | 133 | expect(onProject).not.toHaveBeenCalled(); |
| 134 | 134 | ||
| 135 | radioGroup?.props.onValueChange("all"); | 135 | radioGroup?.props.onValueChange("all"); |
| ... | @@ -159,7 +159,23 @@ describe("pull request filters menu", () => { | ... | @@ -159,7 +159,23 @@ describe("pull request filters menu", () => { |
| 159 | const radioGroup = findValueChange(view); | 159 | const radioGroup = findValueChange(view); |
| 160 | expect(radioGroup).toBeDefined(); | 160 | expect(radioGroup).toBeDefined(); |
| 161 | 161 | ||
| 162 | radioGroup?.props.onValueChange(`env-2 ${projectId}`); | 162 | radioGroup?.props.onValueChange( |
| 163 | pullRequestProjectKey({ id: projectId, environmentId: "env-2" as EnvironmentId }), | ||
| 164 | ); | ||
| 163 | expect(onProject).toHaveBeenCalledWith(projectId, "env-2"); | 165 | expect(onProject).toHaveBeenCalledWith(projectId, "env-2"); |
| 164 | }); | 166 | }); |
| 167 | |||
| 168 | it("does not collide when environment and project ids contain spaces", () => { | ||
| 169 | expect( | ||
| 170 | pullRequestProjectKey({ | ||
| 171 | environmentId: "a b" as EnvironmentId, | ||
| 172 | id: "c" as ProjectId, | ||
| 173 | }), | ||
| 174 | ).not.toBe( | ||
| 175 | pullRequestProjectKey({ | ||
| 176 | environmentId: "a" as EnvironmentId, | ||
| 177 | id: "b c" as ProjectId, | ||
| 178 | }), | ||
| 179 | ); | ||
| 180 | }); | ||
| 165 | }); | 181 | }); |
apps/web/src/components/pullRequest/PullRequestListFilters.tsx+11-9| ... | @@ -122,10 +122,10 @@ const UNFILTERED_VALUE = "all"; | ... | @@ -122,10 +122,10 @@ const UNFILTERED_VALUE = "all"; |
| 122 | * A project's own radio value, carrying the server along with the id: the id alone is only | 122 | * A project's own radio value, carrying the server along with the id: the id alone is only |
| 123 | * unique within its own server, so two rows sharing one would otherwise both read as checked. | 123 | * unique within its own server, so two rows sharing one would otherwise both read as checked. |
| 124 | */ | 124 | */ |
| 125 | const projectMenuValue = (project: { | 125 | export const pullRequestProjectKey = (project: { |
| 126 | readonly id: ProjectId; | 126 | readonly id: ProjectId; |
| 127 | readonly environmentId: EnvironmentId; | 127 | readonly environmentId: EnvironmentId; |
| 128 | }) => `${project.environmentId} ${project.id}`; | 128 | }) => JSON.stringify([project.environmentId, project.id]); |
| 129 | 129 | ||
| 130 | const DRAFT_OPTIONS = [ | 130 | const DRAFT_OPTIONS = [ |
| 131 | { value: UNFILTERED_VALUE, label: "All", Icon: LayersIcon }, | 131 | { value: UNFILTERED_VALUE, label: "All", Icon: LayersIcon }, |
| ... | @@ -247,7 +247,7 @@ export function PullRequestFiltersMenu({ | ... | @@ -247,7 +247,7 @@ export function PullRequestFiltersMenu({ |
| 247 | * the reader is already choosing between projects, rather than as a count above the list | 247 | * the reader is already choosing between projects, rather than as a count above the list |
| 248 | * that says something is missing without saying which. | 248 | * that says something is missing without saying which. |
| 249 | */ | 249 | */ |
| 250 | unavailable: ReadonlyMap<ProjectId, string>; | 250 | unavailable: ReadonlyMap<string, string>; |
| 251 | /** The environment comes with the project id, since picking a row picks a specific server's copy of it. */ | 251 | /** The environment comes with the project id, since picking a row picks a specific server's copy of it. */ |
| 252 | onProject: (projectId: ProjectId | undefined, environmentId: EnvironmentId | undefined) => void; | 252 | onProject: (projectId: ProjectId | undefined, environmentId: EnvironmentId | undefined) => void; |
| 253 | }) { | 253 | }) { |
| ... | @@ -350,7 +350,7 @@ export function PullRequestFiltersMenu({ | ... | @@ -350,7 +350,7 @@ export function PullRequestFiltersMenu({ |
| 350 | value={ | 350 | value={ |
| 351 | projectId === undefined || projectEnvironmentId === undefined | 351 | projectId === undefined || projectEnvironmentId === undefined |
| 352 | ? ALL_PROJECTS_VALUE | 352 | ? ALL_PROJECTS_VALUE |
| 353 | : projectMenuValue({ id: projectId, environmentId: projectEnvironmentId }) | 353 | : pullRequestProjectKey({ id: projectId, environmentId: projectEnvironmentId }) |
| 354 | } | 354 | } |
| 355 | onValueChange={(next) => { | 355 | onValueChange={(next) => { |
| 356 | if (next === ALL_PROJECTS_VALUE) { | 356 | if (next === ALL_PROJECTS_VALUE) { |
| ... | @@ -359,7 +359,7 @@ export function PullRequestFiltersMenu({ | ... | @@ -359,7 +359,7 @@ export function PullRequestFiltersMenu({ |
| 359 | } | 359 | } |
| 360 | // The value carries both halves, since the id alone cannot tell two servers' rows | 360 | // The value carries both halves, since the id alone cannot tell two servers' rows |
| 361 | // apart once they share one. | 361 | // apart once they share one. |
| 362 | const project = projects.find((candidate) => projectMenuValue(candidate) === next); | 362 | const project = projects.find((candidate) => pullRequestProjectKey(candidate) === next); |
| 363 | if ( | 363 | if ( |
| 364 | project !== undefined && | 364 | project !== undefined && |
| 365 | (project.id !== projectId || project.environmentId !== projectEnvironmentId) | 365 | (project.id !== projectId || project.environmentId !== projectEnvironmentId) |
| ... | @@ -379,14 +379,16 @@ export function PullRequestFiltersMenu({ | ... | @@ -379,14 +379,16 @@ export function PullRequestFiltersMenu({ |
| 379 | as a broken menu rather than as a workspace with three unreadable repositories. */} | 379 | as a broken menu rather than as a workspace with three unreadable repositories. */} |
| 380 | {projects | 380 | {projects |
| 381 | .toSorted( | 381 | .toSorted( |
| 382 | (left, right) => Number(unavailable.has(left.id)) - Number(unavailable.has(right.id)), | 382 | (left, right) => |
| 383 | Number(unavailable.has(pullRequestProjectKey(left))) - | ||
| 384 | Number(unavailable.has(pullRequestProjectKey(right))), | ||
| 383 | ) | 385 | ) |
| 384 | .map((project) => { | 386 | .map((project) => { |
| 385 | const reason = unavailable.get(project.id); | 387 | const reason = unavailable.get(pullRequestProjectKey(project)); |
| 386 | return ( | 388 | return ( |
| 387 | <MenuRadioItem | 389 | <MenuRadioItem |
| 388 | key={projectMenuValue(project)} | 390 | key={pullRequestProjectKey(project)} |
| 389 | value={projectMenuValue(project)} | 391 | value={pullRequestProjectKey(project)} |
| 390 | disabled={reason !== undefined} | 392 | disabled={reason !== undefined} |
| 391 | title={reason} | 393 | title={reason} |
| 392 | > | 394 | > |
apps/web/src/components/pullRequest/pullRequestList.logic.test.ts+15-1| ... | @@ -1,4 +1,4 @@ | ... | @@ -1,4 +1,4 @@ |
| 1 | import type { EnvironmentId, PullRequestListEntry } from "@t3tools/contracts"; | 1 | import type { EnvironmentId, ProjectId, PullRequestListEntry } from "@t3tools/contracts"; |
| 2 | import { describe, expect, it } from "vite-plus/test"; | 2 | import { describe, expect, it } from "vite-plus/test"; |
| 3 | 3 | ||
| 4 | import { | 4 | import { |
| ... | @@ -710,6 +710,20 @@ describe("merging the environments' own listings", () => { | ... | @@ -710,6 +710,20 @@ describe("merging the environments' own listings", () => { |
| 710 | ); | 710 | ); |
| 711 | }); | 711 | }); |
| 712 | 712 | ||
| 713 | it("keeps project errors scoped to the environment that reported them", () => { | ||
| 714 | const error = { | ||
| 715 | projectId: "project-1" as ProjectId, | ||
| 716 | projectTitle: "Web", | ||
| 717 | message: "Not signed in", | ||
| 718 | } as const; | ||
| 719 | const merged = mergePullRequestLists([ | ||
| 720 | [ENV_1, answer({ errors: [error] })], | ||
| 721 | [ENV_2, answer()], | ||
| 722 | ]); | ||
| 723 | |||
| 724 | expect(merged?.errors).toEqual([{ ...error, environmentId: ENV_1 }]); | ||
| 725 | }); | ||
| 726 | |||
| 713 | it("folds a host reached from two environments into one switcher row", () => { | 727 | it("folds a host reached from two environments into one switcher row", () => { |
| 714 | const merged = mergePullRequestLists([ | 728 | const merged = mergePullRequestLists([ |
| 715 | [ENV_1, answer()], | 729 | [ENV_1, answer()], |
apps/web/src/components/pullRequest/pullRequestList.logic.ts+14-3| ... | @@ -3,6 +3,7 @@ import * as Schema from "effect/Schema"; | ... | @@ -3,6 +3,7 @@ import * as Schema from "effect/Schema"; |
| 3 | import { | 3 | import { |
| 4 | EnvironmentId, | 4 | EnvironmentId, |
| 5 | PullRequestListEntry, | 5 | PullRequestListEntry, |
| 6 | PullRequestListProjectError, | ||
| 6 | PullRequestListResult, | 7 | PullRequestListResult, |
| 7 | resolvePullRequestAuthorFilter, | 8 | resolvePullRequestAuthorFilter, |
| 8 | } from "@t3tools/contracts"; | 9 | } from "@t3tools/contracts"; |
| ... | @@ -27,6 +28,10 @@ export interface EnvironmentPullRequestStat extends PullRequestDiffStat { | ... | @@ -27,6 +28,10 @@ export interface EnvironmentPullRequestStat extends PullRequestDiffStat { |
| 27 | readonly environmentId: EnvironmentId; | 28 | readonly environmentId: EnvironmentId; |
| 28 | } | 29 | } |
| 29 | 30 | ||
| 31 | export interface EnvironmentPullRequestError extends PullRequestListProjectError { | ||
| 32 | readonly environmentId: EnvironmentId; | ||
| 33 | } | ||
| 34 | |||
| 30 | export type PullRequestGroupKey = "reviewRequested" | "authored" | "others"; | 35 | export type PullRequestGroupKey = "reviewRequested" | "authored" | "others"; |
| 31 | 36 | ||
| 32 | export interface PullRequestGroup<Entry extends PullRequestListEntry = PullRequestListEntry> { | 37 | export interface PullRequestGroup<Entry extends PullRequestListEntry = PullRequestListEntry> { |
| ... | @@ -452,7 +457,7 @@ export interface MergedPullRequestList { | ... | @@ -452,7 +457,7 @@ export interface MergedPullRequestList { |
| 452 | readonly viewers: PullRequestViewers; | 457 | readonly viewers: PullRequestViewers; |
| 453 | readonly providers: PullRequestListResult["providers"]; | 458 | readonly providers: PullRequestListResult["providers"]; |
| 454 | readonly entries: ReadonlyArray<EnvironmentPullRequestEntry>; | 459 | readonly entries: ReadonlyArray<EnvironmentPullRequestEntry>; |
| 455 | readonly errors: PullRequestListResult["errors"]; | 460 | readonly errors: ReadonlyArray<EnvironmentPullRequestError>; |
| 456 | readonly truncated: boolean; | 461 | readonly truncated: boolean; |
| 457 | readonly nextCursors: Readonly<Record<string, PullRequestListCursors>>; | 462 | readonly nextCursors: Readonly<Record<string, PullRequestListCursors>>; |
| 458 | /** | 463 | /** |
| ... | @@ -476,7 +481,7 @@ export function mergePullRequestLists( | ... | @@ -476,7 +481,7 @@ export function mergePullRequestLists( |
| 476 | const truncatedEnvironments: string[] = []; | 481 | const truncatedEnvironments: string[] = []; |
| 477 | const providers = new Map<string, PullRequestListResult["providers"][number]>(); | 482 | const providers = new Map<string, PullRequestListResult["providers"][number]>(); |
| 478 | const entries: EnvironmentPullRequestEntry[] = []; | 483 | const entries: EnvironmentPullRequestEntry[] = []; |
| 479 | const errors: Array<PullRequestListResult["errors"][number]> = []; | 484 | const errors: EnvironmentPullRequestError[] = []; |
| 480 | const nextCursors: Record<string, PullRequestListCursors> = {}; | 485 | const nextCursors: Record<string, PullRequestListCursors> = {}; |
| 481 | let truncated = false; | 486 | let truncated = false; |
| 482 | for (const [environmentId, answer] of answers) { | 487 | for (const [environmentId, answer] of answers) { |
| ... | @@ -498,7 +503,7 @@ export function mergePullRequestLists( | ... | @@ -498,7 +503,7 @@ export function mergePullRequestLists( |
| 498 | ); | 503 | ); |
| 499 | } | 504 | } |
| 500 | entries.push(...answer.entries.map((entry) => ({ ...entry, environmentId }))); | 505 | entries.push(...answer.entries.map((entry) => ({ ...entry, environmentId }))); |
| 501 | errors.push(...answer.errors); | 506 | errors.push(...answer.errors.map((error) => ({ ...error, environmentId }))); |
| 502 | truncated ||= answer.truncated; | 507 | truncated ||= answer.truncated; |
| 503 | if (answer.truncated) truncatedEnvironments.push(environmentId); | 508 | if (answer.truncated) truncatedEnvironments.push(environmentId); |
| 504 | if (Object.keys(answer.nextCursors).length > 0) { | 509 | if (Object.keys(answer.nextCursors).length > 0) { |
| ... | @@ -559,12 +564,18 @@ const EnvironmentPullRequestEntrySchema = Schema.Struct({ | ... | @@ -559,12 +564,18 @@ const EnvironmentPullRequestEntrySchema = Schema.Struct({ |
| 559 | environmentId: EnvironmentId, | 564 | environmentId: EnvironmentId, |
| 560 | }); | 565 | }); |
| 561 | 566 | ||
| 567 | const EnvironmentPullRequestErrorSchema = Schema.Struct({ | ||
| 568 | ...PullRequestListProjectError.fields, | ||
| 569 | environmentId: EnvironmentId, | ||
| 570 | }); | ||
| 571 | |||
| 562 | const decodeSnapshot = Schema.decodeUnknownOption( | 572 | const decodeSnapshot = Schema.decodeUnknownOption( |
| 563 | Schema.Struct({ | 573 | Schema.Struct({ |
| 564 | scope: Schema.String, | 574 | scope: Schema.String, |
| 565 | data: Schema.Struct({ | 575 | data: Schema.Struct({ |
| 566 | ...PullRequestListResult.fields, | 576 | ...PullRequestListResult.fields, |
| 567 | entries: Schema.Array(EnvironmentPullRequestEntrySchema), | 577 | entries: Schema.Array(EnvironmentPullRequestEntrySchema), |
| 578 | errors: Schema.Array(EnvironmentPullRequestErrorSchema), | ||
| 568 | // Per environment here, unlike the wire shape, which is per repository within one. | 579 | // Per environment here, unlike the wire shape, which is per repository within one. |
| 569 | nextCursors: Schema.Record(Schema.String, PullRequestListResult.fields.nextCursors), | 580 | nextCursors: Schema.Record(Schema.String, PullRequestListResult.fields.nextCursors), |
| 570 | truncatedEnvironments: Schema.Array(Schema.String), | 581 | truncatedEnvironments: Schema.Array(Schema.String), |
apps/web/src/routes/_chat.pull-requests.tsx+11-1| ... | @@ -60,6 +60,7 @@ import { | ... | @@ -60,6 +60,7 @@ import { |
| 60 | PullRequestFiltersMenu, | 60 | PullRequestFiltersMenu, |
| 61 | PullRequestSearchInput, | 61 | PullRequestSearchInput, |
| 62 | pullRequestHostLabel, | 62 | pullRequestHostLabel, |
| 63 | pullRequestProjectKey, | ||
| 63 | type PullRequestExpectedHost, | 64 | type PullRequestExpectedHost, |
| 64 | type PullRequestFilterOption, | 65 | type PullRequestFilterOption, |
| 65 | } from "../components/pullRequest/PullRequestListFilters"; | 66 | } from "../components/pullRequest/PullRequestListFilters"; |
| ... | @@ -1250,7 +1251,16 @@ function PullRequestsRouteView() { | ... | @@ -1250,7 +1251,16 @@ function PullRequestsRouteView() { |
| 1250 | 1251 | ||
| 1251 | /** Reported per project rather than as a count, so the reader can see which one it was. */ | 1252 | /** Reported per project rather than as a count, so the reader can see which one it was. */ |
| 1252 | const unavailableProjects = useMemo( | 1253 | const unavailableProjects = useMemo( |
| 1253 | () => new Map(listErrors.map((error) => [error.projectId, error.message] as const)), | 1254 | () => |
| 1255 | new Map( | ||
| 1256 | listErrors.map( | ||
| 1257 | (error) => | ||
| 1258 | [ | ||
| 1259 | pullRequestProjectKey({ id: error.projectId, environmentId: error.environmentId }), | ||
| 1260 | error.message, | ||
| 1261 | ] as const, | ||
| 1262 | ), | ||
| 1263 | ), | ||
| 1254 | [listErrors], | 1264 | [listErrors], |
| 1255 | ); | 1265 | ); |
| 1256 | 1266 |