fix: replace same-named files in place when uploading to the asset library

.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

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<string>();
 
     // 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);

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');
+  });
+});

packages/root-cms/ui/utils/assets.ts

@@ -516,6 +516,33 @@ export async function getAsset(assetId: string): Promise<Asset | null> {
   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<AssetFile | null> {
+  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.