[pull] main from tinacms:main

.changeset/tinacms-pass-branch-on-media-calls.md

@@ -0,0 +1,18 @@
+---
+"tinacms": minor
+"@tinacms/graphql": minor
+---
+
+Forward the editor's current branch to the TinaCloud assets-api on every cloud media call, and fix staging URL handling for multi-segment branches
+
+`TinaMediaStore` now appends `?branch=<encodedBranch>` to its `upload_url`, `list`, and `delete` requests so that — once the assets-api opts an app into branch-aware media — uploads, listings, and deletions are scoped to the branch the editor is on, instead of always hitting the production branch. The branch is read from `Client.branch` (already URL-encoded) and decoded then re-encoded at the use site to avoid double-encoding.
+
+The query parameter is ignored by assets-api versions that do not parse it, so this change is safe to deploy ahead of the server-side rollout. Local mode is unaffected.
+
+`@tinacms/graphql`'s media URL resolver now formats staging URLs as `/__staging/<branch>/__file/<path>` instead of `/__staging/<encoded-branch>/<path>`. The previous form broke for branches containing `/` (e.g. `feat/my-branch`) because CloudFront decodes paths before downstream components see them, so the S3 write key (with a literal `%2F`) wouldn't match the decoded read path. The `__file` delimiter lets the branch contribute its natural `/` segments while still marking where the file path begins.
+
+Note: staging URLs produced by `@tinacms/graphql@2.3.0`–`2.3.1` use the old format and will not round-trip through this version's `resolveMediaCloudToRelative`. Branch-aware media is gated server-side and has not been enabled for any tenant yet, so no persisted data is expected to be affected — but if you turned it on for testing, regenerate the affected field values from the editor after upgrading.
+
+After a successful cloud upload `TinaMediaStore.persist()` now resolves its return value from the assets-api `list` endpoint instead of constructing each `Media.src` locally — the server is the source of truth for the canonical URL (including the staging-branch path and per-stage CDN host). The `MediaStore.persist()` contract is preserved, so the returned items still flow through the media manager and the image-field drop handler.
+
+Also reserves an optional `rename?(from, to)` hook on the `MediaStore` interface as a future extension point — no implementation yet.

packages/@tinacms/graphql/src/resolver/media-utils.test.ts

@@ -138,7 +138,7 @@ describe('resolveMedia', () => {
       schema
     );
     expect(resolvedURL).toEqual(
-      `https://${assetsHost}/${clientId}/__staging/feat%2Fx/llama.png`
+      `https://${assetsHost}/${clientId}/__staging/feat/x/__file/llama.png`
     );
   });
 
@@ -179,8 +179,8 @@ describe('resolveMedia', () => {
       schema
     );
     expect(resolved).toEqual([
-      `https://${assetsHost}/${clientId}/__staging/feat%2Fx/a.png`,
-      `https://${assetsHost}/${clientId}/__staging/feat%2Fx/b.png`,
+      `https://${assetsHost}/${clientId}/__staging/feat/x/__file/a.png`,
+      `https://${assetsHost}/${clientId}/__staging/feat/x/__file/b.png`,
     ]);
   });
 
@@ -196,7 +196,7 @@ describe('resolveMedia', () => {
       mediaBranch: 'main',
     };
 
-    const stagingURL = `https://${assetsHost}/${clientId}/__staging/feat%2Fx/llama.png`;
+    const stagingURL = `https://${assetsHost}/${clientId}/__staging/feat/x/__file/llama.png`;
     const resolvedURL = resolveMediaCloudToRelative(stagingURL, config, schema);
     expect(resolvedURL).toEqual(relativeURL);
   });

packages/@tinacms/graphql/src/resolver/media-utils.ts

@@ -91,16 +91,25 @@ export const resolveMediaRelativeToCloud = (
   }
 };
 
