diff --git a/.changeset/fix-nested-rich-text-getfieldgroup.md b/.changeset/fix-nested-rich-text-getfieldgroup.md new file mode 100644 index 0000000000..53363e2779 --- /dev/null +++ b/.changeset/fix-nested-rich-text-getfieldgroup.md @@ -0,0 +1,5 @@ +--- +"tinacms": patch +--- + +Fix crash in `getFieldGroup` when editing deeply nested rich-text fields (3+ levels) with templates. The method used `findIndex` which always searched from the start of the path array, causing it to resolve the wrong "children"/"props" segments on recursive calls. Replaced with `indexOf` searching from the current position, and added a null guard for graceful fallback on malformed content. diff --git a/.changeset/strip-cloud-url-cross-host.md b/.changeset/strip-cloud-url-cross-host.md new file mode 100644 index 0000000000..988a8f9de1 --- /dev/null +++ b/.changeset/strip-cloud-url-cross-host.md @@ -0,0 +1,11 @@ +--- +"@tinacms/graphql": patch +--- + +Fix `resolveMediaCloudToRelative` so it strips any TinaCloud cloud URL on save, not only ones whose host matches `config.assetsHost`. The match condition is now host-agnostic: the `/…` path prefix is the durable invariant; the host segment can vary across stages. + +This unblocks multi-host setups (PR / stage / personal-dev TinaCloud stages) where the dashboard's default `MediaStore` inserts upload URLs with one host while content-api returns a different one as `assetsHost`. Previously the round-trip silently failed and absolute URLs got committed to the content repo. After this fix, content saves as a relative path regardless of which host the dashboard inserted, matching pre-existing content's format. + +Also covers cross-stage content migration: an absolute URL written against one stage strips correctly when re-saved against another. + +Closes [#6827](https://github.com/tinacms/tinacms/issues/6827). diff --git a/packages/@tinacms/graphql/src/resolver/media-utils.test.ts b/packages/@tinacms/graphql/src/resolver/media-utils.test.ts index 074a918231..b3e3c43e05 100644 --- a/packages/@tinacms/graphql/src/resolver/media-utils.test.ts +++ b/packages/@tinacms/graphql/src/resolver/media-utils.test.ts @@ -217,6 +217,100 @@ describe('resolveMedia', () => { expect(resolvedURL).toEqual(relativeURL); }); + /** + * A cloud URL whose host doesn't match `config.assetsHost` (e.g. content + * uploaded against a different stage, or against the dashboard's hardcoded + * default) should still strip cleanly. The `/…` path prefix is the + * invariant — the host segment can vary across stages. + */ + it('strips cloud URL whose host differs from config.assetsHost', () => { + const config: GraphQLConfig = { + useRelativeMedia: false, + assetsHost, + clientId, + }; + + const otherStageURL = `https://assets.tina.io/${clientId}/llama.png`; + const resolvedURL = resolveMediaCloudToRelative( + otherStageURL, + config, + schema + ); + expect(resolvedURL).toEqual(relativeURL); + }); + + /** + * Array values containing cloud URLs from a mix of stages should each be + * stripped to a relative path regardless of which host they were uploaded + * against. + */ + it('strips array values with mixed cloud-URL hosts', () => { + const config: GraphQLConfig = { + useRelativeMedia: false, + assetsHost, + clientId, + }; + + const resolved = resolveMediaCloudToRelative( + [ + `https://${assetsHost}/${clientId}/a.png`, + `https://assets.tina.io/${clientId}/b.png`, + `https://assets-other-stage.tinajs.dev/${clientId}/c.png`, + ], + config, + schema + ); + expect(resolved).toEqual([ + '/uploads/a.png', + '/uploads/b.png', + '/uploads/c.png', + ]); + }); + + /** + * Cross-host stripping should still respect the `__staging/{branch}` prefix, + * so editorial-branch content migrated between stages round-trips correctly. + */ + it('strips staging prefix from cross-host cloud URLs', () => { + const config: GraphQLConfig = { + useRelativeMedia: false, + assetsHost, + clientId, + branch: 'feat/x', + mediaBranch: 'main', + }; + + const otherStageStagingURL = `https://assets.tina.io/${clientId}/__staging/feat/x/__file/llama.png`; + const resolvedURL = resolveMediaCloudToRelative( + otherStageStagingURL, + config, + schema + ); + expect(resolvedURL).toEqual(relativeURL); + }); + + /** + * URLs whose path doesn't begin with the configured client id (e.g. a + * non-TinaCloud asset URL or content from a different project) must be left + * alone. + */ + it('does not strip URLs that do not match the configured clientId', () => { + const config: GraphQLConfig = { + useRelativeMedia: false, + assetsHost, + clientId, + }; + + const otherClientURL = + 'https://assets.tina.io/00000000-0000-0000-0000-000000000000/llama.png'; + const resolvedURL = resolveMediaCloudToRelative( + otherClientURL, + config, + schema + ); + expect(resolvedURL).toEqual(otherClientURL); + }); + /** * Missing `media: { tina: { ... }}` config should return the value, regardless of `useRelativeMedia` */ diff --git a/packages/@tinacms/graphql/src/resolver/media-utils.ts b/packages/@tinacms/graphql/src/resolver/media-utils.ts index 641075ea27..7284d23ec4 100644 --- a/packages/@tinacms/graphql/src/resolver/media-utils.ts +++ b/packages/@tinacms/graphql/src/resolver/media-utils.ts @@ -26,18 +26,19 @@ export const resolveMediaCloudToRelative = ( } if (hasTinaMediaConfig(schema) === true) { - const assetsURL = `https://${config.assetsHost}/${config.clientId}`; const cleanMediaRoot = cleanUpSlashes(schema.config.media.tina.mediaRoot); + const cloudUrl = cloudUrlPattern(config.clientId); - if (typeof value === 'string' && value.includes(assetsURL)) { + if (typeof value === 'string' && cloudUrl.test(value)) { return `${cleanMediaRoot}${stripStagingPrefix( - value.replace(assetsURL, '') + value.replace(cloudUrl, '') )}`; } if (Array.isArray(value)) { return value.map((v) => { if (!v || typeof v !== 'string') return v; - const strippedURL = v.replace(assetsURL, ''); + if (!cloudUrl.test(v)) return v; + const strippedURL = v.replace(cloudUrl, ''); return `${cleanMediaRoot}${stripStagingPrefix(strippedURL)}`; }); } @@ -116,6 +117,15 @@ const stripStagingPrefix = (path: string): string => { return match ? match[1] : path; }; +const escapeRegExp = (s: string): string => + s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + +// Matches a TinaCloud cloud URL for the given client. The host segment varies +// across stages (e.g. `assets.tina.io`, `assets-{stage}.tinajs.dev`); the +// `/…` path prefix is the durable invariant. +const cloudUrlPattern = (clientId: string): RegExp => + new RegExp(`^https://[^/]+/${escapeRegExp(clientId)}`); + const cleanUpSlashes = (path: string): string => { if (path) { return `/${path.replace(/^\/+|\/+$/gm, '')}`; diff --git a/packages/tinacms/src/toolkit/forms/form.test.ts b/packages/tinacms/src/toolkit/forms/form.test.ts index fc48ba286b..b9a5ee5671 100644 --- a/packages/tinacms/src/toolkit/forms/form.test.ts +++ b/packages/tinacms/src/toolkit/forms/form.test.ts @@ -398,6 +398,275 @@ describe('Form', () => { }); }); }); + + describe('with nested rich-text fields (2 levels deep)', () => { + // The outer Grid is at children[2] and the inner Panel is at + // children[0]. Using different indices is critical: the buggy + // findIndex always resolves the *outer* "children.2" slice when + // processing the inner rich-text, so getIn looks for children[2] + // in the inner content (which only has one element) and returns + // undefined, crashing with "Cannot read properties of undefined + // (reading 'props')". + function makeTwoLevelForm(defaults: FormOptions) { + const panelTemplate = { + inline: false, + name: 'Panel', + fields: [{ type: 'string', name: 'title', required: true }], + }; + return new Form({ + ...defaults, + fields: [ + { + type: 'rich-text', + name: 'body', + isBody: true, + templates: [ + { + inline: false, + name: 'Grid', + fields: [ + { + type: 'rich-text', + name: 'gridBody', + templates: [panelTemplate], + }, + ], + }, + ], + }, + ], + initialValues: { + body: { + type: 'root', + children: [ + { type: 'p', children: [{ type: 'text', text: 'a' }] }, + { type: 'p', children: [{ type: 'text', text: 'b' }] }, + { + type: 'mdxJsxFlowElement', + name: 'Grid', + children: [{ type: 'text', text: '' }], + props: { + gridBody: { + type: 'root', + children: [ + { + type: 'mdxJsxFlowElement', + name: 'Panel', + children: [{ type: 'text', text: '' }], + props: { title: 'Hello' }, + }, + ], + }, + }, + }, + ], + }, + }, + }); + } + + it('does not crash', () => { + const form = makeTwoLevelForm(DEFAULTS); + expect(() => + form.getActiveField( + 'body.children.2.props.gridBody.children.0.props' + ) + ).not.toThrow(); + }); + + it('returns the correct template name', () => { + const form = makeTwoLevelForm(DEFAULTS); + const activeField = form.getActiveField( + 'body.children.2.props.gridBody.children.0.props' + ); + expect(activeField.name).toEqual( + 'body.children.2.props.gridBody.children.0.props' + ); + }); + + it('returns namespaced field names on the inner template', () => { + const form = makeTwoLevelForm(DEFAULTS); + const activeField = form.getActiveField( + 'body.children.2.props.gridBody.children.0.props' + ); + expect(activeField.fields).toEqual([ + { + type: 'string', + name: 'body.children.2.props.gridBody.children.0.props.title', + required: true, + }, + ]); + }); + }); + + describe('with deeply nested rich-text fields (3 levels deep)', () => { + function makeThreeLevelForm(defaults: FormOptions) { + const alertTemplate = { + inline: false, + name: 'Alert', + fields: [{ type: 'string', name: 'message' }], + }; + const panelTemplate = { + inline: false, + name: 'Panel', + fields: [ + { + type: 'rich-text', + name: 'panelBody', + templates: [alertTemplate], + }, + ], + }; + return new Form({ + ...defaults, + fields: [ + { + type: 'rich-text', + name: 'body', + isBody: true, + templates: [ + { + inline: false, + name: 'Grid', + fields: [ + { + type: 'rich-text', + name: 'gridBody', + templates: [panelTemplate], + }, + ], + }, + ], + }, + ], + initialValues: { + body: { + type: 'root', + children: [ + { type: 'p', children: [{ type: 'text', text: 'a' }] }, + { + type: 'mdxJsxFlowElement', + name: 'Grid', + children: [{ type: 'text', text: '' }], + props: { + gridBody: { + type: 'root', + children: [ + { + type: 'p', + children: [{ type: 'text', text: 'b' }], + }, + { + type: 'p', + children: [{ type: 'text', text: 'c' }], + }, + { + type: 'mdxJsxFlowElement', + name: 'Panel', + children: [{ type: 'text', text: '' }], + props: { + panelBody: { + type: 'root', + children: [ + { + type: 'mdxJsxFlowElement', + name: 'Alert', + children: [{ type: 'text', text: '' }], + props: { message: 'Warning!' }, + }, + ], + }, + }, + }, + ], + }, + }, + }, + ], + }, + }, + }); + } + + // Grid at children[1], Panel at children[2], Alert at children[0] + // — all different indices to ensure correct resolution at each level. + const fieldName = + 'body.children.1.props.gridBody.children.2.props.panelBody.children.0.props'; + + it('does not crash', () => { + const form = makeThreeLevelForm(DEFAULTS); + expect(() => form.getActiveField(fieldName)).not.toThrow(); + }); + + it('returns the correct template name', () => { + const form = makeThreeLevelForm(DEFAULTS); + const activeField = form.getActiveField(fieldName); + expect(activeField.name).toEqual(fieldName); + }); + + it('returns namespaced field names on the innermost template', () => { + const form = makeThreeLevelForm(DEFAULTS); + const activeField = form.getActiveField(fieldName); + expect(activeField.fields).toEqual([ + { type: 'string', name: `${fieldName}.message` }, + ]); + }); + }); + + describe('with malformed nested rich-text content', () => { + it('does not crash when inner content is empty', () => { + const form = new Form({ + ...DEFAULTS, + fields: [ + { + type: 'rich-text', + name: 'body', + isBody: true, + templates: [ + { + inline: false, + name: 'Grid', + fields: [ + { + type: 'rich-text', + name: 'gridBody', + templates: [ + { + inline: false, + name: 'Panel', + fields: [{ type: 'string', name: 'title' }], + }, + ], + }, + ], + }, + ], + }, + ], + initialValues: { + body: { + type: 'root', + children: [ + { + type: 'mdxJsxFlowElement', + name: 'Grid', + children: [{ type: 'text', text: '' }], + props: { + gridBody: { type: 'root', children: [] }, + }, + }, + ], + }, + }, + }); + + expect(() => + form.getActiveField( + 'body.children.0.props.gridBody.children.0.props' + ) + ).not.toThrow(); + }); + }); }); describe('when the rich-text child is an img node', () => { diff --git a/packages/tinacms/src/toolkit/forms/form.ts b/packages/tinacms/src/toolkit/forms/form.ts index 145f592bc1..fc3136cc33 100644 --- a/packages/tinacms/src/toolkit/forms/form.ts +++ b/packages/tinacms/src/toolkit/forms/form.ts @@ -486,19 +486,14 @@ export class Form implements Plugin { }), }; } else { - const childrenIndex = namePath.findIndex( - (value) => value === 'children' - ); - // Find the props for the next item, ignoring parent 'props' - const propsIndex = - namePath - .slice(childrenIndex) - .findIndex((value) => value === 'props') + childrenIndex; + const childrenIndex = namePath.indexOf('children', namePathIndex + 1); + const propsIndex = namePath.indexOf('props', childrenIndex + 1); const itemName = namePath.slice(childrenIndex, propsIndex).join('.'); const item = getIn(value, itemName); + if (!item) return formOrObjectField; const props = item.props; const templateString = item.name; - const currentPathIndex = namePathIndex + Math.max(propsIndex, 3); + const currentPathIndex = propsIndex; const isLastItem = currentPathIndex + 1 === namePath.length; const template = field.templates.find( (t) => t.name === templateString @@ -539,8 +534,7 @@ export class Form implements Plugin { formOrObjectField: template, values: props, namePath, - namePathIndex: - namePathIndex + Math.max(4, childrenIndex + propsIndex), + namePathIndex: propsIndex + 1, }); } if (!template) {