mirror of
https://github.com/supermemoryai/supermemory.git
synced 2026-08-28 05:25:33 +00:00
fix(web): keep the document total correct after a bulk delete
The `documents-with-memories` query stores `pagination.totalItems` as the grand
total repeated on every page, and consumers read it from page 0 (e.g.
use-graph-api.ts, memories-grid). The bulk-delete optimistic update decremented
each page's totalItems by the number of documents removed *from that page*, so
deleting documents that live on later pages left page 0's total unchanged and
the UI showed a count that was too high until the refetch landed. The
single-delete path already decrements the grand total correctly.
Extract the three pure cache-update helpers into lib/document-cache-updates.ts
and fix the bulk path to tally the documents removed across all pages, then
subtract that one total from every page. Add unit tests covering removals on
later pages, spread across pages, customId matching, zero-clamping, the flat
{ documents } shape, and the single-delete and optimistic-add helpers.
This commit is contained in:
parent
2e85722cf4
commit
3b2431ea1a
3 changed files with 332 additions and 166 deletions
|
|
@ -11,6 +11,12 @@ import { useAuth } from "@lib/auth-context"
|
|||
import { analytics } from "@/lib/analytics"
|
||||
import { fetchSpaceSettings, spaceSettingsKey } from "@/hooks/use-space-context"
|
||||
import { getBackendUrl } from "@/lib/url-helpers"
|
||||
import {
|
||||
addOptimisticMemoryToQueryData,
|
||||
type OptimisticMemory,
|
||||
removeDocumentFromQueryData,
|
||||
removeDocumentsFromQueryData,
|
||||
} from "@/lib/document-cache-updates"
|
||||
|
||||
/** Pull the human-readable message out of a $fetch error (handles `{error}`/`{message}`/string). */
|
||||
function fetchErrorMessage(err: unknown, fallback: string): string {
|
||||
|
|
@ -23,176 +29,10 @@ function fetchErrorMessage(err: unknown, fallback: string): string {
|
|||
return fallback
|
||||
}
|
||||
|
||||
interface DocumentWithId {
|
||||
id?: string
|
||||
customId?: string | null
|
||||
}
|
||||
|
||||
interface UseDocumentMutationsOptions {
|
||||
onClose?: () => void
|
||||
}
|
||||
|
||||
interface OptimisticMemory {
|
||||
id: string
|
||||
content: string
|
||||
url: string | null
|
||||
title: string
|
||||
description: string
|
||||
containerTags: string[]
|
||||
createdAt: string
|
||||
updatedAt: string
|
||||
status: string
|
||||
type: string
|
||||
metadata: Record<string, unknown>
|
||||
memoryEntries: unknown[]
|
||||
isOptimistic?: boolean
|
||||
}
|
||||
|
||||
function addOptimisticMemoryToQueryData(
|
||||
old: unknown,
|
||||
memory: OptimisticMemory,
|
||||
): unknown {
|
||||
if (!old || typeof old !== "object") return old
|
||||
|
||||
const data = old as Record<string, unknown>
|
||||
|
||||
if ("pages" in data && Array.isArray(data.pages)) {
|
||||
return {
|
||||
...data,
|
||||
pages: data.pages.map((page: unknown, index: number) => {
|
||||
if (index !== 0) return page
|
||||
const p = page as Record<string, unknown>
|
||||
if (!p?.documents || !Array.isArray(p.documents)) return page
|
||||
return {
|
||||
...p,
|
||||
documents: [memory, ...p.documents],
|
||||
pagination: p.pagination
|
||||
? {
|
||||
...(p.pagination as Record<string, unknown>),
|
||||
totalItems:
|
||||
((p.pagination as Record<string, number>).totalItems ?? 0) +
|
||||
1,
|
||||
}
|
||||
: p.pagination,
|
||||
}
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
if ("documents" in data && Array.isArray(data.documents)) {
|
||||
return {
|
||||
...data,
|
||||
documents: [memory, ...data.documents],
|
||||
totalCount: ((data.totalCount as number) ?? 0) + 1,
|
||||
}
|
||||
}
|
||||
|
||||
return old
|
||||
}
|
||||
|
||||
function removeDocumentFromQueryData(
|
||||
old: unknown,
|
||||
documentId: string,
|
||||
): unknown {
|
||||
if (!old || typeof old !== "object") return old
|
||||
|
||||
const data = old as Record<string, unknown>
|
||||
|
||||
if ("pages" in data && Array.isArray(data.pages)) {
|
||||
return {
|
||||
...data,
|
||||
pages: data.pages.map((page: unknown) => {
|
||||
const p = page as Record<string, unknown>
|
||||
if (!p?.documents || !Array.isArray(p.documents)) return page
|
||||
return {
|
||||
...p,
|
||||
documents: (p.documents as DocumentWithId[]).filter(
|
||||
(doc) => doc.id !== documentId && doc.customId !== documentId,
|
||||
),
|
||||
pagination: p.pagination
|
||||
? {
|
||||
...(p.pagination as Record<string, unknown>),
|
||||
totalItems: Math.max(
|
||||
0,
|
||||
((p.pagination as Record<string, number>).totalItems ?? 0) -
|
||||
1,
|
||||
),
|
||||
}
|
||||
: p.pagination,
|
||||
}
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
if ("documents" in data && Array.isArray(data.documents)) {
|
||||
return {
|
||||
...data,
|
||||
documents: (data.documents as DocumentWithId[]).filter(
|
||||
(doc) => doc.id !== documentId && doc.customId !== documentId,
|
||||
),
|
||||
totalCount: Math.max(0, ((data.totalCount as number) ?? 0) - 1),
|
||||
}
|
||||
}
|
||||
|
||||
return old
|
||||
}
|
||||
|
||||
function removeDocumentsFromQueryData(
|
||||
old: unknown,
|
||||
documentIds: Set<string>,
|
||||
): unknown {
|
||||
if (!old || typeof old !== "object" || documentIds.size === 0) return old
|
||||
|
||||
const data = old as Record<string, unknown>
|
||||
|
||||
if ("pages" in data && Array.isArray(data.pages)) {
|
||||
return {
|
||||
...data,
|
||||
pages: data.pages.map((page: unknown) => {
|
||||
const p = page as Record<string, unknown>
|
||||
if (!p?.documents || !Array.isArray(p.documents)) return page
|
||||
const filtered = (p.documents as DocumentWithId[]).filter(
|
||||
(doc) =>
|
||||
!documentIds.has(doc.id ?? "") &&
|
||||
!documentIds.has(doc.customId ?? ""),
|
||||
)
|
||||
const removed =
|
||||
(p.documents as DocumentWithId[]).length - filtered.length
|
||||
return {
|
||||
...p,
|
||||
documents: filtered,
|
||||
pagination: p.pagination
|
||||
? {
|
||||
...(p.pagination as Record<string, unknown>),
|
||||
totalItems: Math.max(
|
||||
0,
|
||||
((p.pagination as Record<string, number>).totalItems ?? 0) -
|
||||
removed,
|
||||
),
|
||||
}
|
||||
: p.pagination,
|
||||
}
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
if ("documents" in data && Array.isArray(data.documents)) {
|
||||
const filtered = (data.documents as DocumentWithId[]).filter(
|
||||
(doc) =>
|
||||
!documentIds.has(doc.id ?? "") && !documentIds.has(doc.customId ?? ""),
|
||||
)
|
||||
const removed =
|
||||
(data.documents as DocumentWithId[]).length - filtered.length
|
||||
return {
|
||||
...data,
|
||||
documents: filtered,
|
||||
totalCount: Math.max(0, ((data.totalCount as number) ?? 0) - removed),
|
||||
}
|
||||
}
|
||||
|
||||
return old
|
||||
}
|
||||
|
||||
async function cancelAndSnapshotQueries(
|
||||
queryClient: QueryClient,
|
||||
): Promise<[unknown, unknown][]> {
|
||||
|
|
|
|||
137
apps/web/lib/document-cache-updates.test.ts
Normal file
137
apps/web/lib/document-cache-updates.test.ts
Normal file
|
|
@ -0,0 +1,137 @@
|
|||
import { describe, expect, it } from "bun:test"
|
||||
import {
|
||||
addOptimisticMemoryToQueryData,
|
||||
type OptimisticMemory,
|
||||
removeDocumentFromQueryData,
|
||||
removeDocumentsFromQueryData,
|
||||
} from "./document-cache-updates"
|
||||
|
||||
type Page = {
|
||||
documents: Array<{ id?: string; customId?: string | null }>
|
||||
pagination: { totalItems: number }
|
||||
}
|
||||
|
||||
const page = (ids: string[], totalItems: number): Page => ({
|
||||
documents: ids.map((id) => ({ id })),
|
||||
pagination: { totalItems },
|
||||
})
|
||||
|
||||
const pagesData = (pages: Page[]) => ({ pages, pageParams: [] })
|
||||
|
||||
const totalsPerPage = (result: unknown): number[] =>
|
||||
(result as { pages: Page[] }).pages.map((p) => p.pagination.totalItems)
|
||||
|
||||
const idsPerPage = (result: unknown): string[][] =>
|
||||
(result as { pages: Page[] }).pages.map((p) =>
|
||||
p.documents.map((d) => d.id ?? ""),
|
||||
)
|
||||
|
||||
describe("removeDocumentsFromQueryData (infinite pages)", () => {
|
||||
it("decrements every page's grand total by the number removed across all pages", () => {
|
||||
// totalItems is the same grand total on every page; consumers read page 0.
|
||||
const data = pagesData([page(["a", "b", "c"], 6), page(["d", "e", "f"], 6)])
|
||||
|
||||
// Delete two documents that live on the SECOND page only.
|
||||
const result = removeDocumentsFromQueryData(data, new Set(["e", "f"]))
|
||||
|
||||
expect(idsPerPage(result)).toEqual([["a", "b", "c"], ["d"]])
|
||||
// Both pages (page 0 included) must drop to 4, not stay at 6 / 6.
|
||||
expect(totalsPerPage(result)).toEqual([4, 4])
|
||||
})
|
||||
|
||||
it("handles deletions spread across multiple pages", () => {
|
||||
const data = pagesData([page(["a", "b"], 5), page(["c", "d", "e"], 5)])
|
||||
|
||||
const result = removeDocumentsFromQueryData(data, new Set(["a", "c", "d"]))
|
||||
|
||||
expect(idsPerPage(result)).toEqual([["b"], ["e"]])
|
||||
expect(totalsPerPage(result)).toEqual([2, 2])
|
||||
})
|
||||
|
||||
it("clamps the total at zero and ignores unknown ids", () => {
|
||||
const data = pagesData([page(["a"], 1)])
|
||||
|
||||
const result = removeDocumentsFromQueryData(
|
||||
data,
|
||||
new Set(["a", "does-not-exist"]),
|
||||
)
|
||||
|
||||
expect(idsPerPage(result)).toEqual([[]])
|
||||
expect(totalsPerPage(result)).toEqual([0])
|
||||
})
|
||||
|
||||
it("matches customId as well as id", () => {
|
||||
const data = {
|
||||
pages: [
|
||||
{
|
||||
documents: [{ id: "1", customId: "cust-1" }, { id: "2" }],
|
||||
pagination: { totalItems: 2 },
|
||||
},
|
||||
],
|
||||
pageParams: [],
|
||||
}
|
||||
|
||||
const result = removeDocumentsFromQueryData(data, new Set(["cust-1"]))
|
||||
|
||||
expect(idsPerPage(result)).toEqual([["2"]])
|
||||
expect(totalsPerPage(result)).toEqual([1])
|
||||
})
|
||||
|
||||
it("updates the flat { documents } shape", () => {
|
||||
const data = {
|
||||
documents: [{ id: "a" }, { id: "b" }, { id: "c" }],
|
||||
totalCount: 3,
|
||||
}
|
||||
|
||||
const result = removeDocumentsFromQueryData(data, new Set(["a", "b"])) as {
|
||||
documents: Array<{ id: string }>
|
||||
totalCount: number
|
||||
}
|
||||
|
||||
expect(result.documents.map((d) => d.id)).toEqual(["c"])
|
||||
expect(result.totalCount).toBe(1)
|
||||
})
|
||||
|
||||
it("returns the input untouched for an empty id set", () => {
|
||||
const data = pagesData([page(["a"], 1)])
|
||||
expect(removeDocumentsFromQueryData(data, new Set())).toBe(data)
|
||||
})
|
||||
})
|
||||
|
||||
describe("removeDocumentFromQueryData (single)", () => {
|
||||
it("drops the doc and decrements the grand total once per page", () => {
|
||||
const data = pagesData([page(["a", "b"], 4), page(["c", "d"], 4)])
|
||||
|
||||
const result = removeDocumentFromQueryData(data, "c")
|
||||
|
||||
expect(idsPerPage(result)).toEqual([["a", "b"], ["d"]])
|
||||
expect(totalsPerPage(result)).toEqual([3, 3])
|
||||
})
|
||||
})
|
||||
|
||||
describe("addOptimisticMemoryToQueryData", () => {
|
||||
const memory: OptimisticMemory = {
|
||||
id: "new",
|
||||
content: "hi",
|
||||
url: null,
|
||||
title: "t",
|
||||
description: "",
|
||||
containerTags: [],
|
||||
createdAt: "",
|
||||
updatedAt: "",
|
||||
status: "queued",
|
||||
type: "note",
|
||||
metadata: {},
|
||||
memoryEntries: [],
|
||||
isOptimistic: true,
|
||||
}
|
||||
|
||||
it("prepends to the first page and bumps the grand total", () => {
|
||||
const data = pagesData([page(["a"], 1), page(["b"], 1)])
|
||||
|
||||
const result = addOptimisticMemoryToQueryData(data, memory)
|
||||
|
||||
expect(idsPerPage(result)).toEqual([["new", "a"], ["b"]])
|
||||
expect(totalsPerPage(result)[0]).toBe(2)
|
||||
})
|
||||
})
|
||||
189
apps/web/lib/document-cache-updates.ts
Normal file
189
apps/web/lib/document-cache-updates.ts
Normal file
|
|
@ -0,0 +1,189 @@
|
|||
/**
|
||||
* Pure helpers for optimistically updating the cached
|
||||
* `documents-with-memories` query data (both the infinite `{ pages }` shape and
|
||||
* the flat `{ documents }` shape).
|
||||
*
|
||||
* `pagination.totalItems` is the grand total repeated on every page, and
|
||||
* consumers read it from page 0 (see `use-graph-api.ts`), so a delete must
|
||||
* decrement the total by the number of documents actually removed across all
|
||||
* pages — not just the ones that happened to sit on a given page.
|
||||
*/
|
||||
|
||||
export interface DocumentWithId {
|
||||
id?: string
|
||||
customId?: string | null
|
||||
}
|
||||
|
||||
export interface OptimisticMemory {
|
||||
id: string
|
||||
content: string
|
||||
url: string | null
|
||||
title: string
|
||||
description: string
|
||||
containerTags: string[]
|
||||
createdAt: string
|
||||
updatedAt: string
|
||||
status: string
|
||||
type: string
|
||||
metadata: Record<string, unknown>
|
||||
memoryEntries: unknown[]
|
||||
isOptimistic?: boolean
|
||||
}
|
||||
|
||||
export function addOptimisticMemoryToQueryData(
|
||||
old: unknown,
|
||||
memory: OptimisticMemory,
|
||||
): unknown {
|
||||
if (!old || typeof old !== "object") return old
|
||||
|
||||
const data = old as Record<string, unknown>
|
||||
|
||||
if ("pages" in data && Array.isArray(data.pages)) {
|
||||
return {
|
||||
...data,
|
||||
pages: data.pages.map((page: unknown, index: number) => {
|
||||
if (index !== 0) return page
|
||||
const p = page as Record<string, unknown>
|
||||
if (!p?.documents || !Array.isArray(p.documents)) return page
|
||||
return {
|
||||
...p,
|
||||
documents: [memory, ...p.documents],
|
||||
pagination: p.pagination
|
||||
? {
|
||||
...(p.pagination as Record<string, unknown>),
|
||||
totalItems:
|
||||
((p.pagination as Record<string, number>).totalItems ?? 0) +
|
||||
1,
|
||||
}
|
||||
: p.pagination,
|
||||
}
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
if ("documents" in data && Array.isArray(data.documents)) {
|
||||
return {
|
||||
...data,
|
||||
documents: [memory, ...data.documents],
|
||||
totalCount: ((data.totalCount as number) ?? 0) + 1,
|
||||
}
|
||||
}
|
||||
|
||||
return old
|
||||
}
|
||||
|
||||
export function removeDocumentFromQueryData(
|
||||
old: unknown,
|
||||
documentId: string,
|
||||
): unknown {
|
||||
if (!old || typeof old !== "object") return old
|
||||
|
||||
const data = old as Record<string, unknown>
|
||||
|
||||
if ("pages" in data && Array.isArray(data.pages)) {
|
||||
return {
|
||||
...data,
|
||||
pages: data.pages.map((page: unknown) => {
|
||||
const p = page as Record<string, unknown>
|
||||
if (!p?.documents || !Array.isArray(p.documents)) return page
|
||||
return {
|
||||
...p,
|
||||
documents: (p.documents as DocumentWithId[]).filter(
|
||||
(doc) => doc.id !== documentId && doc.customId !== documentId,
|
||||
),
|
||||
pagination: p.pagination
|
||||
? {
|
||||
...(p.pagination as Record<string, unknown>),
|
||||
totalItems: Math.max(
|
||||
0,
|
||||
((p.pagination as Record<string, number>).totalItems ?? 0) -
|
||||
1,
|
||||
),
|
||||
}
|
||||
: p.pagination,
|
||||
}
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
if ("documents" in data && Array.isArray(data.documents)) {
|
||||
return {
|
||||
...data,
|
||||
documents: (data.documents as DocumentWithId[]).filter(
|
||||
(doc) => doc.id !== documentId && doc.customId !== documentId,
|
||||
),
|
||||
totalCount: Math.max(0, ((data.totalCount as number) ?? 0) - 1),
|
||||
}
|
||||
}
|
||||
|
||||
return old
|
||||
}
|
||||
|
||||
export function removeDocumentsFromQueryData(
|
||||
old: unknown,
|
||||
documentIds: Set<string>,
|
||||
): unknown {
|
||||
if (!old || typeof old !== "object" || documentIds.size === 0) return old
|
||||
|
||||
const data = old as Record<string, unknown>
|
||||
|
||||
if ("pages" in data && Array.isArray(data.pages)) {
|
||||
// Filter every page first and tally how many documents were actually
|
||||
// removed across all of them, then subtract that single total from each
|
||||
// page's (grand-total) `totalItems`. Decrementing only by each page's own
|
||||
// removals left page 0 — the one consumers read — too high whenever the
|
||||
// deleted documents lived on later pages.
|
||||
let totalRemoved = 0
|
||||
const filteredPages = data.pages.map((page: unknown) => {
|
||||
const p = page as Record<string, unknown>
|
||||
if (!p?.documents || !Array.isArray(p.documents)) {
|
||||
return { page, hasDocuments: false as const }
|
||||
}
|
||||
const kept = (p.documents as DocumentWithId[]).filter(
|
||||
(doc) =>
|
||||
!documentIds.has(doc.id ?? "") &&
|
||||
!documentIds.has(doc.customId ?? ""),
|
||||
)
|
||||
totalRemoved += (p.documents as DocumentWithId[]).length - kept.length
|
||||
return { page: p, documents: kept, hasDocuments: true as const }
|
||||
})
|
||||
|
||||
return {
|
||||
...data,
|
||||
pages: filteredPages.map((entry) => {
|
||||
if (!entry.hasDocuments) return entry.page
|
||||
const p = entry.page as Record<string, unknown>
|
||||
return {
|
||||
...p,
|
||||
documents: entry.documents,
|
||||
pagination: p.pagination
|
||||
? {
|
||||
...(p.pagination as Record<string, unknown>),
|
||||
totalItems: Math.max(
|
||||
0,
|
||||
((p.pagination as Record<string, number>).totalItems ?? 0) -
|
||||
totalRemoved,
|
||||
),
|
||||
}
|
||||
: p.pagination,
|
||||
}
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
if ("documents" in data && Array.isArray(data.documents)) {
|
||||
const filtered = (data.documents as DocumentWithId[]).filter(
|
||||
(doc) =>
|
||||
!documentIds.has(doc.id ?? "") && !documentIds.has(doc.customId ?? ""),
|
||||
)
|
||||
const removed =
|
||||
(data.documents as DocumentWithId[]).length - filtered.length
|
||||
return {
|
||||
...data,
|
||||
documents: filtered,
|
||||
totalCount: Math.max(0, ((data.totalCount as number) ?? 0) - removed),
|
||||
}
|
||||
}
|
||||
|
||||
return old
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue