From ee005e8b473f0d8b13c27d1e32519e970a8c49c8 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Wed, 12 Aug 2026 10:03:57 +0100 Subject: [PATCH 01/16] Removed unused lexicalIndicators labs flag no ref - the flag existed to show L/M badges on the posts list for spotting lexical vs mobiledoc posts during the editor migration; nothing reads it any more now that migration testing is long finished - also removed the orphaned gh-lexical-indicator CSS rules --- .../components/posts-list/list-item-analytics.hbs | 10 ---------- .../app/components/posts-list/list-item.hbs | 10 ---------- apps/ember-admin/app/services/feature.js | 1 - apps/ember-admin/app/styles/layouts/content.css | 6 ------ apps/ember-admin/app/styles/layouts/editor.css | 12 ------------ ghost/core/core/shared/labs.js | 1 - 6 files changed, 40 deletions(-) diff --git a/apps/ember-admin/app/components/posts-list/list-item-analytics.hbs b/apps/ember-admin/app/components/posts-list/list-item-analytics.hbs index e5245dc0d89..7d705da0485 100644 --- a/apps/ember-admin/app/components/posts-list/list-item-analytics.hbs +++ b/apps/ember-admin/app/components/posts-list/list-item-analytics.hbs @@ -72,16 +72,6 @@ {{svg-jar "star-fill" class="gh-featured-post"}} {{/if}} {{@post.title}} - - {{! Display lexical/mobiledoc indicators for easier testing of the feature --}} - {{#if (feature 'lexicalIndicators')}} - {{#if @post.lexical}} - L - {{else if @post.mobiledoc}} - M - {{/if}} - {{/if}} - {{#unless @hideAuthor }}

diff --git a/apps/ember-admin/app/components/posts-list/list-item.hbs b/apps/ember-admin/app/components/posts-list/list-item.hbs index 4dcd6f58b92..1e69154a316 100644 --- a/apps/ember-admin/app/components/posts-list/list-item.hbs +++ b/apps/ember-admin/app/components/posts-list/list-item.hbs @@ -53,16 +53,6 @@ {{svg-jar "star-fill" class="gh-featured-post"}} {{/if}} {{@post.title}} - - {{! Display lexical/mobiledoc indicators for easier testing of the feature --}} - {{#if (feature 'lexicalIndicators')}} - {{#if @post.lexical}} - L - {{else if @post.mobiledoc}} - M - {{/if}} - {{/if}} - {{#unless @hideAuthor }}

diff --git a/apps/ember-admin/app/services/feature.js b/apps/ember-admin/app/services/feature.js index b9c54cafa05..f7fdad436e8 100644 --- a/apps/ember-admin/app/services/feature.js +++ b/apps/ember-admin/app/services/feature.js @@ -80,7 +80,6 @@ export default class FeatureService extends Service { @feature('emailCustomization') emailCustomization; @feature('importMemberTier') importMemberTier; @feature('adminUIRefresh') adminUIRefresh; - @feature('lexicalIndicators') lexicalIndicators; @feature('editorExcerpt') editorExcerpt; @feature('memberDetailsReact') memberDetailsReact; @feature('tagDetailsReact') tagDetailsReact; diff --git a/apps/ember-admin/app/styles/layouts/content.css b/apps/ember-admin/app/styles/layouts/content.css index 100aac14933..60e34c51f19 100644 --- a/apps/ember-admin/app/styles/layouts/content.css +++ b/apps/ember-admin/app/styles/layouts/content.css @@ -357,12 +357,6 @@ overflow: hidden; } -.gh-post-list-title .gh-lexical-indicator { - margin: 2px 8px 0; - padding: 0 8px; - font-size: 1.2rem; -} - .gh-post-list-title .gh-featured-post { flex-shrink: 0; width: 11px; diff --git a/apps/ember-admin/app/styles/layouts/editor.css b/apps/ember-admin/app/styles/layouts/editor.css index bd08ca3e262..8decb8b226f 100644 --- a/apps/ember-admin/app/styles/layouts/editor.css +++ b/apps/ember-admin/app/styles/layouts/editor.css @@ -1093,18 +1093,6 @@ body[data-user-is-dragging] .gh-editor-feature-image-dropzone { } } -.gh-lexical-indicator { - margin: 1px 0 0 8px; - padding: 1px 8px; - background: var(--whitegrey-d1); - color: var(--black); - font-family: var(--font-family-mono); - font-size: 1.25rem; - border-radius: var(--border-radius); -} - - - .gh-editor-preview-trigger { height: 34px; background: var(--white); diff --git a/ghost/core/core/shared/labs.js b/ghost/core/core/shared/labs.js index 006662ff006..a83a741bd79 100644 --- a/ghost/core/core/shared/labs.js +++ b/ghost/core/core/shared/labs.js @@ -46,7 +46,6 @@ const PRIVATE_FEATURES = [ 'stripeAutomaticTax', 'importMemberTier', 'csvContentImporter', - 'lexicalIndicators', 'adminUIRefresh', 'emailCustomization', 'tagsX', From 6e17cad66b8cf7892bcd5abcd9bad70258fdd644 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Wed, 12 Aug 2026 10:15:20 +0100 Subject: [PATCH 02/16] Removed unused emailCustomization labs flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit no ref - nothing reads emailCustomization (or the older emailCustomizationAlpha) at runtime any more — there are no labs/feature conditionals left, and kg-default-nodes' exportDOM never consults options.feature when selecting renderers - removed the flag registration from labs.js, the admin labs toggle, and the ember feature service, and dropped both flags from ExportDOMFeatureOptions - de-flagged the kg-default-nodes tests that passed the flags as inert options, and removed the flag-specific tests whose coverage was already provided by unflagged tests --- .changeset/clean-flags-retire.md | 5 ++ .../advanced/labs/private-features.tsx | 4 -- apps/ember-admin/app/services/feature.js | 1 - ghost/core/core/shared/labs.js | 1 - koenig/kg-default-nodes/src/export-dom.ts | 2 - .../test/generate-decorator-node.test.ts | 35 ++++------- .../renderers/call-to-action-renderer.test.ts | 59 ++++++++---------- .../test/renderers/product-renderer.test.ts | 61 ------------------- 8 files changed, 43 insertions(+), 125 deletions(-) create mode 100644 .changeset/clean-flags-retire.md diff --git a/.changeset/clean-flags-retire.md b/.changeset/clean-flags-retire.md new file mode 100644 index 00000000000..8af68dbcdd8 --- /dev/null +++ b/.changeset/clean-flags-retire.md @@ -0,0 +1,5 @@ +--- +"@tryghost/kg-default-nodes": patch +--- + +Removed the unused emailCustomization and emailCustomizationAlpha feature options. diff --git a/apps/admin/src/settings/app/components/settings/advanced/labs/private-features.tsx b/apps/admin/src/settings/app/components/settings/advanced/labs/private-features.tsx index ae521c2fb8a..9732c3b96a4 100644 --- a/apps/admin/src/settings/app/components/settings/advanced/labs/private-features.tsx +++ b/apps/admin/src/settings/app/components/settings/advanced/labs/private-features.tsx @@ -23,10 +23,6 @@ const features: Feature[] = [{ title: 'Stripe Automatic Tax (private beta)', description: 'Use Stripe Automatic Tax at Stripe Checkout. Needs to be enabled in Stripe', flag: 'stripeAutomaticTax' -}, { - title: 'Email customization (internal beta)', - description: 'Newsletter customization settings that have been released to Ghost\'s own production sites', - flag: 'emailCustomization' }, { title: 'Import Member Tier', description: 'Enables tier to be specified when importing members', diff --git a/apps/ember-admin/app/services/feature.js b/apps/ember-admin/app/services/feature.js index f7fdad436e8..9408422d489 100644 --- a/apps/ember-admin/app/services/feature.js +++ b/apps/ember-admin/app/services/feature.js @@ -77,7 +77,6 @@ export default class FeatureService extends Service { // labs flags @feature('stripeAutomaticTax') stripeAutomaticTax; - @feature('emailCustomization') emailCustomization; @feature('importMemberTier') importMemberTier; @feature('adminUIRefresh') adminUIRefresh; @feature('editorExcerpt') editorExcerpt; diff --git a/ghost/core/core/shared/labs.js b/ghost/core/core/shared/labs.js index a83a741bd79..c3c8dc0e627 100644 --- a/ghost/core/core/shared/labs.js +++ b/ghost/core/core/shared/labs.js @@ -47,7 +47,6 @@ const PRIVATE_FEATURES = [ 'importMemberTier', 'csvContentImporter', 'adminUIRefresh', - 'emailCustomization', 'tagsX', 'emailUniqueid', 'themeTranslation', diff --git a/koenig/kg-default-nodes/src/export-dom.ts b/koenig/kg-default-nodes/src/export-dom.ts index b33d4345d54..9d75c5f09fa 100644 --- a/koenig/kg-default-nodes/src/export-dom.ts +++ b/koenig/kg-default-nodes/src/export-dom.ts @@ -10,8 +10,6 @@ export type ExportDOMOutput< }; export interface ExportDOMFeatureOptions { - emailCustomization?: boolean; - emailCustomizationAlpha?: boolean; [key: string]: unknown; } diff --git a/koenig/kg-default-nodes/test/generate-decorator-node.test.ts b/koenig/kg-default-nodes/test/generate-decorator-node.test.ts index 1b569687560..9204c6ab255 100644 --- a/koenig/kg-default-nodes/test/generate-decorator-node.test.ts +++ b/koenig/kg-default-nodes/test/generate-decorator-node.test.ts @@ -177,27 +177,21 @@ describe('Utils: generateDecoratorNode', function () { expect(() => node.exportDOM(editor)).toThrow('[generateDecoratorNode] versioned-render-test: "defaultRenderFn" for version 2 is required'); })); - ['emailCustomizationAlpha', 'emailCustomization'].forEach((feature) => { - it(`uses custom renderer if passed in (${feature})`, editorTest(function () { - const node = $createNodeWithRender(); - const customRenderer = () => createRenderResult('span', 'custom render'); - - const featureOption: Record = {}; - featureOption[feature] = true; - - const result = node.exportDOM(editor, { - feature: featureOption, - nodeRenderers: { - 'render-test': customRenderer - } - }); + it('uses custom renderer if passed in', editorTest(function () { + const node = $createNodeWithRender(); + const customRenderer = () => createRenderResult('span', 'custom render'); - expect(result.type).toBe('inner'); - expect(expectHtmlElement(result).outerHTML).toBe('custom render'); - })); - }); + const result = node.exportDOM(editor, { + nodeRenderers: { + 'render-test': customRenderer + } + }); - it('throws error when custom versioned renderer is missing for node version (emailCustomizationAlpha)', editorTest(function () { + expect(result.type).toBe('inner'); + expect(expectHtmlElement(result).outerHTML).toBe('custom render'); + })); + + it('throws error when custom versioned renderer is missing for node version', editorTest(function () { const VersionedNode = utils.generateDecoratorNode({ nodeType: 'versioned-render-test', properties: {version: {default: 1}}, @@ -211,9 +205,6 @@ describe('Utils: generateDecoratorNode', function () { const node = new VersionedNode({version: 2}); expect(() => node.exportDOM(editor, { - feature: { - emailCustomizationAlpha: true - }, nodeRenderers: { 'versioned-render-test': { 1: () => createRenderResult('div', 'version 1') diff --git a/koenig/kg-default-nodes/test/renderers/call-to-action-renderer.test.ts b/koenig/kg-default-nodes/test/renderers/call-to-action-renderer.test.ts index a2796e2d42f..7935e755b61 100644 --- a/koenig/kg-default-nodes/test/renderers/call-to-action-renderer.test.ts +++ b/koenig/kg-default-nodes/test/renderers/call-to-action-renderer.test.ts @@ -310,45 +310,36 @@ describe('renderers/call-to-action-renderer', function () { it('skips link to image when button is not shown (email, minimal)', testSkippedImageLink('email', 'minimal')); it('skips link to image when button is not shown (email, immersive)', testSkippedImageLink('email', 'immersive')); - ['emailCustomization', 'emailCustomizationAlpha'].forEach((flag) => { - it(`can render email with ${flag}`, function () { - const result = renderForEmail(getTestData(), {feature: {[flag]: true}}); - - assert.equal(result.element.tagName, 'TABLE'); - assert.ok(result.element.querySelector('table.btn'), 'table.btn element should exist'); + it('can render outline accent buttons', function () { + const result = renderForEmail(getTestData({buttonColor: 'accent'}), { + feature: {}, + design: {buttonStyle: 'outline'} }); - it(`can render outline accent buttons (${flag})`, function () { - const result = renderForEmail(getTestData({buttonColor: 'accent'}), { - feature: {[flag]: true}, - design: {buttonStyle: 'outline'} - }); + // accent buttons are fully styled by the main email template CSS + assert.equal(result.element.querySelector('table.btn td').getAttribute('style'), null); + assert.equal(result.element.querySelector('table.btn a').getAttribute('style'), null); + }); - // accent buttons are fully styled by the main email template CSS - assert.equal(result.element.querySelector('table.btn td').getAttribute('style'), null); - assert.equal(result.element.querySelector('table.btn a').getAttribute('style'), null); + it('can render outline custom buttons', function () { + const result = renderForEmail(getTestData({buttonColor: '#F0F0F0'}), { + feature: {}, + design: {buttonStyle: 'outline'} }); - it(`can render outline custom buttons (${flag})`, function () { - const result = renderForEmail(getTestData({buttonColor: '#F0F0F0'}), { - feature: {[flag]: true}, - design: {buttonStyle: 'outline'} - }); - - assertPrettifiedIncludes(result.html, html` - - - - - - -
- - click me - -
- `); - }); + assertPrettifiedIncludes(result.html, html` + + + + + + +
+ + click me + +
+ `); }); it('handles accent button color', function () { diff --git a/koenig/kg-default-nodes/test/renderers/product-renderer.test.ts b/koenig/kg-default-nodes/test/renderers/product-renderer.test.ts index fcd4660928c..d1ff9331ad7 100644 --- a/koenig/kg-default-nodes/test/renderers/product-renderer.test.ts +++ b/koenig/kg-default-nodes/test/renderers/product-renderer.test.ts @@ -243,67 +243,6 @@ describe('renderers/product-renderer', function () { `); }); - it('renders with emailCustomization feature flag', function () { - const result = renderForEmail({ - productButton: 'Click me', - productButtonEnabled: true, - productDescription: 'This product is ok', - productImageSrc: 'https://example.com/images/ok.jpg', - productImageWidth: null, - productImageHeight: null, - productRatingEnabled: true, - productStarRating: 3, - productTitle: 'Product title!', - productUrl: 'https://example.com/product/ok' - }, {feature: {emailCustomization: true}}); - - assertPrettifiesTo(result.html, html` - - - - - - -
- - - - - - - - - - - - - - - - - - -
- -
-

Product title!

-
- -
This product is ok
- - - - - - -
- Click me -
-
-
- `); - }); - it('renders without rating when star-rating is disabled', function () { const result = renderForEmail({ productButton: 'Click me', From 09a0dd7659af669ccb1dc8d91caf202214689ddd Mon Sep 17 00:00:00 2001 From: Princi Vershwal Date: Wed, 12 Aug 2026 17:16:12 +0530 Subject: [PATCH 03/16] Improved error reporting when routes.yaml or redirects fail to upload (#29488) no ref - uploading to an S3/GCS-backed site answered with "Maximum call stack size exceeded", which told the caller nothing about the real failure - an AWS SDK exception is a circular object graph, and the API error handler deep-clones the error it renders with no cycle detection, so letting one escape the store blew the stack - both S3 stores now convert a non-NotFound failure into a plain Ghost error that holds nothing off the SDK exception by reference, so it is safe to serialise - the origin frames are carried across on the stack, so the underlying storage failure is still diagnosable from the logs and Sentry --- .../adapters/redirects/S3RedirectsStore.ts | 51 ++++++++++++++----- .../route-settings/S3RouteSettingsStore.ts | 51 ++++++++++++++----- .../s3-route-settings-store.test.ts | 8 +-- .../s3-route-settings-store.test.ts | 29 +++++++++-- 4 files changed, 103 insertions(+), 36 deletions(-) diff --git a/ghost/core/core/server/adapters/redirects/S3RedirectsStore.ts b/ghost/core/core/server/adapters/redirects/S3RedirectsStore.ts index 6edbffd61aa..0ca979c9a18 100644 --- a/ghost/core/core/server/adapters/redirects/S3RedirectsStore.ts +++ b/ghost/core/core/server/adapters/redirects/S3RedirectsStore.ts @@ -22,7 +22,8 @@ const messages = { missingBucket: 'S3RedirectsStore requires a bucket name', missingStaticFileURLPrefix: 'S3RedirectsStore requires a staticFileURLPrefix', partialCredentials: 'S3RedirectsStore requires both accessKeyId and secretAccessKey when either is provided', - missingResponseBody: 'S3 GetObject returned no body' + missingResponseBody: 'S3 GetObject returned no body', + requestFailed: 'Something went wrong, please try again.' }; const stripLeadingAndTrailingSlashes = (value = '') => value.replace(/^\/+|\/+$/g, ''); @@ -134,7 +135,7 @@ export default class S3RedirectsStore extends RedirectsStoreBase { if (this._isNotFound(err)) { return []; } - throw err; + throw this._requestError(err); } return parseJson(body); @@ -143,20 +144,24 @@ export default class S3RedirectsStore extends RedirectsStoreBase { async replaceAll(redirects: RedirectConfig[]): Promise { const key = this.buildKey(); - if (await this._canonicalExists()) { - await this.client.send(new CopyObjectCommand({ + try { + if (await this._canonicalExists()) { + await this.client.send(new CopyObjectCommand({ + Bucket: this.bucket, + Key: getBackupRedirectsFilePath(key), + CopySource: `${this.bucket}/${key}` + })); + } + + await this.client.send(new PutObjectCommand({ Bucket: this.bucket, - Key: getBackupRedirectsFilePath(key), - CopySource: `${this.bucket}/${key}` + Key: key, + Body: JSON.stringify(redirects), + ContentType: 'application/json' })); + } catch (err) { + throw this._requestError(err); } - - await this.client.send(new PutObjectCommand({ - Bucket: this.bucket, - Key: key, - Body: JSON.stringify(redirects), - ContentType: 'application/json' - })); } private buildKey(): string { @@ -175,8 +180,26 @@ export default class S3RedirectsStore extends RedirectsStoreBase { if (this._isNotFound(err)) { return false; } - throw err; + throw this._requestError(err); + } + } + + private _requestError(err: unknown): Error { + // The only Ghost errors reaching here are this store's own, which are + // already safe to render — re-wrapping them would hide the reason. + if (err instanceof errors.InternalServerError) { + return err; + } + + const requestError = new errors.InternalServerError({ + message: tpl(messages.requestFailed) + }); + + if (typeof (err as {stack?: string})?.stack === 'string') { + requestError.stack = `${requestError.stack}\n\nCaused by: ${(err as {stack: string}).stack}`; } + + return requestError; } private _isNotFound(err: unknown): boolean { diff --git a/ghost/core/core/server/adapters/route-settings/S3RouteSettingsStore.ts b/ghost/core/core/server/adapters/route-settings/S3RouteSettingsStore.ts index bb71187d12b..ebd4cdea0ac 100644 --- a/ghost/core/core/server/adapters/route-settings/S3RouteSettingsStore.ts +++ b/ghost/core/core/server/adapters/route-settings/S3RouteSettingsStore.ts @@ -30,7 +30,8 @@ const messages = { missingDefaultSettingsBasePath: 'S3RouteSettingsStore requires a defaultSettingsBasePath', partialCredentials: 'S3RouteSettingsStore requires both accessKeyId and secretAccessKey when either is provided', missingResponseBody: 'S3 GetObject returned no body', - ensureDefaults: 'Error trying to access the default settings file in {path}.' + ensureDefaults: 'Error trying to access the default settings file in {path}.', + requestFailed: 'Something went wrong, please try again.' }; const stripLeadingAndTrailingSlashes = (value = '') => value.replace(/^\/+|\/+$/g, ''); @@ -135,7 +136,7 @@ export default class S3RouteSettingsStore extends RouteSettingsStoreBase { const defaultContent = await this.readDefaultSettings(); return parseRouteSettings(parseYaml(defaultContent), defaultContent); } - throw err; + throw this._requestError(err); } return parseRouteSettings(parseYaml(body), body); @@ -144,20 +145,24 @@ export default class S3RouteSettingsStore extends RouteSettingsStoreBase { async replace(settings: RouteSettings): Promise { const key = this.buildKey(); - if (await this._canonicalExists()) { - await this.client.send(new CopyObjectCommand({ + try { + if (await this._canonicalExists()) { + await this.client.send(new CopyObjectCommand({ + Bucket: this.bucket, + Key: getBackupRouteSettingsFilePath(key), + CopySource: `${this.bucket}/${key}` + })); + } + + await this.client.send(new PutObjectCommand({ Bucket: this.bucket, - Key: getBackupRouteSettingsFilePath(key), - CopySource: `${this.bucket}/${key}` + Key: key, + Body: settings.yamlSource, + ContentType: CONTENT_TYPE })); + } catch (err) { + throw this._requestError(err); } - - await this.client.send(new PutObjectCommand({ - Bucket: this.bucket, - Key: key, - Body: settings.yamlSource, - ContentType: CONTENT_TYPE - })); } private buildKey(): string { @@ -176,8 +181,26 @@ export default class S3RouteSettingsStore extends RouteSettingsStoreBase { if (this._isNotFound(err)) { return false; } - throw err; + throw this._requestError(err); + } + } + + private _requestError(err: unknown): Error { + // The only Ghost errors reaching here are this store's own, which are + // already safe to render — re-wrapping them would hide the reason. + if (err instanceof errors.InternalServerError) { + return err; + } + + const requestError = new errors.InternalServerError({ + message: tpl(messages.requestFailed) + }); + + if (typeof (err as {stack?: string})?.stack === 'string') { + requestError.stack = `${requestError.stack}\n\nCaused by: ${(err as {stack: string}).stack}`; } + + return requestError; } private async readDefaultSettings(): Promise { diff --git a/ghost/core/test/integration/adapters/route-settings/s3-route-settings-store.test.ts b/ghost/core/test/integration/adapters/route-settings/s3-route-settings-store.test.ts index 34c2f627e90..a12678912ea 100644 --- a/ghost/core/test/integration/adapters/route-settings/s3-route-settings-store.test.ts +++ b/ghost/core/test/integration/adapters/route-settings/s3-route-settings-store.test.ts @@ -292,7 +292,7 @@ describe('Integration: S3RouteSettingsStore without a live bucket', function () }); }); - it('propagates a non-NotFound error raised while reading', async function () { + it('reports a non-NotFound error raised while reading', async function () { const store = faultyStore(async (command) => { if (command instanceof GetObjectCommand) { throw new Error('connection reset'); @@ -300,10 +300,10 @@ describe('Integration: S3RouteSettingsStore without a live bucket', function () return {}; }); - await assert.rejects(store.get(), /connection reset/); + await assert.rejects(store.get(), /Something went wrong, please try again\./); }); - it('propagates a non-NotFound error raised by the existence check on replace', async function () { + it('reports a non-NotFound error raised by the existence check on replace', async function () { const store = faultyStore(async (command) => { if (command instanceof HeadObjectCommand) { throw new Error('access denied'); @@ -311,6 +311,6 @@ describe('Integration: S3RouteSettingsStore without a live bucket', function () return {}; }); - await assert.rejects(store.replace(fromYaml(SAMPLE_YAML)), /access denied/); + await assert.rejects(store.replace(fromYaml(SAMPLE_YAML)), /Something went wrong, please try again\./); }); }); diff --git a/ghost/core/test/unit/server/adapters/route-settings/s3-route-settings-store.test.ts b/ghost/core/test/unit/server/adapters/route-settings/s3-route-settings-store.test.ts index fbb9aff4c91..aba5ce17257 100644 --- a/ghost/core/test/unit/server/adapters/route-settings/s3-route-settings-store.test.ts +++ b/ghost/core/test/unit/server/adapters/route-settings/s3-route-settings-store.test.ts @@ -12,6 +12,7 @@ import { type S3Client } from '@aws-sdk/client-s3'; +import {utils as errorUtils} from '@tryghost/errors'; import {RouteSettingsStoreBase} from '@tryghost/adapter-base-route-settings'; import S3RouteSettingsStore from '../../../../../core/server/adapters/route-settings/S3RouteSettingsStore'; @@ -222,7 +223,7 @@ describe('UNIT: S3RouteSettingsStore', function () { assert.equal(copyCommands(fake.sent).length, 0); }); - it('propagates a non-NotFound existence-check error and does not overwrite', async function () { + it('reports a non-NotFound existence-check error and does not overwrite', async function () { const sent: S3Command[] = []; const client = stubbedClient(async (command) => { sent.push(command); @@ -232,7 +233,7 @@ describe('UNIT: S3RouteSettingsStore', function () { return {}; }); - await assert.rejects(createStore(client).replace(fromYaml(SAMPLE_YAML)), /access denied/); + await assert.rejects(createStore(client).replace(fromYaml(SAMPLE_YAML)), /Something went wrong, please try again\./); assert.equal(putCommands(sent).length, 0); }); }); @@ -283,12 +284,32 @@ describe('UNIT: S3RouteSettingsStore', function () { }); }); - it('propagates non-NotFound S3 errors instead of falling back to defaults', async function () { + it('reports non-NotFound S3 errors instead of falling back to defaults', async function () { const client = stubbedClient(async () => { throw new Error('AccessDenied'); }); - await assert.rejects(createStore(client).get(), /AccessDenied/); + await assert.rejects(createStore(client).get(), /Something went wrong, please try again\./); + }); + + it('reports an error the API error handler can clone', async function () { + const sdkException = Object.assign(new Error('Access Denied'), {name: 'AccessDenied'}) as Error & {$response?: unknown}; + sdkException.$response = {error: sdkException}; + const client = stubbedClient(async () => { + throw sdkException; + }); + + // Guards the guard: if this stops throwing, the hazard is gone + // upstream and this whole approach can be revisited. + assert.throws(() => errorUtils.prepareStackForUser(sdkException), RangeError); + + await assert.rejects(createStore(client).get(), (err: {message?: string; stack?: string}) => { + assert.equal(err.message, 'Something went wrong, please try again.'); + assert.doesNotThrow(() => errorUtils.prepareStackForUser(err as Error)); + // The S3 failure is still diagnosable from the logs. + assert.match(String(err.stack), /Caused by: AccessDenied: Access Denied/); + return true; + }); }); }); }); From b16b165db0377c3699fd29f449206182755f5cbd Mon Sep 17 00:00:00 2001 From: Austin Burdine Date: Wed, 12 Aug 2026 10:13:51 -0400 Subject: [PATCH 04/16] Updated scheduled release time to Tuesday 3pm UTC (#29874) --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8742fc4b41d..a18d27ae62f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -3,7 +3,7 @@ run-name: "Release — ${{ inputs.bump-type || 'auto' }} from ${{ inputs.branch on: schedule: - - cron: '0 15 * * 5' # Friday 3pm UTC + - cron: '0 15 * * 2' # Tuesday 3pm UTC workflow_dispatch: inputs: branch: From 021a6b0b9297109401b58b16627dbcbb01f4a29a Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Wed, 12 Aug 2026 09:44:51 -0500 Subject: [PATCH 05/16] TypeScriptified test assertion helpers (#29911) no ref --- ghost/core/test/utils/assertions.js | 112 ---------------------------- ghost/core/test/utils/assertions.ts | 81 ++++++++++++++++++++ 2 files changed, 81 insertions(+), 112 deletions(-) delete mode 100644 ghost/core/test/utils/assertions.js create mode 100644 ghost/core/test/utils/assertions.ts diff --git a/ghost/core/test/utils/assertions.js b/ghost/core/test/utils/assertions.js deleted file mode 100644 index f2af6208e2a..00000000000 --- a/ghost/core/test/utils/assertions.js +++ /dev/null @@ -1,112 +0,0 @@ -const assert = require('node:assert/strict'); -const {inspect, isDeepStrictEqual} = require('node:util'); -const {isPlainObject} = require('lodash'); -const {snapshotManager} = require('@tryghost/express-test').snapshot; - -/** - * @template T - * @param {T} value - * @param {string} [message] - * @returns {asserts value is NonNullable} - */ -function assertExists(value, message = 'Value should exist') { - assert( - (value !== undefined) && (value !== null), - message - ); -} - -function assertMatchSnapshot(obj, properties) { - const result = snapshotManager.match(obj, properties); - assert(result.pass, result.message()); -} - -/** - * @template T - * @param {ReadonlyArray} arr - * @param {ReadonlyArray} expectedElements - * @param {string} [message] - * @returns {void} - */ -function assertArrayContainsDeep(arr, expectedElements, message) { - for (const expectedElement of expectedElements) { - assert( - arr.some(el => isDeepStrictEqual(el, expectedElement)), - message || `Expected ${inspect(expectedElement)} to be found` - ); - } -} - -/** - * @template T - * @param {T} obj - * @param {Partial} properties - * @returns {boolean} - */ -function objectMatches(obj, properties) { - for (const [key, value] of Object.entries(properties)) { - const matches = isPlainObject(obj[key]) - ? objectMatches(obj[key], value) - : isDeepStrictEqual(obj[key], value); - if (!matches) { - return false; - } - } - return true; -} - -/** - * @internal - * @template T - * @typedef {( - * T extends Object - * ? {[P in keyof T]?: DeepPartial} - * : T - * )} DeepPartial - */ - -/** - * @template {object} T - * @param {ReadonlyArray} haystack - * @param {ReadonlyArray>} needles - * @returns {void} - */ -function assertArrayMatchesWithoutOrder(haystack, needles) { - assert.equal( - haystack.length, - needles.length, - `Expected ${needles.length} items, but got ${haystack.length}` - ); - for (const a of needles) { - assert(haystack.some(el => objectMatches(el, a))); - } -} - -/** - * @template {object} T - * @param {T} obj - * @param {DeepPartial} properties - * @param {string} [message] - * @returns {void} - */ -function assertObjectMatches(obj, properties, message) { - for (const [key, value] of Object.entries(properties)) { - if (isPlainObject(obj[key])) { - assertObjectMatches(obj[key], value, message); - } else { - assert.deepEqual( - obj[key], - value, - message || `Property mismatch for key "${key}"` - ); - } - } -} - -module.exports = { - assertExists, - assertMatchSnapshot, - assertArrayContainsDeep, - assertArrayMatchesWithoutOrder, - assertObjectMatches -}; diff --git a/ghost/core/test/utils/assertions.ts b/ghost/core/test/utils/assertions.ts new file mode 100644 index 00000000000..4dbd8fe0c35 --- /dev/null +++ b/ghost/core/test/utils/assertions.ts @@ -0,0 +1,81 @@ +import assert from 'node:assert/strict'; +import {inspect, isDeepStrictEqual} from 'node:util'; +import {isPlainObject} from 'lodash'; +// @ts-expect-error @tryghost/express-test lacks type definitions. +import * as expressTest from '@tryghost/express-test'; + +const {snapshotManager} = expressTest.snapshot; + +export function assertExists(value: T, message = 'Value should exist'): asserts value is NonNullable { + assert( + (value !== undefined) && (value !== null), + message + ); +} + +export function assertMatchSnapshot( + obj: Parameters[0], + properties?: Parameters[1] +): void { + const result = snapshotManager.match(obj, properties); + assert(result.pass, result.message()); +} + +export function assertArrayContainsDeep( + arr: ReadonlyArray, + expectedElements: ReadonlyArray, + message?: string +): void { + for (const expectedElement of expectedElements) { + assert( + arr.some(el => isDeepStrictEqual(el, expectedElement)), + message || `Expected ${inspect(expectedElement)} to be found` + ); + } +} + +function objectMatches(obj: T, properties: Partial): boolean { + for (const [key, value] of Object.entries(properties)) { + const objValue = (obj as Record)[key]; + const matches = isPlainObject(objValue) + ? objectMatches(objValue as object, value as object) + : isDeepStrictEqual(objValue, value); + if (!matches) { + return false; + } + } + return true; +} + +type DeepPartial = T extends object ? { + [P in keyof T]?: DeepPartial; +} : T; + +export function assertArrayMatchesWithoutOrder( + haystack: ReadonlyArray, + needles: ReadonlyArray> +): void { + assert.equal( + haystack.length, + needles.length, + `Expected ${needles.length} items, but got ${haystack.length}` + ); + for (const a of needles) { + assert(haystack.some(el => objectMatches(el, a))); + } +} + +export function assertObjectMatches(obj: T, properties: DeepPartial, message?: string): void { + for (const [key, value] of Object.entries(properties)) { + const objValue = (obj as Record)[key]; + if (isPlainObject(objValue)) { + assertObjectMatches(objValue as object, value as DeepPartial, message); + } else { + assert.deepEqual( + objValue, + value, + message || `Property mismatch for key "${key}"` + ); + } + } +} From d563c7a17088da7f5da4e0d5cf634f7fc2bb0636 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Wed, 12 Aug 2026 09:44:51 -0500 Subject: [PATCH 06/16] TypeScriptified several frontend helper tests (#29903) no reef This test-only change converts tests for these helpers: - `total_members` - `total_paid_members` - `color_to_rgba` - `json` - `title` - `encode` - `contrast_text_color` - `facebook_url` - JSON-LD escaping helper --- .../frontend/helpers/color-to-rgba.test.js | 22 ------------- .../frontend/helpers/color-to-rgba.test.ts | 22 +++++++++++++ .../helpers/contrast-text-color.test.js | 32 ------------------- .../helpers/contrast-text-color.test.ts | 32 +++++++++++++++++++ .../{encode.test.js => encode.test.ts} | 9 +++--- ...ebook-url.test.js => facebook-url.test.ts} | 17 +++++----- ....js => ghost-head-jsonld-escaping.test.ts} | 6 ++-- .../helpers/{json.test.js => json.test.ts} | 6 ++-- .../helpers/{title.test.js => title.test.ts} | 9 +++--- .../frontend/helpers/total-members.test.js | 10 ------ .../frontend/helpers/total-members.test.ts | 10 ++++++ .../helpers/total-paid-members.test.js | 10 ------ .../helpers/total-paid-members.test.ts | 10 ++++++ 13 files changed, 96 insertions(+), 99 deletions(-) delete mode 100644 ghost/core/test/unit/frontend/helpers/color-to-rgba.test.js create mode 100644 ghost/core/test/unit/frontend/helpers/color-to-rgba.test.ts delete mode 100644 ghost/core/test/unit/frontend/helpers/contrast-text-color.test.js create mode 100644 ghost/core/test/unit/frontend/helpers/contrast-text-color.test.ts rename ghost/core/test/unit/frontend/helpers/{encode.test.js => encode.test.ts} (61%) rename ghost/core/test/unit/frontend/helpers/{facebook-url.test.js => facebook-url.test.ts} (51%) rename ghost/core/test/unit/frontend/helpers/{ghost-head-jsonld-escaping.test.js => ghost-head-jsonld-escaping.test.ts} (90%) rename ghost/core/test/unit/frontend/helpers/{json.test.js => json.test.ts} (79%) rename ghost/core/test/unit/frontend/helpers/{title.test.js => title.test.ts} (78%) delete mode 100644 ghost/core/test/unit/frontend/helpers/total-members.test.js create mode 100644 ghost/core/test/unit/frontend/helpers/total-members.test.ts delete mode 100644 ghost/core/test/unit/frontend/helpers/total-paid-members.test.js create mode 100644 ghost/core/test/unit/frontend/helpers/total-paid-members.test.ts diff --git a/ghost/core/test/unit/frontend/helpers/color-to-rgba.test.js b/ghost/core/test/unit/frontend/helpers/color-to-rgba.test.js deleted file mode 100644 index 72b257609b2..00000000000 --- a/ghost/core/test/unit/frontend/helpers/color-to-rgba.test.js +++ /dev/null @@ -1,22 +0,0 @@ -const assert = require('node:assert/strict'); -const {assertExists} = require('../../../utils/assertions'); - -const color_to_rgba = require('../../../../core/frontend/helpers/color_to_rgba'); - -describe('{{color_to_rgba}} helper', function () { - it('has color_to_rgba helper', function () { - assertExists(color_to_rgba); - }); - - it('returns an rgba string for a valid color', function () { - assert.equal(color_to_rgba('#FF1A75', 0.25), 'rgba(255, 26, 117, 0.25)'); - }); - - it('clamps alpha into the valid range', function () { - assert.equal(color_to_rgba('#FF1A75', 2), 'rgb(255, 26, 117)'); - }); - - it('falls back for invalid colors', function () { - assert.equal(color_to_rgba('', 0.25), 'rgba(21, 23, 26, 0.25)'); - }); -}); diff --git a/ghost/core/test/unit/frontend/helpers/color-to-rgba.test.ts b/ghost/core/test/unit/frontend/helpers/color-to-rgba.test.ts new file mode 100644 index 00000000000..05b6e746cd6 --- /dev/null +++ b/ghost/core/test/unit/frontend/helpers/color-to-rgba.test.ts @@ -0,0 +1,22 @@ +import assert from 'node:assert/strict'; +import {assertExists} from '../../../utils/assertions'; +// @ts-expect-error color_to_rgba currently lacks type definitions. +import colorToRgba from '../../../../core/frontend/helpers/color_to_rgba'; + +describe('{{color_to_rgba}} helper', function () { + it('has color_to_rgba helper', function () { + assertExists(colorToRgba); + }); + + it('returns an rgba string for a valid color', function () { + assert.equal(colorToRgba('#FF1A75', 0.25), 'rgba(255, 26, 117, 0.25)'); + }); + + it('clamps alpha into the valid range', function () { + assert.equal(colorToRgba('#FF1A75', 2), 'rgb(255, 26, 117)'); + }); + + it('falls back for invalid colors', function () { + assert.equal(colorToRgba('', 0.25), 'rgba(21, 23, 26, 0.25)'); + }); +}); diff --git a/ghost/core/test/unit/frontend/helpers/contrast-text-color.test.js b/ghost/core/test/unit/frontend/helpers/contrast-text-color.test.js deleted file mode 100644 index daed1ab172c..00000000000 --- a/ghost/core/test/unit/frontend/helpers/contrast-text-color.test.js +++ /dev/null @@ -1,32 +0,0 @@ -const assert = require('node:assert/strict'); -const {assertExists} = require('../../../utils/assertions'); - -const contrast_text_color = require('../../../../core/frontend/helpers/contrast_text_color'); - -describe('{{contrast_text_color}} helper', function () { - it('has contrast_text_color helper', function () { - assertExists(contrast_text_color); - }); - - it('returns white for dark backgrounds', function () { - assert.equal(contrast_text_color('#15171A'), '#FFFFFF'); - }); - - it('returns black for light backgrounds', function () { - assert.equal(contrast_text_color('#FFFFFF'), '#000000'); - }); - - it('returns black for other light backgrounds', function () { - ['#dacafe', '#ffa5b1', '#a3e6ff'].forEach(color => { - assert.equal(contrast_text_color(color), '#000000'); - }); - }); - - it('returns white for mid-tone backgrounds', function () { - assert.equal(contrast_text_color('#808080'), '#FFFFFF'); - }); - - it('falls back to white for invalid colors', function () { - assert.equal(contrast_text_color(''), '#FFFFFF'); - }); -}); diff --git a/ghost/core/test/unit/frontend/helpers/contrast-text-color.test.ts b/ghost/core/test/unit/frontend/helpers/contrast-text-color.test.ts new file mode 100644 index 00000000000..cdf8cb263fe --- /dev/null +++ b/ghost/core/test/unit/frontend/helpers/contrast-text-color.test.ts @@ -0,0 +1,32 @@ +import assert from 'node:assert/strict'; +import {assertExists} from '../../../utils/assertions'; +// @ts-expect-error contrast_text_color currently lacks type definitions. +import contrastTextColor from '../../../../core/frontend/helpers/contrast_text_color'; + +describe('{{contrast_text_color}} helper', function () { + it('has contrast_text_color helper', function () { + assertExists(contrastTextColor); + }); + + it('returns white for dark backgrounds', function () { + assert.equal(contrastTextColor('#15171A'), '#FFFFFF'); + }); + + it('returns black for light backgrounds', function () { + assert.equal(contrastTextColor('#FFFFFF'), '#000000'); + }); + + it('returns black for other light backgrounds', function () { + ['#dacafe', '#ffa5b1', '#a3e6ff'].forEach(color => { + assert.equal(contrastTextColor(color), '#000000'); + }); + }); + + it('returns white for mid-tone backgrounds', function () { + assert.equal(contrastTextColor('#808080'), '#FFFFFF'); + }); + + it('falls back to white for invalid colors', function () { + assert.equal(contrastTextColor(''), '#FFFFFF'); + }); +}); diff --git a/ghost/core/test/unit/frontend/helpers/encode.test.js b/ghost/core/test/unit/frontend/helpers/encode.test.ts similarity index 61% rename from ghost/core/test/unit/frontend/helpers/encode.test.js rename to ghost/core/test/unit/frontend/helpers/encode.test.ts index 3cf6871d274..23f0aa62ff6 100644 --- a/ghost/core/test/unit/frontend/helpers/encode.test.js +++ b/ghost/core/test/unit/frontend/helpers/encode.test.ts @@ -1,8 +1,7 @@ -const assert = require('node:assert/strict'); -const {assertExists} = require('../../../utils/assertions'); - -// Stuff we are testing -const encode = require('../../../../core/frontend/helpers/encode'); +import assert from 'node:assert/strict'; +import {assertExists} from '../../../utils/assertions'; +// @ts-expect-error encode currently lacks type definitions. +import encode from '../../../../core/frontend/helpers/encode'; describe('{{encode}} helper', function () { it('can escape URI', function () { diff --git a/ghost/core/test/unit/frontend/helpers/facebook-url.test.js b/ghost/core/test/unit/frontend/helpers/facebook-url.test.ts similarity index 51% rename from ghost/core/test/unit/frontend/helpers/facebook-url.test.js rename to ghost/core/test/unit/frontend/helpers/facebook-url.test.ts index 951ce2dc2e5..ac97a35cd3e 100644 --- a/ghost/core/test/unit/frontend/helpers/facebook-url.test.js +++ b/ghost/core/test/unit/frontend/helpers/facebook-url.test.ts @@ -1,10 +1,9 @@ -const assert = require('node:assert/strict'); - -// Stuff we are testing -const facebook_url = require('../../../../core/frontend/helpers/facebook_url'); +import assert from 'node:assert/strict'; +// @ts-expect-error facebook_url currently lacks type definitions. +import facebookUrl from '../../../../core/frontend/helpers/facebook_url'; describe('{{facebook_url}} helper', function () { - const options = {data: {site: {}}}; + const options: {data: {site: {facebook?: string}}} = {data: {site: {}}}; beforeEach(function () { options.data.site = {facebook: ''}; @@ -13,22 +12,22 @@ describe('{{facebook_url}} helper', function () { it('should output the facebook url for @site, if no other facebook username is provided', function () { options.data.site = {facebook: 'hey'}; - assert.equal(facebook_url.call({}, options), 'https://www.facebook.com/hey'); + assert.equal(facebookUrl.call({}, options), 'https://www.facebook.com/hey'); }); it('should output the facebook url for the local object, if it has one', function () { options.data.site = {facebook: 'hey'}; - assert.equal(facebook_url.call({facebook: 'you/there'}, options), 'https://www.facebook.com/you/there'); + assert.equal(facebookUrl.call({facebook: 'you/there'}, options), 'https://www.facebook.com/you/there'); }); it('should output the facebook url for the provided username when it is explicitly passed in', function () { options.data.site = {facebook: 'hey'}; - assert.equal(facebook_url.call({facebook: 'you/there'}, 'i/see/you/over/there', options), 'https://www.facebook.com/i/see/you/over/there'); + assert.equal(facebookUrl.call({facebook: 'you/there'}, 'i/see/you/over/there', options), 'https://www.facebook.com/i/see/you/over/there'); }); it('should return null if there are no facebook usernames', function () { - assert.equal(facebook_url(options), null); + assert.equal(facebookUrl(options), null); }); }); diff --git a/ghost/core/test/unit/frontend/helpers/ghost-head-jsonld-escaping.test.js b/ghost/core/test/unit/frontend/helpers/ghost-head-jsonld-escaping.test.ts similarity index 90% rename from ghost/core/test/unit/frontend/helpers/ghost-head-jsonld-escaping.test.js rename to ghost/core/test/unit/frontend/helpers/ghost-head-jsonld-escaping.test.ts index 3770137f5be..9ead0853041 100644 --- a/ghost/core/test/unit/frontend/helpers/ghost-head-jsonld-escaping.test.js +++ b/ghost/core/test/unit/frontend/helpers/ghost-head-jsonld-escaping.test.ts @@ -1,6 +1,6 @@ -const assert = require('node:assert/strict'); - -const {escapeJsonLd} = require('../../../../core/frontend/helpers/ghost_head'); +import assert from 'node:assert/strict'; +// @ts-expect-error ghost_head currently lacks type definitions. +import {escapeJsonLd} from '../../../../core/frontend/helpers/ghost_head'; describe('ghost_head escapeJsonLd', function () { it('neutralises a breakout without losing the data', function () { diff --git a/ghost/core/test/unit/frontend/helpers/json.test.js b/ghost/core/test/unit/frontend/helpers/json.test.ts similarity index 79% rename from ghost/core/test/unit/frontend/helpers/json.test.js rename to ghost/core/test/unit/frontend/helpers/json.test.ts index f28a056f013..c8493df27e2 100644 --- a/ghost/core/test/unit/frontend/helpers/json.test.js +++ b/ghost/core/test/unit/frontend/helpers/json.test.ts @@ -1,6 +1,6 @@ -const assert = require('node:assert/strict'); - -const json = require('../../../../core/frontend/helpers/json'); +import assert from 'node:assert/strict'; +// @ts-expect-error json currently lacks type definitions. +import json from '../../../../core/frontend/helpers/json'; describe('{{json}} helper', function () { it('serializes values safely for inline JSON', function () { diff --git a/ghost/core/test/unit/frontend/helpers/title.test.js b/ghost/core/test/unit/frontend/helpers/title.test.ts similarity index 78% rename from ghost/core/test/unit/frontend/helpers/title.test.js rename to ghost/core/test/unit/frontend/helpers/title.test.ts index da45f48ad96..b279af0389f 100644 --- a/ghost/core/test/unit/frontend/helpers/title.test.js +++ b/ghost/core/test/unit/frontend/helpers/title.test.ts @@ -1,8 +1,7 @@ -const assert = require('node:assert/strict'); -const {assertExists} = require('../../../utils/assertions'); - -// Stuff we are testing -const title = require('../../../../core/frontend/helpers/title'); +import assert from 'node:assert/strict'; +import {assertExists} from '../../../utils/assertions'; +// @ts-expect-error title currently lacks type definitions. +import title from '../../../../core/frontend/helpers/title'; describe('{{title}} Helper', function () { it('can render title', function () { diff --git a/ghost/core/test/unit/frontend/helpers/total-members.test.js b/ghost/core/test/unit/frontend/helpers/total-members.test.js deleted file mode 100644 index 145ded3929b..00000000000 --- a/ghost/core/test/unit/frontend/helpers/total-members.test.js +++ /dev/null @@ -1,10 +0,0 @@ -const assert = require('node:assert/strict'); - -const total_members = require('../../../../core/frontend/helpers/total_members'); - -describe('{{total_members}} helper', function () { - it('can render total members', async function () { - const rendered = await total_members.call({total: 50000}); - assert.equal(rendered.string, '50,000+'); - }); -}); diff --git a/ghost/core/test/unit/frontend/helpers/total-members.test.ts b/ghost/core/test/unit/frontend/helpers/total-members.test.ts new file mode 100644 index 00000000000..791b9c93302 --- /dev/null +++ b/ghost/core/test/unit/frontend/helpers/total-members.test.ts @@ -0,0 +1,10 @@ +import assert from 'node:assert/strict'; +// @ts-expect-error total_members currently lacks type definitions. +import totalMembers from '../../../../core/frontend/helpers/total_members'; + +describe('{{total_members}} helper', function () { + it('can render total members', async function () { + const rendered = await totalMembers.call({total: 50000}); + assert.equal(rendered.string, '50,000+'); + }); +}); diff --git a/ghost/core/test/unit/frontend/helpers/total-paid-members.test.js b/ghost/core/test/unit/frontend/helpers/total-paid-members.test.js deleted file mode 100644 index bfb81f471d8..00000000000 --- a/ghost/core/test/unit/frontend/helpers/total-paid-members.test.js +++ /dev/null @@ -1,10 +0,0 @@ -const assert = require('node:assert/strict'); - -const total_paid_members = require('../../../../core/frontend/helpers/total_paid_members'); - -describe('{{total_paid_members}} helper', function () { - it('can render total paid members', async function () { - const rendered = await total_paid_members.call({paid: 3000}); - assert.equal(rendered.string, '3,000+'); - }); -}); diff --git a/ghost/core/test/unit/frontend/helpers/total-paid-members.test.ts b/ghost/core/test/unit/frontend/helpers/total-paid-members.test.ts new file mode 100644 index 00000000000..e2905d4fb34 --- /dev/null +++ b/ghost/core/test/unit/frontend/helpers/total-paid-members.test.ts @@ -0,0 +1,10 @@ +import assert from 'node:assert/strict'; +// @ts-expect-error total_paid_members currently lacks type definitions. +import totalPaidMembers from '../../../../core/frontend/helpers/total_paid_members'; + +describe('{{total_paid_members}} helper', function () { + it('can render total paid members', async function () { + const rendered = await totalPaidMembers.call({paid: 3000}); + assert.equal(rendered.string, '3,000+'); + }); +}); From 0d89154c5493e5b3ec86c2721c1c4332cda948e1 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Wed, 12 Aug 2026 15:55:20 +0100 Subject: [PATCH 07/16] Bumped bookshelf-plugins and removed smarterCounts labs flag (#29910) no ref - @tryghost/bookshelf-plugins 2.3.8 pulls in bookshelf-pagination 2.4.1, where the optimized count(*) query is the default for single-table queries, so the behaviour no longer needs to be gated - removed the smarterCounts labs flag, its gating in the model crud plugin, and the labs UI toggle --- .../advanced/labs/private-features.tsx | 4 - .../core/server/models/base/plugins/crud.js | 5 - ghost/core/core/shared/labs.js | 1 - ghost/core/package.json | 2 +- .../core/test/unit/server/models/post.test.js | 10 +- pnpm-lock.yaml | 173 +++++++++++------- 6 files changed, 115 insertions(+), 80 deletions(-) diff --git a/apps/admin/src/settings/app/components/settings/advanced/labs/private-features.tsx b/apps/admin/src/settings/app/components/settings/advanced/labs/private-features.tsx index 9732c3b96a4..18205b5fa54 100644 --- a/apps/admin/src/settings/app/components/settings/advanced/labs/private-features.tsx +++ b/apps/admin/src/settings/app/components/settings/advanced/labs/private-features.tsx @@ -51,10 +51,6 @@ const features: Feature[] = [{ title: 'Picture Element', description: 'Use the HTML picture element to serve modern image formats (AVIF, WebP) with automatic fallbacks', flag: 'pictureImageFormats' -}, { - title: 'Smarter Counts', - description: 'Use optimized COUNT queries for API pagination when safe', - flag: 'smarterCounts' }, { title: 'Get helper deduplication', description: 'Deduplicate identical {{#get}} helper queries within a single request to avoid redundant database calls', diff --git a/ghost/core/core/server/models/base/plugins/crud.js b/ghost/core/core/server/models/base/plugins/crud.js index 266cc8b4b14..97ae0c125fd 100644 --- a/ghost/core/core/server/models/base/plugins/crud.js +++ b/ghost/core/core/server/models/base/plugins/crud.js @@ -2,7 +2,6 @@ const _ = require('lodash'); const errors = require('@tryghost/errors'); const tpl = require('@tryghost/tpl'); -const labs = require('../../../../shared/labs'); const messages = { couldNotUnderstandRequest: 'Could not understand request.' @@ -180,10 +179,6 @@ module.exports = function (Bookshelf) { options.useBasicCount = unfilteredOptions.useBasicCount; } - if (labs.isSet('smarterCounts')) { - options.useSmartCount = true; - } - if (skipPagination) { applyManualPaginationWindow(itemCollection, options); } diff --git a/ghost/core/core/shared/labs.js b/ghost/core/core/shared/labs.js index c3c8dc0e627..454dab4b2d9 100644 --- a/ghost/core/core/shared/labs.js +++ b/ghost/core/core/shared/labs.js @@ -51,7 +51,6 @@ const PRIVATE_FEATURES = [ 'emailUniqueid', 'themeTranslation', 'pictureImageFormats', - 'smarterCounts', 'getHelperDeduplication', 'memberDetailsReact', 'membersCustomFields', diff --git a/ghost/core/package.json b/ghost/core/package.json index cfe4e517fa1..f17d451e2ef 100644 --- a/ghost/core/package.json +++ b/ghost/core/package.json @@ -92,7 +92,7 @@ "@tryghost/adapter-base-sso": "workspace:*", "@tryghost/admin-api-schema": "workspace:*", "@tryghost/api-framework": "catalog:", - "@tryghost/bookshelf-plugins": "2.3.2", + "@tryghost/bookshelf-plugins": "2.3.8", "@tryghost/brute-knex": "catalog:", "@tryghost/color-utils": "catalog:", "@tryghost/config-url-helpers": "1.0.27", diff --git a/ghost/core/test/unit/server/models/post.test.js b/ghost/core/test/unit/server/models/post.test.js index 0b1bca27a13..f890b17a033 100644 --- a/ghost/core/test/unit/server/models/post.test.js +++ b/ghost/core/test/unit/server/models/post.test.js @@ -41,7 +41,7 @@ describe('Unit: models/post', function () { withRelated: ['tags'] }).then(() => { assert.equal(queries.length, 2); - assert.equal(queries[0].sql, 'select count(distinct posts.id) as aggregate from `posts` where ((`posts`.`id` != ? and `posts`.`id` in (select `posts_tags`.`post_id` from `posts_tags` inner join `tags` on `tags`.`id` = `posts_tags`.`tag_id` where `tags`.`slug` in (?, ?))) and (`posts`.`type` = ? and `posts`.`status` = ?))'); + assert.equal(queries[0].sql, 'select count(*) as aggregate from `posts` where ((`posts`.`id` != ? and `posts`.`id` in (select `posts_tags`.`post_id` from `posts_tags` inner join `tags` on `tags`.`id` = `posts_tags`.`tag_id` where `tags`.`slug` in (?, ?))) and (`posts`.`type` = ? and `posts`.`status` = ?))'); assert.deepEqual(queries[0].bindings, [ testUtils.filterData.data.posts[3].id, 'photo', @@ -76,7 +76,7 @@ describe('Unit: models/post', function () { withRelated: ['authors', 'tags'] }).then(() => { assert.equal(queries.length, 2); - assert.equal(queries[0].sql, 'select count(distinct posts.id) as aggregate from `posts` where (((`posts`.`feature_image` is not null or `posts`.`id` in (select `posts_tags`.`post_id` from `posts_tags` inner join `tags` on `tags`.`id` = `posts_tags`.`tag_id` where `tags`.`slug` = ?)) and `posts`.`id` in (select `posts_authors`.`post_id` from `posts_authors` inner join `users` as `authors` on `authors`.`id` = `posts_authors`.`author_id` where `authors`.`slug` in (?, ?))) and (`posts`.`type` = ? and `posts`.`status` = ?))'); + assert.equal(queries[0].sql, 'select count(*) as aggregate from `posts` where (((`posts`.`feature_image` is not null or `posts`.`id` in (select `posts_tags`.`post_id` from `posts_tags` inner join `tags` on `tags`.`id` = `posts_tags`.`tag_id` where `tags`.`slug` = ?)) and `posts`.`id` in (select `posts_authors`.`post_id` from `posts_authors` inner join `users` as `authors` on `authors`.`id` = `posts_authors`.`author_id` where `authors`.`slug` in (?, ?))) and (`posts`.`type` = ? and `posts`.`status` = ?))'); assert.deepEqual(queries[0].bindings, [ 'hash-audio', 'leslie', @@ -112,7 +112,7 @@ describe('Unit: models/post', function () { withRelated: ['tags'] }).then(() => { assert.equal(queries.length, 2); - assert.equal(queries[0].sql, 'select count(distinct posts.id) as aggregate from `posts` where (`posts`.`published_at` > ? and (`posts`.`type` = ? and `posts`.`status` = ?))'); + assert.equal(queries[0].sql, 'select count(*) as aggregate from `posts` where (`posts`.`published_at` > ? and (`posts`.`type` = ? and `posts`.`status` = ?))'); assert.deepEqual(queries[0].bindings, [ '2015-07-20', 'post', @@ -199,7 +199,7 @@ describe('Unit: models/post', function () { withRelated: ['tags'] }).then(() => { assert.equal(queries.length, 2); - assert.equal(queries[0].sql, 'select count(distinct posts.id) as aggregate from `posts` where ((`posts`.`id` in (select `posts_tags`.`post_id` from `posts_tags` inner join `tags` on `tags`.`id` = `posts_tags`.`tag_id` and `posts_tags`.`sort_order` = 0 where `tags`.`slug` = ? and `tags`.`visibility` = ?)) and (`posts`.`type` = ? and `posts`.`status` = ?))'); + assert.equal(queries[0].sql, 'select count(*) as aggregate from `posts` where ((`posts`.`id` in (select `posts_tags`.`post_id` from `posts_tags` inner join `tags` on `tags`.`id` = `posts_tags`.`tag_id` and `posts_tags`.`sort_order` = 0 where `tags`.`slug` = ? and `tags`.`visibility` = ?)) and (`posts`.`type` = ? and `posts`.`status` = ?))'); assert.deepEqual(queries[0].bindings, [ 'photo', 'public', @@ -232,7 +232,7 @@ describe('Unit: models/post', function () { withRelated: ['authors'] }).then(() => { assert.equal(queries.length, 2); - assert.equal(queries[0].sql, 'select count(distinct posts.id) as aggregate from `posts` where ((`posts`.`id` in (select `posts_authors`.`post_id` from `posts_authors` inner join `users` as `authors` on `authors`.`id` = `posts_authors`.`author_id` and `posts_authors`.`sort_order` = 0 where `authors`.`slug` = ? and `authors`.`visibility` = ?)) and (`posts`.`type` = ? and `posts`.`status` = ?))'); + assert.equal(queries[0].sql, 'select count(*) as aggregate from `posts` where ((`posts`.`id` in (select `posts_authors`.`post_id` from `posts_authors` inner join `users` as `authors` on `authors`.`id` = `posts_authors`.`author_id` and `posts_authors`.`sort_order` = 0 where `authors`.`slug` = ? and `authors`.`visibility` = ?)) and (`posts`.`type` = ? and `posts`.`status` = ?))'); assert.deepEqual(queries[0].bindings, [ 'leslie', 'public', diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a8a8d32b447..07ae69cb8a0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2163,8 +2163,8 @@ importers: specifier: 'catalog:' version: 3.3.5(supports-color@10.2.2) '@tryghost/bookshelf-plugins': - specifier: 2.3.2 - version: 2.3.2(supports-color@10.2.2) + specifier: 2.3.8 + version: 2.3.8(supports-color@10.2.2) '@tryghost/brute-knex': specifier: 'catalog:' version: 3.2.1(better-sqlite3@12.11.1)(express@4.22.2(supports-color@10.2.2))(mysql2@3.22.5(@types/node@22.20.0))(sqlite3@5.1.7(bluebird@3.7.2)(supports-color@10.2.2))(supports-color@10.2.2) @@ -8917,38 +8917,38 @@ packages: '@tryghost/api-framework@3.3.5': resolution: {integrity: sha512-6bDBLtKAHPG82P3ZdY0gT0ZcfpfLbRiK5uwcJhLbr4CfhhH8IWvrecvjkKUskQldL18Tdr2Ct+rfbj2SOYGXog==} - '@tryghost/bookshelf-collision@2.3.2': - resolution: {integrity: sha512-9OQvpAU3Xo/rwlsJGyxe1NmbTpMuu3SZl3A2QF2Q3xy8yvxZOlWsX8JmDmPmK433cAkLW/G5qt21cVq+fxG/bQ==} + '@tryghost/bookshelf-collision@2.3.7': + resolution: {integrity: sha512-CIrIW1BhzyBaybKTKF7Ly6jcnx/AOzCmH1rrLlLhxUjW/5tRZt7qHfXgH7r/SbYbzR8+CVuljq1jGA4vqjM9SA==} - '@tryghost/bookshelf-custom-query@2.3.2': - resolution: {integrity: sha512-uLj8mqaudNyRx6MFO9wXcjDl7PEuoOP9Zi0rJBbdByjHmJfmf5Jc83DIcMH3A7C+s9XS5xHLqIZCLW1fB0qKSw==} + '@tryghost/bookshelf-custom-query@2.3.7': + resolution: {integrity: sha512-i9P24QcI4SadAkFfIwRr5RfK8c0EFOhRlGka0gkXecAOOsnRLJEK36/+f+7eOfVbhdOZq2drOg6zVBAkBjv4HQ==} - '@tryghost/bookshelf-eager-load@2.3.2': - resolution: {integrity: sha512-nb0NdNhkPrbY1wVGD9W5IkXm1FNvfltLuonR7MW5xDQMWdlHZQ2QXOHXjzFvfG8wG7D8G0zXn9o91YF0EXmuKw==} + '@tryghost/bookshelf-eager-load@2.3.7': + resolution: {integrity: sha512-k3eB8Xe665PpZxx+t/ycIh0rqQt9XhV1BtnqCyNWRzgIvnTEcrIx0vQqxUQMOOAEtMoZ0s5rFAqGVzDo1H4KjQ==} - '@tryghost/bookshelf-filter@2.3.2': - resolution: {integrity: sha512-C1sNBp5HfrAm7g+M+LS4kgbrQVcpSrQarXFch3KCnLpJDeEm4Qhrn6lLIYTMmAG4y3KF2NHXv1qhtlcG02mVLA==} + '@tryghost/bookshelf-filter@2.3.7': + resolution: {integrity: sha512-lR51ryZO4NCNvzVcWv6b4CHDpp41XhflbltwdvJZdXgEej5dIf9kYSZ8km7Mrw8CEbZrGqoPUdiAdCxTgeKUZg==} - '@tryghost/bookshelf-has-posts@2.4.2': - resolution: {integrity: sha512-RT+QFPn+qWqXQv3QUjetuYyXhdFynKG+nUT6HmHQqA/hgRbeRVHEsARMdqxIQntZyja2PLg4SXv1I166Tbqz5A==} + '@tryghost/bookshelf-has-posts@2.4.7': + resolution: {integrity: sha512-uWdsQKosBvKhMZnKLArBmK791qH13o6oNnTMcpM9dDRBNWDW3IJBW3XYnZUTJmjDg9HnJ7t5zm7wZxOnKwwP9Q==} - '@tryghost/bookshelf-include-count@2.3.2': - resolution: {integrity: sha512-X1xeAnbOxmZE8HEKL06/JtCC5/h2Gn35zL9HnHCeIUwfnfPPRUgTi6+HKAG4aala1JHjsJgDnCNhXQtHHr2SyA==} + '@tryghost/bookshelf-include-count@2.3.7': + resolution: {integrity: sha512-opncKLBLPtc4/lLgMx+fIhXdjDX34V+4a2OYp259eDsKyFOnrm4H1vYdyxy/VIw8d/WnRy7KpdYA9xlJgZhDsQ==} - '@tryghost/bookshelf-order@2.3.2': - resolution: {integrity: sha512-ZGLoygYHfGCYUrar8qiwsvQo1xX8DzpZretf8nNG/V5RQpkmmXqOBh5bOSqkxyFFr3b3FMLgqOq6MUQ1ekzaqA==} + '@tryghost/bookshelf-order@2.3.7': + resolution: {integrity: sha512-pX4+8ne1W0oo5Kw98gvvgWL7RgzeywywnydnvGe6B48QuSt0NheFqKQ30g9VKZiCanEI7vkyKs9NZkoRTbY9Sw==} - '@tryghost/bookshelf-pagination@2.3.2': - resolution: {integrity: sha512-5CN7fW3xFmvpm+zODM6V+xT+pH8xdKn3ufe2HxoEdEHaKJnzenrQLrJEKmdD6zDkg/I1b9QNIj0B8dFKGKNMOQ==} + '@tryghost/bookshelf-pagination@2.4.1': + resolution: {integrity: sha512-oEFuybdPkEhpadvOBUXM83BgVKyKaiFpdALOUqFHR4fk1xHA4vWgOE75ajntXcDUBUmmTSRW8k/4XZDofFWTxA==} - '@tryghost/bookshelf-plugins@2.3.2': - resolution: {integrity: sha512-4gF429NmprYpxzJiJ+G+nhhsBJ0MceGZSofUTvzOQTmvOf+Stkmz5bzB0+qiRcTyPwqUqraBJ/ucDuECTcqTGw==} + '@tryghost/bookshelf-plugins@2.3.8': + resolution: {integrity: sha512-CSEMOYJtBqXOxE1JccvYcktPDiC1xAXrYQAYSGGoUPXj/5zIRgU0Lr9z5WfmX4AmxEnpTPpFQl1O/9EoDrsxXA==} - '@tryghost/bookshelf-search@2.3.2': - resolution: {integrity: sha512-I9VdWjtnIXAVrVvVkY1TpuTQPTNkPrCuNr0p+IxvwLR6eTy+4MmHdXlcHEsITvWU521Jf2hz1qkFpQV7WGn4Mw==} + '@tryghost/bookshelf-search@2.3.7': + resolution: {integrity: sha512-MAy/aLA72BGIJ7lYoBkCooJMpXGcnCRWI1+QwPDtszNdC4lawDOgBANyEmBzUFH+YveCkK0RAzDPCV4UO0xyPA==} - '@tryghost/bookshelf-transaction-events@2.3.2': - resolution: {integrity: sha512-j1lHAxVp+AZzTM9JlOTiKB5RS0SwbkpFam7aUzzp71DwceKNb6JXhxA7zJhqG1lQS8c71DuG7IRbgQTppFsBSQ==} + '@tryghost/bookshelf-transaction-events@2.3.7': + resolution: {integrity: sha512-m9SP1f999C+pO0HU6wzEVhGAIijXWcv4A4EFooHwTqyYszJTXxSwyzyw3u2dXMaMLHLKO9+iFotlaOBPLZN65g==} '@tryghost/brute-knex@3.2.1': resolution: {integrity: sha512-1Q1CzpDuWNRrejD4sbSZF4CAJ9RTuZWLuQH1BjcC1U6+5sTY3cbnRv4w6mmxGHbUxWJ0kwbP10UM+ODheGaL7A==} @@ -8981,12 +8981,12 @@ packages: '@tryghost/debug@2.3.1': resolution: {integrity: sha512-m35yRGwmmmvHWzs42qJnf0duCvi5Yd456bPa436Gcldv/rxK5YMPehtGDrwjHstJ4K2If0oguXsdHJf3tjgAjw==} - '@tryghost/debug@2.3.2': - resolution: {integrity: sha512-+ux9DG8GapSubq8JRLNVnWYxzou2oga66jM2bit7pq7zCGsNKnmGPV6YMWFbn8xKD22dG1isAUAgLwi3UAHr9A==} - '@tryghost/debug@2.3.5': resolution: {integrity: sha512-t08spG/+SXLb7x1C2zJ0ATw/sM3CC5ssaeCHm/Ew2zPaIF+4SaFzNMynGRxKjzlIb+bxUoWPMJlW0EQaveOPeg==} + '@tryghost/debug@2.3.7': + resolution: {integrity: sha512-qS8QrBLNdDTu1DBgx/RwnNeF/7abDvT1iRZ+bDJ95TIj6gKcHbQN2gQs0rLhZLsnxQs2aC4arb0QjHRy8JyazA==} + '@tryghost/domain-events@3.3.5': resolution: {integrity: sha512-mflHTxYQxZ8j7veZqrLQfhqiBbn17NpvCKX+8gzAYvX3jOAJTZOY1KgsJ6GRDuLrxMrAuT80enuhIjFxfWGcKw==} @@ -9044,9 +9044,15 @@ packages: '@tryghost/mongo-knex@0.10.1': resolution: {integrity: sha512-6LRA4zVpNvCrVrA9hOhs8N4iylAiafGssiKuD8yML/13xg2PvKw6dzOZj6hg0CKQwG7tVp8TA+7Nw3iDOzy/jQ==} + '@tryghost/mongo-knex@0.11.0': + resolution: {integrity: sha512-OWNRZLAqgRDgW3FFFhsZksiDaUMcjgBQVAWM+PzZEB3JpamdqLq+60MWrbkDiI5WynrFHtYI/5rAOxDaz7HY/g==} + '@tryghost/mongo-utils@0.6.5': resolution: {integrity: sha512-fMEfdlVaVkr7SJwVxBxVDfUQ+x4DVF4PMet698PLzabqSnGsaWcruBaQlOuNvtf8ITNXKFUAqZMdmSUTIAHU+Q==} + '@tryghost/mongo-utils@0.6.6': + resolution: {integrity: sha512-CeWSgFKfUhFqD/dk22zEugawFdzVpa9avRgzFExbogiIFmNOfMOuD4G12/GUwFYOGjyCRAdQyKp8xgWSF97SRg==} + '@tryghost/mw-error-handler@1.0.13': resolution: {integrity: sha512-qDRjyiOkF5UNGgMHUDdFKTY0Mcwnxo5N9m71pliwVI+HfYvw77k/GxEIGc8mgikxjCuZkgC+IUnJjWCLuzCvKQ==} @@ -9059,9 +9065,15 @@ packages: '@tryghost/nql-lang@0.6.6': resolution: {integrity: sha512-ZWEUN92fVnxavf+matwASvi7pom0i1jhAtNCuoCN9f3ShtyrVm92m1SfvY+x/XuEvbefXAzCGzSEMcaQ4cqoXw==} + '@tryghost/nql-lang@0.6.7': + resolution: {integrity: sha512-quholb52aCqHWCskYp1ZC2hY/E+BD8XmZtgsrp1TcTHm0si9O3DBV9NGX2GB1GEE1V+QHFL24gdp0j98TrhFIQ==} + '@tryghost/nql@0.13.1': resolution: {integrity: sha512-TQRDur1miWd2ORa3XOhtw8Na8AUj/8h+epNMDgiutOyAH/RTw+d+ksLndMnm/Z9MhnKg7kFy7398peKZ7nF8bQ==} + '@tryghost/nql@0.13.2': + resolution: {integrity: sha512-nlG3xMNJYUO5MvOtnSaYltFKRaSeBTalIAUbIyeGplY4YXlIPVH/KUzCIpM8HyJ5uncR8bXkUey0a29RqCFaeA==} + '@tryghost/pretty-cli@3.3.1': resolution: {integrity: sha512-P7/GffeBG0grjmFxMWEeVFONL6HGt1mWUZvI5G36mhlxC/AXSWCSS6+qiQU91ZfenTwBrP6LZRC9Pyt0gqfzLg==} @@ -9111,6 +9123,9 @@ packages: '@tryghost/root-utils@2.3.5': resolution: {integrity: sha512-7OnoEPEAAaT9LFgR0WNxHUeehC1VSits1k3KuW5FpqitE6NznieiSK9Cpo8v0489Xq7d0Xbkc9qsDytWCCRGCg==} + '@tryghost/root-utils@2.3.7': + resolution: {integrity: sha512-8HR0W95it+s+ERkRI3syWHDLyYcinBFexpVlmLBhMMf8yEcL7r6tio6H3sUAj09L/YE8Kk5uGhvjAZAT3TU77Q==} + '@tryghost/security@1.0.6': resolution: {integrity: sha512-h4FiUK4ndHezlXeuRzfwTZrX/a8ZdCS/FRqmZWCHcMUiBH6lewHv8eIjsPemN6e6Gn9jtsWZsKnuYM2mD0meSQ==} @@ -9136,15 +9151,15 @@ packages: '@tryghost/tpl@2.3.1': resolution: {integrity: sha512-qqa2SvhnBVKFYN+G4hfj2cYZuZXINBDyJ9cTzNBtev/FRNUKniyGarPKAkyb6ZBtvp3Nqlmyvrqe+SFoqHcfrQ==} - '@tryghost/tpl@2.3.2': - resolution: {integrity: sha512-CXx7JCqyTFtsZENp4Bc3Af0cFD6QmQAtoLnM0BL4kTZkPFsPkvJPIhE55JmV3sL8rj9NA0kyz7w8614ViMugiA==} - '@tryghost/tpl@2.3.4': resolution: {integrity: sha512-/sPmwPuTcu1noY5PGwTwDmlE8wZuttl2r1IORMF5vxRKYUC7iWQnyKNstXtYfgh3bgBTYcq0kkfoPdz3B4GEKQ==} '@tryghost/tpl@2.3.5': resolution: {integrity: sha512-wexPVuAfF+k/Ysy1uTLTHPznQVsKe5kImlvhKd3IQU1eA4ZYGwymtD7GaSvJxHvyaS6zRB8FXzbgXl2dpAUyVg==} + '@tryghost/tpl@2.3.7': + resolution: {integrity: sha512-TIDQF9tj4MaQKrIxNB8CxpElShAwLfyByozggIBnuzIVkGudX07gy/o9oWGRYslRselrvdByf+NpYonQ9j3Y0Q==} + '@tryghost/url-utils@5.2.6': resolution: {integrity: sha512-3TQRcseZW/rq3UsSMP94n6WgKYZJqufreNdv9wJT5P4Fm5bNQ04rEHvL8i1AggXWa2kUPmwt2ed7KzNY6xx7zA==} @@ -28231,72 +28246,72 @@ snapshots: transitivePeerDependencies: - supports-color - '@tryghost/bookshelf-collision@2.3.2': + '@tryghost/bookshelf-collision@2.3.7': dependencies: '@tryghost/errors': 3.3.5 lodash: 4.18.1 moment-timezone: 0.5.45 - '@tryghost/bookshelf-custom-query@2.3.2': {} + '@tryghost/bookshelf-custom-query@2.3.7': {} - '@tryghost/bookshelf-eager-load@2.3.2(supports-color@10.2.2)': + '@tryghost/bookshelf-eager-load@2.3.7(supports-color@10.2.2)': dependencies: - '@tryghost/debug': 2.3.2(supports-color@10.2.2) + '@tryghost/debug': 2.3.7(supports-color@10.2.2) lodash: 4.18.1 transitivePeerDependencies: - supports-color - '@tryghost/bookshelf-filter@2.3.2(supports-color@10.2.2)': + '@tryghost/bookshelf-filter@2.3.7(supports-color@10.2.2)': dependencies: - '@tryghost/debug': 2.3.2(supports-color@10.2.2) + '@tryghost/debug': 2.3.7(supports-color@10.2.2) '@tryghost/errors': 3.3.5 - '@tryghost/nql': 0.13.1(supports-color@10.2.2) - '@tryghost/tpl': 2.3.2 + '@tryghost/nql': 0.13.2(supports-color@10.2.2) + '@tryghost/tpl': 2.3.7 transitivePeerDependencies: - supports-color - '@tryghost/bookshelf-has-posts@2.4.2(supports-color@10.2.2)': + '@tryghost/bookshelf-has-posts@2.4.7(supports-color@10.2.2)': dependencies: - '@tryghost/debug': 2.3.2(supports-color@10.2.2) + '@tryghost/debug': 2.3.7(supports-color@10.2.2) lodash: 4.18.1 transitivePeerDependencies: - supports-color - '@tryghost/bookshelf-include-count@2.3.2(supports-color@10.2.2)': + '@tryghost/bookshelf-include-count@2.3.7(supports-color@10.2.2)': dependencies: - '@tryghost/debug': 2.3.2(supports-color@10.2.2) + '@tryghost/debug': 2.3.7(supports-color@10.2.2) lodash: 4.18.1 transitivePeerDependencies: - supports-color - '@tryghost/bookshelf-order@2.3.2': + '@tryghost/bookshelf-order@2.3.7': dependencies: lodash: 4.18.1 - '@tryghost/bookshelf-pagination@2.3.2': + '@tryghost/bookshelf-pagination@2.4.1': dependencies: '@tryghost/errors': 3.3.5 - '@tryghost/tpl': 2.3.2 + '@tryghost/tpl': 2.3.7 lodash: 4.18.1 - '@tryghost/bookshelf-plugins@2.3.2(supports-color@10.2.2)': + '@tryghost/bookshelf-plugins@2.3.8(supports-color@10.2.2)': dependencies: - '@tryghost/bookshelf-collision': 2.3.2 - '@tryghost/bookshelf-custom-query': 2.3.2 - '@tryghost/bookshelf-eager-load': 2.3.2(supports-color@10.2.2) - '@tryghost/bookshelf-filter': 2.3.2(supports-color@10.2.2) - '@tryghost/bookshelf-has-posts': 2.4.2(supports-color@10.2.2) - '@tryghost/bookshelf-include-count': 2.3.2(supports-color@10.2.2) - '@tryghost/bookshelf-order': 2.3.2 - '@tryghost/bookshelf-pagination': 2.3.2 - '@tryghost/bookshelf-search': 2.3.2 - '@tryghost/bookshelf-transaction-events': 2.3.2 + '@tryghost/bookshelf-collision': 2.3.7 + '@tryghost/bookshelf-custom-query': 2.3.7 + '@tryghost/bookshelf-eager-load': 2.3.7(supports-color@10.2.2) + '@tryghost/bookshelf-filter': 2.3.7(supports-color@10.2.2) + '@tryghost/bookshelf-has-posts': 2.4.7(supports-color@10.2.2) + '@tryghost/bookshelf-include-count': 2.3.7(supports-color@10.2.2) + '@tryghost/bookshelf-order': 2.3.7 + '@tryghost/bookshelf-pagination': 2.4.1 + '@tryghost/bookshelf-search': 2.3.7 + '@tryghost/bookshelf-transaction-events': 2.3.7 transitivePeerDependencies: - supports-color - '@tryghost/bookshelf-search@2.3.2': {} + '@tryghost/bookshelf-search@2.3.7': {} - '@tryghost/bookshelf-transaction-events@2.3.2': {} + '@tryghost/bookshelf-transaction-events@2.3.7': {} '@tryghost/brute-knex@3.2.1(better-sqlite3@12.11.1)(express@4.22.2(supports-color@10.2.2))(mysql2@3.22.5(@types/node@22.20.0))(sqlite3@5.1.7(bluebird@3.7.2)(supports-color@10.2.2))(supports-color@10.2.2)': dependencies: @@ -28349,16 +28364,16 @@ snapshots: transitivePeerDependencies: - supports-color - '@tryghost/debug@2.3.2(supports-color@10.2.2)': + '@tryghost/debug@2.3.5(supports-color@10.2.2)': dependencies: - '@tryghost/root-utils': 2.3.2 + '@tryghost/root-utils': 2.3.5 debug: 4.4.3(supports-color@10.2.2) transitivePeerDependencies: - supports-color - '@tryghost/debug@2.3.5(supports-color@10.2.2)': + '@tryghost/debug@2.3.7(supports-color@10.2.2)': dependencies: - '@tryghost/root-utils': 2.3.5 + '@tryghost/root-utils': 2.3.7 debug: 4.4.3(supports-color@10.2.2) transitivePeerDependencies: - supports-color @@ -28508,10 +28523,21 @@ snapshots: transitivePeerDependencies: - supports-color + '@tryghost/mongo-knex@0.11.0(supports-color@10.2.2)': + dependencies: + debug: 4.4.3(supports-color@10.2.2) + lodash: 4.18.1 + transitivePeerDependencies: + - supports-color + '@tryghost/mongo-utils@0.6.5': dependencies: lodash: 4.18.1 + '@tryghost/mongo-utils@0.6.6': + dependencies: + lodash: 4.18.1 + '@tryghost/mw-error-handler@1.0.13(supports-color@10.2.2)': dependencies: '@tryghost/debug': 0.1.40(supports-color@10.2.2) @@ -28594,6 +28620,10 @@ snapshots: dependencies: date-fns: 2.30.0 + '@tryghost/nql-lang@0.6.7': + dependencies: + date-fns: 2.30.0 + '@tryghost/nql@0.13.1(supports-color@10.2.2)': dependencies: '@tryghost/mongo-knex': 0.10.1(supports-color@10.2.2) @@ -28604,6 +28634,16 @@ snapshots: transitivePeerDependencies: - supports-color + '@tryghost/nql@0.13.2(supports-color@10.2.2)': + dependencies: + '@tryghost/mongo-knex': 0.11.0(supports-color@10.2.2) + '@tryghost/mongo-utils': 0.6.6 + '@tryghost/nql-lang': 0.6.7 + lodash: 4.18.1 + mingo: 2.5.3 + transitivePeerDependencies: + - supports-color + '@tryghost/pretty-cli@3.3.1': dependencies: chalk: 5.6.2 @@ -28687,6 +28727,11 @@ snapshots: caller: 1.1.0 find-root: 1.1.0 + '@tryghost/root-utils@2.3.7': + dependencies: + caller: 1.1.0 + find-root: 1.1.0 + '@tryghost/security@1.0.6': dependencies: '@tryghost/string': 0.2.21 @@ -28718,12 +28763,12 @@ snapshots: '@tryghost/tpl@2.3.1': {} - '@tryghost/tpl@2.3.2': {} - '@tryghost/tpl@2.3.4': {} '@tryghost/tpl@2.3.5': {} + '@tryghost/tpl@2.3.7': {} + '@tryghost/url-utils@5.2.6': dependencies: cheerio: 1.2.0 From 05990c514527f35d914e0195ca61e19eedec5d65 Mon Sep 17 00:00:00 2001 From: Renato Costa Date: Thu, 6 Aug 2026 17:39:46 +0300 Subject: [PATCH 08/16] Moved custom field type presentation into shared ref https://linear.app/ghost/issue/BER-3846/ - how a field type looks in a picker (its icon, and the row pairing that icon with its name) is presentation shared by every surface that offers types, and there is about to be a second one in the members import - Settings and the import had rendered the same markup independently, so the option row moves alongside the icon rather than being copied next to it - no behaviour change: the option resolves its own label from the catalog, which is where the local {label, value} shape was reading it from anyway --- .../settings/membership/custom-fields.tsx | 2 +- .../custom-fields/custom-field-modal.tsx | 25 +++---------------- .../custom-field-icon.tsx | 0 .../custom-field-type-option.tsx | 21 ++++++++++++++++ 4 files changed, 26 insertions(+), 22 deletions(-) rename apps/admin/src/{settings/app/components/settings/membership/custom-fields => shared/member-custom-fields}/custom-field-icon.tsx (100%) create mode 100644 apps/admin/src/shared/member-custom-fields/custom-field-type-option.tsx diff --git a/apps/admin/src/settings/app/components/settings/membership/custom-fields.tsx b/apps/admin/src/settings/app/components/settings/membership/custom-fields.tsx index 163851e7de1..f02999d1a86 100644 --- a/apps/admin/src/settings/app/components/settings/membership/custom-fields.tsx +++ b/apps/admin/src/settings/app/components/settings/membership/custom-fields.tsx @@ -1,4 +1,4 @@ -import CustomFieldIcon from './custom-fields/custom-field-icon'; +import CustomFieldIcon from '@/shared/member-custom-fields/custom-field-icon'; import CustomFieldModal from './custom-fields/custom-field-modal'; import NiceModal from '@ebay/nice-modal-react'; import React, {useEffect, useMemo, useRef, useState} from 'react'; diff --git a/apps/admin/src/settings/app/components/settings/membership/custom-fields/custom-field-modal.tsx b/apps/admin/src/settings/app/components/settings/membership/custom-fields/custom-field-modal.tsx index ef9fc2ba5ee..d5d09684ec7 100644 --- a/apps/admin/src/settings/app/components/settings/membership/custom-fields/custom-field-modal.tsx +++ b/apps/admin/src/settings/app/components/settings/membership/custom-fields/custom-field-modal.tsx @@ -1,6 +1,5 @@ -import CustomFieldIcon from './custom-field-icon'; +import {CustomFieldTypeOption} from '@/shared/member-custom-fields/custom-field-type-option'; import NiceModal, {useModal} from '@ebay/nice-modal-react'; -import React from 'react'; import {Button, DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigger, Field, FieldDescription, FieldError, FieldGroup, FieldLabel, Input, Select, SelectContent, SelectItem, SelectTrigger, SelectValue} from '@tryghost/shade/components'; import {LucideIcon} from '@tryghost/shade/utils'; import {SettingsModal} from '@tryghost/shade/patterns'; @@ -11,24 +10,8 @@ import {useConfirmation} from '@/settings/app/components/providers/confirmation- import {useForm, useHandleError} from '@tryghost/admin-x-framework/hooks'; import type {MemberCustomField} from '@tryghost/admin-x-framework/api/member-custom-fields'; -const typeOptions = memberCustomFieldUserTypes.map(userType => ({value: userType.id, label: userType.label})); - const userTypeById = (id: string) => memberCustomFieldUserTypes.find(userType => userType.id === id) || memberCustomFieldUserTypes[0]; -// Fixed-width so option labels align in a column regardless of icon shape. -const TypeTile: React.FC<{userTypeId: string}> = ({userTypeId}) => ( - - - -); - -const renderTypeOption = (option: {label: string; value: string}) => ( - - - {option.label} - -); - const CustomFieldModal = NiceModal.create<{field?: MemberCustomField}>(({field}) => { const modal = useModal(); const {confirm} = useConfirmation(); @@ -78,7 +61,7 @@ const CustomFieldModal = NiceModal.create<{field?: MemberCustomField}>(({field}) }); const isArchived = field?.status === 'archived'; - const selectedType = typeOptions.find(option => option.value === formState.userTypeId); + const selectedType = userTypeById(formState.userTypeId); // The modal's third action mirrors the field's state: an active field can // be archived, an archived one reactivated. Both confirm first (the @@ -214,10 +197,10 @@ const CustomFieldModal = NiceModal.create<{field?: MemberCustomField}>(({field}) updateForm(state => ({...state, userTypeId: userTypeById(value).id})); }}> - {selectedType && renderTypeOption(selectedType)} + - {typeOptions.map(option => {renderTypeOption(option)})} + {memberCustomFieldUserTypes.map(userType => )} {isEdit && Type can’t be changed after creation} diff --git a/apps/admin/src/settings/app/components/settings/membership/custom-fields/custom-field-icon.tsx b/apps/admin/src/shared/member-custom-fields/custom-field-icon.tsx similarity index 100% rename from apps/admin/src/settings/app/components/settings/membership/custom-fields/custom-field-icon.tsx rename to apps/admin/src/shared/member-custom-fields/custom-field-icon.tsx diff --git a/apps/admin/src/shared/member-custom-fields/custom-field-type-option.tsx b/apps/admin/src/shared/member-custom-fields/custom-field-type-option.tsx new file mode 100644 index 00000000000..6b725848a83 --- /dev/null +++ b/apps/admin/src/shared/member-custom-fields/custom-field-type-option.tsx @@ -0,0 +1,21 @@ +import CustomFieldIcon from './custom-field-icon'; +import {userTypeForFieldType} from '@tryghost/admin-x-framework/api/member-custom-fields'; +import type {MemberCustomField} from '@tryghost/admin-x-framework/api/member-custom-fields'; + +/** + * A field type as it appears in a picker: its icon and its name. + * + * Shared rather than owned by Settings so that wherever a publisher is offered the field + * types, they read the same, and so a type's icon is decided in one place. + */ +export function CustomFieldTypeOption({type}: {type: MemberCustomField['type']}) { + return ( + + {/* Fixed width so labels line up in a column whatever shape the icon is. */} + + + + {userTypeForFieldType(type).label} + + ); +} From 4f724b5889f5ce1ca1d8301489191b9fb2ac9880 Mon Sep 17 00:00:00 2001 From: Renato Costa Date: Thu, 6 Aug 2026 17:39:15 +0300 Subject: [PATCH 09/16] Tied a custom field type's labels and values to its value schema ref https://linear.app/ghost/issue/BER-3859/ A field type's parts were described in two places that nothing held together. The shared catalog derives them from the type's own value schema, while admin hand-writes the label for each one, and the only thing checking that the two still agreed was a hand-applied constraint on the address type alone. A part added upstream reached a publisher as its raw key, and a composite nobody had labelled at all read as a scalar, which is worse: a caller would never think to ask which part it was holding. Presentation is now compile-forced against the value schema for every type, so a part added, removed or renamed upstream fails the build here instead, and the fallback to a raw key is gone. The value a member write may carry was hand-listed the same way, naming address as though it were the only composite there could be; it is derived from the schemas now, so a type added to them is writable without anyone editing that line. The CSV column labels come from the same source, paired to each column by the part it holds rather than by two lists being built in the same order. --- .../src/api/member-custom-fields.ts | 74 ++++++++++++++----- apps/admin-x-framework/src/api/members.ts | 11 ++- .../unit/api/member-custom-fields.test.ts | 43 ++++++++++- packages/custom-field-types/src/csv.ts | 13 +++- packages/custom-field-types/src/index.ts | 8 ++ packages/custom-field-types/test/csv.test.ts | 20 ++--- 6 files changed, 133 insertions(+), 36 deletions(-) diff --git a/apps/admin-x-framework/src/api/member-custom-fields.ts b/apps/admin-x-framework/src/api/member-custom-fields.ts index a01328c136d..30ea6f824bf 100644 --- a/apps/admin-x-framework/src/api/member-custom-fields.ts +++ b/apps/admin-x-framework/src/api/member-custom-fields.ts @@ -1,4 +1,4 @@ -import {FIELD_TYPE_IDS, type Address, type FieldType} from '@tryghost/custom-field-types'; +import {FIELD_TYPES, FIELD_TYPE_IDS, subFieldsOf, type FieldType} from '@tryghost/custom-field-types'; import {csvColumnsForField} from '@tryghost/custom-field-types/csv'; import {Meta, createMutation, createQuery, createQueryWithId} from '../utils/api/hooks'; @@ -41,21 +41,37 @@ export type MemberCustomFieldUserType = { // Which control collects/edits a value of this type input: 'text' | 'textarea' | 'address'; // Composite types only: label per sub-field, keyed by the sub-field key the shared - // value schema defines. Kept with the type's other presentation, not a parallel map. + // value schema defines. Widened from the catalog below, which is exact, because a + // caller resolving a type at runtime cannot know which one it holds. subFields?: Record; }; -// Presentation for every field type in the shared catalog. The explicit -// Record annotation keeps this exhaustive: adding a field type -// upstream fails to compile here until it has a presentation. -const fieldTypePresentation: Record> = { +/** The parts a type's value schema declares, or never for a type whose value is one thing. */ +type PartKeys = + typeof FIELD_TYPES[T] extends {fields: infer F} ? Extract : never; + +/** + * How one field type is presented, constrained by what its value is: a composite names + * every part its schema declares and no others, a scalar names none. + * + * The shared catalog owns which parts exist; this one owns what they are called, so adding, + * removing or renaming a part upstream fails the build here rather than reaching a publisher + * as a raw key. Enforced against a literal, which is how the catalog below is written; a + * pre-widened `Record` would satisfy it. + */ +export type FieldTypePresentation = { + label: string; + input: MemberCustomFieldUserType['input']; +} & ([PartKeys] extends [never] ? {subFields?: never} : {subFields: Record, string>}); + +// Presentation for every field type in the shared catalog. The mapped type keeps this +// exhaustive: adding a field type upstream fails to compile here until it has one. +const fieldTypePresentation: {[T in FieldType]: FieldTypePresentation} = { short_text: {label: 'Short text', input: 'text'}, long_text: {label: 'Long text', input: 'textarea'}, address: { label: 'Address', input: 'address', - // Keyed to the shared value schema's parts (`satisfies`), so a part added, removed, - // or mistyped upstream is a compile error here rather than a silently missing label. subFields: { line1: 'Address line 1', line2: 'Address line 2', @@ -63,10 +79,18 @@ const fieldTypePresentation: Record + } } }; +/** + * A type's part labels, keyed by part; empty for a type with no parts. + * + * Total for every key the value schema declares, which is the only kind of key that + * reaches it: `FieldTypePresentation` refuses to compile a catalog missing one. + */ +const partLabelsFor = (type: FieldType): Record => fieldTypePresentation[type].subFields ?? {}; + // The catalog in the shared catalog's declared order, so every admin surface // offers and renders the field types in the same order. export const memberCustomFieldUserTypes: MemberCustomFieldUserType[] = @@ -93,18 +117,32 @@ export type MemberCustomFieldCsvColumn = {label: string; value: string}; */ export const memberCustomFieldCsvColumns = (fields: MemberCustomField[]): MemberCustomFieldCsvColumn[] => { return fields.flatMap((field) => { - const columns = csvColumnsForField({key: field.key, type: field.type}); - return columns.map((column) => { - if (columns.length === 1) { - return {label: field.name, value: column}; - } - const sub = column.slice(column.lastIndexOf('.') + 1); - const subLabel = userTypeForFieldType(field.type).subFields?.[sub] ?? sub; - return {label: `${field.name} (${subLabel})`, value: column}; - }); + const labels = partLabelsFor(field.type); + return csvColumnsForField({key: field.key, type: field.type}).map(({column, subField}) => ({ + label: subField === null ? field.name : `${field.name} (${labels[subField]})`, + value: column + })); }); }; +/** One part of a composite field type: the key the value schema declares, and its label. */ +export type MemberCustomFieldPart = {key: string; label: string}; + +/** + * The parts of a composite field type, or null for a scalar. + * + * Which parts exist, and in what order, comes from the value schema; naming them is this + * catalog's job. + */ +export const memberCustomFieldParts = (type: FieldType): MemberCustomFieldPart[] | null => { + const partKeys = subFieldsOf(type); + if (!partKeys) { + return null; + } + const labels = partLabelsFor(type); + return partKeys.map(key => ({key, label: labels[key]})); +}; + export interface MemberCustomFieldsResponseType { meta?: Meta; members_custom_fields: MemberCustomField[]; diff --git a/apps/admin-x-framework/src/api/members.ts b/apps/admin-x-framework/src/api/members.ts index ecf04830eeb..ac37fbcbd7d 100644 --- a/apps/admin-x-framework/src/api/members.ts +++ b/apps/admin-x-framework/src/api/members.ts @@ -2,7 +2,7 @@ import {InfiniteData, useIsFetching, useQueryClient} from '@tanstack/react-query import {useEffect} from 'react'; import {Meta, createInfiniteQuery, createMutation, createQuery, createQueryWithId} from '../utils/api/hooks'; import {apiUrl} from '../utils/api/fetch-api'; -import type {Address} from '@tryghost/custom-field-types'; +import type {FieldValue} from '@tryghost/custom-field-types'; import {useCurrentUser} from './current-user'; import {canManageMembers} from './users'; @@ -507,11 +507,10 @@ export interface EditMemberData { newsletters?: Array<{id: string}>; tiers?: Array<{id: string; expiry_at?: string | null}>; // Merge semantics: only the keys present are written; `null` clears a - // value. Values are strings for text-backed fields and composite objects - // for address — every sub-field of which is optional, the server asking - // only that one of them is filled in. Requires the `membersCustomFields` - // flag server-side. - custom_fields?: Record; + // value. The value union is derived from the shared schemas, so a field type + // added there is writable here without this line being edited. Requires the + // `membersCustomFields` flag server-side. + custom_fields?: Record; } export const useEditMember = createMutation({ diff --git a/apps/admin-x-framework/test/unit/api/member-custom-fields.test.ts b/apps/admin-x-framework/test/unit/api/member-custom-fields.test.ts index 89b2f15cc27..16f5472ad05 100644 --- a/apps/admin-x-framework/test/unit/api/member-custom-fields.test.ts +++ b/apps/admin-x-framework/test/unit/api/member-custom-fields.test.ts @@ -1,4 +1,25 @@ -import {type MemberCustomField, memberCustomFieldCsvColumns} from '../../../src/api/member-custom-fields'; +import {type FieldTypePresentation, type MemberCustomField, memberCustomFieldCsvColumns, memberCustomFieldParts} from '../../../src/api/member-custom-fields'; + +// Compile-time cases: the build failing is the assertion. Each `@ts-expect-error` fails the +// build if the case it names stops being an error. Declared on one line each, because the +// directive only covers the line below it and a spread literal reports on its inner line. +const composite = {line1: 'a', line2: 'b', city: 'c', state: 'd', postal_code: 'e', country: 'f'}; + +const labelled: FieldTypePresentation<'address'> = {label: 'Address', input: 'address', subFields: composite}; + +// @ts-expect-error a composite missing one of the parts its value schema declares +const missingPart: FieldTypePresentation<'address'> = {label: 'Address', input: 'address', subFields: {line1: 'a', line2: 'b', city: 'c', state: 'd', country: 'f'}}; + +// @ts-expect-error a composite naming a part its value schema does not declare +const unknownPart: FieldTypePresentation<'address'> = {label: 'Address', input: 'address', subFields: {...composite, county: 'g'}}; + +// @ts-expect-error a composite with no part labels at all +const unlabelled: FieldTypePresentation<'address'> = {label: 'Address', input: 'address'}; + +// @ts-expect-error a type whose value is one thing has no parts to name +const scalarWithParts: FieldTypePresentation<'short_text'> = {label: 'Short text', input: 'text', subFields: {line1: 'a'}}; + +export {labelled, missingPart, unknownPart, unlabelled, scalarWithParts}; const field = (overrides: Partial): MemberCustomField => ({ key: 'nickname', @@ -35,4 +56,24 @@ describe('member custom fields api helpers', () => { expect(memberCustomFieldCsvColumns([])).toEqual([]); }); }); + + describe('memberCustomFieldParts', () => { + // The null is the contract a caller branches on to tell a composite from a + // scalar, so it is pinned by name rather than only through the CSV columns. + it('has no parts for a scalar type', () => { + expect(memberCustomFieldParts('short_text')).toBeNull(); + expect(memberCustomFieldParts('long_text')).toBeNull(); + }); + + it('names a composite type\'s parts in the order its value schema declares them', () => { + expect(memberCustomFieldParts('address')).toEqual([ + {key: 'line1', label: 'Address line 1'}, + {key: 'line2', label: 'Address line 2'}, + {key: 'city', label: 'City'}, + {key: 'state', label: 'State'}, + {key: 'postal_code', label: 'Postal code'}, + {key: 'country', label: 'Country'} + ]); + }); + }); }); diff --git a/packages/custom-field-types/src/csv.ts b/packages/custom-field-types/src/csv.ts index e36c1876070..3f8a1945d12 100644 --- a/packages/custom-field-types/src/csv.ts +++ b/packages/custom-field-types/src/csv.ts @@ -45,12 +45,21 @@ function isBlank(cell: string): boolean { return cell.trim() === ''; } +/** A column a field occupies, and which part of the field's value it holds. */ +export interface CsvFieldColumn { + column: string; + /** The part this column holds, or null where the field's whole value is one column. */ + subField: string | null; +} + // Shares csvCellsForFields' column derivation, so a field is written, read, and offered // as a mapping target under one set of column names. -export function csvColumnsForField(field: CsvField): string[] { +export function csvColumnsForField(field: CsvField): CsvFieldColumn[] { const column = `${NAMESPACE}${SEPARATOR}${field.key}`; const subFields = subFieldsOf(field.type); - return subFields ? subFields.map(sub => `${column}${SEPARATOR}${sub}`) : [column]; + return subFields + ? subFields.map(sub => ({column: `${column}${SEPARATOR}${sub}`, subField: sub})) + : [{column, subField: null}]; } export function isCustomFieldColumn(column: string): boolean { diff --git a/packages/custom-field-types/src/index.ts b/packages/custom-field-types/src/index.ts index 70543d36275..5d515fbf86c 100644 --- a/packages/custom-field-types/src/index.ts +++ b/packages/custom-field-types/src/index.ts @@ -185,6 +185,14 @@ export const FIELD_TYPES = defineFieldTypes({ export const AddressValue = FIELD_TYPES.address.value; export type Address = z.infer; +/** + * A value of any field type, as a caller holding a field of unknown type must accept it. + * + * Derived from the schemas rather than listed, so a type added here widens it without + * anyone remembering to. + */ +export type FieldValue = {[T in FieldType]: z.infer}[FieldType]; + /** The parts of a record type in declaration order, or null for a type with none. */ export function subFieldsOf(type: FieldType): string[] | null { // Through the interface, not the literal: a type with no parts has no `fields` key. diff --git a/packages/custom-field-types/test/csv.test.ts b/packages/custom-field-types/test/csv.test.ts index 0decfe55090..9e6fc0b2976 100644 --- a/packages/custom-field-types/test/csv.test.ts +++ b/packages/custom-field-types/test/csv.test.ts @@ -80,18 +80,20 @@ describe('custom field CSV cells', function () { // The column names are the vocabulary the admin offers as import mapping targets and // the error report echoes, so they are derived from the same primitives the cells are. describe('custom field CSV columns', function () { - it('gives a scalar field one namespaced column', function () { - assert.deepEqual(csvColumnsForField({key: 'nickname', type: 'short_text'}), ['custom_fields.nickname']); + it('gives a scalar field one namespaced column holding no particular part', function () { + assert.deepEqual(csvColumnsForField({key: 'nickname', type: 'short_text'}), [ + {column: 'custom_fields.nickname', subField: null} + ]); }); - it('gives a composite field one column per sub-field', function () { + it('gives a composite field one column per sub-field, each naming the part it holds', function () { assert.deepEqual(csvColumnsForField({key: 'shipping_address', type: 'address'}), [ - 'custom_fields.shipping_address.line1', - 'custom_fields.shipping_address.line2', - 'custom_fields.shipping_address.city', - 'custom_fields.shipping_address.state', - 'custom_fields.shipping_address.postal_code', - 'custom_fields.shipping_address.country' + {column: 'custom_fields.shipping_address.line1', subField: 'line1'}, + {column: 'custom_fields.shipping_address.line2', subField: 'line2'}, + {column: 'custom_fields.shipping_address.city', subField: 'city'}, + {column: 'custom_fields.shipping_address.state', subField: 'state'}, + {column: 'custom_fields.shipping_address.postal_code', subField: 'postal_code'}, + {column: 'custom_fields.shipping_address.country', subField: 'country'} ]); }); From 399acf3bf01a0ac83346a226df7cf5b8de31f118 Mon Sep 17 00:00:00 2001 From: Rob Lester Date: Wed, 12 Aug 2026 14:53:22 +0100 Subject: [PATCH 10/16] Read the address editor's parts from the shared catalog ref https://linear.app/ghost/issue/BER-3859/ The member detail editor kept its own list of the address composite's parts, so the parts existed in three places: the value schema that declares them, the catalog that labels them, and this tuple. The tuple also typed the editable value through a Pick, which catches a part being removed or renamed upstream but not one being added, so a new part would have been quietly missing from the editor and from the trimming both the draft and the save run over it. The editor now takes the parts, in the order the schema declares them and under the labels every other surface uses, from the same accessor the import mapping reads. Part keys carry their own type out of the shared catalog now, so a consumer indexes a value by one without restating which parts exist. The formatted address line still names its parts one by one, because where each sits in the sentence is a fact about how an address reads rather than one the schema can supply. --- .../src/api/member-custom-fields.ts | 12 +++------ .../detail/member-custom-fields-field.tsx | 11 +++----- .../src/members/detail/member-detail-edit.ts | 26 ++++++++++--------- .../members/detail/member-detail-format.ts | 8 +++++- packages/custom-field-types/src/index.ts | 22 +++++++++++++--- 5 files changed, 48 insertions(+), 31 deletions(-) diff --git a/apps/admin-x-framework/src/api/member-custom-fields.ts b/apps/admin-x-framework/src/api/member-custom-fields.ts index 30ea6f824bf..93328278431 100644 --- a/apps/admin-x-framework/src/api/member-custom-fields.ts +++ b/apps/admin-x-framework/src/api/member-custom-fields.ts @@ -1,4 +1,4 @@ -import {FIELD_TYPES, FIELD_TYPE_IDS, subFieldsOf, type FieldType} from '@tryghost/custom-field-types'; +import {FIELD_TYPE_IDS, subFieldsOf, type FieldType, type PartsOf} from '@tryghost/custom-field-types'; import {csvColumnsForField} from '@tryghost/custom-field-types/csv'; import {Meta, createMutation, createQuery, createQueryWithId} from '../utils/api/hooks'; @@ -46,10 +46,6 @@ export type MemberCustomFieldUserType = { subFields?: Record; }; -/** The parts a type's value schema declares, or never for a type whose value is one thing. */ -type PartKeys = - typeof FIELD_TYPES[T] extends {fields: infer F} ? Extract : never; - /** * How one field type is presented, constrained by what its value is: a composite names * every part its schema declares and no others, a scalar names none. @@ -62,7 +58,7 @@ type PartKeys = export type FieldTypePresentation = { label: string; input: MemberCustomFieldUserType['input']; -} & ([PartKeys] extends [never] ? {subFields?: never} : {subFields: Record, string>}); +} & ([PartsOf] extends [never] ? {subFields?: never} : {subFields: Record, string>}); // Presentation for every field type in the shared catalog. The mapped type keeps this // exhaustive: adding a field type upstream fails to compile here until it has one. @@ -126,7 +122,7 @@ export const memberCustomFieldCsvColumns = (fields: MemberCustomField[]): Member }; /** One part of a composite field type: the key the value schema declares, and its label. */ -export type MemberCustomFieldPart = {key: string; label: string}; +export type MemberCustomFieldPart = {key: PartsOf; label: string}; /** * The parts of a composite field type, or null for a scalar. @@ -134,7 +130,7 @@ export type MemberCustomFieldPart = {key: string; label: string}; * Which parts exist, and in what order, comes from the value schema; naming them is this * catalog's job. */ -export const memberCustomFieldParts = (type: FieldType): MemberCustomFieldPart[] | null => { +export const memberCustomFieldParts = (type: T): MemberCustomFieldPart[] | null => { const partKeys = subFieldsOf(type); if (!partKeys) { return null; diff --git a/apps/admin/src/members/detail/member-custom-fields-field.tsx b/apps/admin/src/members/detail/member-custom-fields-field.tsx index fe62c5e9b0c..744cb898548 100644 --- a/apps/admin/src/members/detail/member-custom-fields-field.tsx +++ b/apps/admin/src/members/detail/member-custom-fields-field.tsx @@ -2,10 +2,10 @@ import React from 'react'; import {Button, Card, CardContent, Dialog, DialogContent, DialogFooter, DialogHeader, DialogTitle, Input, Label, LoadingIndicator, Textarea} from '@tryghost/shade/components'; import {LucideIcon} from '@tryghost/shade/utils'; import {dequal} from 'dequal'; -import {ADDRESS_SUBFIELD_KEYS, buildCustomFieldSavePayload, getCustomFieldValidationErrors, getEditableCustomFieldValues, parseCustomFieldServerErrors} from './member-detail-edit'; +import {ADDRESS_PARTS, buildCustomFieldSavePayload, getCustomFieldValidationErrors, getEditableCustomFieldValues, parseCustomFieldServerErrors} from './member-detail-edit'; import {formatAddressValue} from './member-detail-format'; import {toast} from 'sonner'; -import {useBrowseMemberCustomFields, userTypeForField, userTypeForFieldType} from '@tryghost/admin-x-framework/api/member-custom-fields'; +import {useBrowseMemberCustomFields, userTypeForField} from '@tryghost/admin-x-framework/api/member-custom-fields'; import {useEditMember} from '@tryghost/admin-x-framework/api/members'; import type {EditableAddressValue, EditableCustomFieldValue} from './member-detail-edit'; import type {MemberCustomField} from '@tryghost/admin-x-framework/api/member-custom-fields'; @@ -19,9 +19,6 @@ interface MemberCustomFieldsFieldProps { disabled?: boolean; } -// Shared with the CSV import mapping so a sub-field reads the same on every surface. -const ADDRESS_SUBFIELD_LABELS = userTypeForFieldType('address').subFields ?? {}; - // role='alert': after a save-attempt these render while focus stays on the Save // button, so an assertive live region is the only way a screen reader hears the // failure. @@ -43,12 +40,12 @@ const AddressInput: React.FC<{ }> = ({inputId, value, errors, disabled, onChange}) => { return (
- {ADDRESS_SUBFIELD_KEYS.map((subfield) => { + {ADDRESS_PARTS.map(({key: subfield, label}) => { const subfieldId = `${inputId}-${subfield}`; const error = errors?.[subfield]; return (
- + >; +// The parts of the address composite, in the order its value schema declares them, each +// with the label every other surface shows it under. +export const ADDRESS_PARTS = memberCustomFieldParts('address') ?? []; + +// Partial because a draft mid-edit (or a normalized sparse value) may hold any subset; the +// shared AddressValue schema — enforced by the server — decides completeness. +export type EditableAddressValue = Partial; export type EditableCustomFieldValue = string | EditableAddressValue; export interface MemberEditableLabel { @@ -99,10 +101,10 @@ export function getEditableCustomFieldValues(customFields: Record): EditableAddressValue | undefined { const address: EditableAddressValue = {}; - for (const subfield of ADDRESS_SUBFIELD_KEYS) { - const subvalue = value[subfield]; + for (const {key} of ADDRESS_PARTS) { + const subvalue = value[key]; if (typeof subvalue === 'string') { - address[subfield] = subvalue.trim(); + address[key] = subvalue.trim(); } } return Object.values(address).some(part => part !== '') ? address : undefined; @@ -115,10 +117,10 @@ function addressToSave(value: Record): EditableAddressValue | u */ function normalizeAddressValue(value: Record): EditableAddressValue | undefined { const address: EditableAddressValue = {}; - for (const subfield of ADDRESS_SUBFIELD_KEYS) { - const subvalue = value[subfield]; + for (const {key} of ADDRESS_PARTS) { + const subvalue = value[key]; if (typeof subvalue === 'string' && subvalue.trim() !== '') { - address[subfield] = subvalue.trim(); + address[key] = subvalue.trim(); } } return Object.keys(address).length ? address : undefined; diff --git a/apps/admin/src/members/detail/member-detail-format.ts b/apps/admin/src/members/detail/member-detail-format.ts index 2e31d6c7d7d..383915186e4 100644 --- a/apps/admin/src/members/detail/member-detail-format.ts +++ b/apps/admin/src/members/detail/member-detail-format.ts @@ -1,3 +1,5 @@ +import type {MemberCustomFieldAddress} from '@tryghost/admin-x-framework/api/member-custom-fields'; + export interface MemberGeolocation { country_code?: string; country?: string; @@ -54,8 +56,12 @@ export function formatMemberLocation(rawGeolocation: string | null | undefined): * "1 Main St, 12 apt B, New York, NY 00001, US". State and postal code pair * up the way people write them; whatever sub-fields are missing simply drop * out, so a partial address still reads naturally. + * + * The parts are named one by one rather than walked, because where each sits in the + * sentence is a fact about how an address reads, not one the value schema can supply. A + * part added upstream will not appear here until someone decides where it belongs. */ -export function formatAddressValue(address: Partial>): string { +export function formatAddressValue(address: Partial): string { const statePostal = [address.state, address.postal_code].filter(Boolean).join(' '); return [address.line1, address.line2, address.city, statePostal, address.country] .filter(Boolean) diff --git a/packages/custom-field-types/src/index.ts b/packages/custom-field-types/src/index.ts index 5d515fbf86c..74755f01686 100644 --- a/packages/custom-field-types/src/index.ts +++ b/packages/custom-field-types/src/index.ts @@ -193,9 +193,25 @@ export type Address = z.infer; */ export type FieldValue = {[T in FieldType]: z.infer}[FieldType]; -/** The parts of a record type in declaration order, or null for a type with none. */ -export function subFieldsOf(type: FieldType): string[] | null { +/** + * The parts a record type declares, or never for a type whose value is a single thing. + * + * Distributed over `T`, so a caller holding a type it only knows as `FieldType` gets every + * part any type declares rather than the empty intersection of all of them. + */ +export type PartsOf = T extends FieldType + ? typeof FIELD_TYPES[T] extends {fields: infer F} ? Extract : never + : never; + +/** + * The parts of a record type in declaration order, or null for a type with none. + * + * Typed to the parts the caller's type declares, so a caller holding one of these can + * index a value of that type without restating which parts exist. + */ +export function subFieldsOf(type: T): PartsOf[] | null { // Through the interface, not the literal: a type with no parts has no `fields` key. const {fields}: FieldTypeDefinition = FIELD_TYPES[type]; - return fields ? Object.keys(fields) : null; + // The keys are `PartsOf` by construction: `fields` is the object it reads `keyof` from. + return fields ? Object.keys(fields) as PartsOf[] : null; } From dd087ab80794df1425ff8e51d39601f69a612652 Mon Sep 17 00:00:00 2001 From: Rob Lester Date: Wed, 12 Aug 2026 15:28:45 +0100 Subject: [PATCH 11/16] Tightened what the custom field catalogs claim about themselves ref https://linear.app/ghost/issue/BER-3859/ Three small things the previous commits left slightly untrue. The presentation catalog's own comment said it owned a type's icon, which it never has, so a reader consolidating presentation would have moved things toward a file that does not hold them. The widened sub-field map on the exported user type had no reader left once labels were resolved from the catalog directly, and leaving it there invited the next consumer to take the untyped path around the accessor that was just added. The editor's control switch fell through to rendering nothing for an input it did not recognise, which reads as tolerance but guards nothing, because the input comes from the catalog rather than the server; typed as never, a control added to the catalog now fails the build instead. The shared package also records where the line between it and admin actually falls, which is whether more than one renderer has to agree on a string rather than presentation against validity, since it already ships the sentences a broken rule shows. --- .../src/api/member-custom-fields.ts | 10 ++++------ .../detail/member-custom-fields-field.tsx | 16 ++++++++++++---- packages/custom-field-types/src/index.ts | 5 +++++ 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/apps/admin-x-framework/src/api/member-custom-fields.ts b/apps/admin-x-framework/src/api/member-custom-fields.ts index 93328278431..580f903f6ed 100644 --- a/apps/admin-x-framework/src/api/member-custom-fields.ts +++ b/apps/admin-x-framework/src/api/member-custom-fields.ts @@ -30,20 +30,18 @@ export type MemberCustomField = { * The user-type catalog: the presentation layer over the shared field types. * * The shared catalog (@tryghost/custom-field-types) owns what a field type *is* - * - its storage and validation. This catalog owns what it *looks like* in admin: - * label, icon, and which control collects a value. Admin surfaces (settings + * - its storage and validation. This catalog owns what a publisher is told it is: + * its name, and which control collects a value. Admin surfaces (settings * list/modal, member detail) render from here so every surface presents fields * identically. The backend never sees any of this. + * + * The icon is not here: it is a component, so it sits with admin's, under the same type ids. */ export type MemberCustomFieldUserType = { id: FieldType; label: string; // Which control collects/edits a value of this type input: 'text' | 'textarea' | 'address'; - // Composite types only: label per sub-field, keyed by the sub-field key the shared - // value schema defines. Widened from the catalog below, which is exact, because a - // caller resolving a type at runtime cannot know which one it holds. - subFields?: Record; }; /** diff --git a/apps/admin/src/members/detail/member-custom-fields-field.tsx b/apps/admin/src/members/detail/member-custom-fields-field.tsx index 744cb898548..a81d66fc591 100644 --- a/apps/admin/src/members/detail/member-custom-fields-field.tsx +++ b/apps/admin/src/members/detail/member-custom-fields-field.tsx @@ -61,10 +61,15 @@ const AddressInput: React.FC<{ ); }; +// Exists so the switch has to account for every control the catalog can name. +function assertNoControl(input: never): null { + void input; + return null; +} + // The control for one field, picked from the presentation catalog's `input` // hint — the same hint the collection forms render from, so the editor and -// the member-facing forms never diverge per type. Unknown future inputs -// render nothing rather than degrading to a wrong text input. +// the member-facing forms never diverge per type. const CustomFieldInput: React.FC<{ field: MemberCustomField; inputId: string; @@ -75,7 +80,8 @@ const CustomFieldInput: React.FC<{ onChange: (value: EditableCustomFieldValue) => void; }> = ({field, inputId, value, errors, disabled, onChange}) => { const fieldError = errors?.['']; - switch (userTypeForField(field).input) { + const {input} = userTypeForField(field); + switch (input) { case 'text': return onChange(e.target.value)} />; case 'textarea': @@ -93,7 +99,9 @@ const CustomFieldInput: React.FC<{ /> ); default: - return null; + // Unreachable: `input` comes from the catalog, not the server. Typed never so a + // control added to the catalog fails the build here rather than rendering nothing. + return assertNoControl(input); } }; diff --git a/packages/custom-field-types/src/index.ts b/packages/custom-field-types/src/index.ts index 74755f01686..daffba0bd19 100644 --- a/packages/custom-field-types/src/index.ts +++ b/packages/custom-field-types/src/index.ts @@ -48,6 +48,11 @@ import {z} from 'zod'; * rule was broken. No storage: columns and codecs belong to the backend. One exception * lives in `./csv` — how a value maps onto CSV columns — because both tiers need the same * answer and a disagreement between them is a file that silently stops round-tripping. + * + * The line is whether more than one renderer must agree on a string, not presentation + * against validity — the sentences above are presentation. Admin is the only renderer + * today, so a part's label lives there, held against this file by a type. A Portal + * collection form, which cannot reach admin's packages, moves the labels here. */ /** The source for the union type, the zod enum and the `FIELD_TYPES` keys alike. */ From 6cebc38ea2b6dea935e0e1f76af6e17e1a1afbc5 Mon Sep 17 00:00:00 2001 From: Rob Lester Date: Wed, 12 Aug 2026 15:31:24 +0100 Subject: [PATCH 12/16] Stopped a custom field type from the future taking the surface down ref https://linear.app/ghost/issue/BER-3859/ A field's type arrives as a string off the wire and is asserted rather than checked, so an admin build older than the server it talks to is handed a type it has never heard of. Two places read that type by indexing a catalog and using the result immediately, which threw on the way to building the import mapping targets. The editor and its validation had already decided what to do about a type this build does not know, twice, in comments that say the server stays authoritative, so the reading now agrees with them: an unheard-of type has no parts and no part labels, and degrades to a single whole-field column instead of failing the surface. This crash predates the rest of this branch and is pinned by a test that reproduces it. --- apps/admin-x-framework/src/api/member-custom-fields.ts | 8 ++++++-- .../test/unit/api/member-custom-fields.test.ts | 10 ++++++++++ packages/custom-field-types/src/index.ts | 6 ++++-- packages/custom-field-types/test/index.test.ts | 8 ++++++++ 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/apps/admin-x-framework/src/api/member-custom-fields.ts b/apps/admin-x-framework/src/api/member-custom-fields.ts index 580f903f6ed..b289bf03ea0 100644 --- a/apps/admin-x-framework/src/api/member-custom-fields.ts +++ b/apps/admin-x-framework/src/api/member-custom-fields.ts @@ -78,12 +78,16 @@ const fieldTypePresentation: {[T in FieldType]: FieldTypePresentation} = { }; /** - * A type's part labels, keyed by part; empty for a type with no parts. + * A type's part labels, keyed by part; empty for a type with no parts, and for one this + * build has never heard of. * * Total for every key the value schema declares, which is the only kind of key that * reaches it: `FieldTypePresentation` refuses to compile a catalog missing one. */ -const partLabelsFor = (type: FieldType): Record => fieldTypePresentation[type].subFields ?? {}; +const partLabelsFor = (type: FieldType): Record => { + const labels: Record | undefined = fieldTypePresentation[type]?.subFields; + return labels ?? {}; +}; // The catalog in the shared catalog's declared order, so every admin surface // offers and renders the field types in the same order. diff --git a/apps/admin-x-framework/test/unit/api/member-custom-fields.test.ts b/apps/admin-x-framework/test/unit/api/member-custom-fields.test.ts index 16f5472ad05..e1d0dc8b8d3 100644 --- a/apps/admin-x-framework/test/unit/api/member-custom-fields.test.ts +++ b/apps/admin-x-framework/test/unit/api/member-custom-fields.test.ts @@ -55,6 +55,16 @@ describe('member custom fields api helpers', () => { it('returns no targets for an empty field set', () => { expect(memberCustomFieldCsvColumns([])).toEqual([]); }); + + // An admin build older than the server it talks to is handed a type it has no + // presentation for. The mapping picker offering one fewer column beats it throwing. + it('offers a whole-column target for a type it has never heard of', () => { + const future = field({key: 'mystery', name: 'Mystery', type: 'a_type_from_the_future' as MemberCustomField['type']}); + + expect(memberCustomFieldCsvColumns([future])).toEqual([ + {label: 'Mystery', value: 'custom_fields.mystery'} + ]); + }); }); describe('memberCustomFieldParts', () => { diff --git a/packages/custom-field-types/src/index.ts b/packages/custom-field-types/src/index.ts index daffba0bd19..f6fee677999 100644 --- a/packages/custom-field-types/src/index.ts +++ b/packages/custom-field-types/src/index.ts @@ -216,7 +216,9 @@ export type PartsOf = T extends FieldType */ export function subFieldsOf(type: T): PartsOf[] | null { // Through the interface, not the literal: a type with no parts has no `fields` key. - const {fields}: FieldTypeDefinition = FIELD_TYPES[type]; + // Optional because a caller built against an older catalog than the server it talks to + // reaches here with a type this build has never heard of, which reads as no parts. + const definition: FieldTypeDefinition | undefined = FIELD_TYPES[type]; // The keys are `PartsOf` by construction: `fields` is the object it reads `keyof` from. - return fields ? Object.keys(fields) as PartsOf[] : null; + return definition?.fields ? Object.keys(definition.fields) as PartsOf[] : null; } diff --git a/packages/custom-field-types/test/index.test.ts b/packages/custom-field-types/test/index.test.ts index ebd6aba49b7..a7c02e4f109 100644 --- a/packages/custom-field-types/test/index.test.ts +++ b/packages/custom-field-types/test/index.test.ts @@ -19,6 +19,14 @@ describe('custom-field-types catalog', function () { }); }); + it('reads a type it has never heard of as having no parts', function () { + // Only reachable by lying about the type, which is what an admin build older than + // the server it talks to does: the type is a string off the wire, asserted not + // checked. Failing here would take the surface down over a field it merely cannot + // render, so it degrades and the server stays authoritative. + assert.equal(subFieldsOf('a_type_from_the_future' as FieldType), null); + }); + describe('text is trimmed, whatever type it belongs to', function () { // Trimming decides whether a value is stored at all: a value that trims to // nothing is a clear. Two text types disagreeing about that would mean the same From 4ca74b07329a41bbd3d6a6e2b94918d41c727d17 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Wed, 12 Aug 2026 16:20:22 +0100 Subject: [PATCH 13/16] Excluded GitHub docs from Core CI tests (#29914) Documentation-only changes to `.github/CONTRIBUTING.md` currently trigger the MySQL and SQLite acceptance and legacy test matrices because `.github/**` is shared by the Core filter. Those files cannot affect Ghost runtime behaviour, so excluding them will reduce unnecessary CI time on documentation changes. --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f3df26033d9..e438206b80e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -145,6 +145,10 @@ jobs: - 'scripts/test/check-agent-skill-links.test.js' core: - *shared + # Repository documentation and ownership metadata do not affect + # Ghost runtime behaviour, even though they live in .github. + - '!.github/**/*.md' + - '!.github/CODEOWNERS' - 'ghost/**' - '!ghost/core/core/server/data/tinybird/**' # Unit tests + vitest config are exercised only by job_unit-tests; From bfb342eed5b782b4fbcb2da9f385b3f07cc865d9 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Wed, 12 Aug 2026 16:39:57 +0100 Subject: [PATCH 14/16] Added foundational codebase documentation (#29913) Established `/docs` as the canonical home for documentation about working on the Ghost codebase. Added current guides for development setup, contribution workflow, testing, shipping, and monorepo structure. Kept `.github/CONTRIBUTING.md` focused on contributor-specific requirements, with direct links into the codebase documentation. Updated the root README and docs index so contributors, developers, and self-hosters are routed to the appropriate documentation. Removed duplicated setup, command, and monorepo guidance from `AGENTS.md`. Shared facts now live in human-readable codebase docs, while `AGENTS.md` retains agent-specific workflow and implementation constraints. --- .github/CONTRIBUTING.md | 48 ++--- AGENTS.md | 243 ++----------------------- README.md | 6 +- docs/README.md | 127 +++++++------ docs/codebase/monorepo-structure.md | 129 +++++++++++++ docs/contributing/development-setup.md | 189 +++++++++++++++++++ docs/contributing/shipping.md | 87 +++++++++ docs/contributing/testing.md | 152 ++++++++++++++++ docs/contributing/workflow.md | 152 ++++++++++++++++ 9 files changed, 816 insertions(+), 317 deletions(-) create mode 100644 docs/codebase/monorepo-structure.md create mode 100644 docs/contributing/development-setup.md create mode 100644 docs/contributing/shipping.md create mode 100644 docs/contributing/testing.md create mode 100644 docs/contributing/workflow.md diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 558a85019a8..631f889097b 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -6,28 +6,21 @@ For **help**, **support**, **questions** and **ideas** please use **[our forum]( ## Where to Start -If you're a developer looking to contribute, but you're not sure where to begin: Check out the [good first issue](https://github.com/TryGhost/Ghost/labels/good%20first%20issue) label on Github, which contains small piece of work that have been specifically flagged as being friendly to new contributors. +The [codebase documentation](../docs/README.md) explains how to set up the +monorepo and find your way around it. Start with the +[development setup guide](../docs/contributing/development-setup.md), then use +the [contribution workflow](../docs/contributing/workflow.md) when you are ready +to make a change. -After that, if you're looking for something a little more challenging to sink your teeth into, there's a broader [help wanted](https://github.com/TryGhost/Ghost/labels/help%20wanted) label encompassing issues which need some love. +If you're not sure what to work on, start with +[good first issues](https://github.com/TryGhost/Ghost/labels/good%20first%20issue) +or browse the broader +[help wanted](https://github.com/TryGhost/Ghost/labels/help%20wanted) list. -If you've got an idea for a new feature, please start by suggesting it in the [forum](https://forum.ghost.org), as adding new features to Ghost first requires generating consensus around a design and spec. +Discuss new features and substantial product or architectural changes in the +[forum](https://forum.ghost.org) before implementing them. - -## Working on Ghost Core - -If you're going to work on Ghost core you'll need to go through a slightly more involved install and setup process than the usual Ghost CLI version. - -First you'll need to fork [Ghost](https://github.com/tryghost/ghost) to your personal Github account, and then follow the detailed [install from source](https://ghost.org/docs/install/source/) setup guide. - - -### Branching Guide - -`main` on the main repository always contains the latest changes. This means that it is WIP for the next minor version and should NOT be considered stable. Stable versions are tagged using [semantic versioning](http://semver.org/). - -On your local repository, you should always work on a branch to make keeping up-to-date and submitting pull requests easier, but in most cases you should submit your pull requests to `main`. Where necessary, for example if multiple people are contributing on a large feature, or if a feature requires a database change, we make use of feature branches. - - -### Commit Messages +## Commit Messages We have a handful of simple standards for commit messages which help us to generate readable changelogs. Please follow this wherever possible and mention the associated issue number. @@ -58,10 +51,9 @@ There is no need to include what modules have changed in the commit message, as [Good example](https://github.com/TryGhost/Ghost/commit/95751a0e5fb719bb5bca74cb97fb5f29b225094f) +## Changesets -### Changesets - -Ghost publishes several workspace packages to npm — the `@tryghost/*` editor and adapter packages under `koenig/` and `packages/`. When your change touches one of these publishable packages, add a **changeset** so it gets a version bump and a changelog entry: +Ghost publishes several workspace packages to npm — the `@tryghost/*` editor and adapter packages under `koenig/` and `packages/`. When your change affects one of these publishable packages, including by changing a catalog entry it consumes, add a **changeset** so it gets a version bump and a changelog entry: ```bash pnpm change @@ -73,18 +65,18 @@ This records which packages changed and the bump type (patch / minor / major); t pnpm change --bump none ``` -CI enforces this — the **Check app version bump** job fails a pull request that modifies a publishable package without a covering changeset. The pre-commit hook prints a non-blocking reminder locally, and `pnpm change status` shows what's currently pending. +CI enforces this — the **Check app version bump** job fails a pull request that affects a publishable package without a covering changeset. The pre-commit hook prints a non-blocking reminder locally, and `pnpm change status` shows what's currently pending. +For more detail, see the [contribution workflow](../docs/contributing/workflow.md). -### Submitting Pull Requests +## Submitting Pull Requests -We aim to merge any straightforward, well-understood bug fixes or improvements immediately, as long as they pass our tests (run `pnpm test` to check locally). We generally don’t merge new features and larger changes without prior discussion with the core product team for tech/design specification. +We aim to merge any straightforward, well-understood bug fixes or improvements immediately, as long as they pass our tests (run `pnpm check` to ensure everything works). We generally don’t merge new features and larger changes without prior discussion with the core product team for tech/design specification. Please provide plenty of context and reasoning around your changes, to help us merge quickly. Closing an already open issue is our preferred workflow. If your PR gets out of date, we may ask you to rebase as you are more familiar with your changes than we will be. -### Sharing feedback on Documentation - -While the Docs are no longer Open Source, we welcome revisions and ideas on the forum! Please create a Post with your questions or suggestions in the [Contributing to Ghost Category](https://forum.ghost.org/c/contributing/27). Thank you for helping us keep the Docs relevant and up-to-date. +For branch, validation, and pull request details, follow the +[contribution workflow](../docs/contributing/workflow.md). --- diff --git a/AGENTS.md b/AGENTS.md index faf348dc65e..4ee650360a1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,223 +2,33 @@ This file provides guidance to AI Agents when working with code in this repository. -## Package Manager - -**Always use `pnpm` for all commands.** This repository uses pnpm workspaces, not npm. - -Shared dependency versions are pinned in `pnpm-workspace.yaml` under `catalog:` and referenced as `"pkg": "catalog:"` (or `catalog:` for named catalogs). `catalogMode` is `strict`, so `pnpm add` routes new deps into the catalog automatically — don't inline the version. - -## Monorepo Structure - -Ghost is a pnpm + Nx monorepo with four workspace groups: - -### ghost/* - Core Ghost packages -- **ghost/core** - Main Ghost application (Node.js/Express backend) - - Core server: `ghost/core/core/server/` - - Frontend rendering: `ghost/core/core/frontend/` - -### apps/* - React-based UI applications -Two categories of apps: - -**Admin Apps** (embedded in Ghost Admin): -- `ember-admin` - Ember.js admin client (legacy, being migrated to React) -- `admin` - The consolidated React admin shell, organized by domain (`src/{analytics,members,posts,tags,comments,automations,settings,...}`) -- `activitypub` - ActivityPub integration (route-composed into `admin`) -- Built with Vite + React + `@tanstack/react-query` - -**Public Apps** (served to site visitors): -- `portal`, `comments-ui`, `signup-form`, `sodo-search`, `announcement-bar` -- Built as UMD bundles, loaded via CDN in site themes - -**Foundation Libraries**: -- `admin-x-framework` - Shared API hooks, routing, utilities -- `admin-x-design-system` - Legacy design system (being phased out) -- `shade` - New design system (shadcn/ui + Radix UI + react-hook-form + zod) - -### koenig/* - Ghost editor (Koenig) packages -Merged from the former TryGhost/Koenig repo with full git history: - -- **koenig-lexical** - The Lexical-based rich text editor UI. Bundled into - Ghost Admin at build time (`apps/ember-admin` copies its UMD build into admin - assets; `apps/admin` imports it directly) -- **kg-*** - Editor support packages: server-side renderers and converters - consumed by `ghost/core` (kg-default-nodes, kg-lexical-html-renderer, - kg-html-to-lexical, ...) plus frontend helpers (kg-unsplash-selector) - -All Koenig packages resolve via `workspace:` — nothing in dev, CI, or the -release archive installs them from npm. They are published to npm for -external consumers only, automatically as part of the Ghost release lane -(see `publish_koenig_packages` in ci.yml). - -**Zero-build dev via the `source` export condition.** The `kg-*` libraries -consumed by `ghost/core` declare a `source` condition in their `package.json` -`exports` that points at the raw `src/*.ts`, listed *before* -`types`/`import`/`require`: - -```jsonc -".": { - "source": "./src/index.ts", // dev/test: read raw TS - "types": "./build/esm/index.d.ts", - "import": "./build/esm/index.js", - "require": "./build/cjs/index.js" // prod/published: compiled JS -} -``` - -`ghost/core`'s dev runner (`nodemon.json`: `node --conditions=source --import=tsx`) -and its Vitest configs (`resolve.conditions: ['source', 'node']` + -`--import tsx --conditions=source`) activate this condition, so a source change -in a `kg-*` package is picked up with **no `tsc` rebuild**. Production and the -published npm tarball run plain `node`, which ignores `source` and uses -`build/` — and `src/` is excluded from each package's `files` array, so it is -never shipped. The separate ESM and CommonJS outputs are part of Koenig's public -package contract; new internal packages use the ESM-only shape documented below. - -### packages/* - Shared workspace libraries -Backend and shared libraries. Internal packages are consumed via `workspace:*`; -selected adapter bases also have supported public releases: - -Read [`packages/README.md`](packages/README.md) before creating or modernizing an -internal package. It is the canonical lifetime contract; `packages/_template` -is its scaffold. - -- **i18n** - Centralized internationalization for all apps -- **parse-email-address** - Email address parsing -- **adapters/** - Adapter base classes (`adapter-base-*`: scheduling, storage, - SSO, redirects, route settings) -- **custom-field-types**, **testing** - Shared field-type definitions and test - helpers -- **_template** - Scaffold for new packages; excluded from the workspace - -### e2e/ - End-to-end tests -- Playwright-based E2E tests with Docker container isolation -- See `e2e/CLAUDE.md` for detailed testing guidance - -## Common Commands - -### Development -```bash -corepack enable pnpm # Enable corepack to use the correct pnpm version -pnpm run setup # First-time setup (installs deps + submodules + builds workspace packages) -pnpm dev # Start development (Docker backend + host frontend dev servers) -``` - -> **Fresh worktree / first run — run `pnpm setup` before anything else.** It installs deps and syncs submodules. `pnpm fix` does a clean reinstall if anything misbehaves after a branch switch. - -### Building -```bash -pnpm build # Build all packages (Nx handles dependencies) -pnpm build:clean # Clean build artifacts and rebuild -``` - -### Testing -```bash -# Unit tests (from root) -pnpm test:unit # Run all unit tests in all packages -pnpm test:watch # Watch mode — unified Vitest watcher (ghost/core + all apps) - -# Ghost core tests (from ghost/core/) -cd ghost/core -pnpm test:unit # Unit tests only (Vitest, run once) -pnpm test:watch # Watch mode — ghost/core unit tests only -pnpm test:integration # Integration tests -pnpm test:e2e # Server-side e2e suites (webhooks/server/frontend/api) — not browser -pnpm test:all # All test types - -# These run on sqlite with no extra services. The Redis/MinIO/S3 adapter suites -# probe for their service and auto-skip when it's down (run `pnpm dev:storage` -# etc. to exercise them); they always run in CI, which starts the services. - -# E2E browser tests (from root) -pnpm test:e2e # Run e2e/ Playwright tests - -# Running a single test -cd ghost/core -pnpm test:single test/unit/path/to/test.test.js # routes test/unit/* → unit config, test/* → DB config - -# Watch a single DB-backed file (integration/e2e) — the default test:watch only -# covers unit tests, so point it at the DB config explicitly: -pnpm exec vitest -c vitest.config.db.ts test/integration/path/to/test.test.js - -# Ember Admin tests (from the repository root) -pnpm nx run ghost-admin:test - -# Run one Ember Admin test file. Paths are relative to apps/ember-admin. -# The explicit `1` supplies the numeric value required by the test script's -# trailing `--parallel` option before additional Ember Exam arguments. -pnpm nx run ghost-admin:test -- 1 --file-path=tests/acceptance/editor/publish-flow-test.js -``` - -> **Always run Ember Admin tests through Nx.** Running `ember test` or -> `ember exam` directly from `apps/ember-admin` skips the dependency build -> graph and commonly fails in fresh worktrees with missing outputs such as -> `koenig-lexical.umd.js`, `@tryghost/admin-x-framework/hooks`, or -> `@tryghost/kg-converters`. For focused runs, use Ember Exam's `--file-path` -> as shown above rather than appending `--filter` to the package script. - -### Linting -```bash -pnpm lint # Lint all packages -cd ghost/core && pnpm lint # Lint Ghost core (server, shared, frontend, tests) -cd apps/ember-admin && pnpm lint # Lint Ember admin -``` - -### Database -```bash -pnpm knex-migrator migrate # Run database migrations -pnpm reset:data # Reset database with test data (1000 members, 100 posts) (requires pnpm dev running) -pnpm reset:data:empty # Reset database with no data (requires pnpm dev running) -``` - -### Docker -```bash -pnpm docker:build # Build Docker images -pnpm docker:clean # Stop containers, remove volumes and local images -pnpm docker:down # Stop containers -``` - -### How `pnpm dev` works - -The `pnpm dev` command uses a **hybrid Docker + host development** setup: - -**What runs in Docker:** -- Ghost Core backend (with hot-reload via mounted source) -- MySQL, Redis, Mailpit -- Caddy gateway/reverse proxy +Human-readable setup, workflow, testing, shipping, and architecture guidance +lives in the [codebase documentation](docs/README.md). Treat those guides and +nearby package READMEs as the source of truth for facts shared by humans and +agents. This file adds agent-specific execution rules and code constraints. -**What runs on host by default:** -- Admin, legacy Ember admin, Portal, and foundation library dev watchers -- Optional public UMD app watchers can be added when needed +Start with: -**Setup:** -```bash -# Start Ghost backend, Admin, Portal, and Docker services -pnpm dev +- [Development setup](docs/contributing/development-setup.md) +- [Contribution workflow](docs/contributing/workflow.md) +- [Testing](docs/contributing/testing.md) +- [Shipping](docs/contributing/shipping.md) +- [Monorepo structure](docs/codebase/monorepo-structure.md) -# Add optional public apps (comments-ui, sodo-search, signup-form, admin-toolbar) -pnpm dev:public +## Package Manager -# Develop the Koenig editor against Ghost Admin (adds a koenig-lexical rebuild -# watcher + preview server; Admin loads the editor from your local build) -pnpm dev:lexical +**Always use `pnpm` for all commands.** This repository uses pnpm workspaces, not npm. -# With optional services (uses Docker Compose file composition) -pnpm dev:analytics # Include Tinybird analytics -pnpm dev:storage # Include MinIO S3-compatible object storage -pnpm dev:stripe # Include Stripe webhook forwarding -pnpm dev:full # Include analytics, storage, Stripe, and public app watchers +Shared dependency versions are pinned in `pnpm-workspace.yaml` under `catalog:` and referenced as `"pkg": "catalog:"` (or `catalog:` for named catalogs). `catalogMode` is `strict`, so `pnpm add` routes new deps into the catalog automatically — don't inline the version. -# Everything available -pnpm dev:all # -``` +## Required Workflow -**Accessing Services:** -- Ghost: `http://localhost:2368` (database: `ghost_dev`) -- Mailpit UI: `http://localhost:8025` (email testing) -- MySQL: `localhost:3306` -- Redis: `localhost:6379` -- Tinybird: `http://localhost:7181` (when analytics enabled) -- MinIO Console: `http://localhost:9001` (when storage enabled) -- MinIO S3 API: `http://localhost:9000` (when storage enabled) +- Run `pnpm setup` before other commands in a fresh checkout or worktree. +- Use `pnpm check` as the default full validation command. Follow the + [testing guide](docs/contributing/testing.md) for focused commands and the + browser E2E and Ember Admin suites that run separately. +- Read the nearest `AGENTS.md`, `CLAUDE.md`, and README files before changing a + package or subsystem. More specific instructions override this file. ## Architecture Patterns @@ -387,16 +197,3 @@ Conventions: - **Config:** Add Tinybird config to `ghost/core/config.development.json` - **Scripts:** `ghost/core/core/server/data/tinybird/scripts/` - **Datafiles:** `ghost/core/core/server/data/tinybird/` - -## Troubleshooting - -### Build Issues -```bash -pnpm fix # Clean cache + node_modules + reinstall -pnpm build:clean # Clean build artifacts -pnpm nx reset # Reset Nx cache -``` - -### Test Issues -- **E2E failures:** Check `e2e/CLAUDE.md` for debugging tips -- **Docker issues:** `pnpm docker:clean && pnpm docker:build` diff --git a/README.md b/README.md index 3a4c70331c4..9df2c0f09ea 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ Ghost.org • Forum • Docs • - Contributing • + Contributing • Twitter

@@ -79,7 +79,9 @@ Check out our [official documentation](https://ghost.org/docs/) for more informa ### Contributors & advanced developers -For anyone wishing to contribute to Ghost or to hack/customize core files we recommend following our full development setup guides: [Contributor guide](https://ghost.org/docs/contributing/) • [Developer setup](https://ghost.org/docs/install/source/) +To contribute to Ghost, start with the +[contributing guide](.github/CONTRIBUTING.md). To work on the monorepo, see the +[codebase documentation](docs/README.md).   diff --git a/docs/README.md b/docs/README.md index 031c7305e5d..a8df41a1fc0 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1,86 +1,62 @@ -# Ghost Contributor Documentation +# Ghost Codebase Documentation -Welcome to the Ghost contributor documentation! This guide will help you understand the codebase, set up your development environment, and start contributing to Ghost. +Welcome to the Ghost codebase documentation! These docs are for anyone wanting +to work on the Ghost codebase. For self-hosting, themes, or using Ghost APIs, +see the [official Ghost documentation](https://ghost.org/docs/). ## Quick Start -### Prerequisites - -- **Node.js** - Recommended to install via [nvm](https://github.com/nvm-sh/nvm) -- **pnpm** - Package manager -- **Docker** - For MySQL database and development services - -### Initial Setup - -#### 1. Fork and Clone - -First, [fork the Ghost repository](https://github.com/TryGhost/Ghost/fork) on GitHub, then: +With the [prerequisites](contributing/development-setup.md#prerequisites) +installed: ```bash -# Clone your fork with submodules -git clone --recurse-submodules git@github.com:/Ghost.git +git clone --recurse-submodules git@github.com:TryGhost/Ghost.git cd Ghost -# Configure remotes -git remote rename origin upstream -git remote add origin git@github.com:/Ghost.git -``` - -#### 2. Install and Setup - -```bash -# Install dependencies and initialize submodules -corepack enable pnpm -pnpm run setup -``` - -#### 3. Start Ghost - -```bash -# Start development (runs Docker backend services + frontend dev servers) +pnpm setup pnpm dev ``` Ghost will be available at: -- **Main site**: http://localhost:2368/ -- **Admin panel**: http://localhost:2368/ghost/ -### Troubleshooting Setup +- **Main site**: [http://localhost:2368](http://localhost:2368) +- **Admin panel**: [http://localhost:2368/ghost/](http://localhost:2368/ghost/) +- **Development email**: [http://localhost:8025](http://localhost:8025) -If you encounter issues during setup: +`pnpm dev` also starts the supporting MySQL and Redis containers, plus Admin and +Portal development watchers. -```bash -# Fix dependency issues -pnpm fix - -# Update to latest main branch -pnpm main - -# Reset running dev data -pnpm reset:data -``` +For more detail, see the +[development setup guide](contributing/development-setup.md) including first-run +setup, development variants, and troubleshooting. ## Repository Structure -``` +```text Ghost/ -├── apps/ # Frontend applications -│ ├── admin-x-*/ # New React-based admin apps -│ ├── portal/ # Member portal -│ ├── comments-ui/ # Comments widget -│ ├── signup-form/ # Signup form widget -│ └── ... -├── ghost/ # Core Ghost application -│ ├── core/ # Main Ghost backend -│ ├── admin/ # Admin build output -│ └── i18n/ # Internationalization -├── koenig/ # Ghost editor (Koenig) packages -│ ├── koenig-lexical/ # Lexical-based rich text editor UI -│ └── kg-*/ # Editor renderers, converters, and support packages -└── e2e/ # End-to-end tests +├── apps/ # Admin and public frontend apps +│ ├── admin/ # React Admin +│ ├── ember-admin/ # Legacy Ember Admin +│ ├── portal/ # Member Portal +│ ├── comments-ui/ # Comments +│ └── shade/ # Admin design system +├── ghost/core/ # Ghost server and frontend rendering +│ ├── core/server/ # APIs, models, and services +│ ├── core/frontend/ # Theme rendering and helpers +│ ├── content/ # Default themes, adapters, and local content +│ └── test/ # Server tests +├── koenig/ # Editor and content-format packages +├── packages/ # Shared libraries and adapter contracts +├── configs/ # Shared build, lint, test, and TypeScript config +├── e2e/ # Browser end-to-end tests +├── docker/ # Local development containers and services +└── scripts/ # Repository tooling ``` -## Contributing +pnpm links the workspaces and Nx runs their tasks in dependency order. For more +detail, see the [monorepo structure guide](codebase/monorepo-structure.md). + +## Contributing a change Before contributing, please read: @@ -94,13 +70,36 @@ Before contributing, please read: ### Development Workflow -1. **Fork and clone** the repository +1. **Clone** the repository 2. **Create a branch** for your changes 3. **Make your changes** and write tests -4. **Run tests** to ensure everything works +4. **Run `pnpm check`** to ensure everything works 5. **Commit** following our commit message conventions 6. **Submit a pull request** to the `main` branch +For more detail, see the [contribution workflow](contributing/workflow.md). + +### Testing + +Use `pnpm check` as the default one-stop command for linting and testing. Add +tests at the closest layer to the behavior you changed. Browser end-to-end tests +and Ember Admin tests run separately from `pnpm check`. + +For more detail, see the [testing guide](contributing/testing.md) including how +to choose a test suite, run focused tests, and use the separate browser and +Ember Admin test lanes. + +### Shipping + +Admin uses continuous delivery on Ghost(Pro), so every commit to `main` can ship +before the next server release. Keep Admin compatible with server versions that +are still live. Public Ghost releases include Admin and the server every +Tuesday. + +For more detail, see the [shipping guide](contributing/shipping.md) including +when changes reach Ghost(Pro), self-hosted installs, npm, jsDelivr, and the +Docker Official Image. + ## Additional Resources - **[Official Documentation](https://ghost.org/docs/)** - User and developer docs diff --git a/docs/codebase/monorepo-structure.md b/docs/codebase/monorepo-structure.md new file mode 100644 index 00000000000..25c9d4bc27c --- /dev/null +++ b/docs/codebase/monorepo-structure.md @@ -0,0 +1,129 @@ +# Monorepo structure + +Ghost is a pnpm workspace. Nx runs build, lint, test, and development tasks +across the workspace dependency graph. + +## Top-level directories + +| Directory | Contains | +| --- | --- | +| `apps/` | Admin applications, public browser apps, and frontend libraries | +| `ghost/core/` | The Ghost server, frontend rendering, migrations, and server tests | +| `koenig/` | The Koenig editor and packages for storing, converting, and rendering content | +| `packages/` | Shared libraries, schemas, translations, test data, and adapter contracts | +| `configs/` | Shared ESLint, TypeScript, Vite, and Vitest configuration | +| `e2e/` | Playwright tests for complete Admin and public-site journeys | +| `docker/` | Containers and supporting services for local development and CI | +| `scripts/` | Repository setup, validation, build, and release tooling | + +[`pnpm-workspace.yaml`](../../pnpm-workspace.yaml) is the source of truth for +which directories are workspaces. Read the README beside an app, package, or +service before changing it. + +## Frontend applications + +`apps/` contains several types of frontend project: + +- `admin/` is the React Admin application. +- `ember-admin/` is the legacy Ember Admin application. Routes are moving from + Ember to React over time. +- `activitypub/` is a React application included in Admin. +- `portal/`, `comments-ui/`, `signup-form/`, `sodo-search/`, + `announcement-bar/`, and `admin-toolbar/` are public apps published to npm + and loaded through the CDN. +- `shade/` is the current Admin design system. +- `admin-x-framework/` provides shared Admin API hooks, routing, and utilities. + +Admin combines the React and Ember applications into one interface. See +[`apps/admin/README.md`](../../apps/admin/README.md) for the current integration +boundary. + +## Ghost Core + +`ghost/core/` is the main `ghost` package. The most common paths are: + +| Path | Contains | +| --- | --- | +| `ghost/core/core/server/` | APIs, models, services, data access, and server startup | +| `ghost/core/core/frontend/` | Theme rendering, helpers, middleware, and public assets | +| `ghost/core/core/shared/` | Configuration and code shared across server boundaries | +| `ghost/core/content/` | Default themes, adapters, settings, images, and runtime content | +| `ghost/core/test/` | Unit, integration, and server E2E tests | + +Built Admin assets are copied into `ghost/core/core/built/admin/` for the Ghost +release. Treat `built/`, `build/`, `dist/`, and `umd/` as generated output unless +a nearby README says otherwise. + +## Koenig + +`koenig/` contains the Lexical editor UI and the `kg-*` packages used to store, +convert, and render Ghost content. These packages were moved into this monorepo +and are local workspace dependencies during development and CI. + +The editor is bundled into Admin. Ghost Core also consumes server-side Koenig +packages for content conversion and rendering. See +[`koenig/README.md`](../../koenig/README.md) for the package map and development +commands. + +## Shared packages and configuration + +`packages/` contains shared libraries used by Ghost Core and the apps. Adapter +base packages define contracts for storage, scheduling, redirects, caching, and +other replaceable services. + +Read [`packages/README.md`](../../packages/README.md) before adding or +modernizing an internal package. New internal packages start from +`packages/_template/`; the template itself is excluded from the workspace. + +`configs/` contains shared configuration packages. Workspaces depend on them by +package name instead of copying configuration into each project. + +## Workspace dependencies + +Workspace packages declare local dependencies with `workspace:` versions. pnpm +links those packages from this checkout, so development and CI do not install a +separate npm copy. + +External dependency versions are managed in the catalog in +[`pnpm-workspace.yaml`](../../pnpm-workspace.yaml). Use `catalog:` in workspace +manifests rather than adding the same version in several packages. + +## Nx tasks + +Nx reads the package dependency graph and runs tasks in the required order. For +example, a build runs dependency builds before the project that consumes them. +Nx also caches declared task outputs. + +Useful commands from the repository root are: + +```bash +pnpm nx show projects +pnpm nx show project +pnpm nx graph +pnpm nx run : +``` + +The root `pnpm build`, `pnpm lint`, and `pnpm test` commands use Nx to run the +matching targets across the monorepo. + +## Source and production builds + +Some TypeScript packages expose a `source` export condition alongside their +compiled output. Ghost Core's development server and tests enable this +condition, so they can load raw TypeScript from packages such as +`@tryghost/kg-default-nodes` without rebuilding after every change. + +Production does not enable the `source` condition. It uses the compiled files +from `build/`, and the Ghost release contains those production files rather than +package source. Browser applications also use their normal build outputs. + +Koenig's published `kg-*` packages retain separate ESM and CommonJS outputs as +part of their existing public package contract: `import` resolves from +`build/esm/` and `require` resolves from `build/cjs/`. Their package `files` +lists include `build/` but not `src/`, so the raw TypeScript used by the source +condition is never published. New internal packages use the ESM-only contract +documented in [`packages/README.md`](../../packages/README.md). + +This means a source edit may work immediately in development while a production +build still needs `pnpm build`. Run the build when changing package exports, +build configuration, or code included in a release artifact. diff --git a/docs/contributing/development-setup.md b/docs/contributing/development-setup.md new file mode 100644 index 00000000000..b2c729b14a7 --- /dev/null +++ b/docs/contributing/development-setup.md @@ -0,0 +1,189 @@ +# Development setup + +This guide runs the Ghost monorepo in its standard development configuration: +Ghost Core and its backing services run in Docker, while frontend build watchers +run on the host. + +## Prerequisites + +Install: + +- [Git](https://git-scm.com/) +- Node.js `22.23.1` (the version in [`.nvmrc`](../../.nvmrc) and + [`.node-version`](../../.node-version)) +- [Docker](https://docs.docker.com/get-docker/) with Docker Compose v2 +- [Corepack](https://nodejs.org/api/corepack.html), included with supported + Node.js distributions + +The default environment binds ports `80`, `2368`, `3306`, `6379`, `8025`, and +`8026`. Stop local services using those ports before starting Ghost. + +The repository pins its pnpm version in `package.json`. Activate that version +before first use rather than installing a separate global version of pnpm: + +```bash +corepack enable pnpm +``` + +## Clone the repository + +Clone the canonical repository with its submodules: + +```bash +git clone --recurse-submodules git@github.com:TryGhost/Ghost.git +cd Ghost +``` + +If you already cloned without submodules, the setup command in the next section +initializes them. Contributors without write access can create a fork and add it +as a remote when they are ready to submit a pull request; a fork is not required +to run Ghost locally. + +## Install the workspace + +From the repository root: + +```bash +pnpm setup +``` + +`pnpm setup` installs the workspace and initializes all Git submodules. Run it +after a fresh clone and whenever a branch changes workspace dependencies or +submodules. + +## Start Ghost + +```bash +pnpm dev +``` + +The first run builds the development image and may take longer than subsequent +starts. The command starts: + +- Ghost Core, MySQL, Redis, and Mailpit in Docker +- a Caddy gateway in Docker on `http://localhost:2368` +- Admin and Portal development watchers on the host + +Wait for Docker Compose to report healthy services, then open: + +- Site: [http://localhost:2368](http://localhost:2368) +- Admin: [http://localhost:2368/ghost/](http://localhost:2368/ghost/) +- Development email: [http://localhost:8025](http://localhost:8025) + +On a new database, the Admin URL opens Ghost's setup screen. Create a local owner +account there; the development environment does not define shared login +credentials. + +As a quick health check, confirm that the site and Admin load and that +`docker compose -f compose.dev.yaml ps` reports the Docker services as running or +healthy. + +Press `Ctrl+C` in the development process to stop its watchers and containers. +Docker volumes preserve the database and uploaded development content between +runs. + +## Accessing services + +| Service | Address | +| --- | --- | +| Ghost site | [http://localhost:2368](http://localhost:2368) | +| Ghost site (gateway alias) | [http://localhost](http://localhost) | +| Ghost Admin | [http://localhost:2368/ghost/](http://localhost:2368/ghost/) | +| Mailpit | [http://localhost:8025](http://localhost:8025) | +| Mailpit (E2E) | [http://localhost:8026](http://localhost:8026) | +| MySQL | `localhost:3306` using the `ghost_dev` database | +| Redis | `localhost:6379` | +| Tinybird | [http://localhost:7181](http://localhost:7181) with `pnpm dev:analytics` | +| MinIO console | [http://localhost:9001](http://localhost:9001) with `pnpm dev:storage` | +| MinIO S3 API | [http://localhost:9000](http://localhost:9000) with `pnpm dev:storage` | + +## Development variants + +Run one root command at a time. Each variant includes the standard development +environment and adds the listed tooling: + +| Command | Use it when working on | +| --- | --- | +| `pnpm dev` | Ghost Core, Admin, or Portal | +| `pnpm dev:public` | Comments UI, Signup Form, Search, Announcement Bar, or Admin Toolbar | +| `pnpm dev:lexical` | Koenig's Lexical editor inside Ghost Admin | +| `pnpm dev:analytics` | Tinybird-backed analytics; also exposes Tinybird on port `7181` | +| `pnpm dev:storage` | S3-compatible storage through MinIO on ports `9000` and `9001` | +| `pnpm dev:stripe` | Stripe webhooks; requires `STRIPE_SECRET_KEY` in the environment or a local `.env` file | +| `pnpm dev:full` | Public app watchers plus analytics, storage, and Stripe | + +Copy [`.env.example`](../../.env.example) to `.env` only when you need an +optional integration. Never commit credentials or the local `.env` file. + +## Data and email + +After creating the local owner account, populate a development site with stable +sample data: + +```bash +pnpm reset:data +``` + +This clears the development database while preserving the owner, then creates +1,000 members and 100 posts. Use `pnpm reset:data:empty` for an empty site. Both +commands are destructive and require the Docker development environment to be +running. + +When developing a database migration, apply pending migrations to the running +development database with: + +```bash +pnpm migrate:db +``` + +Development email is captured by Mailpit rather than delivered. Open +[http://localhost:8025](http://localhost:8025) to inspect messages. + +## Updating and recovering + +Before starting new work, update your local `main` from the canonical repository: + +```bash +git fetch origin +git switch main +git pull --ff-only origin main +pnpm setup +``` + +If dependencies or Nx state become inconsistent after switching branches, run: + +```bash +pnpm fix +``` + +This prunes the pnpm store, removes workspace `node_modules` directories, +reinstalls dependencies, and resets Nx state. + +For narrower build and cache problems, use: + +```bash +pnpm nx reset # Clear the Nx cache +pnpm build:clean # Clear the Nx cache and Ghost build output +pnpm docker:build # Rebuild the local development images +``` + +To stop containers outside a running `pnpm dev` process: + +```bash +pnpm docker:down +``` + +As a last resort, `pnpm docker:clean` removes the development containers, +volumes, and locally built images. This deletes the local development database +and uploaded content; do not use it when you need to preserve that data. + +If startup fails, inspect `docker compose -f compose.dev.yaml ps` and +`docker compose -f compose.dev.yaml logs SERVICE-NAME`. Check for occupied ports, +an unhealthy Docker daemon, and stale dependencies before resetting data or +volumes. + +## Next steps + +Use the README beside the area you are changing for its focused commands and +architecture. The [codebase documentation index](../README.md) links to the +main workspace guides. diff --git a/docs/contributing/shipping.md b/docs/contributing/shipping.md new file mode 100644 index 00000000000..bb83540ed19 --- /dev/null +++ b/docs/contributing/shipping.md @@ -0,0 +1,87 @@ +# Shipping Ghost + +Ghost treats `main` as always green and working, so Admin and server changes +must remain compatible without assuming they ship together as we move towards +continuous delivery. + +## At a glance + +| Track | Current cadence | +| --- | --- | +| Admin (Pro only) | Every commit to `main` after its Admin release-path checks pass | +| Portal and other public apps | When the app changes on `main` | +| Admin + Server (Public Release) | Weekly on Tuesdays | +| Admin + Server (Docker Official Image) | Follows public release after Docker team review | +| Server (Pro only) | Daily rollout on weekdays | + +## Admin + +Admin on Ghost(Pro) uses continuous delivery. Every commit to `main` publishes a +new Admin build and makes it live after its Admin build and Docker release-path +checks pass. + +The public Ghost release contains a Ghost Admin build to keep self-hosted +installs easy to manage. + +## Public apps + +These public apps are npm packages served through jsDelivr: + +- `@tryghost/portal` +- `@tryghost/sodo-search` +- `@tryghost/comments-ui` +- `@tryghost/signup-form` +- `@tryghost/announcement-bar` +- `@tryghost/admin-toolbar` + +The full list lives in +[`scripts/public-apps.json`](../../scripts/public-apps.json). When one changes on +`main`, CI publishes a new patch version to npm and clears the jsDelivr cache. +Sites load the latest patch in the major/minor line configured by Ghost, so a +patch can go live without a Ghost release. + +A minor or major app release is different. It changes the major/minor version +pinned by Ghost. The new line becomes the default when that change is included +in a public Ghost release and the site upgrades. See the app's README for that +release process. + +## Public Ghost releases + +An automated workflow releases a new public Ghost version every Tuesday. + +A version tag starts the release jobs in +[`ci.yml`](../../.github/workflows/ci.yml). CI: + +- builds and tests the Ghost package with Ghost-CLI +- publishes that package to npm +- creates the GitHub release and release notes +- starts the Docker Official Image update + +For a normal `ghost install` or `ghost update`, Ghost-CLI downloads the `ghost` +package from npm and checks its published checksum. It does not use the source +ZIP generated by GitHub. + +Ghost-CLI also accepts a local `.zip`, `.tgz`, or `.tar.gz` through its +`--archive` option. This is useful for testing a CI build, but it is not the +normal release path. + +## Docker Official Image + +After the npm package is published: + +1. [`TryGhost/docker-library-ghost`](https://github.com/TryGhost/docker-library-ghost) + updates its Dockerfiles for the new Ghost and Ghost-CLI versions. +2. Automation opens a pull request in + [`docker-library/official-images`](https://github.com/docker-library/official-images). +3. A Docker Official Images maintainer must approve and merge the pull request. +4. Docker builds and publishes the official `ghost` images on Docker Hub. + +The approval must come from the Docker team, not the Ghost team. This means the +Docker image can appear later than the npm and GitHub release. + +## Ghost(Pro) Server + +In an effort to move continuous delivery beyond Admin, we currently roll out +the latest Ghost server to Ghost(Pro) daily on weekdays. + +We're investigating adding a public "nightly" build in the near future. diff --git a/docs/contributing/testing.md b/docs/contributing/testing.md new file mode 100644 index 00000000000..555a366a19a --- /dev/null +++ b/docs/contributing/testing.md @@ -0,0 +1,152 @@ +# Testing Ghost + +Ghost has several test suites across the monorepo. Start with the suite closest +to the behavior you changed, then run the broader checks before submitting your +pull request. + +## Default Check + +From the repository root, run: + +```bash +pnpm check +``` + +This is the default one-stop command for linting and testing. It runs +`pnpm lint` followed by `pnpm test` across the monorepo. + +`pnpm check` does not run the Playwright browser end-to-end suite or Ember +Admin's test suite. Run those separately when your change affects those areas. + +## Choose a Test Suite + +Put tests as close as possible to the code and behavior under test: + +- **Unit tests** cover a function, component, or package in isolation. Most + workspaces use Vitest and expose a `test` or `test:unit` target. +- **Ghost Core integration tests** cover interactions between server modules + and live against a test database. They live in `ghost/core/test/integration/`. +- **Ghost Core server E2E tests** exercise the server, frontend rendering, + webhooks, and APIs against a running Ghost instance and test database. They + live under `ghost/core/test/e2e-*/`. These are Vitest suites, not browser + tests. +- **App acceptance tests** exercise an individual app through its UI. The + framework and command vary by app, so use that workspace's + `test:acceptance` target. +- **Browser E2E tests** use Playwright to cover complete journeys across Ghost + Admin and the public site. They live in `e2e/`. +- **Ember Admin tests** cover the legacy Ember application in + `apps/ember-admin/` and run through Ember Exam via Nx. + +When a regression crosses several layers, prefer a focused test at the lowest +layer that proves the fix. Add a broader acceptance or browser E2E test when the +integration between layers is itself the behavior being protected. + +## Run Focused Tests + +Nx can run a target for one workspace from the repository root: + +```bash +pnpm nx test +pnpm nx test:unit +pnpm nx test:acceptance +``` + +Check the workspace's `package.json` or list its Nx targets when you are unsure +which targets it provides: + +```bash +pnpm nx show project +``` + +For Ghost Core, run its suites from `ghost/core/`: + +```bash +cd ghost/core + +pnpm test:unit +pnpm test:integration +pnpm test:e2e +pnpm test:all +``` + +`test:all` runs Ghost Core's unit, integration, server E2E, and lint targets. To +run one Ghost Core test file, use: + +```bash +pnpm test:single test/unit/path/to/test.test.js +pnpm test:single test/integration/path/to/test.test.js +``` + +Watch mode at the repository root covers unit tests across the workspace: + +```bash +pnpm test:watch +``` + +To watch a single database-backed Ghost Core file, point Vitest at the database +configuration explicitly: + +```bash +cd ghost/core +pnpm exec vitest -c vitest.config.db.ts test/integration/path/to/test.test.js +``` + +Ghost Core's database-backed suites use SQLite by default locally. Tests for +optional Redis and object-storage adapters skip when their services are not +available; start the relevant development services when you need to exercise +those adapters. + +## Run Browser E2E Tests + +The browser suite needs its test infrastructure running. For the normal +development flow, keep `pnpm dev` running in one terminal and run the suite from +another: + +```bash +# Terminal 1, from the repository root +pnpm dev + +# Terminal 2, from the repository root +pnpm test:e2e +``` + +Run a specific file or match a test title by passing Playwright arguments: + +```bash +pnpm test:e2e tests/admin/posts.spec.ts +pnpm test:e2e --grep "publish a post" +``` + +Use `pnpm test:e2e:debug` for Ghost E2E debug logs. See the +[browser E2E guide](../../e2e/README.md) for infrastructure modes, test +isolation, fixtures, selectors, and debugging. + +## Run Ember Admin Tests + +Always run Ember Admin tests through Nx so its dependency graph is built first: + +```bash +# From the repository root +pnpm nx run ghost-admin:test +``` + +For one file, pass the numeric parallel value required by the Ember Admin test +script before the Ember Exam arguments: + +```bash +pnpm nx run ghost-admin:test -- 1 \ + --file-path=tests/acceptance/editor/publish-flow-test.js +``` + +Do not run `ember test` or `ember exam` directly from `apps/ember-admin/`. +Doing so bypasses Nx's dependency builds and can leave required Admin and +Koenig outputs missing. + +## Before Opening a Pull Request + +Run `pnpm check` to ensure everything works. Also run the relevant browser E2E, +app acceptance, or Ember Admin suite when your change affects those areas. + +If a full suite is impractical locally, run the most relevant focused tests and +state exactly what you ran in the pull request. diff --git a/docs/contributing/workflow.md b/docs/contributing/workflow.md new file mode 100644 index 00000000000..c41d88fc59e --- /dev/null +++ b/docs/contributing/workflow.md @@ -0,0 +1,152 @@ +# Contribution workflow + +This guide covers the path from a working development environment to a reviewed +pull request. Use the [development setup](development-setup.md) first if Ghost is +not already running locally. + +## Choose work + +Issues labelled +[good first issue](https://github.com/TryGhost/Ghost/labels/good%20first%20issue) +are intended to be approachable first contributions. The broader +[help wanted](https://github.com/TryGhost/Ghost/labels/help%20wanted) list contains +other contributions the project would welcome. + +Discuss new features and substantial product or architectural changes in the +[Ghost Forum](https://forum.ghost.org/) before implementing them. A focused bug +fix or agreed improvement can usually proceed directly. + +## Start from current `main` + +Update the canonical checkout, then create a descriptive branch: + +```bash +git fetch origin +git switch main +git pull --ff-only origin main +pnpm setup + +git switch -c concise-change-name +``` + +Keep unrelated changes on separate branches and in separate pull requests. If a +larger effort needs a shared or release branch, agree that with the maintainers +first; ordinary pull requests target `main`. + +## Make and validate the change + +Add or update automated tests when behavior changes. Run the most focused checks +for the code you touched, following the README beside that workspace. Before +handing off a change, use the repository's one-stop lint and test command: + +```bash +pnpm check +``` + +`pnpm check` runs `pnpm lint` followed by `pnpm test`. It does not include the +browser E2E suite or Ember Admin tests, so run those separately when the affected +area requires them. CI uses the Nx affected graph and path filters to select the +relevant lint, unit, integration, acceptance, build, and browser-test jobs for a +pull request. + +## Record package release intent + +Changes that affect a publishable `@tryghost/*` package under `koenig/` or +`packages/`, including changes to catalog entries consumed by that package, need +a changeset so the package receives an appropriate version and changelog entry: + +```bash +pnpm change +``` + +Choose patch, minor, or major according to the package's public compatibility +impact. The summary becomes the changelog entry, so describe the result for the +package's consumers. + +If a changed publishable package genuinely requires no release—for example, a +test-only or internal tooling change—record that explicitly: + +```bash +pnpm change --bump none +``` + +Use `pnpm change status` to inspect pending release intent. CI rejects changes +that affect publishable packages without a covering changeset. Changes that do +not affect a publishable package do not need one. + +## Commit Messages + +We have a handful of simple standards for commit messages which help us to generate readable changelogs. Please follow this wherever possible and mention the associated issue number. + +- **1st line:** Max 80 character summary + - Written in past tense e.g. “Fixed the thing” not “Fixes the thing” + - Start with one of: Fixed, Changed, Updated, Improved, Added, Removed, Reverted, Moved, Released, Bumped, Cleaned +- **2nd line:** [Always blank] +- **3rd line:** `ref `, `fixes `, `closes ` or blank +- **4th line:** Why this change was made - the code includes the what, the commit message should describe the context of why - why this, why now, why not something else? + +If your change is **user-facing** please prepend the first line of your commit with **an emoji key**. If the commit is for an alpha feature, no emoji is needed. We are following [gitmoji](https://gitmoji.carloscuesta.me/). + +**Main emojis we are using:** + +- ✨ Feature +- 🎨 Improvement / change +- 🐛 Bug Fix +- 🌐 i18n (translation) submissions [[See Translating Ghost docs for more detail](https://www.notion.so/5af2858289b44f9194f73f8a1e17af59?pvs=25#bef8c9988e294a4b9a6dd624136de36f)] +- 💡 Anything else flagged to users or whoever is writing release notes + +Good commit message examples: [new feature](https://github.com/TryGhost/Ghost/commit/61db6defde3b10a4022c86efac29cf15ae60983f), [bug fix](https://github.com/TryGhost/Ghost/commit/6ef835bb5879421ae9133541ebf8c4e560a4a90e) and [translation](https://github.com/TryGhost/Ghost/commit/83904c1611ae7ab3257b3b7d55f03e50cead62d7). + +**Bumping @tryghost dependencies** + +When bumping `@tryghost/*` dependencies, the first line should follow the above format and say what has changed, not say what has been bumped. + +There is no need to include what modules have changed in the commit message, as this is _very_ clear from the contents of the commit. The commit should focus on surfacing the underlying changes from the dependencies - what actually changed as a result of this dependency bump? + +[Good example](https://github.com/TryGhost/Ghost/commit/95751a0e5fb719bb5bca74cb97fb5f29b225094f) + + +## Publish the branch + +Everyone can clone, run, and modify the canonical repository without a fork. The +publication step depends on whether you can push branches to `TryGhost/Ghost`. + +### Maintainers + +Push the current branch directly: + +```bash +git push -u origin HEAD +gh pr create --base main +``` + +### External contributors + +Create a fork when the change is ready to publish. With the GitHub CLI, the fork +can be created from the existing canonical checkout and added as a separate +remote: + +```bash +gh repo fork --remote --remote-name fork +git push -u fork HEAD +gh pr create --repo TryGhost/Ghost --base main +``` + +The equivalent GitHub web or desktop flow is also fine. Tooling or a coding agent +may perform these steps on your behalf; check the proposed remote, branch, and +pull request before authorizing a push. + +## Open the pull request + +Target `main` unless a maintainer has asked for another base branch. The pull +request should explain: + +- why the change is needed; +- what behavior or contract changes; +- how it was tested; +- any compatibility, release, migration, or rollout considerations. + +Link the issue when one exists and include screenshots for visible UI changes. +Keep the branch current if requested and respond to review feedback with new +commits. CI must pass before merge; skipped jobs are expected when they are not +relevant to the changed paths. From 3783a38240898d51a5ac9c0f3b96908ec17caa88 Mon Sep 17 00:00:00 2001 From: Austin Burdine Date: Wed, 12 Aug 2026 11:42:34 -0400 Subject: [PATCH 15/16] Replaced nrwl/nx-set-shas action with in-repo script (#29918) no ref - nx-set-shas makes a high number of calls to the Github API - transient issues or rate limiting errors are masked as a failure to lookup a successful workflow run - adding a custom script that only needs one call to the Github API simplifies the setup and leverages the local git checkout more --- .github/workflows/ci.yml | 38 +++--- pnpm-lock.yaml | 135 +++++++++++++++++++ pnpm-workspace.yaml | 3 + scripts/nx-set-shas.js | 225 +++++++++++++++++++++++++++++++ scripts/package.json | 3 + scripts/test/nx-set-shas.test.js | 110 +++++++++++++++ 6 files changed, 498 insertions(+), 16 deletions(-) create mode 100644 scripts/nx-set-shas.js create mode 100644 scripts/test/nx-set-shas.test.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e438206b80e..06bf1df3222 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -78,12 +78,30 @@ jobs: echo "GITHUB_EVENT_NAME: ${{ github.event_name }}" echo "GITHUB_CONTEXT: ${{ toJson(github.event) }}" + - uses: pnpm/action-setup@0ebf47130e4866e96fce0953f49152a61190b271 # v6.0.9 + - name: Set up Node + uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6 + env: + FORCE_COLOR: 0 + with: + node-version: ${{ env.NODE_VERSION }} + cache: pnpm + + - name: Install dependencies + run: pnpm install --frozen-lockfile --ignore-scripts + + # Replaced nrwl/nx-set-shas, which verified each candidate commit over the + # API and hid the errors — see scripts/nx-set-shas.js. - name: Set SHAs for Nx Commands if: env.IS_TAG != 'true' - uses: nrwl/nx-set-shas@afb73a62d26e41464e9254689e1fd6122ee683c1 # v5.0.1 - with: - main-branch-name: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.ref || github.ref_name }} - error-on-no-successful-workflow: ${{ env.IS_MAIN == 'true' && github.repository == 'TryGhost/Ghost' }} + env: + GITHUB_TOKEN: ${{ github.token }} + BRANCH: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.ref || github.ref_name }} + # Canonical main is the one branch where too narrow a base means + # untested commits land, so there a lookup that comes up empty fails + # the run rather than falling back to the previous commit. + ON_MISSING: ${{ (env.IS_MAIN == 'true' && github.repository == 'TryGhost/Ghost') && 'error' || 'previous-commit' }} + run: node scripts/nx-set-shas.js --branch "$BRANCH" --head "$HEAD_COMMIT" --on-missing "$ON_MISSING" - name: Check user org membership id: check_user_org_membership @@ -200,18 +218,6 @@ jobs: run: | echo 'matrix=["22.23.1"]' >> $GITHUB_OUTPUT - - uses: pnpm/action-setup@0ebf47130e4866e96fce0953f49152a61190b271 # v6.0.9 - - name: Set up Node - uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6 - env: - FORCE_COLOR: 0 - with: - node-version: ${{ env.NODE_VERSION }} - cache: pnpm - - - name: Install dependencies - run: pnpm install --frozen-lockfile --ignore-scripts - - name: Start Nx Cloud CI run run: pnpm nx start-ci-run diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 07ae69cb8a0..5e398c86af3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -66,6 +66,15 @@ catalogs: '@faker-js/faker': specifier: 10.5.0 version: 10.5.0 + '@octokit/core': + specifier: 7.0.7 + version: 7.0.7 + '@octokit/plugin-retry': + specifier: 8.1.1 + version: 8.1.1 + '@octokit/plugin-throttling': + specifier: 11.0.5 + version: 11.0.5 '@playwright/test': specifier: 1.61.1 version: 1.61.1 @@ -4046,6 +4055,15 @@ importers: scripts: dependencies: + '@octokit/core': + specifier: 'catalog:' + version: 7.0.7 + '@octokit/plugin-retry': + specifier: 'catalog:' + version: 8.1.1(@octokit/core@7.0.7) + '@octokit/plugin-throttling': + specifier: 'catalog:' + version: 11.0.5(@octokit/core@7.0.7) '@pnpm/releasing.versioning': specifier: 1100.1.0 version: 1100.1.0(@pnpm/logger@1100.0.0) @@ -6597,6 +6615,48 @@ packages: cpu: [x64] os: [win32] + '@octokit/auth-token@6.0.0': + resolution: {integrity: sha512-P4YJBPdPSpWTQ1NU4XYdvHvXJJDxM6YwpS0FZHRgP7YFkdVxsWcpWGy/NVqlAA7PcPCnMacXlRm1y2PFZRWL/w==} + engines: {node: '>= 20'} + + '@octokit/core@7.0.7': + resolution: {integrity: sha512-DcB0M3KFgr9ECI328lhBMVsyFT2DnmNucSBTqEN3exyNKUzkkpUSCHmTRcunF41Eou2TIQKW4seewri8ON9bSA==} + engines: {node: '>= 20'} + + '@octokit/endpoint@11.0.4': + resolution: {integrity: sha512-f1cOWoHPmxryJFknxbtDdjODWfV8A9tc8Aae6ermXPNgHFZ/x91AtHIz4gicEjL8hkJiip+u21QHJORfBv/qiA==} + engines: {node: '>= 20'} + + '@octokit/graphql@9.0.4': + resolution: {integrity: sha512-5s15CCiY8XXQ+FG+b1YQcl6Z2FA++nwAz/tg2VUrTmnMncP+2nnGUEYANImdnxsA2Fnq+Mbl7hDjUTw7cFAwcg==} + engines: {node: '>= 20'} + + '@octokit/openapi-types@28.0.0': + resolution: {integrity: sha512-0rFyLuyHvIj6uuZWuDslxkowFYdPXoNIkeAv4b27dzm2Tf4vGWXnPsMcxs7d65kLdMERgP3wc1AEPlqMz8e1cQ==} + + '@octokit/plugin-retry@8.1.1': + resolution: {integrity: sha512-VCVvZ/R1+u3WuiBWpNavZ0mY4aaJNAsENrpBP9aLSR2QyOpQgd7DhM5j4AW7z4MQpnJYgwBPf0XqPQoNBRdQwg==} + engines: {node: '>= 20'} + peerDependencies: + '@octokit/core': '>=7' + + '@octokit/plugin-throttling@11.0.5': + resolution: {integrity: sha512-LIdrkrUv+DWbKeg/49rGuFJ3SU0d3hUS+B4MhNZLepBoNUFXms8Ic9edJjrlx+zycqJHjrMRudVpVb/bAXM2Lw==} + engines: {node: '>= 20'} + peerDependencies: + '@octokit/core': ^7.0.0 + + '@octokit/request-error@7.1.1': + resolution: {integrity: sha512-+eaY7G2VVpSf2pc5Gn1+mph837V/d/TYTJAgWL9Tb0ogGYcpN3IlAVFgjL+Vv93F/sevrxkvsYCedtpLdcFLzA==} + engines: {node: '>= 20'} + + '@octokit/request@10.0.13': + resolution: {integrity: sha512-v2269YxL9Yf+x3d+gRI63FP0vFQEiWgLyBzxe/Y+0yFDg2B/Tzf5dhh9VNfccVAQnfcfwQWyk/y6Bn7rUXXs7A==} + engines: {node: '>= 20'} + + '@octokit/types@17.0.0': + resolution: {integrity: sha512-ByP1v7YL5SMveFPP7+sj0/ZuWCOOg/Chs4NafOMpq6WNIM/hdGY0S7C0TCGDBWu1aGmOxmUIhMx3cO+IdwYZ1Q==} + '@one-ini/wasm@0.1.1': resolution: {integrity: sha512-XuySG1E38YScSJoMlqovLru4KTUNSjgVTIjyh7qMX6aNN5HY5Ct5LhRJdxO79JtTzKfzV/bnWpz+zquYrISsvw==} @@ -10997,6 +11057,9 @@ packages: resolution: {integrity: sha512-GlF5wPWnSa/X5LKM1o0wz0suXIINz1iHRLvTS+sLyi7XPbe5ycmYI3DlZqVGZZtDgl4DmasFg7gOB3JYbphV5g==} hasBin: true + before-after-hook@4.0.0: + resolution: {integrity: sha512-q6tR3RPqIB1pMiTRMFcZwuG5T8vwp+vUvEG0vuI6B+Rikh5BfPp2fQ82c925FOs+b0lcFQ8CFrL+KbilfZFhOQ==} + better-path-resolve@1.0.0: resolution: {integrity: sha512-pbnl5XzGBdrFU/wT4jqmJVPn2B6UHPBOhzMQkY/SPUPB6QtUXtmBHBIwCbXJol93mOpGMnQyP/+BB19q04xj7g==} engines: {node: '>=4'} @@ -11084,6 +11147,9 @@ packages: resolution: {integrity: sha512-d0II/GO9uf9lfUHH2BQsjxzRJZBdsjgsBiW4BvhWk/3qoKwQFjIDVN19PfX8F2D/r9PCMTtLWjYVCFrpeYUzsw==} deprecated: Package no longer supported. Contact Support at https://www.npmjs.com/support for more info. + bottleneck@2.19.5: + resolution: {integrity: sha512-VHiNCbI1lKdl44tGrhNfU3lup0Tj/ZBMJB5/2ZbNXRCPuRCO7ed2mgcK4r17y+KB2EfuYuRaVlwNbAeaWGSpbw==} + boundary@2.0.0: resolution: {integrity: sha512-rJKn5ooC9u8q13IMCrW0RSp31pxBCHE3y9V/tp3TdWSLf8Em3p6Di4NBpfzbJge9YjjFEsD0RtFEjtvHL5VyEA==} @@ -16380,6 +16446,9 @@ packages: json-stringify-safe@5.0.1: resolution: {integrity: sha512-ZClg6AaYvamvYEE82d3Iyd3vSSIjQ+odgjaTzRuO3s7toCdFKczob2i0zCh7JE8kWn17yvAWhUVxvqGwUalsRA==} + json-with-bigint@3.5.10: + resolution: {integrity: sha512-Vcx+JVNEBts/xfcoCS69sKrOhOk/3TVlvlT+XzUOefVKnnrbYSCKpDCm10pohsJFtsJVYnwa/cXRZ4eElzaM6w==} + json5@1.0.2: resolution: {integrity: sha512-g1MWMLBiz8FKi1e4w0UyVL3w+iJceWAFBAaBnnGKOpNa5f8TLktkbre1+s6oICydWAm+HRUGTmI+//xv2hvXYA==} hasBin: true @@ -21804,6 +21873,9 @@ packages: unist-util-visit@5.1.0: resolution: {integrity: sha512-m+vIdyeCOpdr/QeQCu2EzxX/ohgS8KbnPDgFni4dQsfSCtpz8UqDyY5GjRru8PDKuYn7Fq19j1CQ+nJSsGKOzg==} + universal-user-agent@7.0.3: + resolution: {integrity: sha512-TmnEAEAsBJVZM/AADELsK76llnwcf9vMKuPz8JflO1frO8Lchitr0fNaN9d+Ap0BjKtqWqd/J17qeDnXh8CL2A==} + universalify@0.1.2: resolution: {integrity: sha512-rBJeI5CXAlmy1pV+617WB9J63U6XcazHHF2f2dbJix4XzpUF0RS3Zbj0FGIOCAva5P/d/GBOYaACQ1w+0azUkg==} engines: {node: '>= 4.0.0'} @@ -25920,6 +25992,61 @@ snapshots: '@nx/nx-win32-x64-msvc@23.0.1': optional: true + '@octokit/auth-token@6.0.0': {} + + '@octokit/core@7.0.7': + dependencies: + '@octokit/auth-token': 6.0.0 + '@octokit/graphql': 9.0.4 + '@octokit/request': 10.0.13 + '@octokit/request-error': 7.1.1 + '@octokit/types': 17.0.0 + before-after-hook: 4.0.0 + universal-user-agent: 7.0.3 + + '@octokit/endpoint@11.0.4': + dependencies: + '@octokit/types': 17.0.0 + universal-user-agent: 7.0.3 + + '@octokit/graphql@9.0.4': + dependencies: + '@octokit/request': 10.0.13 + '@octokit/types': 17.0.0 + universal-user-agent: 7.0.3 + + '@octokit/openapi-types@28.0.0': {} + + '@octokit/plugin-retry@8.1.1(@octokit/core@7.0.7)': + dependencies: + '@octokit/core': 7.0.7 + '@octokit/request-error': 7.1.1 + '@octokit/types': 17.0.0 + bottleneck: 2.19.5 + + '@octokit/plugin-throttling@11.0.5(@octokit/core@7.0.7)': + dependencies: + '@octokit/core': 7.0.7 + '@octokit/types': 17.0.0 + bottleneck: 2.19.5 + + '@octokit/request-error@7.1.1': + dependencies: + '@octokit/types': 17.0.0 + + '@octokit/request@10.0.13': + dependencies: + '@octokit/endpoint': 11.0.4 + '@octokit/request-error': 7.1.1 + '@octokit/types': 17.0.0 + content-type: 2.0.0 + json-with-bigint: 3.5.10 + universal-user-agent: 7.0.3 + + '@octokit/types@17.0.0': + dependencies: + '@octokit/openapi-types': 28.0.0 + '@one-ini/wasm@0.1.1': optional: true @@ -31464,6 +31591,8 @@ snapshots: bcryptjs@3.0.3: {} + before-after-hook@4.0.0: {} + better-path-resolve@1.0.0: dependencies: is-windows: 1.0.2 @@ -31583,6 +31712,8 @@ snapshots: boolean@3.2.0: {} + bottleneck@2.19.5: {} + boundary@2.0.0: {} bower-config@1.4.3: @@ -39260,6 +39391,8 @@ snapshots: json-stringify-safe@5.0.1: {} + json-with-bigint@3.5.10: {} + json5@1.0.2: dependencies: minimist: 1.2.8 @@ -46057,6 +46190,8 @@ snapshots: unist-util-is: 6.0.1 unist-util-visit-parents: 6.0.2 + universal-user-agent@7.0.3: {} + universalify@0.1.2: {} universalify@0.2.0: {} diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 8b1ffcef706..587cd8d7c8c 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -184,6 +184,9 @@ catalog: '@dnd-kit/utilities': 3.2.2 shell-quote: 1.10.0 unidecode: 1.1.0 + '@octokit/core': 7.0.7 + '@octokit/plugin-retry': 8.1.1 + '@octokit/plugin-throttling': 11.0.5 catalogs: react17: diff --git a/scripts/nx-set-shas.js b/scripts/nx-set-shas.js new file mode 100644 index 00000000000..2ab41860d8d --- /dev/null +++ b/scripts/nx-set-shas.js @@ -0,0 +1,225 @@ +// Resolves NX_BASE / NX_HEAD — the commit window `nx affected` and the CI path +// filters diff against. +// +// Replaces nrwl/nx-set-shas, which re-verified every candidate commit over the +// GitHub API (two calls per candidate, up to 60 a run) and swallowed every +// error, so a transient API failure was indistinguishable from a rewritten +// branch and hard-failed the run. Ancestry is answerable locally — job_setup +// checks out with fetch-depth 0 — so the API is only asked which runs passed: +// one request, with the retry/throttling plugins handling rate limits. + +import {appendFileSync} from 'node:fs'; +import {spawnSync} from 'node:child_process'; +import {parseArgs} from 'node:util'; + +import {Octokit} from '@octokit/core'; +import {retry} from '@octokit/plugin-retry'; +import {throttling} from '@octokit/plugin-throttling'; + +// Hash of git's empty tree — diffing against it marks everything as changed. +const EMPTY_TREE_SHA = '4b825dc642cb6eb9a060e54bf8d69288fbee4904'; +const PULL_REQUEST_EVENTS = new Set(['pull_request', 'pull_request_target']); +const ON_MISSING_MODES = new Set(['error', 'previous-commit']); +// A primary rate limit can reset an hour out; waiting that long would just burn +// the job's timeout, so past this we fail with the real status instead. +const MAX_THROTTLE_WAIT_SECONDS = 60; +const MAX_THROTTLE_RETRIES = 2; + +function git(args) { + const {status, stdout} = spawnSync('git', args, {encoding: 'utf8'}); + + return {ok: status === 0, stdout: (stdout ?? '').trim()}; +} + +function isAncestor(sha, headSha) { + // Exits 128 rather than 1 when the commit isn't in the local history, which + // for our purposes is the same answer: not a usable base. + return git(['merge-base', '--is-ancestor', sha, headSha]).ok; +} + +export function createOctokit({token, baseUrl = process.env.GITHUB_API_URL} = {}) { + const CiOctokit = Octokit.plugin(retry, throttling); + const onLimit = (retryAfter, options, octokit, retryCount) => { + octokit.log.warn(`Rate limited on ${options.method} ${options.url}, waiting ${retryAfter}s`); + + return retryAfter <= MAX_THROTTLE_WAIT_SECONDS && retryCount < MAX_THROTTLE_RETRIES; + }; + + return new CiOctokit({ + auth: token, + ...(baseUrl ? {baseUrl} : {}), + throttle: {onRateLimit: onLimit, onSecondaryRateLimit: onLimit} + }); +} + +/** + * The head SHAs of the workflow's successful runs on a branch, newest first. + * + * @param {object} options + * @param {object} options.octokit + * @param {string} options.repo - owner/name + * @param {string} options.workflow - workflow file name, e.g. ci.yml + * @param {string} options.branch + * @param {string} [options.event] - the trigger to match, defaults to push + * @returns {Promise} + */ +export async function fetchSuccessfulRunShas({octokit, repo, workflow, branch, event = 'push'}) { + const [owner, name] = repo.split('/'); + const {data} = await octokit.request('GET /repos/{owner}/{repo}/actions/workflows/{workflow_id}/runs', { + owner, + repo: name, + workflow_id: workflow, + branch, + event, + status: 'success', + per_page: 100, + exclude_pull_requests: true + }); + + return data.workflow_runs.map(run => run.head_sha); +} + +/** + * The newest run SHA usable as a base: still in this branch's history, and not + * the commit under test — re-running a commit that already passed should + * re-test it, not diff it against itself. Commits whose run was cancelled by a + * faster follow-up push have no successful run, so they're skipped over and + * their changes stay inside the window rather than going untested. + * + * @param {string[]} shas - candidate SHAs, newest first + * @param {object} options + * @param {string} options.headSha + * @param {Function} [options.ancestorCheck] - injectable for tests + * @returns {string|null} + */ +export function selectBaseSha(shas, {headSha, ancestorCheck = isAncestor}) { + for (const sha of shas) { + if (sha !== headSha && ancestorCheck(sha, headSha)) { + return sha; + } + } + + return null; +} + +function previousCommit(headSha) { + const previous = git(['rev-parse', `${headSha}~1`]); + + if (previous.ok && previous.stdout) { + return previous.stdout; + } + + console.log(`${headSha}~1 does not exist, using the empty tree as base`); + return EMPTY_TREE_SHA; +} + +function exportShas(base, head) { + console.log(`NX_BASE=${base}`); + console.log(`NX_HEAD=${head}`); + + if (process.env.GITHUB_ENV) { + appendFileSync(process.env.GITHUB_ENV, `NX_BASE=${base}\nNX_HEAD=${head}\n`); + } +} + +/** + * @param {object} options + * @param {object} options.octokit + * @param {string} options.branch - the PR's base branch, or the pushed branch + * @param {string} options.headSha + * @param {string} options.event - GITHUB_EVENT_NAME + * @param {string} options.workflow + * @param {string} options.repo + * @param {string} options.onMissing - error | previous-commit + * @param {Function} [options.ancestorCheck] - injectable for tests + * @returns {Promise} + */ +export async function resolveBase({octokit, branch, headSha, event, workflow, repo, onMissing, ancestorCheck}) { + if (PULL_REQUEST_EVENTS.has(event)) { + const mergeBase = git(['merge-base', `origin/${branch}`, headSha]); + + if (!mergeBase.ok || !mergeBase.stdout) { + throw new Error(`Could not find the merge base of origin/${branch} and ${headSha}`); + } + + return mergeBase.stdout; + } + + const shas = await fetchSuccessfulRunShas({octokit, repo, workflow, branch}); + const base = selectBaseSha(shas, {headSha, ancestorCheck}); + + if (base) { + console.log(`Last successful ${workflow} run on ${branch}: ${base}`); + return base; + } + + if (onMissing === 'error') { + throw new Error(shas.length === 0 ? + `No successful ${workflow} run found on ${branch}` : + `None of the ${shas.length} successful ${workflow} runs on ${branch} point at a commit in this ` + + `branch's history — was ${branch} rebased?` + ); + } + + console.log(`No successful ${workflow} run found on ${branch}, falling back to the previous commit`); + return previousCommit(headSha); +} + +export async function main(argv = process.argv.slice(2)) { + const {values} = parseArgs({ + args: argv, + options: { + branch: {type: 'string'}, + head: {type: 'string'}, + event: {type: 'string', default: process.env.GITHUB_EVENT_NAME ?? 'push'}, + workflow: {type: 'string', default: 'ci.yml'}, + repo: {type: 'string', default: process.env.GITHUB_REPOSITORY}, + // What to do when no successful run can be found: fail, for a + // canonical branch where too narrow a base means untested commits + // land, or fall back to HEAD~1 for forks, which may have no run + // history at all. + 'on-missing': {type: 'string', default: 'previous-commit'} + } + }); + + const onMissing = values['on-missing']; + + if (!ON_MISSING_MODES.has(onMissing)) { + throw new Error(`--on-missing must be one of: ${[...ON_MISSING_MODES].join(', ')}`); + } + + if (!values.branch) { + throw new Error('--branch is required'); + } + + if (!values.repo) { + throw new Error('--repo is required when GITHUB_REPOSITORY is unset'); + } + + const headSha = values.head || git(['rev-parse', 'HEAD']).stdout; + + if (!headSha) { + throw new Error('Could not resolve HEAD'); + } + + const base = await resolveBase({ + octokit: createOctokit({token: process.env.GITHUB_TOKEN || process.env.GH_TOKEN}), + branch: values.branch, + headSha, + event: values.event, + workflow: values.workflow, + repo: values.repo, + onMissing + }); + + exportShas(base, headSha); +} + +if (import.meta.main) { + try { + await main(); + } catch (error) { + console.error(error.message); + process.exit(1); + } +} diff --git a/scripts/package.json b/scripts/package.json index 3a24bb3ee1b..33357d47d5e 100644 --- a/scripts/package.json +++ b/scripts/package.json @@ -11,6 +11,9 @@ "test": "pnpm run '/^test:/'" }, "dependencies": { + "@octokit/core": "catalog:", + "@octokit/plugin-retry": "catalog:", + "@octokit/plugin-throttling": "catalog:", "@pnpm/releasing.versioning": "1100.1.0", "@pnpm/workspace.workspace-manifest-reader": "1100.1.0", "camelcase-keys": "10.0.2", diff --git a/scripts/test/nx-set-shas.test.js b/scripts/test/nx-set-shas.test.js new file mode 100644 index 00000000000..20dbee003c5 --- /dev/null +++ b/scripts/test/nx-set-shas.test.js @@ -0,0 +1,110 @@ +import {describe, it} from 'node:test'; +import assert from 'node:assert'; +import {fetchSuccessfulRunShas, resolveBase, selectBaseSha} from '../nx-set-shas.js'; + +const HEAD = 'head0000000000000000000000000000000000000'; +const GREEN = 'green000000000000000000000000000000000000'; +const OLDER = 'older000000000000000000000000000000000000'; +const REBASED = 'gone0000000000000000000000000000000000000'; + +// The branch as CI sees it: HEAD plus the two commits behind it. +const ancestors = new Set([HEAD, GREEN, OLDER]); +const ancestorCheck = sha => ancestors.has(sha); + +function octokitReturning(shas) { + const calls = []; + + return { + calls, + request: async (route, params) => { + calls.push({route, params}); + return {data: {workflow_runs: shas.map(sha => ({head_sha: sha}))}}; + } + }; +} + +describe('selectBaseSha', () => { + it('takes the newest successful run still on the branch', () => { + assert.strictEqual(selectBaseSha([GREEN, OLDER], {headSha: HEAD, ancestorCheck}), GREEN); + }); + + it('skips commits that are no longer on the branch', () => { + assert.strictEqual(selectBaseSha([REBASED, GREEN], {headSha: HEAD, ancestorCheck}), GREEN); + }); + + it('skips a successful run of the commit under test so a re-run re-tests it', () => { + assert.strictEqual(selectBaseSha([HEAD, GREEN], {headSha: HEAD, ancestorCheck}), GREEN); + }); + + it('returns null when nothing is usable', () => { + assert.strictEqual(selectBaseSha([REBASED], {headSha: HEAD, ancestorCheck}), null); + assert.strictEqual(selectBaseSha([], {headSha: HEAD, ancestorCheck}), null); + }); +}); + +describe('fetchSuccessfulRunShas', () => { + it('asks for successful runs of the workflow on the branch', async () => { + const octokit = octokitReturning([GREEN, OLDER]); + const shas = await fetchSuccessfulRunShas({ + octokit, + repo: 'TryGhost/Ghost', + workflow: 'ci.yml', + branch: 'main' + }); + + assert.deepStrictEqual(shas, [GREEN, OLDER]); + assert.strictEqual(octokit.calls.length, 1); + assert.deepStrictEqual(octokit.calls[0].params, { + owner: 'TryGhost', + repo: 'Ghost', + workflow_id: 'ci.yml', + branch: 'main', + event: 'push', + status: 'success', + per_page: 100, + exclude_pull_requests: true + }); + }); +}); + +describe('resolveBase', () => { + const pushRun = { + branch: 'main', + headSha: HEAD, + event: 'push', + workflow: 'ci.yml', + repo: 'TryGhost/Ghost', + onMissing: 'error', + ancestorCheck + }; + + it('resolves to the last successful run on the branch', async () => { + const base = await resolveBase({...pushRun, octokit: octokitReturning([GREEN])}); + + assert.strictEqual(base, GREEN); + }); + + it('errors rather than narrowing the window when nothing is usable', async () => { + await assert.rejects( + resolveBase({...pushRun, octokit: octokitReturning([REBASED])}), + /was main rebased/ + ); + }); + + it('errors when the branch has no successful runs at all', async () => { + await assert.rejects( + resolveBase({...pushRun, octokit: octokitReturning([])}), + /No successful ci.yml run found on main/ + ); + }); + + it('surfaces an API failure instead of treating it as no successful run', async () => { + const octokit = { + request: async () => { + throw new Error('GitHub API responded 403'); + } + }; + + await assert.rejects(resolveBase({...pushRun, octokit}), /403/); + }); +}); From ada263958d074e1682bafb7e574d88d69a93f334 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Wed, 12 Aug 2026 16:46:21 +0100 Subject: [PATCH 16/16] Removed the customFonts labs flag (#29886) Removes the obsolete `customFonts` Labs registration, which is entirely unused. Custom fonts remain generally available - no runtime behavior is changed. --- ghost/core/core/shared/labs.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ghost/core/core/shared/labs.js b/ghost/core/core/shared/labs.js index 454dab4b2d9..e794cede8db 100644 --- a/ghost/core/core/shared/labs.js +++ b/ghost/core/core/shared/labs.js @@ -27,8 +27,7 @@ const messages = { // flags in this list always return `true`, allows quick global enable prior to full flag removal const GA_FEATURES = [ - 'automationAnalytics', - 'customFonts' + 'automationAnalytics' ]; // These features are considered publicly available and can be enabled/disabled by users