mirror of
https://github.com/block/buzz.git
synced 2026-10-31 06:21:34 +00:00
fix(desktop): retain one selected review across Projects chrome (#6429)
Resolve the selected review once during pull-request refetch so the workspace, diff query, breadcrumb, context card, and agent payload stay on the same identity until a completed result omits it. Signed-off-by: Thomas Petersen <thomasp@squareup.com>
This commit is contained in:
@@ -188,6 +188,7 @@ export function ProjectDetailScreen(props: ProjectDetailScreenProps) {
|
||||
activeRepoPullRequest,
|
||||
openBranchPullRequest,
|
||||
selectedBranchPullRequest,
|
||||
selectedPullRequest,
|
||||
} = useRetainedPullRequestSelection({
|
||||
activeBranch,
|
||||
isFetching: pullRequestsQuery.isFetching,
|
||||
@@ -688,9 +689,6 @@ export function ProjectDetailScreen(props: ProjectDetailScreenProps) {
|
||||
const displayedRepositoryContributors =
|
||||
displayedRepositorySnapshot?.contributors ?? repoContributors;
|
||||
const displayedRepositoryFiles = displayedRepositorySnapshot?.files ?? [];
|
||||
const selectedPullRequest =
|
||||
pullRequestsQuery.data?.find((item) => item.id === selectedPullRequestId) ??
|
||||
null;
|
||||
const selectedIssue =
|
||||
issuesQuery.data?.find((item) => item.id === selectedIssueId) ?? null;
|
||||
const displayedSnapshotCommits =
|
||||
@@ -910,13 +908,13 @@ export function ProjectDetailScreen(props: ProjectDetailScreenProps) {
|
||||
repoDiffLoading={displayedRepoDiffLoading}
|
||||
pullRequests={pullRequestsQuery.data ?? []}
|
||||
pullRequestsError={pullRequestsQuery.error}
|
||||
pullRequestsFetching={pullRequestsQuery.isFetching}
|
||||
pullRequestsLoading={pullRequestsQuery.isLoading}
|
||||
repoContributors={repoContributors}
|
||||
repoHost={repoRemote.host}
|
||||
repoSource={repoSource}
|
||||
selectedCommitHash={selectedCommitHash}
|
||||
selectedIssueId={selectedIssueId}
|
||||
selectedPullRequest={selectedPullRequest}
|
||||
selectedPullRequestId={selectedPullRequestId}
|
||||
sharedHeaderBackdrop={sharedHeaderBackdrop}
|
||||
snapshot={displayedRepoSnapshot}
|
||||
|
||||
@@ -26,12 +26,7 @@ import {
|
||||
} from "@/features/projects/lib/projectContributorMatching";
|
||||
import { repositoryDiscussionQuery } from "@/features/projects/lib/discussionChannels";
|
||||
import type { ProjectRepoHost } from "@/features/projects/lib/projectRepoHost";
|
||||
import {
|
||||
currentPullRequestForSelection,
|
||||
projectReviewFilesChangedBody,
|
||||
retainLatestByKey,
|
||||
shouldReplaceRetainedPullRequest,
|
||||
} from "@/features/projects/lib/projectReviewDisplay";
|
||||
import { projectReviewFilesChangedBody } from "@/features/projects/lib/projectReviewDisplay";
|
||||
import { projectRepoUnavailableReason } from "@/features/projects/lib/projectRepoAvailability";
|
||||
import type { UserProfileLookup } from "@/features/profile/lib/identity";
|
||||
import { normalizePubkey } from "@/shared/lib/pubkey";
|
||||
@@ -113,11 +108,11 @@ export function WorkspaceTabs({
|
||||
repoDiffLoading,
|
||||
selectedCommitHash,
|
||||
selectedIssueId,
|
||||
selectedPullRequest,
|
||||
selectedPullRequestId,
|
||||
sharedHeaderBackdrop,
|
||||
pullRequests,
|
||||
pullRequestsError,
|
||||
pullRequestsFetching,
|
||||
pullRequestsLoading,
|
||||
onSelectedCommitHashChange,
|
||||
onFilesContextChange,
|
||||
@@ -160,12 +155,11 @@ export function WorkspaceTabs({
|
||||
repoDiffLoading: boolean;
|
||||
selectedCommitHash: string | null;
|
||||
selectedIssueId: string | null;
|
||||
selectedPullRequest: ProjectPullRequest | null;
|
||||
selectedPullRequestId: string | null;
|
||||
sharedHeaderBackdrop?: boolean;
|
||||
pullRequests: ProjectPullRequest[];
|
||||
pullRequestsError: unknown;
|
||||
/** True while a pull-request list fetch (including refetch) is in flight. */
|
||||
pullRequestsFetching: boolean;
|
||||
pullRequestsLoading: boolean;
|
||||
onSelectedCommitHashChange: (hash: string | null) => void;
|
||||
onFilesContextChange?: (context: {
|
||||
@@ -245,21 +239,6 @@ export function WorkspaceTabs({
|
||||
retryPending={sourceControls?.fetchPending}
|
||||
/>
|
||||
) : null;
|
||||
const currentSelectedPullRequest = currentPullRequestForSelection({
|
||||
pullRequests,
|
||||
selectedPullRequestId,
|
||||
});
|
||||
const selectedPullRequestCacheKey = `${project.id}:${selectedPullRequestId ?? "none"}`;
|
||||
const selectedPullRequestCache = React.useRef({
|
||||
key: selectedPullRequestCacheKey,
|
||||
value: currentSelectedPullRequest,
|
||||
});
|
||||
const selectedPullRequest = retainLatestByKey(
|
||||
selectedPullRequestCache,
|
||||
selectedPullRequestCacheKey,
|
||||
currentSelectedPullRequest,
|
||||
(next) => shouldReplaceRetainedPullRequest(next, pullRequestsFetching),
|
||||
);
|
||||
const filesChangedBody = projectReviewFilesChangedBody({
|
||||
hasPopulatedDiff: (repoDiff?.files.length ?? 0) > 0,
|
||||
hasSelectedPullRequest: Boolean(selectedPullRequest),
|
||||
|
||||
@@ -0,0 +1,161 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { after, afterEach, before, test } from "node:test";
|
||||
|
||||
import { JSDOM } from "jsdom";
|
||||
|
||||
import { buildProjectDetailAgentContext } from "../lib/projectDetailAgentContext.ts";
|
||||
import { projectDetailSelectionItem } from "../lib/projectDetailSelectionItem.ts";
|
||||
import { reviewDiffWorkspaceBranch } from "../lib/projectReviewDisplay.ts";
|
||||
import { buildProjectDetailCrumbs } from "./useProjectDetailCrumbs.ts";
|
||||
|
||||
const OWNER = "a".repeat(64);
|
||||
const REVIEW_A_ID = "b".repeat(64);
|
||||
|
||||
const repository = {
|
||||
id: `${OWNER}:buzz`,
|
||||
cloneUrls: ["https://example.com/buzz.git"],
|
||||
defaultBranch: "main",
|
||||
channelId: "trusted-repository-channel",
|
||||
name: "buzz",
|
||||
repoAddress: `30617:${OWNER}:buzz`,
|
||||
};
|
||||
|
||||
const reviewA = {
|
||||
id: REVIEW_A_ID,
|
||||
title: "Ship the retained review",
|
||||
author: "alice",
|
||||
status: "Open",
|
||||
branchName: "feature-a",
|
||||
targetBranch: "main",
|
||||
cloneUrls: repository.cloneUrls,
|
||||
repoAddress: repository.repoAddress,
|
||||
channelId: "forged-origin-channel",
|
||||
};
|
||||
|
||||
const noop = () => {};
|
||||
|
||||
function productionConsumers({ activeRepoPullRequest, selectedPullRequest }) {
|
||||
const crumbs = buildProjectDetailCrumbs({
|
||||
activeTab: "prs",
|
||||
commit: null,
|
||||
issue: null,
|
||||
pullRequest: selectedPullRequest,
|
||||
setRequestedTab: noop,
|
||||
setSelectedCommitHash: noop,
|
||||
setSelectedIssueId: noop,
|
||||
setSelectedPullRequestId: noop,
|
||||
setTabsResetKey: noop,
|
||||
});
|
||||
const contextItem = projectDetailSelectionItem({
|
||||
projectChannelId: "trusted-project-channel",
|
||||
projectId: "project-id",
|
||||
pullRequest: selectedPullRequest,
|
||||
repository,
|
||||
});
|
||||
const agent = buildProjectDetailAgentContext({
|
||||
activeTab: "prs",
|
||||
branch: "feature-a",
|
||||
project: { name: "buzz" },
|
||||
repository: {
|
||||
name: repository.name,
|
||||
repoAddress: repository.repoAddress,
|
||||
},
|
||||
source: "remote",
|
||||
workItems: [null, null, selectedPullRequest],
|
||||
});
|
||||
return {
|
||||
agentReviewId: agent.workItem?.id ?? null,
|
||||
crumbTitle: crumbs.activeWorkItemCrumb?.title ?? null,
|
||||
contextId: contextItem?.id ?? null,
|
||||
diffQueryId: activeRepoPullRequest?.id ?? null,
|
||||
diffWorkspaceBranch: reviewDiffWorkspaceBranch({
|
||||
activeBranch: "feature-a",
|
||||
defaultBranch: repository.defaultBranch,
|
||||
pullRequest: activeRepoPullRequest,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
const dom = new JSDOM("<!doctype html><html><body></body></html>", {
|
||||
url: "http://localhost",
|
||||
});
|
||||
|
||||
before(() => {
|
||||
Object.assign(globalThis, {
|
||||
document: dom.window.document,
|
||||
HTMLElement: dom.window.HTMLElement,
|
||||
IS_REACT_ACT_ENVIRONMENT: true,
|
||||
window: dom.window,
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
const { cleanup } = await import("@testing-library/react");
|
||||
cleanup();
|
||||
});
|
||||
|
||||
after(() => dom.window.close());
|
||||
|
||||
let hookModule;
|
||||
before(async () => {
|
||||
hookModule = await import("./useRetainedProjectGitViews.ts");
|
||||
});
|
||||
|
||||
async function renderSelection(initialProps) {
|
||||
const { renderHook } = await import("@testing-library/react");
|
||||
return renderHook(
|
||||
(props) => hookModule.useRetainedPullRequestSelection(props),
|
||||
{ initialProps },
|
||||
);
|
||||
}
|
||||
|
||||
test("selected review chrome and diff query stay aligned across fetch phases", async () => {
|
||||
const populated = {
|
||||
activeBranch: "feature-a",
|
||||
isFetching: false,
|
||||
pullRequests: [reviewA],
|
||||
repository,
|
||||
selectedPullRequestId: REVIEW_A_ID,
|
||||
};
|
||||
const { rerender, result } = await renderSelection(populated);
|
||||
|
||||
const populatedConsumers = productionConsumers(result.current);
|
||||
assert.equal(result.current.selectedPullRequest, reviewA);
|
||||
assert.equal(result.current.activeRepoPullRequest, reviewA);
|
||||
assert.deepEqual(populatedConsumers, {
|
||||
agentReviewId: REVIEW_A_ID,
|
||||
crumbTitle: "Ship the retained review",
|
||||
contextId: `review:${REVIEW_A_ID}`,
|
||||
diffQueryId: REVIEW_A_ID,
|
||||
diffWorkspaceBranch: "main",
|
||||
});
|
||||
|
||||
rerender({
|
||||
...populated,
|
||||
isFetching: true,
|
||||
pullRequests: [],
|
||||
});
|
||||
const fetchingConsumers = productionConsumers(result.current);
|
||||
assert.equal(result.current.selectedPullRequest, reviewA);
|
||||
assert.equal(
|
||||
result.current.selectedPullRequest,
|
||||
result.current.activeRepoPullRequest,
|
||||
);
|
||||
assert.deepEqual(fetchingConsumers, populatedConsumers);
|
||||
|
||||
rerender({
|
||||
...populated,
|
||||
isFetching: false,
|
||||
pullRequests: [],
|
||||
});
|
||||
const completedConsumers = productionConsumers(result.current);
|
||||
assert.equal(result.current.selectedPullRequest, null);
|
||||
assert.equal(result.current.activeRepoPullRequest, null);
|
||||
assert.deepEqual(completedConsumers, {
|
||||
agentReviewId: null,
|
||||
crumbTitle: null,
|
||||
contextId: null,
|
||||
diffQueryId: null,
|
||||
diffWorkspaceBranch: "feature-a",
|
||||
});
|
||||
});
|
||||
@@ -64,29 +64,32 @@ export function useRetainedPullRequestSelection({
|
||||
selectedBranchPullRequest?.status === "Draft"
|
||||
? selectedBranchPullRequest
|
||||
: null;
|
||||
const currentRepoPullRequest = currentPullRequestForSelection({
|
||||
fallback: selectedBranchPullRequest,
|
||||
const currentSelectedPullRequest = currentPullRequestForSelection({
|
||||
pullRequests,
|
||||
selectedPullRequestId,
|
||||
});
|
||||
const activeRepoPullRequestKey = [
|
||||
const selectedPullRequestKey = [
|
||||
repository?.id ?? "none",
|
||||
selectedPullRequestId ?? `branch:${activeBranch}`,
|
||||
selectedPullRequestId ?? "none",
|
||||
].join(":");
|
||||
const activeRepoPullRequestCache = React.useRef({
|
||||
key: activeRepoPullRequestKey,
|
||||
value: currentRepoPullRequest,
|
||||
const selectedPullRequestCache = React.useRef({
|
||||
key: selectedPullRequestKey,
|
||||
value: currentSelectedPullRequest,
|
||||
});
|
||||
const activeRepoPullRequest = retainLatestByKey(
|
||||
activeRepoPullRequestCache,
|
||||
activeRepoPullRequestKey,
|
||||
currentRepoPullRequest,
|
||||
const selectedPullRequest = retainLatestByKey(
|
||||
selectedPullRequestCache,
|
||||
selectedPullRequestKey,
|
||||
currentSelectedPullRequest,
|
||||
(next) => shouldReplaceRetainedPullRequest(next, isFetching),
|
||||
);
|
||||
const activeRepoPullRequest = selectedPullRequestId
|
||||
? selectedPullRequest
|
||||
: selectedBranchPullRequest;
|
||||
return {
|
||||
activeRepoPullRequest,
|
||||
openBranchPullRequest,
|
||||
selectedBranchPullRequest,
|
||||
selectedPullRequest,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user