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.