+// Branches may contain `/` (e.g. `feat/my-branch`). Storing them URL-encoded
+// would break the CDN read path because CloudFront decodes paths before
+// downstream components see them — the write would have `%2F` literal in the
+// S3 key but every read would look for the decoded `/` form. Instead we let
+// the branch contribute its natural `/` segments and use a `__file` prefix to
+// delimit where the (possibly multi-segment) branch ends and the file path
+// begins.
 const stagingPrefix = (config: {
   branch?: string;
   mediaBranch?: string;
 }): string =>
   config.branch && config.branch !== config.mediaBranch
-    ? `/__staging/${encodeURIComponent(config.branch)}`
+    ? `/__staging/${config.branch}/__file`
     : '';
 
-// Matches `/__staging/<encoded-branch>/…` and captures everything after the branch segment.
-const STAGING_SEGMENT = /^\/__staging\/[^/]+(\/.*)$/;
+// Matches `/__staging/<branch (possibly multi-segment)>/__file/<rest>` and
+// captures everything after the `__file` segment. Non-greedy so the branch
+// can span multiple `/` segments.
+const STAGING_SEGMENT = /^\/__staging\/.+?\/__file(\/.*)$/;
 
 const stripStagingPrefix = (path: string): string => {
   const match = path.match(STAGING_SEGMENT);

packages/tinacms/src/toolkit/components/media/media-manager.tsx

@@ -13,11 +13,6 @@ import React, { useEffect, useState, forwardRef, useRef } from 'react';
 import { createContext, useContext } from 'react';
 import * as dropzone from 'react-dropzone';
 import type { FileError } from 'react-dropzone';
-import { captureEvent } from '../../../lib/posthog/posthogProvider';
-import {
-  MediaManagerContentUploadedEvent,
-  MediaManagerContentDeletedEvent,
-} from '../../../lib/posthog/posthog';
 import {
   BiArrowToBottom,
   BiCloudUpload,
@@ -30,9 +25,14 @@ import {
 } from 'react-icons/bi';
 import { BiFile } from 'react-icons/bi';
 import { IoMdRefresh } from 'react-icons/io';
+import {
+  MediaManagerContentDeletedEvent,
+  MediaManagerContentUploadedEvent,
+} from '../../../lib/posthog/posthog';
+import { captureEvent } from '../../../lib/posthog/posthogProvider';
 import { Breadcrumb } from './breadcrumb';
 import { CopyField } from './copy-field';
-import { checkerboardStyle, GridMediaItem, ListMediaItem } from './media-item';
+import { GridMediaItem, ListMediaItem, checkerboardStyle } from './media-item';
 import { DeleteModal, NewFolderModal } from './modal';
 import {
   DEFAULT_MEDIA_UPLOAD_TYPES,
@@ -312,6 +312,7 @@ export function MediaPicker({
             );
           });
         }
+
         // if there are media items, set the first one as active and prepend all the items to the list
         if (mediaItems.length !== 0) {
           const extensions = [

packages/tinacms/src/toolkit/core/media-store.default.test.ts

@@ -0,0 +1,234 @@
+import type { Media, MediaUploadOptions } from './media';
+import { TinaMediaStore } from './media-store.default';
+
+type FetchWithTokenMock = ReturnType<typeof vi.fn>;
+
+const makeJsonResponse = (status: number, body: unknown) =>
+  ({
+    ok: status >= 200 && status < 300,
+    status,
+    json: vi.fn().mockResolvedValue(body),
+  }) as any;
+
+const buildStore = ({
+  branch = 'main',
+  isLocalMode = false,
+  authenticated = true,
+}: {
+  branch?: string | undefined;
+  isLocalMode?: boolean;
+  authenticated?: boolean;
+} = {}) => {
+  const fetchWithToken: FetchWithTokenMock = vi.fn();
+  const authProvider = {
+    fetchWithToken,
+    isAuthenticated: vi.fn().mockResolvedValue(authenticated),
+  };
+  const api = {
+    branch,
+    clientId: 'test-client',
+    contentApiUrl:
+      'https://content.tinajs.io/1.1/content/test-client/github/main',
+    assetsApiUrl: 'https://assets.tinajs.io',
+    isLocalMode,
+    authProvider,
+    options: {},
+    getRequestStatus: vi.fn().mockResolvedValue({ error: false }),
+    schema: { schema: { config: { media: { tina: {} } } } },
+  };
+  const cms = { api: { tina: api } } as any;
+  const store = new TinaMediaStore(cms);
+  return { store, fetchWithToken, api };
+};
+
+describe('TinaMediaStore — branch query param', () => {
+  describe('list()', () => {
+    it('appends single-encoded branch for a simple branch', async () => {
+      const { store, fetchWithToken } = buildStore({ branch: 'main' });
+      fetchWithToken.mockResolvedValueOnce(
+        makeJsonResponse(200, { files: [], directories: [], cursor: 0 })
+      );
+
+      await store.list({ directory: '', thumbnailSizes: [] } as any);
+
+      const calledUrl = fetchWithToken.mock.calls[0][0];
+      expect(calledUrl).toContain('&branch=main');
+      expect(calledUrl).not.toContain('branch=undefined');
+    });
+
+    it('single-encodes a branch containing `/` (already encoded on the Client)', async () => {
+      // `Client.setBranch('feat/x')` stores `'feat%2Fx'`; list() should
+      // forward exactly `branch=feat%2Fx`, never the double-encoded form.
+      const { store, fetchWithToken } = buildStore({ branch: 'feat%2Fx' });
+      fetchWithToken.mockResolvedValueOnce(
+        makeJsonResponse(200, { files: [], directories: [], cursor: 0 })
+      );
+
+      await store.list({ directory: '', thumbnailSizes: [] } as any);
+
+      const calledUrl = fetchWithToken.mock.calls[0][0];
+      expect(calledUrl).toContain('&branch=feat%2Fx');
+      expect(calledUrl).not.toContain('feat%252Fx');
+    });
+
+    it('omits the branch param when branch is the literal string "undefined"', async () => {
+      // `Client.setBranch(undefined)` runs `encodeURIComponent(undefined)`,
+      // which returns the literal 9-char string "undefined". We must not
+      // forward that to the assets-api as a real branch.
+      const { store, fetchWithToken } = buildStore({ branch: 'undefined' });
+      fetchWithToken.mockResolvedValueOnce(
+        makeJsonResponse(200, { files: [], directories: [], cursor: 0 })
+      );
+
+      await store.list({ directory: '', thumbnailSizes: [] } as any);
+
+      const calledUrl = fetchWithToken.mock.calls[0][0];
+      expect(calledUrl).not.toContain('branch=');
+    });
+
+    it('omits the branch param when branch is empty', async () => {
+      const { store, fetchWithToken } = buildStore({ branch: '' });
+      fetchWithToken.mockResolvedValueOnce(
+        makeJsonResponse(200, { files: [], directories: [], cursor: 0 })
+      );
+
+      await store.list({ directory: '', thumbnailSizes: [] } as any);
+
+      const calledUrl = fetchWithToken.mock.calls[0][0];
+      expect(calledUrl).not.toContain('branch=');
+    });
+  });
+
+  describe('delete()', () => {
+    beforeEach(() => {
+      vi.useFakeTimers();
+    });
+
+    afterEach(() => {
+      vi.useRealTimers();
+    });
+
+    it('appends branch as the first query param', async () => {
+      const { store, fetchWithToken } = buildStore({ branch: 'feat%2Fx' });
+      fetchWithToken.mockResolvedValueOnce(
+        makeJsonResponse(200, { requestId: 'req-1' })
+      );
+
+      const deletePromise = store.delete({
+        directory: 'images',
+        filename: 'a.png',
+      } as Media);
+      await vi.advanceTimersByTimeAsync(1100);
+      await deletePromise;
+
+      const calledUrl = fetchWithToken.mock.calls[0][0];
+      expect(calledUrl).toContain('/images/a.png?branch=feat%2Fx');
+    });
+
+    it('omits the branch param when branch is unset', async () => {
+      const { store, fetchWithToken } = buildStore({ branch: '' });
+      fetchWithToken.mockResolvedValueOnce(
+        makeJsonResponse(200, { requestId: 'req-1' })
+      );
+
+      const deletePromise = store.delete({
+        directory: 'images',
+        filename: 'a.png',
+      } as Media);
+      await vi.advanceTimersByTimeAsync(1100);
+      await deletePromise;
+
+      const calledUrl = fetchWithToken.mock.calls[0][0];
+      expect(calledUrl).toContain('/images/a.png');
+      expect(calledUrl).not.toContain('branch=');
+    });
+  });
+
+  describe('persist()', () => {
+    beforeEach(() => {
+      vi.useFakeTimers();
+    });
+
+    afterEach(() => {
+      vi.useRealTimers();
+      vi.unstubAllGlobals();
+    });
+
+    const stubFetchGlobal = (status: number, body: unknown) => {
+      const fetchMock = vi.fn().mockResolvedValue({
+        ok: status >= 200 && status < 300,
+        status,
+        text: vi.fn().mockResolvedValue(''),
+        json: vi.fn().mockResolvedValue(body),
+      });
+      vi.stubGlobal('fetch', fetchMock);
+      return fetchMock;
+    };
+
+    it('forwards the encoded branch on the upload_url request and resolves canonical entries via list()', async () => {
+      const { store, fetchWithToken } = buildStore({ branch: 'feat%2Fx' });
+      // 1) upload_url response
+      fetchWithToken.mockResolvedValueOnce(
+        makeJsonResponse(200, {
+          signedUrl: 'https://s3.example/signed',
+          requestId: 'req-1',
+        })
+      );
+      // 2) the post-upload list() call inside fetchUploadedEntries
+      fetchWithToken.mockResolvedValueOnce(
+        makeJsonResponse(200, {
+          files: [
+            {
+              filename: 'llama.png',
+              src: 'https://assets.tina.io/test-client/__staging/feat/x/__file/uploads/llama.png',
+            },
+          ],
+          directories: [],
+          cursor: 0,
+        })
+      );
+      stubFetchGlobal(200, {});
+
+      const uploads: MediaUploadOptions[] = [
+        {
+          directory: 'uploads',
+          file: new File(['x'], 'llama.png', { type: 'image/png' }),
+        },
+      ];
+
+      const persistPromise = store.persist(uploads);
+      // Advance past the 1s polling sleep inside the upload loop.
+      await vi.advanceTimersByTimeAsync(1100);
+      const result = await persistPromise;
+
+      // First call is upload_url with branch query.
+      const uploadUrl = fetchWithToken.mock.calls[0][0];
+      expect(uploadUrl).toContain(
+        '/upload_url/uploads/llama.png?branch=feat%2Fx'
+      );
+
+      // Result contains the canonical entry from the list endpoint, not a
+      // locally-constructed `https://assets.tina.io/<clientId>/<path>` URL.
+      expect(result).toHaveLength(1);
+      expect(result[0].filename).toBe('llama.png');
+      expect(result[0].src).toBe(
+        'https://assets.tina.io/test-client/__staging/feat/x/__file/uploads/llama.png'
+      );
+    });
+
+    it('returns [] when not authenticated, without making upload calls', async () => {
+      const { store, fetchWithToken } = buildStore({ authenticated: false });
+
+      const uploads: MediaUploadOptions[] = [
+        {
+          directory: 'uploads',
+          file: new File(['x'], 'llama.png', { type: 'image/png' }),
+        },
+      ];
+
+      const result = await store.persist(uploads);
+      expect(result).toEqual([]);
+      expect(fetchWithToken).not.toHaveBeenCalled();
+    });
+  });
+});