From 238e54122029ffd7f1b70607b8ba4ce5fc552bca Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 16:04:58 +0000 Subject: [PATCH] fix: replace same-named files in place when uploading to the asset library Dropping or uploading a file into an asset library folder that already had a file with the same name created a second entry with a duplicate name instead of updating the existing one. The upload flow now looks up an existing file by parent folder and name, replaces its file in place and fans the change out to docs that use the asset, matching the "Replace file" action in the asset details modal. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016L52aaKKrhphsmLBEupcti --- .changeset/asset-upload-replace-same-name.md | 5 + .../components/AssetBrowser/AssetBrowser.tsx | 46 +++++-- .../root-cms/ui/utils/assets.find.test.ts | 122 ++++++++++++++++++ packages/root-cms/ui/utils/assets.ts | 27 ++++ 4 files changed, 192 insertions(+), 8 deletions(-) create mode 100644 .changeset/asset-upload-replace-same-name.md create mode 100644 packages/root-cms/ui/utils/assets.find.test.ts diff --git a/.changeset/asset-upload-replace-same-name.md b/.changeset/asset-upload-replace-same-name.md new file mode 100644 index 000000000..2152dcab2 --- /dev/null +++ b/.changeset/asset-upload-replace-same-name.md @@ -0,0 +1,5 @@ +--- +'@blinkk/root-cms': patch +--- + +fix: replace same-named files in place when uploading to the asset library diff --git a/packages/root-cms/ui/components/AssetBrowser/AssetBrowser.tsx b/packages/root-cms/ui/components/AssetBrowser/AssetBrowser.tsx index 28bd0f195..5136c196d 100644 --- a/packages/root-cms/ui/components/AssetBrowser/AssetBrowser.tsx +++ b/packages/root-cms/ui/components/AssetBrowser/AssetBrowser.tsx @@ -57,6 +57,7 @@ import { createAssetFolder, createAssetFolderPaths, deleteAsset, + findAssetFile, findDocsUsingAsset, getAsset, getFolderId, @@ -67,6 +68,7 @@ import { moveAsset, parseFolderPath, renameAsset, + replaceAssetFile, sortAssets, syncAssetToDocs, updateAssetAltDisabled, @@ -297,7 +299,9 @@ export function AssetBrowser(props: AssetBrowserProps) { * Uploads files into the current folder. Entries that came from a folder * upload carry the folder path they were nested under, which is mirrored in * the asset library (creating any missing folders) before the files are - * uploaded into it. + * uploaded into it. A file whose destination folder already has a file of + * the same name replaces that file in place (and the change fans out to + * docs that use it) rather than creating a duplicate entry. */ async function uploadFiles(entries: UploadFileEntry[]) { if (entries.length === 0 || uploading) { @@ -322,6 +326,10 @@ export function AssetBrowser(props: AssetBrowserProps) { }); const uploaded: AssetFile[] = []; const failed: string[] = []; + // Existing assets that were replaced by a same-named upload. + const replaced: AssetFile[] = []; + // Docs that failed to pick up a replaced asset's new file. + const failedDocIds = new Set(); // Resolve each file's destination folder up front so that files with a // path the asset library can't represent are skipped individually. @@ -375,12 +383,25 @@ export function AssetBrowser(props: AssetBrowserProps) { disallowClose: true, }); try { + const existing = await findAssetFile( + item.parent, + item.file.name.trim() + ); const uploadedFile = await uploadFileToGCS(item.file); - const asset = await createAssetFile({ - parent: item.parent, - file: uploadedFile, - }); - uploaded.push(asset); + if (existing) { + const previousFile = existing.file; + const asset = await replaceAssetFile(existing, uploadedFile); + uploaded.push(asset); + replaced.push(asset); + const res = await syncAssetToDocs(asset, {previousFile}); + res.failedDocIds.forEach((docId) => failedDocIds.add(docId)); + } else { + const asset = await createAssetFile({ + parent: item.parent, + file: uploadedFile, + }); + uploaded.push(asset); + } } catch (err) { console.error(`failed to upload ${item.label}:`, err); failed.push(item.label); @@ -396,9 +417,18 @@ export function AssetBrowser(props: AssetBrowserProps) { autoClose: false, }); } else { + const parts = [`Uploaded ${uploaded.length} file(s).`]; + if (replaced.length > 0) { + parts.push(`Replaced ${replaced.length} existing file(s).`); + } + showNotification({message: parts.join(' '), color: 'green'}); + } + if (failedDocIds.size > 0) { showNotification({ - message: `Uploaded ${uploaded.length} file(s).`, - color: 'green', + title: 'Some docs failed to update', + message: `Failed to update: ${Array.from(failedDocIds).join(', ')}. Re-save the asset to retry.`, + color: 'red', + autoClose: false, }); } await reload(folder); diff --git a/packages/root-cms/ui/utils/assets.find.test.ts b/packages/root-cms/ui/utils/assets.find.test.ts new file mode 100644 index 000000000..f5e1b9b11 --- /dev/null +++ b/packages/root-cms/ui/utils/assets.find.test.ts @@ -0,0 +1,122 @@ +import {beforeEach, describe, expect, it, vi} from 'vitest'; +import {Asset, findAssetFile} from './assets.js'; + +const mocks = vi.hoisted(() => ({ + collection: vi.fn(), + getDocs: vi.fn(), + query: vi.fn(), + where: vi.fn(), +})); + +vi.mock('firebase/firestore', () => ({ + Timestamp: {now: () => ({type: 'timestamp'})}, + collection: mocks.collection, + deleteDoc: vi.fn(), + deleteField: vi.fn(), + doc: vi.fn(), + getDoc: vi.fn(), + getDocs: mocks.getDocs, + limit: vi.fn(), + query: mocks.query, + serverTimestamp: vi.fn(), + setDoc: vi.fn(), + updateDoc: vi.fn(), + where: mocks.where, + writeBatch: vi.fn(), +})); + +vi.mock('./actions.js', () => ({logAction: vi.fn()})); +vi.mock('./doc-cache.js', () => ({removeDocsFromCache: vi.fn()})); + +/** The asset docs the mocked firestore queries read from. */ +let assetDocs: Asset[] = []; + +function testAsset(type: 'file' | 'folder', parent: string, name: string) { + return { + id: `${type}-${parent}-${name}`, + type: type, + parent: parent, + name: name, + } as Asset; +} + +/** Wires the firestore mocks up to the in-memory `assetDocs`. */ +function setupFirestoreMocks() { + vi.clearAllMocks(); + assetDocs = []; + window.__ROOT_CTX = {rootConfig: {projectId: 'test-project'}} as any; + window.firebase = { + db: {type: 'mock-db'}, + user: {email: 'editor@example.com'}, + } as any; + mocks.collection.mockImplementation( + (_db: unknown, ...path: string[]) => `col:${path.join('/')}` + ); + mocks.where.mockImplementation((field: string, op: string, value: any) => ({ + field: field, + op: op, + value: value, + })); + mocks.query.mockImplementation((_colRef: unknown, ...constraints: any[]) => ({ + constraints: constraints, + })); + mocks.getDocs.mockImplementation(async (q: any) => { + const matches = assetDocs.filter((asset: any) => + (q.constraints || []).every( + (c: any) => c.op === '==' && asset[c.field] === c.value + ) + ); + return { + forEach: (cb: (snap: any) => void) => + matches.forEach((asset) => cb({data: () => asset})), + }; + }); +} + +describe('findAssetFile', () => { + beforeEach(() => { + setupFirestoreMocks(); + }); + + it('finds a file by name within a folder', async () => { + assetDocs = [ + testAsset('file', 'marketing', 'hero.png'), + testAsset('file', 'marketing', 'logo.png'), + ]; + + const res = await findAssetFile('marketing', 'hero.png'); + + expect(res?.id).toEqual('file-marketing-hero.png'); + }); + + it('returns null when no file has that name', async () => { + assetDocs = [testAsset('file', 'marketing', 'hero.png')]; + + expect(await findAssetFile('marketing', 'banner.png')).toBeNull(); + expect(await findAssetFile('marketing', 'HERO.png')).toBeNull(); + }); + + it('ignores same-named files in other folders', async () => { + assetDocs = [ + testAsset('file', '', 'hero.png'), + testAsset('file', 'marketing/q1', 'hero.png'), + ]; + + expect(await findAssetFile('marketing', 'hero.png')).toBeNull(); + expect((await findAssetFile('', 'hero.png'))?.parent).toEqual(''); + }); + + it('ignores folders with the same name', async () => { + assetDocs = [testAsset('folder', 'marketing', 'hero.png')]; + + expect(await findAssetFile('marketing', 'hero.png')).toBeNull(); + }); + + it('normalizes a trailing slash on the parent path', async () => { + assetDocs = [testAsset('file', 'marketing', 'hero.png')]; + + const res = await findAssetFile('marketing/', 'hero.png'); + + expect(res?.id).toEqual('file-marketing-hero.png'); + }); +}); diff --git a/packages/root-cms/ui/utils/assets.ts b/packages/root-cms/ui/utils/assets.ts index 231e87bb7..a4afe509c 100644 --- a/packages/root-cms/ui/utils/assets.ts +++ b/packages/root-cms/ui/utils/assets.ts @@ -516,6 +516,33 @@ export async function getAsset(assetId: string): Promise { return snapshot.data() as Asset; } +/** + * Finds the file asset named `name` directly within a folder, or null if the + * folder has no file by that name. Used by uploads to replace an existing + * file in place rather than creating a duplicate entry with the same name. + */ +export async function findAssetFile( + parent: string, + name: string +): Promise { + const colRef = getAssetsDbCollection(); + const snapshot = await getDocs( + query( + colRef, + where('parent', '==', normalizeParentPath(parent)), + where('name', '==', name) + ) + ); + let res: AssetFile | null = null; + snapshot.forEach((snap) => { + const data = snap.data(); + if (!res && isValidAsset(data) && data.type === 'file') { + res = data; + } + }); + return res; +} + /** * Creates a folder within the asset library. No-op if the folder already * exists.