mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-09 18:06:45 +08:00
fix(issues): stop a comment create failing on sibling issue caches (MUL-6394) (#7211)
`onIssueAuxiliaryRevision` scans issue caches by key PREFIX, but every prefix also covers siblings with a different shape: `tableAll` holds the grouped (infinite) and facet caches next to the row pages, `myAll` holds the assignee-grouped caches. Reading `data.rows` / `data.byStatus` off those threw "Cannot read properties of undefined (reading 'some')". `useCreateComment` calls it from onSuccess, so the throw rejected `mutateAsync` after the server had already created the comment and enqueued the agent task: the composer showed an error toast and skipped the timeline append, and the comment only appeared on reload. Every scan now goes through the shape-filtered helpers the cache coordinator already had (`bucketedListEntries` / `flatListEntries` / `tableRowEntries`, plus a new `issueArrayEntries`), so the check can't be forgotten at a new call site. Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]>
This commit is contained in:
co-authored by
J
multica-agent
parent
4c1c8709c3
commit
c8048a0c8a
@@ -122,7 +122,20 @@ function listContractFromKey(key: QueryKey): {
|
||||
};
|
||||
}
|
||||
|
||||
function bucketedListEntries(
|
||||
/**
|
||||
* SHAPE-FILTERED CACHE SCANS.
|
||||
*
|
||||
* `getQueriesData` matches a key PREFIX, and every issue-surface prefix also
|
||||
* covers sibling queries that hold a different shape: `myAll` covers the
|
||||
* assignee-grouped caches, `tableAll` covers the grouped (infinite) and facet
|
||||
* caches next to the row pages, `flatAll` covers the export window. Reading
|
||||
* `data.rows` / `data.pages` / `data.byStatus` off those siblings throws
|
||||
* ("Cannot read properties of undefined"), and inside a mutation's onSuccess
|
||||
* that throw surfaces as a failed write the server already accepted
|
||||
* (MUL-6394). Every scan goes through these helpers so the shape check can't
|
||||
* be forgotten at a new call site.
|
||||
*/
|
||||
export function bucketedListEntries(
|
||||
qc: QueryClient,
|
||||
wsId: string,
|
||||
): [QueryKey, ListIssuesCache][] {
|
||||
@@ -134,7 +147,7 @@ function bucketedListEntries(
|
||||
);
|
||||
}
|
||||
|
||||
function flatListEntries(
|
||||
export function flatListEntries(
|
||||
qc: QueryClient,
|
||||
wsId: string,
|
||||
): [QueryKey, IssueFlatCache][] {
|
||||
@@ -146,7 +159,7 @@ function flatListEntries(
|
||||
);
|
||||
}
|
||||
|
||||
function tableRowEntries(
|
||||
export function tableRowEntries(
|
||||
qc: QueryClient,
|
||||
wsId: string,
|
||||
): [QueryKey, IssueTableRowCache][] {
|
||||
@@ -160,6 +173,17 @@ function tableRowEntries(
|
||||
);
|
||||
}
|
||||
|
||||
/** Caches under `prefix` that hold a plain `Issue[]` — per-parent children and
|
||||
* the project Gantt list. */
|
||||
export function issueArrayEntries(
|
||||
qc: QueryClient,
|
||||
prefix: readonly unknown[],
|
||||
): [QueryKey, Issue[]][] {
|
||||
return qc
|
||||
.getQueriesData<Issue[]>({ queryKey: prefix })
|
||||
.filter((entry): entry is [QueryKey, Issue[]] => Array.isArray(entry[1]));
|
||||
}
|
||||
|
||||
function flatContractFromKey(key: QueryKey): {
|
||||
scope: string | undefined;
|
||||
filter: IssueFlatFilter;
|
||||
|
||||
@@ -10,6 +10,7 @@ import { setApiInstance } from "../api";
|
||||
import type { ApiClient } from "../api/client";
|
||||
import {
|
||||
useBatchUpdateIssues,
|
||||
useCreateComment,
|
||||
useDeleteComment,
|
||||
useResolveComment,
|
||||
useUpdateComment,
|
||||
@@ -948,3 +949,76 @@ describe("useResolveComment", () => {
|
||||
expect(resolvedIds(qc)).toEqual(["a1"]);
|
||||
});
|
||||
});
|
||||
|
||||
// MUL-6394: posting a comment while the Table view's grouped/facet caches are
|
||||
// loaded rejected `mutateAsync` with "Cannot read properties of undefined
|
||||
// (reading 'some')" — the comment WAS created server-side (the agent task
|
||||
// started), but the composer showed an error toast and never appended the
|
||||
// entry, so it only appeared after a reload.
|
||||
describe("useCreateComment — sibling caches under a shared key prefix", () => {
|
||||
const ISSUE_ID = "issue-1";
|
||||
const tableQuery = {
|
||||
scope: { kind: "workspace" },
|
||||
filters: {},
|
||||
sort: { field: "position", direction: "asc" },
|
||||
} as const;
|
||||
|
||||
let qc: QueryClient;
|
||||
|
||||
beforeEach(() => {
|
||||
qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||
setApiInstance({
|
||||
createComment: vi.fn().mockResolvedValue({
|
||||
id: "comment-1",
|
||||
issue_id: ISSUE_ID,
|
||||
author_type: "member",
|
||||
author_id: "user-1",
|
||||
content: "hello",
|
||||
type: "comment",
|
||||
parent_id: null,
|
||||
reactions: [],
|
||||
attachments: [],
|
||||
created_at: "2026-08-19T00:00:00Z",
|
||||
updated_at: "2026-08-19T00:00:00Z",
|
||||
resolved_at: null,
|
||||
resolved_by_type: null,
|
||||
resolved_by_id: null,
|
||||
issue_revision: 7,
|
||||
}),
|
||||
} as unknown as ApiClient);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
qc.clear();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("appends the created comment even when non-row table caches are loaded", async () => {
|
||||
qc.setQueryData<TimelineEntry[]>(issueKeys.timeline(ISSUE_ID), []);
|
||||
// Grouped rows are an infinite cache and facets a plain object — both live
|
||||
// under the `table-query` prefix next to the row pages, and neither has a
|
||||
// `rows` array.
|
||||
qc.setQueryData(issueKeys.tableGroups(WS_ID, tableQuery, { kind: "status" }), {
|
||||
pages: [
|
||||
{ query_fingerprint: "sha256:groups", total: 0, groups: [], next_cursor: null },
|
||||
],
|
||||
pageParams: [null],
|
||||
});
|
||||
qc.setQueryData(
|
||||
issueKeys.tableFacets(WS_ID, { query: tableQuery, facets: [{ kind: "status" }] }),
|
||||
{ query_fingerprint: "sha256:facets", total: 0, facets: [] },
|
||||
);
|
||||
|
||||
const { result } = renderHook(() => useCreateComment(ISSUE_ID), {
|
||||
wrapper: createWrapper(qc),
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.mutateAsync({ content: "hello" });
|
||||
});
|
||||
|
||||
expect(
|
||||
qc.getQueryData<TimelineEntry[]>(issueKeys.timeline(ISSUE_ID))?.map((e) => e.id),
|
||||
).toEqual(["comment-1"]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -220,3 +220,102 @@ describe("issue realtime revision admission", () => {
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// MUL-6394: `getQueriesData` matches a key PREFIX, so a scan over `tableAll` /
|
||||
// `myAll` also sees sibling caches with a different shape. Reading `rows` /
|
||||
// `byStatus` off them threw "Cannot read properties of undefined (reading
|
||||
// 'some')" out of `useCreateComment`'s onSuccess — the comment was created,
|
||||
// the UI reported a failure.
|
||||
describe("auxiliary revision — sibling caches under a shared key prefix", () => {
|
||||
const query = {
|
||||
scope: { kind: "workspace" },
|
||||
filters: {},
|
||||
sort: { field: "position", direction: "asc" },
|
||||
} as const;
|
||||
|
||||
const rowPageKey = [
|
||||
...issueKeys.tableRows(
|
||||
"ws-1",
|
||||
query,
|
||||
{ kind: "none" },
|
||||
null,
|
||||
false,
|
||||
null,
|
||||
),
|
||||
"page",
|
||||
null,
|
||||
] as const;
|
||||
|
||||
function seedSiblingCaches(qc: QueryClient) {
|
||||
// Grouped rows are an infinite cache, facets a plain object; neither has
|
||||
// a `rows` array, and both sit under the `table-query` prefix.
|
||||
qc.setQueryData(issueKeys.tableGroups("ws-1", query, { kind: "status" }), {
|
||||
pages: [
|
||||
{
|
||||
query_fingerprint: "sha256:groups",
|
||||
total: 1,
|
||||
groups: [],
|
||||
next_cursor: null,
|
||||
},
|
||||
],
|
||||
pageParams: [null],
|
||||
});
|
||||
qc.setQueryData(
|
||||
issueKeys.tableFacets("ws-1", { query, facets: [{ kind: "status" }] }),
|
||||
{ query_fingerprint: "sha256:facets", total: 1, facets: [] },
|
||||
);
|
||||
// The assignee-grouped caches share the `my` prefix with the bucketed
|
||||
// ones but carry no `byStatus`.
|
||||
qc.setQueryData(issueKeys.myAssigneeGroups("ws-1", "assigned", {}), {
|
||||
groups: [],
|
||||
});
|
||||
}
|
||||
|
||||
it("skips them instead of throwing, and still invalidates stale row pages", () => {
|
||||
const qc = new QueryClient();
|
||||
seedSiblingCaches(qc);
|
||||
qc.setQueryData(rowPageKey, {
|
||||
query_fingerprint: "sha256:rows",
|
||||
group_key: null,
|
||||
parent_id: null,
|
||||
total: 1,
|
||||
rows: [{ issue: issue(2, "stale row"), direct_child_count: 0 }],
|
||||
branch_total: 1,
|
||||
next_cursor: null,
|
||||
});
|
||||
|
||||
expect(() =>
|
||||
onIssueAuxiliaryRevision(qc, "ws-1", "issue-1", 3),
|
||||
).not.toThrow();
|
||||
|
||||
expect(qc.getQueryState(rowPageKey)?.isInvalidated).toBe(true);
|
||||
expect(
|
||||
qc.getQueryState(
|
||||
issueKeys.tableGroups("ws-1", query, { kind: "status" }),
|
||||
)?.isInvalidated,
|
||||
).toBe(false);
|
||||
expect(
|
||||
qc.getQueryState(
|
||||
issueKeys.myAssigneeGroups("ws-1", "assigned", {}),
|
||||
)?.isInvalidated,
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("leaves an up-to-date row page alone", () => {
|
||||
const qc = new QueryClient();
|
||||
seedSiblingCaches(qc);
|
||||
qc.setQueryData(rowPageKey, {
|
||||
query_fingerprint: "sha256:rows",
|
||||
group_key: null,
|
||||
parent_id: null,
|
||||
total: 1,
|
||||
rows: [{ issue: issue(3, "current row"), direct_child_count: 0 }],
|
||||
branch_total: 1,
|
||||
next_cursor: null,
|
||||
});
|
||||
|
||||
onIssueAuxiliaryRevision(qc, "ws-1", "issue-1", 3);
|
||||
|
||||
expect(qc.getQueryState(rowPageKey)?.isInvalidated).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -5,10 +5,14 @@ import { labelKeys } from "../labels/queries";
|
||||
import { projectKeys } from "../projects/queries";
|
||||
import {
|
||||
applyIssueChange,
|
||||
bucketedListEntries,
|
||||
flatListEntries,
|
||||
invalidateIssueDerivatives,
|
||||
invalidateLastActivitySortedIssueLists,
|
||||
invalidateStaleListKeys,
|
||||
invalidateUpdatedAtSortedIssueLists,
|
||||
issueArrayEntries,
|
||||
tableRowEntries,
|
||||
type IssueFlatCache,
|
||||
} from "./cache-coordinator";
|
||||
import {
|
||||
@@ -128,10 +132,7 @@ function patchIssueInFlatCaches(
|
||||
patch: Partial<Issue>,
|
||||
orderRevision?: number,
|
||||
) {
|
||||
for (const [key, data] of qc.getQueriesData<IssueFlatCache>({
|
||||
queryKey: issueKeys.flatAll(wsId),
|
||||
})) {
|
||||
if (!data?.pages) continue;
|
||||
for (const [key, data] of flatListEntries(qc, wsId)) {
|
||||
qc.setQueryData<IssueFlatCache>(key, {
|
||||
...data,
|
||||
pages: data.pages.map((page) => ({
|
||||
@@ -154,10 +155,8 @@ function patchIssueInChildrenCaches(
|
||||
patch: Partial<Issue>,
|
||||
orderRevision?: number,
|
||||
) {
|
||||
for (const [key, data] of qc.getQueriesData<Issue[]>({
|
||||
queryKey: issueKeys.childrenAll(wsId),
|
||||
})) {
|
||||
if (!data || !data.some((child) => child.id === issueId)) continue;
|
||||
for (const [key, data] of issueArrayEntries(qc, issueKeys.childrenAll(wsId))) {
|
||||
if (!data.some((child) => child.id === issueId)) continue;
|
||||
qc.setQueryData<Issue[]>(
|
||||
key,
|
||||
data.map((child) =>
|
||||
@@ -176,17 +175,7 @@ function patchIssueInTableCaches(
|
||||
patch: Partial<Issue>,
|
||||
orderRevision?: number,
|
||||
) {
|
||||
for (const [key, data] of qc.getQueriesData<unknown>({
|
||||
queryKey: issueKeys.tableAll(wsId),
|
||||
})) {
|
||||
if (
|
||||
!data ||
|
||||
typeof data !== "object" ||
|
||||
!Array.isArray((data as IssueTableRowsResponse).rows)
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
const page = data as IssueTableRowsResponse;
|
||||
for (const [key, page] of tableRowEntries(qc, wsId)) {
|
||||
if (!page.rows.some((row) => row.issue.id === issueId)) continue;
|
||||
qc.setQueryData<IssueTableRowsResponse>(key, {
|
||||
...page,
|
||||
@@ -209,18 +198,15 @@ function patchIssueSnapshot(
|
||||
patch: Partial<Issue>,
|
||||
orderRevision?: number,
|
||||
) {
|
||||
for (const prefix of [issueKeys.list(wsId), issueKeys.myAll(wsId)]) {
|
||||
for (const [key, data] of qc.getQueriesData<ListIssuesCache>({ queryKey: prefix })) {
|
||||
if (!data) continue;
|
||||
const location = findIssueLocation(data, issueId);
|
||||
if (
|
||||
!location ||
|
||||
mergeIssuePatch(location.issue, patch, orderRevision) === location.issue
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
qc.setQueryData<ListIssuesCache>(key, patchIssueInBuckets(data, issueId, patch));
|
||||
for (const [key, data] of bucketedListEntries(qc, wsId)) {
|
||||
const location = findIssueLocation(data, issueId);
|
||||
if (
|
||||
!location ||
|
||||
mergeIssuePatch(location.issue, patch, orderRevision) === location.issue
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
qc.setQueryData<ListIssuesCache>(key, patchIssueInBuckets(data, issueId, patch));
|
||||
}
|
||||
patchIssueInFlatCaches(qc, wsId, issueId, patch, orderRevision);
|
||||
patchIssueInTableCaches(qc, wsId, issueId, patch, orderRevision);
|
||||
@@ -228,10 +214,10 @@ function patchIssueSnapshot(
|
||||
qc.setQueryData<Issue>(issueKeys.detail(wsId, issueId), (old) =>
|
||||
old ? mergeIssuePatch(old, patch, orderRevision) : old,
|
||||
);
|
||||
for (const [key, data] of qc.getQueriesData<Issue[]>({
|
||||
queryKey: issueKeys.projectGanttAll(wsId),
|
||||
})) {
|
||||
if (!data) continue;
|
||||
for (const [key, data] of issueArrayEntries(
|
||||
qc,
|
||||
issueKeys.projectGanttAll(wsId),
|
||||
)) {
|
||||
qc.setQueryData<Issue[]>(
|
||||
key,
|
||||
data.map((issue) =>
|
||||
@@ -254,27 +240,20 @@ function freshestCachedIssueRevision(
|
||||
freshest = issue.revision;
|
||||
}
|
||||
};
|
||||
for (const prefix of [issueKeys.list(wsId), issueKeys.myAll(wsId)]) {
|
||||
for (const [, data] of qc.getQueriesData<ListIssuesCache>({ queryKey: prefix })) {
|
||||
consider(data ? findIssueLocation(data, issueId)?.issue : undefined);
|
||||
}
|
||||
for (const [, data] of bucketedListEntries(qc, wsId)) {
|
||||
consider(findIssueLocation(data, issueId)?.issue);
|
||||
}
|
||||
for (const [, data] of qc.getQueriesData<IssueFlatCache>({
|
||||
queryKey: issueKeys.flatAll(wsId),
|
||||
})) {
|
||||
for (const page of data?.pages ?? []) {
|
||||
for (const [, data] of flatListEntries(qc, wsId)) {
|
||||
for (const page of data.pages) {
|
||||
consider(page.issues.find((issue) => issue.id === issueId));
|
||||
}
|
||||
}
|
||||
for (const [, data] of qc.getQueriesData<IssueTableRowsResponse>({
|
||||
queryKey: issueKeys.tableAll(wsId),
|
||||
})) {
|
||||
if (!data || !Array.isArray(data.rows)) continue;
|
||||
for (const [, data] of tableRowEntries(qc, wsId)) {
|
||||
consider(data.rows.find((row) => row.issue.id === issueId)?.issue);
|
||||
}
|
||||
for (const prefix of [issueKeys.childrenAll(wsId), issueKeys.projectGanttAll(wsId)]) {
|
||||
for (const [, data] of qc.getQueriesData<Issue[]>({ queryKey: prefix })) {
|
||||
consider(data?.find((issue) => issue.id === issueId));
|
||||
for (const [, data] of issueArrayEntries(qc, prefix)) {
|
||||
consider(data.find((issue) => issue.id === issueId));
|
||||
}
|
||||
}
|
||||
return freshest;
|
||||
@@ -307,30 +286,24 @@ function invalidateIssueOwnerProjectionsWhere(
|
||||
const detailKey = issueKeys.detail(wsId, issueId);
|
||||
if (shouldInvalidate(qc.getQueryData<Issue>(detailKey))) invalidateExact(detailKey);
|
||||
|
||||
for (const prefix of [issueKeys.list(wsId), issueKeys.myAll(wsId)]) {
|
||||
for (const [key, data] of qc.getQueriesData<ListIssuesCache>({ queryKey: prefix })) {
|
||||
if (shouldInvalidate(data ? findIssueLocation(data, issueId)?.issue : undefined)) {
|
||||
invalidateExact(key);
|
||||
}
|
||||
}
|
||||
}
|
||||
for (const [key, data] of qc.getQueriesData<IssueFlatCache>({
|
||||
queryKey: issueKeys.flatAll(wsId),
|
||||
})) {
|
||||
if (data?.pages.some((page) => shouldInvalidate(page.issues.find((issue) => issue.id === issueId)))) {
|
||||
for (const [key, data] of bucketedListEntries(qc, wsId)) {
|
||||
if (shouldInvalidate(findIssueLocation(data, issueId)?.issue)) {
|
||||
invalidateExact(key);
|
||||
}
|
||||
}
|
||||
for (const [key, data] of qc.getQueriesData<IssueTableRowsResponse>({
|
||||
queryKey: issueKeys.tableAll(wsId),
|
||||
})) {
|
||||
if (data?.rows.some((row) => row.issue.id === issueId && shouldInvalidate(row.issue))) {
|
||||
for (const [key, data] of flatListEntries(qc, wsId)) {
|
||||
if (data.pages.some((page) => shouldInvalidate(page.issues.find((issue) => issue.id === issueId)))) {
|
||||
invalidateExact(key);
|
||||
}
|
||||
}
|
||||
for (const [key, data] of tableRowEntries(qc, wsId)) {
|
||||
if (data.rows.some((row) => row.issue.id === issueId && shouldInvalidate(row.issue))) {
|
||||
invalidateExact(key);
|
||||
}
|
||||
}
|
||||
for (const prefix of [issueKeys.childrenAll(wsId), issueKeys.projectGanttAll(wsId)]) {
|
||||
for (const [key, data] of qc.getQueriesData<Issue[]>({ queryKey: prefix })) {
|
||||
if (data?.some((issue) => issue.id === issueId && shouldInvalidate(issue))) {
|
||||
for (const [key, data] of issueArrayEntries(qc, prefix)) {
|
||||
if (data.some((issue) => issue.id === issueId && shouldInvalidate(issue))) {
|
||||
invalidateExact(key);
|
||||
}
|
||||
}
|
||||
@@ -394,10 +367,8 @@ function findIssueInFlatCaches(
|
||||
wsId: string,
|
||||
issueId: string,
|
||||
) {
|
||||
for (const [, data] of qc.getQueriesData<IssueFlatCache>({
|
||||
queryKey: issueKeys.flatAll(wsId),
|
||||
})) {
|
||||
for (const page of data?.pages ?? []) {
|
||||
for (const [, data] of flatListEntries(qc, wsId)) {
|
||||
for (const page of data.pages) {
|
||||
const issue = page.issues.find((candidate) => candidate.id === issueId);
|
||||
if (issue) return issue;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user