diff --git a/src/features/tech-log/domain/content-format/asset-evidence-catalog.ts b/src/features/tech-log/domain/content-format/asset-evidence-catalog.ts index 2170c52..e9fd615 100644 --- a/src/features/tech-log/domain/content-format/asset-evidence-catalog.ts +++ b/src/features/tech-log/domain/content-format/asset-evidence-catalog.ts @@ -63,6 +63,40 @@ function outranks(candidate: ResolvableAsset, incumbent: ResolvableAsset): boole return candidate.id > incumbent.id; } +/** + * Grows the set of Assets an evidence key can resolve against, never shrinks + * it. + * + * Alignment follow-up, item 2. The editor screen's Asset array feeds two + * consumers whose needs are opposite: the Picker's *displayed* list, which a + * search must narrow, and Instant Preview's *resolution catalog*, which a + * search must never narrow -- `instantPreviewEvidenceKeyGate` rejects any key + * no loaded Asset backs, so an Asset dropped from the array turns a directive + * that rendered a moment ago into an error panel. Handing the Picker's search + * results straight to `setAssets` would do exactly that: type a query that + * excludes an already-inserted figure and the live preview blanks, purely + * because the author touched a search box. + * + * Making the screen's callback *additive* is what keeps the two apart, and it + * has to be additive by construction rather than by convention -- the + * replacing version was one `setAssets` reference away and read as correct. + * `incoming` is placed first and `current` deduped behind it, so a re-fetched + * Asset's fresher fields win while every previously observed Asset survives; + * that is also exactly the expression a single freshly uploaded Asset needs, + * so both callers share one rule. + * + * Order is not load-bearing for resolution -- `findResolvableAsset` is + * explicitly order-independent -- so this only has to be total and stable. + */ +export function mergeAssetCatalog( + current: readonly Asset[], + incoming: readonly Asset[], +): readonly Asset[] { + if (incoming.length === 0) return current; + const arriving = new Set(incoming.map((asset) => asset.id)); + return [...incoming, ...current.filter((asset) => !arriving.has(asset.id))]; +} + /** * The evidence-key gate every caller uses. A key is referenceable when a real * Asset resolves it, or when it is the caller's own legacy static key -- the diff --git a/src/features/tech-log/presentation/studio/components/asset-picker.tsx b/src/features/tech-log/presentation/studio/components/asset-picker.tsx index a97944b..cab9dc7 100644 --- a/src/features/tech-log/presentation/studio/components/asset-picker.tsx +++ b/src/features/tech-log/presentation/studio/components/asset-picker.tsx @@ -1,8 +1,11 @@ -import { useEffect, useState } from "react"; +import { useEffect, useId, useState, type FormEvent } from "react"; import type { Asset } from "../../../contracts/studio/contract.ts"; import type { StudioAssetGateway } from "../../../application/ports/studio-asset-gateway.ts"; +/** One screenful of candidates; searching, not scrolling, reaches the rest. */ +const PAGE_SIZE = 50; + /** * A directive attribute value is delimited by double quotes (see * `parse-case-content.ts`'s `attributesOf`), so a value containing one would @@ -15,6 +18,10 @@ function attributeValue(raw: string): string { return raw.replaceAll('"', "").replace(/\s+/gu, " ").trim(); } +function isAbortError(error: unknown): boolean { + return error instanceof DOMException && error.name === "AbortError"; +} + export function buildEvidenceDirective( input: Readonly<{ assetKey: string; alt: string; caption: string; zoom: boolean }>, ): string { @@ -27,45 +34,100 @@ export function buildEvidenceDirective( export function AssetPicker({ gateway, onInsert, - onLoaded, + onAssetsObserved, }: Readonly<{ gateway: StudioAssetGateway; onInsert: (directive: string) => void; /** - * Optional so Task 10 Step 1's original test (which renders `AssetPicker` - * without it) keeps passing. When supplied, the editor screen uses this to - * own the same Asset list the Instant Preview resolver reads -- so a - * directive this Picker just inserted renders immediately instead of as a - * placeholder. + * Every Asset this Picker has *seen* -- the first page, and each search's + * results -- reported as it arrives. Deliberately not "the Assets to show": + * the editor screen merges these into a resolution catalog that only grows + * (`mergeAssetCatalog`), because Instant Preview resolves `:::evidence` + * directives against that catalog and a narrowing search must never make an + * already-inserted figure stop rendering. Optional so Task 10 Step 1's + * original test (which renders `AssetPicker` on its own) keeps passing. */ - onLoaded?: (assets: readonly Asset[]) => void; + onAssetsObserved?: (assets: readonly Asset[]) => void; }>) { const [assets, setAssets] = useState([]); - const [failed, setFailed] = useState(false); + const [status, setStatus] = useState<"LOADING" | "ERROR" | "READY">("LOADING"); + // Alignment follow-up, item 2. `searchDraft` is what the author is typing, + // `q` the query actually submitted -- only `q` is an effect dependency. That + // is a stronger guarantee than debouncing (zero requests while typing rather + // than fewer, and no trailing request after the author stops), and it is the + // pair `document-list.tsx` already uses for the same job. + const [searchDraft, setSearchDraft] = useState(""); + const [q, setQ] = useState(""); + const searchId = useId(); useEffect(() => { const controller = new AbortController(); + // `RequestOptions.signal` is advisory: an adapter that ignores it still + // resolves, and an abandoned query's response would then replace the newer + // one's results. This flag flips synchronously when `q` changes, so an + // older search can never land last. + let active = true; + setStatus("LOADING"); gateway - .listAssets({ managementStatus: "READY", limit: 50 }, { signal: controller.signal }) + .listAssets( + { managementStatus: "READY", ...(q ? { q } : {}), limit: PAGE_SIZE }, + { signal: controller.signal }, + ) .then((page) => { + if (!active) return; setAssets(page.items); - onLoaded?.(page.items); + setStatus("READY"); + onAssetsObserved?.(page.items); }) - .catch(() => setFailed(true)); - return () => controller.abort(); - }, [gateway, onLoaded]); + .catch((error: unknown) => { + if (!active || isAbortError(error)) return; + setStatus("ERROR"); + }); + return () => { + active = false; + controller.abort(); + }; + }, [gateway, onAssetsObserved, q]); // READY만 삽입 후보다. 서버 필터를 신뢰하되 방어적으로 한 번 더 거른다. + // A query does not relax this: an asset under review is never insertable, + // however it was found. const selectable = assets.filter((asset) => asset.managementStatus === "READY"); - if (failed) return

Asset 목록을 불러오지 못했습니다.

; + const submitSearch = (event: FormEvent) => { + event.preventDefault(); + setQ(searchDraft.trim()); + }; - if (selectable.length === 0) { - return

삽입할 수 있는 Asset이 없습니다. 먼저 업로드하세요.

; - } + const listMessage = status === "LOADING" + ? "Asset 목록을 불러오는 중입니다." + : selectable.length > 0 + ? `삽입할 수 있는 Asset ${selectable.length}개` + : q + ? "검색 결과가 없습니다. 다른 검색어를 입력하세요." + : "삽입할 수 있는 Asset이 없습니다. 먼저 업로드하세요."; return
- : null}
; } diff --git a/src/features/tech-log/presentation/studio/components/case-fields.tsx b/src/features/tech-log/presentation/studio/components/case-fields.tsx index 988a835..6ec28db 100644 --- a/src/features/tech-log/presentation/studio/components/case-fields.tsx +++ b/src/features/tech-log/presentation/studio/components/case-fields.tsx @@ -37,13 +37,17 @@ function insertAtCursor( export function CaseFields({ draft, onChange, - onAssetsLoaded, + onAssetsObserved, onAssetUploaded, }: { draft: CaseInput; onChange(draft: CaseInput): void; - /** The editor screen owns the loaded Asset list so the Picker and Instant Preview read the same array. */ - onAssetsLoaded?: (assets: readonly Asset[]) => void; + /** + * The editor screen owns the resolution catalog Instant Preview reads, and + * merges what the Picker observes into it -- it never adopts the Picker's + * (searchable, and so narrowable) displayed list wholesale. + */ + onAssetsObserved?: (assets: readonly Asset[]) => void; onAssetUploaded?: (asset: Asset) => void; }) { // Asset UI must use the throwing accessor, not the nullable `assetGateway` @@ -103,7 +107,7 @@ export function CaseFields({ Asset 업로드 - + {uploadOpen ? ( ([]); + const observeAssets = useCallback((observed: readonly Asset[]) => { + setAssets((current) => mergeAssetCatalog(current, observed)); + }, []); + const observeUploadedAsset = useCallback((asset: Asset) => { + setAssets((current) => mergeAssetCatalog(current, [asset])); + }, []); useEffect(() => { const request = new AbortController(); @@ -156,10 +171,8 @@ export function DocumentEditorScreen({ documentId }: { documentId: string }) { controller={controller!} catalog={currentResult.catalog} assets={assets} - onAssetsLoaded={setAssets} - onAssetUploaded={(asset) => - setAssets((current) => [asset, ...current.filter((existing) => existing.id !== asset.id)]) - } + onAssetsObserved={observeAssets} + onAssetUploaded={observeUploadedAsset} /> ); } diff --git a/src/features/tech-log/presentation/studio/components/document-editor.tsx b/src/features/tech-log/presentation/studio/components/document-editor.tsx index 519c128..5884d16 100644 --- a/src/features/tech-log/presentation/studio/components/document-editor.tsx +++ b/src/features/tech-log/presentation/studio/components/document-editor.tsx @@ -17,14 +17,14 @@ export function DocumentEditor({ controller, catalog, assets, - onAssetsLoaded, + onAssetsObserved, onAssetUploaded, }: { controller: DocumentEditorController; catalog: CatalogEntry[]; /** Owned by `DocumentEditorScreen` so the CASE editor's Picker/Upload and Instant Preview share one list. */ assets: readonly Asset[]; - onAssetsLoaded: (assets: readonly Asset[]) => void; + onAssetsObserved: (assets: readonly Asset[]) => void; onAssetUploaded: (asset: Asset) => void; }) { const [tab, setTab] = useState<"EDIT" | "PREVIEW">("EDIT"); @@ -64,7 +64,7 @@ export function DocumentEditor({ {controller.draft.kind === "CASE" - ? + ? : controller.draft.kind === "REFERENCE" ? : controller.draft.kind === "QUESTION" diff --git a/src/features/tech-log/presentation/styles/studio-editor.css b/src/features/tech-log/presentation/styles/studio-editor.css index ac8f562..bbbe564 100644 --- a/src/features/tech-log/presentation/styles/studio-editor.css +++ b/src/features/tech-log/presentation/styles/studio-editor.css @@ -78,6 +78,14 @@ .studio-app .studio-asset-panel-actions button { min-height: 44px; padding-inline: 13px; border: 1px solid var(--line-strong); border-radius: 5px; background: var(--paper); color: var(--ink); font-weight: 650; } .studio-app .asset-picker ul { display: flex; flex-wrap: wrap; gap: 8px; padding: 0; margin: 0; list-style: none; } .studio-app .asset-picker button { min-height: 36px; padding-inline: 12px; border: 1px solid var(--line-strong); border-radius: 999px; background: var(--paper); color: var(--ink); font-size: 13px; } +/* Declared after `.asset-picker button` on purpose: the pill rule above also + matches this form's submit button, and both selectors carry the same + specificity, so only source order tells them apart. */ +.studio-app .asset-picker-search { display: grid; max-width: 420px; gap: 8px; margin-bottom: 14px; color: var(--muted); font-size: 12px; font-weight: 650; } +.studio-app .asset-picker-search div { display: flex; gap: 8px; } +.studio-app .asset-picker-search input { min-width: 0; flex: 1; min-height: 44px; padding-inline: 12px; border: 1px solid var(--line-strong); border-radius: 5px; background: var(--paper); color: var(--ink); font-size: 14px; } +.studio-app .asset-picker-search button { min-height: 44px; padding-inline: 16px; border-radius: 5px; font-size: 13px; font-weight: 650; } +.studio-app .asset-picker ul:not(:empty) { margin-top: 12px; } .studio-app .studio-error, .studio-app .studio-asset-picker-empty { margin: 0; color: var(--muted); font-size: 13px; } .studio-app .studio-instant-preview { min-width: 0; overflow: clip; border: 1px solid var(--line); } diff --git a/tests/features/tech-log/asset-picker.test.tsx b/tests/features/tech-log/asset-picker.test.tsx index 5dc92d6..df34b8e 100644 --- a/tests/features/tech-log/asset-picker.test.tsx +++ b/tests/features/tech-log/asset-picker.test.tsx @@ -2,7 +2,7 @@ import assert from "node:assert/strict"; import { afterAll, beforeAll, test } from "vitest"; -import { fireEvent, render, screen, waitFor, within } from "@testing-library/react"; +import { act, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { MemoryRouter } from "react-router-dom"; @@ -10,6 +10,7 @@ import { AssetPicker, buildEvidenceDirective, } from "../../../src/features/tech-log/presentation/studio/components/asset-picker.tsx"; +import { mergeAssetCatalog } from "../../../src/features/tech-log/domain/content-format/asset-evidence-catalog.ts"; import { AssetUploadDialog, stateForError, @@ -151,7 +152,7 @@ test("reports the loaded Asset list once listAssets resolves", async () => { {}} - onLoaded={(assets) => (loaded as Asset[][]).push([...assets])} + onAssetsObserved={(assets) => (loaded as Asset[][]).push([...assets])} />, ); @@ -1561,3 +1562,276 @@ test("the dialog refuses to upload a non-decorative asset with no alt text", asy await waitFor(() => assert.equal(screen.getByRole("status", { name: "업로드 상태" }).textContent, "업로드했습니다.")); assert.equal(calls, 1); }); + +// --- Alignment follow-up, item 2: the Picker searches the server --- +// +// The Picker asked for `{ managementStatus: "READY", limit: 50 }` and ignored +// `nextCursor`, so the 51st-oldest READY asset onward could not be inserted at +// all. The Picker sits inside the editing flow, where scrolling a long list is +// the wrong interaction, so it gets search rather than a "더 보기" control -- +// but it still loads a first page, because an empty panel until you type is +// hostile to an author who just wants the asset they uploaded a minute ago. + +function searchableGateway(items: ReadonlyArray>) { + const calls: Array> = []; + const gateway: StudioAssetGateway = { + async listAssets(query) { + calls.push({ ...query }); + const q = (query.q ?? "").trim().toLocaleLowerCase("ko-KR"); + const matched = items.filter( + (item) => + (!query.managementStatus || + item.managementStatus === query.managementStatus) && + (!q || + String(item.assetKey).toLocaleLowerCase("ko-KR").includes(q)), + ); + return { items: matched, nextCursor: null } as never; + }, + async uploadAsset() { + throw new Error("not used"); + }, + async getAsset() { + throw new Error("not used"); + }, + async updateAssetMetadata() { + throw new Error("not used"); + }, + async deleteAsset() {}, + }; + return { gateway, calls }; +} + +const PICKER_A = { + ...READY, + id: "44444444-4444-4444-8444-444444444441", + assetKey: "inserted-diagram", + altText: "삽입한 다이어그램", + publicPath: "/media/inserted-diagram.svg", +}; +const PICKER_B = { + ...READY, + id: "44444444-4444-4444-8444-444444444442", + assetKey: "other-diagram", + altText: "다른 다이어그램", + publicPath: "/media/other-diagram.svg", +}; + +test("the Picker shows a first page before anything is typed, then narrows to the submitted query", async () => { + const user = userEvent.setup(); + const { gateway, calls } = searchableGateway([PICKER_A, PICKER_B]); + render( {}} />); + + // Useful before the author types: both assets are offered. + await screen.findByRole("button", { name: /inserted-diagram/ }); + assert.ok(screen.getByRole("button", { name: /other-diagram/ })); + + await user.type(screen.getByLabelText("Asset 검색"), "other"); + await user.click(screen.getByRole("button", { name: "검색" })); + + await waitFor(() => + assert.equal(screen.queryByRole("button", { name: /inserted-diagram/ }), null), + ); + assert.ok(screen.getByRole("button", { name: /other-diagram/ })); + assert.equal(calls.at(-1)?.q, "other"); + // The server filter must survive search: an asset under review is never + // insertable, whatever the query. + assert.equal(calls.at(-1)?.managementStatus, "READY"); +}); + +test("typing in the Picker's search box never reaches the gateway on its own", async () => { + const user = userEvent.setup(); + const { gateway, calls } = searchableGateway([PICKER_A]); + render( {}} />); + + await screen.findByRole("button", { name: /inserted-diagram/ }); + assert.equal(calls.length, 1); + + await user.type(screen.getByLabelText("Asset 검색"), "inserted"); + + assert.equal(calls.length, 1); +}); + +test("a QUARANTINED asset the server wrongly returns for a query is still not offered", async () => { + const user = userEvent.setup(); + const unsafe = { ...QUARANTINED, assetKey: "unsafe-diagram" }; + const gateway: StudioAssetGateway = { + async listAssets() { + // Deliberately ignores `managementStatus`: the screen's own filter is + // the thing under test. + return { items: [PICKER_A, unsafe], nextCursor: null } as never; + }, + async uploadAsset() { + throw new Error("not used"); + }, + async getAsset() { + throw new Error("not used"); + }, + async updateAssetMetadata() { + throw new Error("not used"); + }, + async deleteAsset() {}, + }; + render( {}} />); + + await screen.findByRole("button", { name: /inserted-diagram/ }); + await user.type(screen.getByLabelText("Asset 검색"), "diagram"); + await user.click(screen.getByRole("button", { name: "검색" })); + + await waitFor(() => + assert.ok(screen.getByRole("button", { name: /inserted-diagram/ })), + ); + assert.equal(screen.queryByRole("button", { name: /unsafe-diagram/ }), null); +}); + +test("the Picker reports search results too, so the catalog grows with every query", async () => { + const user = userEvent.setup(); + const observed: Asset[][] = []; + const { gateway } = searchableGateway([PICKER_A, PICKER_B]); + render( + {}} + onAssetsObserved={(assets) => observed.push([...assets])} + />, + ); + + await screen.findByRole("button", { name: /inserted-diagram/ }); + await user.type(screen.getByLabelText("Asset 검색"), "other"); + await user.click(screen.getByRole("button", { name: "검색" })); + + await waitFor(() => assert.equal(observed.length, 2)); + assert.deepEqual( + observed[1]!.map((asset) => asset.assetKey), + ["other-diagram"], + ); +}); + +test("a slow earlier Picker search never overwrites the newer query's results", async () => { + const user = userEvent.setup(); + const pending: Array<{ settle: (page: unknown) => void }> = []; + const gateway: StudioAssetGateway = { + async listAssets() { + return new Promise((resolve) => { + pending.push({ settle: resolve }); + }) as never; + }, + async uploadAsset() { + throw new Error("not used"); + }, + async getAsset() { + throw new Error("not used"); + }, + async updateAssetMetadata() { + throw new Error("not used"); + }, + async deleteAsset() {}, + }; + render( {}} />); + + await waitFor(() => assert.equal(pending.length, 1)); + await act(async () => { + pending[0]!.settle({ items: [PICKER_A], nextCursor: null }); + }); + await screen.findByRole("button", { name: /inserted-diagram/ }); + + const input = screen.getByLabelText("Asset 검색"); + await user.type(input, "slow"); + await user.click(screen.getByRole("button", { name: "검색" })); + await waitFor(() => assert.equal(pending.length, 2)); + + await user.clear(input); + await user.type(input, "other"); + await user.click(screen.getByRole("button", { name: "검색" })); + await waitFor(() => assert.equal(pending.length, 3)); + + await act(async () => { + pending[2]!.settle({ items: [PICKER_B], nextCursor: null }); + }); + await screen.findByRole("button", { name: /other-diagram/ }); + + await act(async () => { + pending[1]!.settle({ items: [PICKER_A], nextCursor: null }); + }); + + assert.equal(screen.queryByRole("button", { name: /inserted-diagram/ }), null); + assert.ok(screen.getByRole("button", { name: /other-diagram/ })); +}); + +// --- The trap this item creates --- +// +// The editor screen's Asset array feeds two consumers with opposite needs: the +// Picker's *displayed* list (which a search must narrow) and Instant Preview's +// *resolution catalog* (which a search must never shrink -- its gate rejects a +// key no loaded asset backs). Wiring the Picker's results straight back into +// the array the screen owns makes typing a search term blank out evidence +// figures that rendered a moment earlier: a worse bug than the one being +// fixed. The two are kept apart by making the screen's callback additive -- +// the Picker reports what it *observed*, the screen merges it into a catalog +// that only ever grows. + +test("a Picker search never drops an already-inserted asset out of Instant Preview's resolution", async () => { + const user = userEvent.setup(); + const gateway = createMockStudioGateway(); + const { gateway: assetGateway } = searchableGateway([PICKER_A, PICKER_B]); + + render( + + gateway} createAssetGateway={() => assetGateway}> + + + , + ); + + await screen.findByLabelText("본문 Markdown"); + await user.click(await screen.findByRole("button", { name: /inserted-diagram/ })); + const body = (screen.getByLabelText("본문 Markdown") as HTMLTextAreaElement).value; + assert.ok(body.includes('key="inserted-diagram"'), body); + + // Now search for something that excludes the asset just inserted. The Picker + // must narrow; the preview must not. + await user.type(screen.getByLabelText("Asset 검색"), "other"); + await user.click(screen.getByRole("button", { name: "검색" })); + await waitFor(() => + assert.equal(screen.queryByRole("button", { name: /inserted-diagram/ }), null), + ); + + await user.click(screen.getByRole("tab", { name: "즉시 미리보기" })); + const panel = screen.getByRole("tabpanel", { name: "즉시 미리보기" }); + + assert.equal( + within(panel).queryByRole("alert")?.textContent ?? null, + null, + "the preview refused a directive it resolved before the search", + ); + const [image] = within(panel).getAllByAltText("삽입한 다이어그램"); + assert.equal(image!.getAttribute("src"), "/media/inserted-diagram.svg"); +}); + +test("mergeAssetCatalog keeps every previously known asset and lets a fresher copy win", () => { + const known = assetFixture({ id: "50505050-5050-4050-8050-505050505051", assetKey: "known" }); + const staleCopy = assetFixture({ + id: "50505050-5050-4050-8050-505050505052", + assetKey: "replaced", + publicPath: "/media/replaced-old.svg", + updatedAt: "2026-08-14T00:00:00.000Z", + }); + const freshCopy = { ...staleCopy, publicPath: "/media/replaced-new.svg", version: 2 }; + const arrival = assetFixture({ id: "50505050-5050-4050-8050-505050505053", assetKey: "arrival" }); + + const merged = mergeAssetCatalog([known, staleCopy], [freshCopy, arrival]); + + assert.deepEqual( + [...merged].map((asset) => asset.assetKey).sort(), + ["arrival", "known", "replaced"], + ); + assert.equal( + merged.find((asset) => asset.id === staleCopy.id)?.publicPath, + "/media/replaced-new.svg", + ); +}); + +test("mergeAssetCatalog can only grow: an empty arrival keeps everything", () => { + const known = assetFixture({ id: "60606060-6060-4060-8060-606060606061", assetKey: "kept" }); + + assert.deepEqual(mergeAssetCatalog([known], []).map((asset) => asset.id), [known.id]); +});