| author | |
| committer | |
| log | 6a2d2466673c010a15cb530fa2a8f77b363e046d |
| tree | d930f265560b3d4227144c6c4862c343f059b138 |
| parent | c52b8d96e4b34201f19b5e5bb12c6b2a77bfaa9a |
| signature | Signed by PGP key B5690EEEBB952194 |
5 files changed, 427 insertions(+), 40 deletions(-)
apps/web/src/components/pullRequest/PullRequestLabelPicker.tsx+2-7| ... | @@ -33,15 +33,12 @@ export function PullRequestLabelPicker({ | ... | @@ -33,15 +33,12 @@ export function PullRequestLabelPicker({ |
| 33 | environmentId, | 33 | environmentId, |
| 34 | reference, | 34 | reference, |
| 35 | allowed, | 35 | allowed, |
| 36 | onChanged, | ||
| 37 | }: { | 36 | }: { |
| 38 | environmentId: EnvironmentId; | 37 | environmentId: EnvironmentId; |
| 39 | reference: PullRequestRef; | 38 | reference: PullRequestRef; |
| 40 | /** False where the host would refuse this account's change. Disabled with the reason rather | 39 | /** False where the host would refuse this account's change. Disabled with the reason rather |
| 41 | * than hidden, like the reviewer control beside it. */ | 40 | * than hidden, like the reviewer control beside it. */ |
| 42 | allowed: boolean; | 41 | allowed: boolean; |
| 43 | /** The detail carries the labels, so it is re-read once the host has taken the change. */ | ||
| 44 | onChanged: () => void; | ||
| 45 | }) { | 42 | }) { |
| 46 | const [open, setOpen] = useState(false); | 43 | const [open, setOpen] = useState(false); |
| 47 | const [query, setQuery] = useState(""); | 44 | const [query, setQuery] = useState(""); |
| ... | @@ -79,8 +76,6 @@ export function PullRequestLabelPicker({ | ... | @@ -79,8 +76,6 @@ export function PullRequestLabelPicker({ |
| 79 | }); | 76 | }); |
| 80 | return; | 77 | return; |
| 81 | } | 78 | } |
| 82 | onChanged(); | ||
| 83 | candidatesQuery.refresh(); | ||
| 84 | }; | 79 | }; |
| 85 | 80 | ||
| 86 | return ( | 81 | return ( |
| ... | @@ -94,8 +89,8 @@ export function PullRequestLabelPicker({ | ... | @@ -94,8 +89,8 @@ export function PullRequestLabelPicker({ |
| 94 | query={query} | 89 | query={query} |
| 95 | onQueryChange={setQuery} | 90 | onQueryChange={setQuery} |
| 96 | searchLabel="Search labels" | 91 | searchLabel="Search labels" |
| 97 | isPending={candidatesQuery.isPending} | 92 | isPending={candidatesQuery.isPending && candidatesQuery.data === null} |
| 98 | error={candidatesQuery.error} | 93 | error={candidatesQuery.data === null ? candidatesQuery.error : null} |
| 99 | candidates={candidates} | 94 | candidates={candidates} |
| 100 | emptyLabel="This repository has no labels." | 95 | emptyLabel="This repository has no labels." |
| 101 | noMatchLabel="No label matches that." | 96 | noMatchLabel="No label matches that." |
apps/web/src/components/pullRequest/PullRequestReviewerPicker.tsx+2-7| ... | @@ -38,15 +38,12 @@ export function PullRequestReviewerPicker({ | ... | @@ -38,15 +38,12 @@ export function PullRequestReviewerPicker({ |
| 38 | environmentId, | 38 | environmentId, |
| 39 | reference, | 39 | reference, |
| 40 | allowed, | 40 | allowed, |
| 41 | onRequested, | ||
| 42 | }: { | 41 | }: { |
| 43 | environmentId: EnvironmentId; | 42 | environmentId: EnvironmentId; |
| 44 | reference: PullRequestRef; | 43 | reference: PullRequestRef; |
| 45 | /** False where the host would refuse this account's request, which is worth saying rather than | 44 | /** False where the host would refuse this account's request, which is worth saying rather than |
| 46 | * hiding: the control disabled with a reason answers the question its absence would raise. */ | 45 | * hiding: the control disabled with a reason answers the question its absence would raise. */ |
| 47 | allowed: boolean; | 46 | allowed: boolean; |
| 48 | /** The detail carries who is requested, so it is re-read once the host has taken the change. */ | ||
| 49 | onRequested: () => void; | ||
| 50 | }) { | 47 | }) { |
| 51 | const [open, setOpen] = useState(false); | 48 | const [open, setOpen] = useState(false); |
| 52 | const [query, setQuery] = useState(""); | 49 | const [query, setQuery] = useState(""); |
| ... | @@ -96,8 +93,6 @@ export function PullRequestReviewerPicker({ | ... | @@ -96,8 +93,6 @@ export function PullRequestReviewerPicker({ |
| 96 | ? `Review request to ${candidate.login} taken back` | 93 | ? `Review request to ${candidate.login} taken back` |
| 97 | : `Review requested from ${candidate.login}`, | 94 | : `Review requested from ${candidate.login}`, |
| 98 | }); | 95 | }); |
| 99 | onRequested(); | ||
| 100 | candidatesQuery.refresh(); | ||
| 101 | }; | 96 | }; |
| 102 | 97 | ||
| 103 | return ( | 98 | return ( |
| ... | @@ -111,8 +106,8 @@ export function PullRequestReviewerPicker({ | ... | @@ -111,8 +106,8 @@ export function PullRequestReviewerPicker({ |
| 111 | query={query} | 106 | query={query} |
| 112 | onQueryChange={setQuery} | 107 | onQueryChange={setQuery} |
| 113 | searchLabel="Search people with access" | 108 | searchLabel="Search people with access" |
| 114 | isPending={candidatesQuery.isPending} | 109 | isPending={candidatesQuery.isPending && candidatesQuery.data === null} |
| 115 | error={candidatesQuery.error} | 110 | error={candidatesQuery.data === null ? candidatesQuery.error : null} |
| 116 | candidates={candidates} | 111 | candidates={candidates} |
| 117 | emptyLabel="Nobody else has access to this repository." | 112 | emptyLabel="Nobody else has access to this repository." |
| 118 | noMatchLabel="Nobody with access matches that." | 113 | noMatchLabel="Nobody with access matches that." |
apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx-2| ... | @@ -683,7 +683,6 @@ export function PullRequestSummaryTab({ | ... | @@ -683,7 +683,6 @@ export function PullRequestSummaryTab({ |
| 683 | environmentId={environmentId} | 683 | environmentId={environmentId} |
| 684 | reference={reference} | 684 | reference={reference} |
| 685 | allowed={detail.viewerPermissions.requestReviewers} | 685 | allowed={detail.viewerPermissions.requestReviewers} |
| 686 | onRequested={onRefresh} | ||
| 687 | /> | 686 | /> |
| 688 | ) : null} | 687 | ) : null} |
| 689 | </span> | 688 | </span> |
| ... | @@ -718,7 +717,6 @@ export function PullRequestSummaryTab({ | ... | @@ -718,7 +717,6 @@ export function PullRequestSummaryTab({ |
| 718 | environmentId={environmentId} | 717 | environmentId={environmentId} |
| 719 | reference={reference} | 718 | reference={reference} |
| 720 | allowed={detail.viewerPermissions.labels !== false} | 719 | allowed={detail.viewerPermissions.labels !== false} |
| 721 | onChanged={onRefresh} | ||
| 722 | /> | 720 | /> |
| 723 | ) : null} | 721 | ) : null} |
| 724 | </span> | 722 | </span> |
packages/client-runtime/src/state/pullRequests.test.ts+242| ... | @@ -1,5 +1,6 @@ | ... | @@ -1,5 +1,6 @@ |
| 1 | import { EnvironmentId, ProjectId, WS_METHODS, type PullRequestStack } from "@t3tools/contracts"; | 1 | import { EnvironmentId, ProjectId, WS_METHODS, type PullRequestStack } from "@t3tools/contracts"; |
| 2 | import { expect, it } from "@effect/vitest"; | 2 | import { expect, it } from "@effect/vitest"; |
| 3 | import * as Data from "effect/Data"; | ||
| 3 | import * as Effect from "effect/Effect"; | 4 | import * as Effect from "effect/Effect"; |
| 4 | import * as Latch from "effect/Latch"; | 5 | import * as Latch from "effect/Latch"; |
| 5 | import * as Layer from "effect/Layer"; | 6 | import * as Layer from "effect/Layer"; |
| ... | @@ -26,6 +27,8 @@ import { | ... | @@ -26,6 +27,8 @@ import { |
| 26 | import { PullRequestDiffLoader } from "./pullRequestDiffHttp.ts"; | 27 | import { PullRequestDiffLoader } from "./pullRequestDiffHttp.ts"; |
| 27 | import { executeAtomQuery } from "./runtime.ts"; | 28 | import { executeAtomQuery } from "./runtime.ts"; |
| 28 | 29 | ||
| 30 | class MutationRefused extends Data.TaggedError("MutationRefused") {} | ||
| 31 | |||
| 29 | const TARGET = new PrimaryConnectionTarget({ | 32 | const TARGET = new PrimaryConnectionTarget({ |
| 30 | environmentId: EnvironmentId.make("environment-1"), | 33 | environmentId: EnvironmentId.make("environment-1"), |
| 31 | label: "Test environment", | 34 | label: "Test environment", |
| ... | @@ -216,6 +219,245 @@ it.effect("refreshes pull request activity after a comment is updated", () => | ... | @@ -216,6 +219,245 @@ it.effect("refreshes pull request activity after a comment is updated", () => |
| 216 | ), | 219 | ), |
| 217 | ); | 220 | ); |
| 218 | 221 | ||
| 222 | it.effect("updates cached labels after successful edits without rereading the host", () => | ||
| 223 | Effect.scoped( | ||
| 224 | Effect.gen(function* () { | ||
| 225 | let detailReads = 0; | ||
| 226 | let candidateReads = 0; | ||
| 227 | let refuse = false; | ||
| 228 | let failDetail = false; | ||
| 229 | const existing = { name: "existing", color: "111111" }; | ||
| 230 | const addedLabel = { name: "new", color: "abcdef" }; | ||
| 231 | const detailRefreshStarted = yield* Latch.make(); | ||
| 232 | const releaseDetailRefresh = yield* Latch.make(); | ||
| 233 | const client = { | ||
| 234 | [WS_METHODS.pullRequestsSubscribeRefreshes]: () => Stream.never, | ||
| 235 | [WS_METHODS.pullRequestsDetail]: () => | ||
| 236 | Effect.gen(function* () { | ||
| 237 | detailReads++; | ||
| 238 | if (failDetail) { | ||
| 239 | yield* detailRefreshStarted.open; | ||
| 240 | yield* releaseDetailRefresh.await; | ||
| 241 | return yield* Effect.fail(new MutationRefused()); | ||
| 242 | } | ||
| 243 | return { title: "keep this title", labels: [existing] }; | ||
| 244 | }), | ||
| 245 | [WS_METHODS.pullRequestsLabelCandidates]: () => | ||
| 246 | Effect.sync(() => { | ||
| 247 | candidateReads++; | ||
| 248 | return { | ||
| 249 | candidates: [ | ||
| 250 | { ...existing, description: null, isApplied: true }, | ||
| 251 | { ...addedLabel, description: "description", isApplied: false }, | ||
| 252 | ], | ||
| 253 | truncated: false, | ||
| 254 | }; | ||
| 255 | }), | ||
| 256 | [WS_METHODS.pullRequestsSetLabels]: () => | ||
| 257 | refuse ? Effect.fail(new MutationRefused()) : Effect.void, | ||
| 258 | } as unknown as WsRpcProtocolClient; | ||
| 259 | const { atoms, registry } = yield* makeTestRuntime(client); | ||
| 260 | const target = { | ||
| 261 | environmentId: TARGET.environmentId, | ||
| 262 | input: { | ||
| 263 | projectId: ProjectId.make("project-1"), | ||
| 264 | repository: "acme/web", | ||
| 265 | number: 1, | ||
| 266 | host: "github.example.com", | ||
| 267 | }, | ||
| 268 | }; | ||
| 269 | const detail = atoms.detail(target); | ||
| 270 | const candidates = atoms.labelCandidates(target); | ||
| 271 | registry.mount(detail); | ||
| 272 | const unmountCandidates = registry.mount(candidates); | ||
| 273 | yield* AtomRegistry.getResult(registry, detail, { suspendOnWaiting: true }); | ||
| 274 | yield* AtomRegistry.getResult(registry, candidates, { suspendOnWaiting: true }); | ||
| 275 | |||
| 276 | const added = yield* Effect.promise(() => | ||
| 277 | atoms.setLabels.run(registry, { | ||
| 278 | ...target, | ||
| 279 | input: { | ||
| 280 | host: target.input.host, | ||
| 281 | projectId: target.input.projectId, | ||
| 282 | repository: target.input.repository, | ||
| 283 | number: target.input.number, | ||
| 284 | labels: ["new"], | ||
| 285 | applied: true, | ||
| 286 | }, | ||
| 287 | }), | ||
| 288 | ); | ||
| 289 | expect(AsyncResult.isSuccess(added)).toBe(true); | ||
| 290 | expect(yield* AtomRegistry.getResult(registry, detail)).toEqual({ | ||
| 291 | title: "keep this title", | ||
| 292 | labels: [existing, addedLabel], | ||
| 293 | }); | ||
| 294 | unmountCandidates(); | ||
| 295 | registry.mount(atoms.labelCandidates(target)); | ||
| 296 | expect((yield* AtomRegistry.getResult(registry, candidates)).candidates[1]).toEqual({ | ||
| 297 | ...addedLabel, | ||
| 298 | description: "description", | ||
| 299 | isApplied: true, | ||
| 300 | }); | ||
| 301 | |||
| 302 | for (const name of ["existing", "new"]) { | ||
| 303 | refuse = name === "new"; | ||
| 304 | const result = yield* Effect.promise(() => | ||
| 305 | atoms.setLabels.run(registry, { | ||
| 306 | ...target, | ||
| 307 | input: { ...target.input, labels: [name], applied: false }, | ||
| 308 | }), | ||
| 309 | ); | ||
| 310 | expect(result._tag).toBe(refuse ? "Failure" : "Success"); | ||
| 311 | expect((yield* AtomRegistry.getResult(registry, detail)).labels).toEqual([addedLabel]); | ||
| 312 | expect((yield* AtomRegistry.getResult(registry, candidates)).candidates).toMatchObject([ | ||
| 313 | { name: "existing", isApplied: false }, | ||
| 314 | { name: "new", isApplied: true }, | ||
| 315 | ]); | ||
| 316 | } | ||
| 317 | expect(detailReads).toBe(1); | ||
| 318 | expect(candidateReads).toBe(1); | ||
| 319 | |||
| 320 | failDetail = true; | ||
| 321 | registry.refresh(detail); | ||
| 322 | yield* detailRefreshStarted.await; | ||
| 323 | expect(registry.get(detail).waiting).toBe(true); | ||
| 324 | expect(Option.getOrThrow(AsyncResult.value(registry.get(detail))).labels).toEqual([ | ||
| 325 | addedLabel, | ||
| 326 | ]); | ||
| 327 | yield* releaseDetailRefresh.open; | ||
| 328 | yield* Effect.exit(AtomRegistry.getResult(registry, detail, { suspendOnWaiting: true })); | ||
| 329 | expect(AsyncResult.isFailure(registry.get(detail))).toBe(true); | ||
| 330 | expect(Option.getOrThrow(AsyncResult.value(registry.get(detail))).labels).toEqual([ | ||
| 331 | addedLabel, | ||
| 332 | ]); | ||
| 333 | failDetail = false; | ||
| 334 | registry.refresh(detail); | ||
| 335 | expect( | ||
| 336 | (yield* AtomRegistry.getResult(registry, detail, { suspendOnWaiting: true })).labels, | ||
| 337 | ).toEqual([existing]); | ||
| 338 | expect(detailReads).toBe(3); | ||
| 339 | }), | ||
| 340 | ), | ||
| 341 | ); | ||
| 342 | |||
| 343 | it.effect("updates reviewer requests and enriched reviewers without rereading the host", () => | ||
| 344 | Effect.scoped( | ||
| 345 | Effect.gen(function* () { | ||
| 346 | let reads = 0; | ||
| 347 | let refuse = false; | ||
| 348 | const actor = { login: "reviewer", name: "Reviewer", avatarUrl: null }; | ||
| 349 | const hostActor = { ...actor, login: "Reviewer" }; | ||
| 350 | let hostRequested = false; | ||
| 351 | let reviewed = false; | ||
| 352 | let pauseActivity = false; | ||
| 353 | const activityStarted = yield* Latch.make(); | ||
| 354 | const client = { | ||
| 355 | [WS_METHODS.pullRequestsSubscribeRefreshes]: () => Stream.never, | ||
| 356 | [WS_METHODS.pullRequestsDetail]: () => | ||
| 357 | Effect.sync(() => { | ||
| 358 | reads++; | ||
| 359 | return { reviewers: hostRequested ? [hostActor] : [] }; | ||
| 360 | }), | ||
| 361 | [WS_METHODS.pullRequestsActivity]: () => | ||
| 362 | Effect.gen(function* () { | ||
| 363 | reads++; | ||
| 364 | if (pauseActivity) { | ||
| 365 | pauseActivity = false; | ||
| 366 | yield* activityStarted.open; | ||
| 367 | return yield* Effect.never; | ||
| 368 | } | ||
| 369 | return { | ||
| 370 | reviewers: hostRequested ? [hostActor] : [], | ||
| 371 | comments: reviewed ? [{ kind: "review-comment", author: hostActor }] : [], | ||
| 372 | }; | ||
| 373 | }), | ||
| 374 | [WS_METHODS.pullRequestsReviewerCandidates]: (input: { number: number }) => | ||
| 375 | input.number === 2 | ||
| 376 | ? Effect.never | ||
| 377 | : Effect.sync(() => { | ||
| 378 | reads++; | ||
| 379 | return { | ||
| 380 | candidates: [{ ...actor, id: "12", kind: "user", isRequested: false }], | ||
| 381 | truncated: false, | ||
| 382 | }; | ||
| 383 | }), | ||
| 384 | [WS_METHODS.pullRequestsRequestReviewers]: (input: { requested: boolean }) => | ||
| 385 | refuse | ||
| 386 | ? Effect.fail(new MutationRefused()) | ||
| 387 | : Effect.sync(() => { | ||
| 388 | hostRequested = input.requested; | ||
| 389 | }), | ||
| 390 | } as unknown as WsRpcProtocolClient; | ||
| 391 | const { atoms, registry } = yield* makeTestRuntime(client); | ||
| 392 | const target = { | ||
| 393 | environmentId: TARGET.environmentId, | ||
| 394 | input: { | ||
| 395 | projectId: ProjectId.make("project-1"), | ||
| 396 | repository: "acme/web", | ||
| 397 | number: 1, | ||
| 398 | host: "github.example.com", | ||
| 399 | }, | ||
| 400 | }; | ||
| 401 | const detail = atoms.detail(target); | ||
| 402 | const activity = atoms.activity(target); | ||
| 403 | const candidates = atoms.reviewerCandidates(target); | ||
| 404 | registry.mount(detail); | ||
| 405 | registry.mount(activity); | ||
| 406 | registry.mount(candidates); | ||
| 407 | yield* AtomRegistry.getResult(registry, detail, { suspendOnWaiting: true }); | ||
| 408 | yield* AtomRegistry.getResult(registry, activity, { suspendOnWaiting: true }); | ||
| 409 | yield* AtomRegistry.getResult(registry, candidates, { suspendOnWaiting: true }); | ||
| 410 | const request = (requested: boolean, reference = target) => | ||
| 411 | Effect.promise(() => | ||
| 412 | atoms.requestReviewers.run(registry, { | ||
| 413 | ...reference, | ||
| 414 | input: { ...reference.input, reviewers: [{ id: "12", kind: "user" }], requested }, | ||
| 415 | }), | ||
| 416 | ); | ||
| 417 | for (const operation of ["request", "refuse", "remove"]) { | ||
| 418 | refuse = operation === "refuse"; | ||
| 419 | expect((yield* request(operation === "request"))._tag).toBe(refuse ? "Failure" : "Success"); | ||
| 420 | const expected = operation === "remove" ? [] : [actor]; | ||
| 421 | expect((yield* AtomRegistry.getResult(registry, detail)).reviewers).toEqual(expected); | ||
| 422 | expect((yield* AtomRegistry.getResult(registry, activity)).reviewers).toEqual(expected); | ||
| 423 | expect((yield* AtomRegistry.getResult(registry, candidates)).candidates).toMatchObject([ | ||
| 424 | { isRequested: operation !== "remove" }, | ||
| 425 | ]); | ||
| 426 | } | ||
| 427 | expect(reads).toBe(3); | ||
| 428 | // A slow activity read started before the write must not hide the new request. | ||
| 429 | pauseActivity = true; | ||
| 430 | registry.refresh(activity); | ||
| 431 | yield* activityStarted.await; | ||
| 432 | expect(AsyncResult.isSuccess(yield* request(true))).toBe(true); | ||
| 433 | expect( | ||
| 434 | (yield* AtomRegistry.getResult(registry, activity, { suspendOnWaiting: true })).reviewers, | ||
| 435 | ).toEqual([hostActor]); | ||
| 436 | expect(reads).toBe(5); | ||
| 437 | expect(AsyncResult.isSuccess(yield* request(false))).toBe(true); | ||
| 438 | expect((yield* AtomRegistry.getResult(registry, activity)).reviewers).toEqual([]); | ||
| 439 | |||
| 440 | reviewed = true; | ||
| 441 | yield* request(true); | ||
| 442 | registry.refresh(activity); | ||
| 443 | yield* AtomRegistry.getResult(registry, activity, { suspendOnWaiting: true }); | ||
| 444 | yield* request(false); | ||
| 445 | expect((yield* AtomRegistry.getResult(registry, activity)).reviewers).toEqual([hostActor]); | ||
| 446 | |||
| 447 | // A caller without an open picker still needs authoritative reviewer identities. | ||
| 448 | const otherTarget = { ...target, input: { ...target.input, number: 2 } }; | ||
| 449 | const otherDetail = atoms.detail(otherTarget); | ||
| 450 | registry.mount(otherDetail); | ||
| 451 | yield* AtomRegistry.getResult(registry, otherDetail, { suspendOnWaiting: true }); | ||
| 452 | yield* request(true, otherTarget); | ||
| 453 | expect( | ||
| 454 | (yield* AtomRegistry.getResult(registry, otherDetail, { suspendOnWaiting: true })) | ||
| 455 | .reviewers, | ||
| 456 | ).toEqual([hostActor]); | ||
| 457 | }), | ||
| 458 | ), | ||
| 459 | ); | ||
| 460 | |||
| 219 | it.effect("refreshes stack state after reopening and head SHAs after a turn", () => | 461 | it.effect("refreshes stack state after reopening and head SHAs after a turn", () => |
| 220 | Effect.scoped( | 462 | Effect.scoped( |
| 221 | Effect.gen(function* () { | 463 | Effect.gen(function* () { |
packages/client-runtime/src/state/pullRequests.ts+181-24| ... | @@ -1,7 +1,10 @@ | ... | @@ -1,7 +1,10 @@ |
| 1 | import { | 1 | import { |
| 2 | WS_METHODS, | 2 | WS_METHODS, |
| 3 | type EnvironmentId, | ||
| 4 | type PullRequestActor, | ||
| 3 | type PullRequestDetail, | 5 | type PullRequestDetail, |
| 4 | type PullRequestDiffInput, | 6 | type PullRequestDiffInput, |
| 7 | type PullRequestRef, | ||
| 5 | type PullRequestSummary, | 8 | type PullRequestSummary, |
| 6 | type VcsStatusResult, | 9 | type VcsStatusResult, |
| 7 | } from "@t3tools/contracts"; | 10 | } from "@t3tools/contracts"; |
| ... | @@ -9,7 +12,7 @@ import * as Data from "effect/Data"; | ... | @@ -9,7 +12,7 @@ import * as Data from "effect/Data"; |
| 9 | import * as Effect from "effect/Effect"; | 12 | import * as Effect from "effect/Effect"; |
| 10 | import * as Option from "effect/Option"; | 13 | import * as Option from "effect/Option"; |
| 11 | import * as SubscriptionRef from "effect/SubscriptionRef"; | 14 | import * as SubscriptionRef from "effect/SubscriptionRef"; |
| 12 | import { Atom } from "effect/unstable/reactivity"; | 15 | import { AsyncResult, Atom, AtomRegistry } from "effect/unstable/reactivity"; |
| 13 | 16 | ||
| 14 | import { | 17 | import { |
| 15 | createAtomCommandScheduler, | 18 | createAtomCommandScheduler, |
| ... | @@ -36,6 +39,52 @@ export class EnvironmentHttpConnectionNotReadyError extends Data.TaggedError( | ... | @@ -36,6 +39,52 @@ export class EnvironmentHttpConnectionNotReadyError extends Data.TaggedError( |
| 36 | 39 | ||
| 37 | const LINKED_PULL_REQUEST_IDLE_TTL_MS = 5_000; | 40 | const LINKED_PULL_REQUEST_IDLE_TTL_MS = 5_000; |
| 38 | 41 | ||
| 42 | /** Keep confirmed edits on the same cached reference regardless of input property order. */ | ||
| 43 | function writableQueryFamily<A, E>( | ||
| 44 | family: (target: { | ||
| 45 | readonly environmentId: EnvironmentId; | ||
| 46 | readonly input: PullRequestRef; | ||
| 47 | }) => Atom.Atom<AsyncResult.AsyncResult<A, E>>, | ||
| 48 | ) { | ||
| 49 | const writable = Atom.family((source: Atom.Atom<AsyncResult.AsyncResult<A, E>>) => | ||
| 50 | Atom.writable( | ||
| 51 | (get) => { | ||
| 52 | const result = get(source); | ||
| 53 | if (result._tag === "Success" && !result.waiting) return result; | ||
| 54 | const previous = get.self<AsyncResult.AsyncResult<A, E>>(); | ||
| 55 | const value = Option.flatMap(previous, AsyncResult.value); | ||
| 56 | if (Option.isNone(value)) return result; | ||
| 57 | return result._tag === "Failure" | ||
| 58 | ? AsyncResult.failureWithPrevious(result.cause, { previous, waiting: result.waiting }) | ||
| 59 | : AsyncResult.success<A, E>(value.value, result); | ||
| 60 | }, | ||
| 61 | (context, value: AsyncResult.AsyncResult<A, E>) => context.setSelf(value), | ||
| 62 | (refresh) => refresh(source), | ||
| 63 | ).pipe(Atom.setIdleTTL(5 * 60_000)), | ||
| 64 | ); | ||
| 65 | return ({ | ||
| 66 | environmentId, | ||
| 67 | input: { projectId, host, repository, number }, | ||
| 68 | }: Parameters<typeof family>[0]) => | ||
| 69 | writable( | ||
| 70 | family({ | ||
| 71 | environmentId, | ||
| 72 | input: { projectId, ...(host === undefined ? {} : { host }), repository, number }, | ||
| 73 | }), | ||
| 74 | ); | ||
| 75 | } | ||
| 76 | |||
| 77 | /** Restart pre-mutation reads before patching so they cannot restore stale values. */ | ||
| 78 | function updateCached<A, E>( | ||
| 79 | registry: AtomRegistry.AtomRegistry, | ||
| 80 | atom: Atom.Writable<AsyncResult.AsyncResult<A, E>>, | ||
| 81 | update: (value: A) => A, | ||
| 82 | refresh = false, | ||
| 83 | ) { | ||
| 84 | if (refresh || registry.get(atom).waiting) registry.refresh(atom); | ||
| 85 | registry.update(atom, AsyncResult.map(update)); | ||
| 86 | } | ||
| 87 | |||
| 39 | function createPullRequestRefreshAtomFamily<R, E>( | 88 | function createPullRequestRefreshAtomFamily<R, E>( |
| 40 | runtime: Atom.AtomRuntime<EnvironmentRegistry | R, E>, | 89 | runtime: Atom.AtomRuntime<EnvironmentRegistry | R, E>, |
| 41 | ) { | 90 | ) { |
| ... | @@ -92,7 +141,7 @@ export function pullRequestDetailToVcsStatus( | ... | @@ -92,7 +141,7 @@ export function pullRequestDetailToVcsStatus( |
| 92 | /** | 141 | /** |
| 93 | * Reopening a PR within a minute reuses detail and activity. Explicit refreshes and | 142 | * Reopening a PR within a minute reuses detail and activity. Explicit refreshes and |
| 94 | * turn notifications still revalidate. Mutations run serially per environment: actions on the same | 143 | * turn notifications still revalidate. Mutations run serially per environment: actions on the same |
| 95 | * pull request are order-sensitive, and the detail view refetches after each one. | 144 | * pull request are order-sensitive. Confirmed label and reviewer edits update cached state. |
| 96 | */ | 145 | */ |
| 97 | export function createPullRequestEnvironmentAtoms<R, E>( | 146 | export function createPullRequestEnvironmentAtoms<R, E>( |
| 98 | runtime: Atom.AtomRuntime<EnvironmentRegistry | PullRequestDiffLoader | R, E>, | 147 | runtime: Atom.AtomRuntime<EnvironmentRegistry | PullRequestDiffLoader | R, E>, |
| ... | @@ -103,12 +152,36 @@ export function createPullRequestEnvironmentAtoms<R, E>( | ... | @@ -103,12 +152,36 @@ export function createPullRequestEnvironmentAtoms<R, E>( |
| 103 | mode: "serial", | 152 | mode: "serial", |
| 104 | key: ({ environmentId }: { readonly environmentId: string }) => environmentId, | 153 | key: ({ environmentId }: { readonly environmentId: string }) => environmentId, |
| 105 | } as const; | 154 | } as const; |
| 106 | const activity = createEnvironmentRpcQueryAtomFamily(runtime, { | 155 | const activity = writableQueryFamily( |
| 107 | label: "environment-data:pull-requests:activity", | 156 | createEnvironmentRpcQueryAtomFamily(runtime, { |
| 108 | tag: WS_METHODS.pullRequestsActivity, | 157 | label: "environment-data:pull-requests:activity", |
| 109 | staleTimeMs: 60_000, | 158 | tag: WS_METHODS.pullRequestsActivity, |
| 110 | refreshTrigger: ({ environmentId }) => refreshes({ environmentId, input: {} }), | 159 | staleTimeMs: 60_000, |
| 111 | }); | 160 | refreshTrigger: ({ environmentId }) => refreshes({ environmentId, input: {} }), |
| 161 | }), | ||
| 162 | ); | ||
| 163 | const detail = writableQueryFamily( | ||
| 164 | createEnvironmentRpcQueryAtomFamily(runtime, { | ||
| 165 | label: "environment-data:pull-requests:detail", | ||
| 166 | tag: WS_METHODS.pullRequestsDetail, | ||
| 167 | staleTimeMs: 60_000, | ||
| 168 | refreshTrigger: ({ environmentId }) => refreshes({ environmentId, input: {} }), | ||
| 169 | }), | ||
| 170 | ); | ||
| 171 | const labelCandidates = writableQueryFamily( | ||
| 172 | createEnvironmentRpcQueryAtomFamily(runtime, { | ||
| 173 | label: "environment-data:pull-requests:label-candidates", | ||
| 174 | tag: WS_METHODS.pullRequestsLabelCandidates, | ||
| 175 | staleTimeMs: 60_000, | ||
| 176 | }), | ||
| 177 | ); | ||
| 178 | const reviewerCandidates = writableQueryFamily( | ||
| 179 | createEnvironmentRpcQueryAtomFamily(runtime, { | ||
| 180 | label: "environment-data:pull-requests:reviewer-candidates", | ||
| 181 | tag: WS_METHODS.pullRequestsReviewerCandidates, | ||
| 182 | staleTimeMs: 60_000, | ||
| 183 | }), | ||
| 184 | ); | ||
| 112 | return { | 185 | return { |
| 113 | refreshes, | 186 | refreshes, |
| 114 | linkedThreads: createEnvironmentRpcQueryAtomFamily(runtime, { | 187 | linkedThreads: createEnvironmentRpcQueryAtomFamily(runtime, { |
| ... | @@ -137,12 +210,7 @@ export function createPullRequestEnvironmentAtoms<R, E>( | ... | @@ -137,12 +210,7 @@ export function createPullRequestEnvironmentAtoms<R, E>( |
| 137 | staleTimeMs: 60_000, | 210 | staleTimeMs: 60_000, |
| 138 | refreshTrigger: ({ environmentId }) => refreshes({ environmentId, input: {} }), | 211 | refreshTrigger: ({ environmentId }) => refreshes({ environmentId, input: {} }), |
| 139 | }), | 212 | }), |
| 140 | detail: createEnvironmentRpcQueryAtomFamily(runtime, { | 213 | detail, |
| 141 | label: "environment-data:pull-requests:detail", | ||
| 142 | tag: WS_METHODS.pullRequestsDetail, | ||
| 143 | staleTimeMs: 60_000, | ||
| 144 | refreshTrigger: ({ environmentId }) => refreshes({ environmentId, input: {} }), | ||
| 145 | }), | ||
| 146 | activity, | 214 | activity, |
| 147 | threadComments: createEnvironmentRpcCommand(runtime, { | 215 | threadComments: createEnvironmentRpcCommand(runtime, { |
| 148 | label: "environment-data:pull-requests:thread-comments", | 216 | label: "environment-data:pull-requests:thread-comments", |
| ... | @@ -241,28 +309,117 @@ export function createPullRequestEnvironmentAtoms<R, E>( | ... | @@ -241,28 +309,117 @@ export function createPullRequestEnvironmentAtoms<R, E>( |
| 241 | * for a minute, because who has access to a repository changes far more slowly than the | 309 | * for a minute, because who has access to a repository changes far more slowly than the |
| 242 | * change request it is being read for. | 310 | * change request it is being read for. |
| 243 | */ | 311 | */ |
| 244 | reviewerCandidates: createEnvironmentRpcQueryAtomFamily(runtime, { | 312 | reviewerCandidates, |
| 245 | label: "environment-data:pull-requests:reviewer-candidates", | ||
| 246 | tag: WS_METHODS.pullRequestsReviewerCandidates, | ||
| 247 | staleTimeMs: 60_000, | ||
| 248 | }), | ||
| 249 | requestReviewers: createEnvironmentRpcCommand(runtime, { | 313 | requestReviewers: createEnvironmentRpcCommand(runtime, { |
| 250 | label: "environment-data:pull-requests:request-reviewers", | 314 | label: "environment-data:pull-requests:request-reviewers", |
| 251 | tag: WS_METHODS.pullRequestsRequestReviewers, | 315 | tag: WS_METHODS.pullRequestsRequestReviewers, |
| 252 | scheduler: commandScheduler, | 316 | scheduler: commandScheduler, |
| 253 | concurrency: serialPerEnvironment, | 317 | concurrency: serialPerEnvironment, |
| 318 | onSuccess: (target, registry) => | ||
| 319 | Effect.sync(() => { | ||
| 320 | const { reviewers, requested } = target.input; | ||
| 321 | const candidatesAtom = reviewerCandidates(target); | ||
| 322 | const candidates = Option.getOrNull(AsyncResult.value(registry.get(candidatesAtom))); | ||
| 323 | const selected = | ||
| 324 | candidates?.candidates.filter((candidate) => | ||
| 325 | reviewers.some( | ||
| 326 | (reviewer) => reviewer.id === candidate.id && reviewer.kind === candidate.kind, | ||
| 327 | ), | ||
| 328 | ) ?? []; | ||
| 329 | const missingIdentities = selected.length < reviewers.length; | ||
| 330 | updateCached(registry, candidatesAtom, (value) => ({ | ||
| 331 | ...value, | ||
| 332 | candidates: value.candidates.map((candidate) => | ||
| 333 | selected.includes(candidate) ? { ...candidate, isRequested: requested } : candidate, | ||
| 334 | ), | ||
| 335 | })); | ||
| 336 | const selectedLogins = new Set( | ||
| 337 | selected.map((candidate) => candidate.login.toLowerCase()), | ||
| 338 | ); | ||
| 339 | const updateReviewers = ( | ||
| 340 | actors: ReadonlyArray<PullRequestActor>, | ||
| 341 | keep = (_actor: PullRequestActor) => false, | ||
| 342 | ) => | ||
| 343 | requested | ||
| 344 | ? [ | ||
| 345 | ...actors, | ||
| 346 | ...selected | ||
| 347 | .filter( | ||
| 348 | (candidate) => | ||
| 349 | !actors.some( | ||
| 350 | (actor) => actor.login.toLowerCase() === candidate.login.toLowerCase(), | ||
| 351 | ), | ||
| 352 | ) | ||
| 353 | .map(({ login, name, avatarUrl }) => ({ login, name, avatarUrl })), | ||
| 354 | ] | ||
| 355 | : actors.filter( | ||
| 356 | (actor) => !selectedLogins.has(actor.login.toLowerCase()) || keep(actor), | ||
| 357 | ); | ||
| 358 | updateCached( | ||
| 359 | registry, | ||
| 360 | detail(target), | ||
| 361 | (value) => ({ | ||
| 362 | ...value, | ||
| 363 | reviewers: updateReviewers(value.reviewers), | ||
| 364 | }), | ||
| 365 | missingIdentities, | ||
| 366 | ); | ||
| 367 | updateCached( | ||
| 368 | registry, | ||
| 369 | activity(target), | ||
| 370 | (value) => ({ | ||
| 371 | ...value, | ||
| 372 | reviewers: | ||
| 373 | value.reviewers === undefined | ||
| 374 | ? undefined | ||
| 375 | : updateReviewers(value.reviewers, (actor) => | ||
| 376 | value.comments.some( | ||
| 377 | (comment) => | ||
| 378 | (comment.kind === "review" || comment.kind === "review-comment") && | ||
| 379 | comment.author?.login.toLowerCase() === actor.login.toLowerCase(), | ||
| 380 | ), | ||
| 381 | ), | ||
| 382 | }), | ||
| 383 | missingIdentities, | ||
| 384 | ); | ||
| 385 | }), | ||
| 254 | }), | 386 | }), |
| 255 | /** Read when the label menu opens, and kept for a minute, like the reviewer candidates. */ | 387 | /** Read when the label menu opens, and kept for a minute, like the reviewer candidates. */ |
| 256 | labelCandidates: createEnvironmentRpcQueryAtomFamily(runtime, { | 388 | labelCandidates, |
| 257 | label: "environment-data:pull-requests:label-candidates", | ||
| 258 | tag: WS_METHODS.pullRequestsLabelCandidates, | ||
| 259 | staleTimeMs: 60_000, | ||
| 260 | }), | ||
| 261 | setLabels: createEnvironmentRpcCommand(runtime, { | 389 | setLabels: createEnvironmentRpcCommand(runtime, { |
| 262 | label: "environment-data:pull-requests:set-labels", | 390 | label: "environment-data:pull-requests:set-labels", |
| 263 | tag: WS_METHODS.pullRequestsSetLabels, | 391 | tag: WS_METHODS.pullRequestsSetLabels, |
| 264 | scheduler: commandScheduler, | 392 | scheduler: commandScheduler, |
| 265 | concurrency: serialPerEnvironment, | 393 | concurrency: serialPerEnvironment, |
| 394 | onSuccess: (target, registry) => | ||
| 395 | Effect.sync(() => { | ||
| 396 | const { labels, applied } = target.input; | ||
| 397 | const candidatesAtom = labelCandidates(target); | ||
| 398 | const candidates = Option.getOrNull(AsyncResult.value(registry.get(candidatesAtom))); | ||
| 399 | const names = new Set(labels); | ||
| 400 | updateCached(registry, candidatesAtom, (value) => ({ | ||
| 401 | ...value, | ||
| 402 | candidates: value.candidates.map((candidate) => | ||
| 403 | names.has(candidate.name) ? { ...candidate, isApplied: applied } : candidate, | ||
| 404 | ), | ||
| 405 | })); | ||
| 406 | updateCached(registry, detail(target), (value) => ({ | ||
| 407 | ...value, | ||
| 408 | labels: applied | ||
| 409 | ? [ | ||
| 410 | ...value.labels, | ||
| 411 | ...labels | ||
| 412 | .filter((name) => !value.labels.some((label) => label.name === name)) | ||
| 413 | .map((name) => ({ | ||
| 414 | name, | ||
| 415 | color: | ||
| 416 | candidates?.candidates.find((candidate) => candidate.name === name) | ||
| 417 | ?.color ?? null, | ||
| 418 | })), | ||
| 419 | ] | ||
| 420 | : value.labels.filter((label) => !names.has(label.name)), | ||
| 421 | })); | ||
| 422 | }), | ||
| 266 | }), | 423 | }), |
| 267 | setThreadResolution: createEnvironmentRpcCommand(runtime, { | 424 | setThreadResolution: createEnvironmentRpcCommand(runtime, { |
| 268 | label: "environment-data:pull-requests:set-thread-resolution", | 425 | label: "environment-data:pull-requests:set-thread-resolution", |