From 7859dc9928889192fe11ca8b7dee72f3e3d993a6 Mon Sep 17 00:00:00 2001 From: Paul Davis Date: Wed, 26 Aug 2026 12:38:36 +0100 Subject: [PATCH 01/11] Added CSV import post deduplication, authors and tags (#30307) ref https://linear.app/ghost/project/4b2edbd66469/ - Milestone 5 Each commit references an individual Linear issue - Skips duplicate posts and updates existing posts when imported data is newer - Matches posts by source ID or slug - Reconciles and creates authors and tags, with safe Owner fallbacks and warnings - Adds full coverage across all changed CSV importer modules --- .../content-import/mapping.test.ts | 32 +- .../migration-tools/content-import/mapping.ts | 9 + .../universal-import-modal.test.tsx | 46 ++ .../content-import/import/importer.ts | 58 +- .../content-import/import/post-data.ts | 5 +- .../content-import/import/post-repository.ts | 138 +++++ .../content-import/import/relations.ts | 224 +++++++ .../services/content-import/import/row.ts | 12 + .../services/content-import/import/store.ts | 3 +- .../server/services/content-import/index.ts | 5 +- .../test/e2e-api/admin/posts-importer.test.js | 373 +++++++++++- .../test/e2e-webhooks/posts-importer.test.ts | 22 +- .../content-import/import/importer.test.ts | 178 +++++- .../content-import/import/post-data.test.ts | 38 ++ .../import/post-repository.test.ts | 562 ++++++++++++++++++ .../content-import/import/reader.test.ts | 9 +- .../content-import/import/relations.test.ts | 483 +++++++++++++++ .../content-import/import/row.test.ts | 9 + .../content-import/import/schema.test.ts | 8 + .../content-import/import/store.test.ts | 14 + 20 files changed, 2178 insertions(+), 50 deletions(-) create mode 100644 ghost/core/core/server/services/content-import/import/post-repository.ts create mode 100644 ghost/core/core/server/services/content-import/import/relations.ts create mode 100644 ghost/core/test/unit/server/services/content-import/import/post-repository.test.ts create mode 100644 ghost/core/test/unit/server/services/content-import/import/relations.test.ts diff --git a/apps/admin/src/settings/advanced/migration-tools/content-import/mapping.test.ts b/apps/admin/src/settings/advanced/migration-tools/content-import/mapping.test.ts index a3b2dcecccb..69ad2dd7396 100644 --- a/apps/admin/src/settings/advanced/migration-tools/content-import/mapping.test.ts +++ b/apps/admin/src/settings/advanced/migration-tools/content-import/mapping.test.ts @@ -15,12 +15,24 @@ describe('ContentFieldMapping', () => { expect(mapping.toJSON()).toEqual({ 'First title': '', 'Second title': 'title' }); }); + it('reads and clears individual mappings', () => { + const mapping = ContentFieldMapping.detect(['title', 'Other']); + + expect(mapping.get('title')).toBe('title'); + expect(mapping.get('Other')).toBeNull(); + expect(mapping.update('title', null).get('title')).toBeNull(); + }); + it('detects exact field-name headers', () => { const mapping = ContentFieldMapping.detect([ 'title', 'html', 'markdown', 'published_at', + 'comment_id', + 'authors', + 'author_emails', + 'tags', 'Something else', ]); @@ -29,6 +41,10 @@ describe('ContentFieldMapping', () => { html: 'html', markdown: 'markdown', published_at: 'published_at', + comment_id: 'comment_id', + authors: 'authors', + author_emails: 'author_emails', + tags: 'tags', 'Something else': '', }); }); @@ -51,6 +67,7 @@ describe('ContentFieldMapping', () => { expect(CONTENT_FIELD_GROUPS.map((group) => group.label)).toEqual([ 'Content', 'Publishing', + 'Authors & tags', 'Images', 'SEO', 'Social', @@ -69,6 +86,9 @@ describe('ContentFieldMapping', () => { 'created_at', 'updated_at', 'published_at', + 'authors', + 'author_emails', + 'tags', 'feature_image', 'feature_image_alt', 'feature_image_caption', @@ -82,22 +102,14 @@ describe('ContentFieldMapping', () => { 'twitter_image', 'twitter_title', 'twitter_description', + 'comment_id', 'custom_template', 'codeinjection_head', 'codeinjection_foot', 'frontmatter', ]); expect(CONTENT_FIELD_MAPPINGS.map((field) => field.value)).not.toEqual( - expect.arrayContaining([ - 'authors', - 'tags', - 'comment_id', - 'newsletter_id', - 'email', - 'tiers', - 'id', - 'lexical', - ]), + expect.arrayContaining(['newsletter_id', 'email', 'tiers', 'id', 'lexical']), ); }); }); diff --git a/apps/admin/src/settings/advanced/migration-tools/content-import/mapping.ts b/apps/admin/src/settings/advanced/migration-tools/content-import/mapping.ts index 2917bf05d93..092e5fa8c0d 100644 --- a/apps/admin/src/settings/advanced/migration-tools/content-import/mapping.ts +++ b/apps/admin/src/settings/advanced/migration-tools/content-import/mapping.ts @@ -32,6 +32,14 @@ export const CONTENT_FIELD_GROUPS: readonly ContentFieldGroup[] = [ { label: 'Published at', value: 'published_at', required: false }, ], }, + { + label: 'Authors & tags', + fields: [ + { label: 'Authors', value: 'authors', required: false }, + { label: 'Author emails', value: 'author_emails', required: false }, + { label: 'Tags', value: 'tags', required: false }, + ], + }, { label: 'Images', fields: [ @@ -67,6 +75,7 @@ export const CONTENT_FIELD_GROUPS: readonly ContentFieldGroup[] = [ { label: 'Advanced', fields: [ + { label: 'Source ID', value: 'comment_id', required: false }, { label: 'Custom template', value: 'custom_template', required: false }, { label: 'Code injection head', value: 'codeinjection_head', required: false }, { label: 'Code injection foot', value: 'codeinjection_foot', required: false }, diff --git a/apps/admin/src/settings/advanced/migration-tools/universal-import-modal.test.tsx b/apps/admin/src/settings/advanced/migration-tools/universal-import-modal.test.tsx index 3ee66a3c286..6b2b520b3db 100644 --- a/apps/admin/src/settings/advanced/migration-tools/universal-import-modal.test.tsx +++ b/apps/admin/src/settings/advanced/migration-tools/universal-import-modal.test.tsx @@ -212,6 +212,52 @@ describe('UniversalImportModal', () => { ); }); + it('maps author names, author emails, and tags from the grouped picker', async () => { + mockUseFeatureFlag.mockReturnValue(true); + showModal(); + + const file = new File( + [ + 'title,Bylines,Emails,Topics\nHello,"Alice, Bob","alice@example.com, bob@example.com","News, Features"', + ], + 'posts.csv', + { type: 'text/csv' }, + ); + await dropFile(file); + + fireEvent.click(await screen.findByRole('combobox', { name: /Field for Bylines/ })); + expect(screen.getByText('Authors & tags')).toBeInTheDocument(); + fireEvent.change(screen.getByPlaceholderText('Search post fields...'), { + target: { value: 'Authors' }, + }); + fireEvent.click(screen.getByText('Authors')); + + fireEvent.click(screen.getByRole('combobox', { name: /Field for Emails/ })); + fireEvent.change(screen.getByPlaceholderText('Search post fields...'), { + target: { value: 'Author emails' }, + }); + fireEvent.click(screen.getByText('Author emails')); + + fireEvent.click(screen.getByRole('combobox', { name: /Field for Topics/ })); + fireEvent.change(screen.getByPlaceholderText('Search post fields...'), { + target: { value: 'Tags' }, + }); + fireEvent.click(screen.getByText('Tags')); + fireEvent.click(screen.getByRole('button', { name: 'Import' })); + + await waitFor(() => + expect(mockImportContentCSV).toHaveBeenCalledWith({ + file, + mapping: { + title: 'title', + Bylines: 'authors', + Emails: 'author_emails', + Topics: 'tags', + }, + }), + ); + }); + it('still sends JSON files to the db import when csvContentImporter is enabled', async () => { mockUseFeatureFlag.mockReturnValue(true); showModal(); diff --git a/ghost/core/core/server/services/content-import/import/importer.ts b/ghost/core/core/server/services/content-import/import/importer.ts index 51827daaa9f..d870b854123 100644 --- a/ghost/core/core/server/services/content-import/import/importer.ts +++ b/ghost/core/core/server/services/content-import/import/importer.ts @@ -7,6 +7,7 @@ import buildPostData, { type PostData, } from './post-data'; import type { PostImportRow } from './row'; +import type { PostsRepository, WrittenPost } from './post-repository'; import type { ImportRequest } from './schema'; import type { Clock, ImportRunStore, RowOutcome } from './store'; import type { PreparedImportSource } from './source'; @@ -27,15 +28,6 @@ export interface ImportAccepted { total: number; } -export interface CreatedPost { - id: string; - toJSON(): Record; -} - -export interface PostsRepository { - create(data: PostData, options: object): Promise; -} - // Must not throw: it is called from catch blocks that exist to stop an error escaping. export type FailureReporter = (error: unknown) => void; @@ -47,7 +39,7 @@ const messages = { tooManyPosts: 'This file contains more than {max} posts. Imports are temporarily limited to {max} posts at a time — please split the file into smaller files and try again.', allWritesFailed: 'Content import failed to write all {count} attempted {postNoun}.', - urlResolutionFailed: 'Content import could not resolve a URL for {count} created {postNoun}.', + urlResolutionFailed: 'Content import could not resolve a URL for {count} imported {postNoun}.', }; function logLifecycle(message: string): void { @@ -71,7 +63,7 @@ interface ImporterDeps { addJob: (job: { job: () => Promise; offloaded: boolean; name: string }) => void; report: FailureReporter; store: ImportRunStore; - urlForPost: (post: CreatedPost) => string; + urlForPost: (post: WrittenPost) => string; newRunId: () => string; getTimezone: () => string; now?: Clock; @@ -94,7 +86,7 @@ class ContentCSVImporter { private _addJob: ImporterDeps['addJob']; private _report: FailureReporter; private _store: ImportRunStore; - private _urlForPost: (post: CreatedPost) => string; + private _urlForPost: (post: WrittenPost) => string; private _newRunId: () => string; private _getTimezone: () => string; private _now: Clock; @@ -194,7 +186,6 @@ class ContentCSVImporter { const htmlToLexical = this._getHtmlToLexical(); const markdownToHtml = this._getMarkdownToHtml(); const cleanHTML = this._getCleanHTML(); - let attemptedWrites = 0; let successfulWrites = 0; let failedWrites = 0; let firstWriteFailure: unknown; @@ -221,8 +212,9 @@ class ContentCSVImporter { throw error; } - attemptedWrites += 1; - let post: CreatedPost; + let post: WrittenPost; + let writeStatus: 'created' | 'updated'; + let warnings: string[]; try { // options.importing preserves the supplied timestamps and keeps the import silent: // the webhook, Slack, IndexNow and mention consumers all stand down on it, and a @@ -230,10 +222,33 @@ class ContentCSVImporter { // it, one reason status is never 'scheduled'). Pinned by // test/e2e-webhooks/posts-importer.test.js. A fresh options object per row: the // model layer mutates it. - post = await this._posts.create(data, { - importing: true, - context: { internal: true }, - }); + const result = await this._posts.write( + data, + { + importing: true, + context: { internal: true }, + }, + { + sourceUpdatedAt: row.updated_at, + authorNames: row.authors, + authorEmails: row.author_emails, + tagNames: row.tags, + }, + ); + + if (result.status === 'skipped') { + this._store.record(runId, { + line, + title: row.title, + status: 'skipped', + reason: result.reason, + }); + continue; + } + + post = result.post; + writeStatus = result.status; + warnings = result.warnings; successfulWrites += 1; } catch (error) { if (failedWrites === 0) { @@ -252,8 +267,9 @@ class ContentCSVImporter { const outcome: RowOutcome = { line, title: row.title, - status: 'created', + status: writeStatus, postId: post.id, + ...(warnings.length > 0 ? { warnings } : {}), }; try { outcome.url = this._urlForPost(post); @@ -266,7 +282,7 @@ class ContentCSVImporter { this._store.record(runId, outcome); } - if (attemptedWrites > 0 && successfulWrites === 0 && failedWrites === attemptedWrites) { + if (failedWrites > 0 && successfulWrites === 0) { this._report( new errors.InternalServerError({ message: tpl(messages.allWritesFailed, { diff --git a/ghost/core/core/server/services/content-import/import/post-data.ts b/ghost/core/core/server/services/content-import/import/post-data.ts index 4792f0db7c7..4265676185d 100644 --- a/ghost/core/core/server/services/content-import/import/post-data.ts +++ b/ghost/core/core/server/services/content-import/import/post-data.ts @@ -36,6 +36,7 @@ export interface PostsMetaData { export interface PostData { title: string; slug: string; + comment_id?: string; lexical?: string; custom_excerpt?: string; feature_image?: string; @@ -51,11 +52,13 @@ export interface PostData { status: 'draft' | 'published'; type: 'post' | 'page'; visibility: 'public' | 'members' | 'paid'; - tags: Array<{ name: string }>; + authors?: Array<{ id: string }>; + tags: Array<{ id: string } | { name: string }>; posts_meta?: PostsMetaData; } const DIRECT_OPTIONAL_FIELDS = [ + 'comment_id', 'custom_excerpt', 'feature_image', 'canonical_url', diff --git a/ghost/core/core/server/services/content-import/import/post-repository.ts b/ghost/core/core/server/services/content-import/import/post-repository.ts new file mode 100644 index 00000000000..4e2d46ef47d --- /dev/null +++ b/ghost/core/core/server/services/content-import/import/post-repository.ts @@ -0,0 +1,138 @@ +import type { PostData } from './post-data'; +import { + BookshelfPostRelationsResolver, + type PostRelationSource, + type PostRelationsResolver, + type RelationModels, +} from './relations'; + +export interface WrittenPost { + id: string; + toJSON(): Record; +} + +export type PostWriteResult = + | { status: 'created'; post: WrittenPost; warnings: string[] } + | { status: 'updated'; post: WrittenPost; warnings: string[] } + | { status: 'skipped'; reason: string }; + +export interface PostWriteMetadata extends PostRelationSource { + sourceUpdatedAt?: string; +} + +export interface PostsRepository { + write(data: PostData, options: object, metadata?: PostWriteMetadata): Promise; +} + +interface Models extends RelationModels { + Base: { + transaction(callback: (transacting: object) => Promise): Promise; + }; + Post: { + findOne(data: object, options: object): Promise; + add(data: PostData, options: object): Promise; + edit(data: object, options: object): Promise; + }; +} + +export class BookshelfPostsRepository implements PostsRepository { + private _models: Models; + private _relations: PostRelationsResolver; + + constructor( + models: Models, + relations: PostRelationsResolver = new BookshelfPostRelationsResolver(models), + ) { + this._models = models; + this._relations = relations; + } + + write( + data: PostData, + options: object, + metadata: PostWriteMetadata = {}, + ): Promise { + return this._models.Base.transaction(async (transacting) => { + const writeOptions = { ...options, transacting }; + const lookupOptions = { ...writeOptions, forUpdate: true }; + let existingMatch: { post: WrittenPost; duplicateReason: string } | undefined; + + if (data.comment_id) { + const existing = await this._models.Post.findOne( + { comment_id: data.comment_id, status: 'all' }, + lookupOptions, + ); + + if (existing) { + existingMatch = { + post: existing, + duplicateReason: `A post with the source ID "${data.comment_id}" already exists.`, + }; + } + } + + if (!existingMatch) { + const existing = await this._models.Post.findOne( + { slug: data.slug, status: 'all' }, + lookupOptions, + ); + + if (existing) { + existingMatch = { + post: existing, + duplicateReason: `A post with the slug "${data.slug}" already exists.`, + }; + } + } + + if (existingMatch) { + const { post: existing, duplicateReason } = existingMatch; + if (!metadata.sourceUpdatedAt) { + return { status: 'skipped', reason: duplicateReason }; + } + + const incomingInstant = new Date(metadata.sourceUpdatedAt).getTime(); + const storedUpdatedAt = existing.toJSON().updated_at; + const storedInstant = storedUpdatedAt + ? new Date(storedUpdatedAt as string | number | Date).getTime() + : undefined; + + if ( + Number.isNaN(incomingInstant) || + (storedInstant !== undefined && incomingInstant <= storedInstant) + ) { + return { + status: 'skipped', + reason: 'The existing post is newer than or as recent as the imported row.', + }; + } + + // Ghost's collision plugin treats updated_at as the client's version token. + // First update the content against the locked server version, then persist + // the incoming source timestamp on its own. The second edit is safe because + // timestamp-only importing edits are excluded from collision detection. + const resolved = await this._relations.resolve(data, metadata, writeOptions); + const collisionSafeData: Record = { ...resolved.data }; + if (storedUpdatedAt) { + collisionSafeData.updated_at = storedUpdatedAt; + } else { + delete collisionSafeData.updated_at; + } + const editOptions = { + ...writeOptions, + id: existing.id, + }; + await this._models.Post.edit(collisionSafeData, { ...editOptions }); + const post = await this._models.Post.edit( + { updated_at: metadata.sourceUpdatedAt }, + { ...editOptions }, + ); + return { status: 'updated', post, warnings: resolved.warnings }; + } + + const resolved = await this._relations.resolve(data, metadata, writeOptions); + const post = await this._models.Post.add(resolved.data, writeOptions); + return { status: 'created', post, warnings: resolved.warnings }; + }); + } +} diff --git a/ghost/core/core/server/services/content-import/import/relations.ts b/ghost/core/core/server/services/content-import/import/relations.ts new file mode 100644 index 00000000000..b37d5674804 --- /dev/null +++ b/ghost/core/core/server/services/content-import/import/relations.ts @@ -0,0 +1,224 @@ +import type { PostData } from './post-data'; + +const { slugify } = require('@tryghost/string'); +const validator = require('@tryghost/validator'); + +interface RelationModel { + id: string; +} + +export interface RelationModels { + User: { + findOne(data: object, options: object): Promise; + getByEmail(email: string, options: object): Promise; + add(data: object, options: object): Promise; + getOwnerUser(options: object): Promise; + }; + Tag: { + findOne(data: object, options: object): Promise; + add(data: object, options: object): Promise; + }; +} + +export interface PostRelationSource { + authorNames?: string; + authorEmails?: string; + tagNames?: string; +} + +export interface AuthorReference { + name?: string; + email?: string; +} + +export interface PostRelationsResolver { + resolve(data: PostData, source: PostRelationSource, options: object): Promise; +} + +export interface ResolvedRelations { + data: PostData; + warnings: string[]; +} + +export function parseAuthorReferences( + authorNames?: string, + authorEmails?: string, +): AuthorReference[] { + const names = splitList(authorNames); + const emails = splitList(authorEmails); + return Array.from({ length: Math.max(names.length, emails.length) }, (_, index) => ({ + ...(names[index] ? { name: names[index] } : {}), + ...(emails[index] ? { email: emails[index] } : {}), + })); +} + +export function parseTagReferences(tagNames?: string): string[] { + return splitList(tagNames).filter((tag): tag is string => Boolean(tag)); +} + +export class BookshelfPostRelationsResolver implements PostRelationsResolver { + private _models: RelationModels; + + constructor(models: RelationModels) { + this._models = models; + } + + async resolve( + data: PostData, + source: PostRelationSource, + options: object, + ): Promise { + const { authors, warnings } = await this.resolveAuthors(source, options); + const tags = await this.resolveTags(source, options); + const resolved: PostData = { + ...data, + tags: [...tags, ...data.tags], + }; + + if (authors.length > 0) { + resolved.authors = authors; + } + + return { data: resolved, warnings }; + } + + private async resolveAuthors( + source: PostRelationSource, + options: object, + ): Promise<{ authors: Array<{ id: string }>; warnings: string[] }> { + const authors: Array<{ id: string }> = []; + const warnings: string[] = []; + const seen = new Set(); + const authorsByEmail = new Map(); + let owner: RelationModel | undefined; + + const useOwner = async (warning: string) => { + owner ??= await this._models.User.getOwnerUser({ ...options }); + warnings.push(warning); + if (!seen.has(owner.id)) { + seen.add(owner.id); + authors.push({ id: owner.id }); + } + }; + + for (const reference of parseAuthorReferences(source.authorNames, source.authorEmails)) { + let author: RelationModel | null = null; + if (reference.email) { + if (!validator.isEmail(reference.email)) { + await useOwner(`Author email "${reference.email}" is invalid; assigned Owner instead.`); + continue; + } + + const emailKey = reference.email.toLowerCase(); + author = authorsByEmail.get(emailKey) ?? null; + if (!author) { + author = (await this._models.User.getByEmail(emailKey, { ...options })) ?? null; + } + + if (!author && reference.name) { + author = await this._models.User.add( + { + name: reference.name, + email: emailKey, + roles: ['Contributor'], + }, + { ...options }, + ); + } + + if (!author) { + await useOwner(`Author email "${reference.email}" has no name; assigned Owner instead.`); + continue; + } + + authorsByEmail.set(emailKey, author); + } else if (reference.name) { + author = await this._models.User.findOne({ slug: slugify(reference.name) }, { ...options }); + + if (!author) { + await useOwner(`Author "${reference.name}" has no email; assigned Owner instead.`); + continue; + } + } else { + await useOwner('An empty author entry was assigned to Owner instead.'); + continue; + } + + if (author && !seen.has(author.id)) { + seen.add(author.id); + authors.push({ id: author.id }); + } + } + + return { authors, warnings }; + } + + private async resolveTags( + source: PostRelationSource, + options: object, + ): Promise> { + const tags: Array<{ id: string }> = []; + const seen = new Set(); + + for (const reference of parseTagReferences(source.tagNames)) { + const normalizedSlug = slugify(reference); + const lookups = [{ name: reference }, { slug: reference }]; + if (normalizedSlug && normalizedSlug !== reference) { + lookups.push({ slug: normalizedSlug }); + } + + const findTag = async (lookupOptions: object = {}) => { + for (const lookup of lookups) { + const tag = await this._models.Tag.findOne(lookup, { + ...options, + ...lookupOptions, + }); + if (tag) { + return tag; + } + } + return null; + }; + + let tag = await findTag(); + if (!tag) { + try { + tag = await this._models.Tag.add({ name: reference }, { ...options }); + } catch (error) { + if (!isUniqueConstraintError(error)) { + throw error; + } + + // A concurrent row or import may have created the same slug after our + // lookup. A locking read sees that committed row under MySQL's default + // repeatable-read isolation, where another ordinary select may not. + tag = await findTag({ forUpdate: true }); + if (!tag) { + throw error; + } + } + } + + if (!seen.has(tag.id)) { + seen.add(tag.id); + tags.push({ id: tag.id }); + } + } + + return tags; + } +} + +function splitList(value?: string): Array { + return value?.split(',').map((part) => part.trim() || undefined) ?? []; +} + +function isUniqueConstraintError(error: unknown): boolean { + const code = + typeof error === 'object' && error !== null && 'code' in error + ? (error as { code?: unknown }).code + : undefined; + return ( + code === 'ER_DUP_ENTRY' || (typeof code === 'string' && code.startsWith('SQLITE_CONSTRAINT')) + ); +} diff --git a/ghost/core/core/server/services/content-import/import/row.ts b/ghost/core/core/server/services/content-import/import/row.ts index d5dbe4f8ea7..6eabd975872 100644 --- a/ghost/core/core/server/services/content-import/import/row.ts +++ b/ghost/core/core/server/services/content-import/import/row.ts @@ -14,6 +14,9 @@ export const EDITORIAL_POST_FIELDS = [ 'created_at', 'updated_at', 'published_at', + 'authors', + 'author_emails', + 'tags', 'feature_image', 'feature_image_alt', 'feature_image_caption', @@ -27,6 +30,7 @@ export const EDITORIAL_POST_FIELDS = [ 'twitter_image', 'twitter_title', 'twitter_description', + 'comment_id', 'custom_template', 'codeinjection_head', 'codeinjection_foot', @@ -58,6 +62,9 @@ export const postImportRowSchema = z created_at: optionalCell, updated_at: optionalCell, published_at: optionalCell, + authors: optionalCell, + author_emails: optionalCell, + tags: optionalCell, feature_image: optionalCell, feature_image_alt: optionalCell, feature_image_caption: optionalCell, @@ -71,6 +78,7 @@ export const postImportRowSchema = z twitter_image: optionalCell, twitter_title: optionalCell, twitter_description: optionalCell, + comment_id: optionalCell, custom_template: optionalCell, codeinjection_head: optionalCell, codeinjection_foot: optionalCell, @@ -99,6 +107,10 @@ export const importableRowSchema = postImportRowSchema.superRefine((row, ctx) => ctx.addIssue({ code: 'custom', message: 'title must be 255 characters or fewer' }); } + if (row.comment_id && row.comment_id.length > 50) { + ctx.addIssue({ code: 'custom', message: 'comment_id must be 50 characters or fewer' }); + } + for (const field of DATE_FIELDS) { const value = row[field]; if (value && !isValidDate(value)) { diff --git a/ghost/core/core/server/services/content-import/import/store.ts b/ghost/core/core/server/services/content-import/import/store.ts index bdceece620b..a3c5c8a4318 100644 --- a/ghost/core/core/server/services/content-import/import/store.ts +++ b/ghost/core/core/server/services/content-import/import/store.ts @@ -6,7 +6,7 @@ // failed = the write was attempted and lost. export type Clock = () => Date; -export type RowStatus = 'created' | 'skipped' | 'failed'; +export type RowStatus = 'created' | 'updated' | 'skipped' | 'failed'; export interface RowOutcome { // Source line number as a publisher sees it in a spreadsheet: the header is @@ -15,6 +15,7 @@ export interface RowOutcome { title: string | null; status: RowStatus; reason?: string; + warnings?: string[]; postId?: string; url?: string; } diff --git a/ghost/core/core/server/services/content-import/index.ts b/ghost/core/core/server/services/content-import/index.ts index 313597aea69..5ef2ed4ce6a 100644 --- a/ghost/core/core/server/services/content-import/index.ts +++ b/ghost/core/core/server/services/content-import/index.ts @@ -1,5 +1,6 @@ import { z } from 'zod'; import ContentCSVImporter, { type ImportAccepted, type FailureReporter } from './import/importer'; +import { BookshelfPostsRepository } from './import/post-repository'; import readPostRows from './import/reader'; import { importRequestSchema, type ImportRequest } from './import/schema'; import { ImportRunStore } from './import/store'; @@ -43,9 +44,7 @@ function makeImporter(): ContentCSVImporter { return new ContentCSVImporter({ readRows: readPostRows, prepareSource: prepareImportSource, - posts: { - create: (data, options) => models.Post.add(data, options), - }, + posts: new BookshelfPostsRepository(models), getHtmlToLexical: () => lexicalLib.htmlToLexicalConverter, getMarkdownToHtml: () => require('@tryghost/kg-markdown-html-renderer').render, getCleanHTML: () => require('@tryghost/mg-clean-html').cleanHTML, diff --git a/ghost/core/test/e2e-api/admin/posts-importer.test.js b/ghost/core/test/e2e-api/admin/posts-importer.test.js index 4a7f0657bc3..bb6a8c95ac0 100644 --- a/ghost/core/test/e2e-api/admin/posts-importer.test.js +++ b/ghost/core/test/e2e-api/admin/posts-importer.test.js @@ -96,6 +96,34 @@ describe('Posts Importer API', function () { .expect(cacheInvalidateHeaderNotSet()); }); + it('Keeps content import initialization idempotent and rejects invalid service requests', async function () { + const contentImportService = + await import('../../../core/server/services/content-import/index.ts?coverage-lifecycle'); + + assert.throws( + () => contentImportService.importCSV({ filePath: '/tmp/posts.csv', fileName: 'posts.csv' }), + /Content import service used before init/, + ); + contentImportService.init(); + contentImportService.init(); + + assert.throws( + () => contentImportService.importCSV({ filePath: '', fileName: '' }), + (error) => { + assert.equal(error.errorType, 'ValidationError'); + assert.match(error.message, /Too small/); + return true; + }, + ); + await assert.rejects( + contentImportService.importCSV({ + filePath: path.join(tmpDir, 'missing.csv'), + fileName: 'missing.csv', + }), + /The file could not be parsed as a CSV file/, + ); + }); + it('Can upload a posts CSV as Administrator', async function () { await agent.loginAsAdmin(); @@ -506,8 +534,8 @@ describe('Posts Importer API', function () { reason: /Invalid CSV header mapping: "constructor"/, }, { - mapping: { First: 'title', Second: 'authors' }, - reason: /Unknown post field mapping: "authors"/, + mapping: { First: 'title', Second: 'newsletter_id' }, + reason: /Unknown post field mapping: "newsletter_id"/, }, { mapping: { First: 'title', Second: 'title' }, @@ -613,6 +641,130 @@ describe('Posts Importer API', function () { assert.equal(two.get('updated_at').toISOString(), '2024-06-15T18:45:00.000Z'); }); + it('Skips existing post slugs when the same CSV is imported again', async function () { + await agent.loginAsOwner(); + + const duplicateCsvPath = await csvFile( + 'posts-import-deduplication.csv', + 'title,slug,html\n' + + 'CSV deduplication check,csv-deduplication-check,

Only one copy

\n', + ); + + await agent.post('posts/upload/').attach('postsfile', duplicateCsvPath).expectStatus(202); + await jobsService.allSettled(); + await agent.post('posts/upload/').attach('postsfile', duplicateCsvPath).expectStatus(202); + await jobsService.allSettled(); + + const { data: posts } = await models.Post.findPage({ + filter: "slug:'csv-deduplication-check'", + status: 'all', + limit: 'all', + }); + + assert.equal(posts.length, 1); + assert.equal(posts[0].get('title'), 'CSV deduplication check'); + assert.match(posts[0].get('html'), /Only one copy/); + }); + + it('Matches explicit CSV source IDs before falling back to slugs', async function () { + await agent.loginAsOwner(); + + const originalCsvPath = await csvFile( + 'posts-import-source-id-original.csv', + 'title,slug,comment_id\n' + + 'CSV source ID original,csv-source-id-original,m5-source-id-primary\n', + ); + await agent.post('posts/upload/').attach('postsfile', originalCsvPath).expectStatus(202); + await jobsService.allSettled(); + + const comparisonCsvPath = await csvFile( + 'posts-import-source-id-comparisons.csv', + 'Headline,Address,Source\n' + + 'CSV source ID duplicate,csv-source-id-different,m5-source-id-primary\n' + + 'CSV source ID slug fallback,csv-source-id-original,m5-source-id-unmatched\n' + + `CSV source ID too long,csv-source-id-too-long,${'x'.repeat(51)}\n` + + 'CSV source ID distinct,csv-source-id-distinct,m5-source-id-distinct\n', + ); + const form = new FormData(); + form.append('mapping[Headline]', 'title'); + form.append('mapping[Address]', 'slug'); + form.append('mapping[Source]', 'comment_id'); + form.append('postsfile', await fs.readFile(comparisonCsvPath), { + filename: path.basename(comparisonCsvPath), + contentType: 'text/csv', + }); + await agent.post('posts/upload/').body(form).expectStatus(202); + await jobsService.allSettled(); + + const original = await models.Post.findOne({ slug: 'csv-source-id-original', status: 'all' }); + const sourceDuplicate = await models.Post.findOne({ + slug: 'csv-source-id-different', + status: 'all', + }); + const tooLong = await models.Post.findOne({ + slug: 'csv-source-id-too-long', + status: 'all', + }); + const distinct = await models.Post.findOne({ slug: 'csv-source-id-distinct', status: 'all' }); + + assert.ok(original); + assert.equal(original.get('title'), 'CSV source ID original'); + assert.equal(original.get('comment_id'), 'm5-source-id-primary'); + assert.equal(sourceDuplicate, null, 'the source ID match takes precedence over its new slug'); + assert.equal(tooLong, null, 'an invalid source ID skips only its row'); + assert.ok(distinct, 'a valid row after the invalid source ID is still imported'); + assert.equal(distinct.get('comment_id'), 'm5-source-id-distinct'); + }); + + it('Updates matching posts only when the CSV has a newer explicit updated_at', async function () { + await agent.loginAsOwner(); + + const originalCsvPath = await csvFile( + 'posts-import-update-originals.csv', + 'title,slug,comment_id,updated_at\n' + + 'CSV update original,csv-update-original,m5-update-source,2025-01-01T00:00:00.000Z\n' + + 'CSV update slug original,csv-update-by-slug,,2025-01-01T00:00:00.000Z\n', + ); + await agent.post('posts/upload/').attach('postsfile', originalCsvPath).expectStatus(202); + await jobsService.allSettled(); + + const updatesCsvPath = await csvFile( + 'posts-import-update-comparisons.csv', + 'title,slug,comment_id,updated_at\n' + + 'CSV update newer,csv-update-newer,m5-update-source,2025-02-01T00:00:00.000Z\n' + + 'CSV update equal,csv-update-equal,m5-update-source,2025-02-01T01:00:00.000+01:00\n' + + 'CSV update older,csv-update-older,m5-update-source,2025-01-31T23:59:59.999Z\n' + + 'CSV update by slug,csv-update-by-slug,,2025-03-01T00:00:00.000Z\n' + + 'CSV update invalid date,csv-update-invalid-date,,not-a-date\n' + + 'CSV update after invalid,csv-update-after-invalid,,2025-04-01T00:00:00.000Z\n', + ); + await agent.post('posts/upload/').attach('postsfile', updatesCsvPath).expectStatus(202); + await jobsService.allSettled(); + + const newer = await models.Post.findOne({ + comment_id: 'm5-update-source', + status: 'all', + }); + const updatedBySlug = await models.Post.findOne({ + slug: 'csv-update-by-slug', + status: 'all', + }); + const invalid = await models.Post.findOne({ slug: 'csv-update-invalid-date', status: 'all' }); + const afterInvalid = await models.Post.findOne({ + slug: 'csv-update-after-invalid', + status: 'all', + }); + + assert.ok(newer); + assert.equal(newer.get('updated_at').toISOString(), '2025-02-01T00:00:00.000Z'); + assert.equal(newer.get('title'), 'CSV update newer'); + assert.ok(updatedBySlug); + assert.equal(updatedBySlug.get('title'), 'CSV update by slug'); + assert.equal(updatedBySlug.get('updated_at').toISOString(), '2025-03-01T00:00:00.000Z'); + assert.equal(invalid, null, 'an invalid updated_at skips only its row'); + assert.ok(afterInvalid, 'a valid row after the invalid date is still imported'); + }); + it('Imports public posts even when the site default visibility is paid', async function () { mockManager.mockSetting('default_content_visibility', 'paid'); await agent.loginAsOwner(); @@ -689,6 +841,223 @@ describe('Posts Importer API', function () { assert.match(post.get('html'), /Mapped body/); }); + it('Reconciles mapped CSV authors and tags with existing records', async function () { + await agent.loginAsOwner(); + + const emailAuthor = fixtureManager.get('users', 1); + const nameAuthor = fixtureManager.get('users', 3); + const tagOptions = { context: { internal: true } }; + const exactTag = await models.Tag.add( + { name: 'CSV exact relation', slug: 'csv-exact-relation-stored' }, + tagOptions, + ); + const explicitSlugTag = await models.Tag.add( + { name: 'Stored explicit relation', slug: 'csv-explicit-relation' }, + tagOptions, + ); + const normalizedSlugTag = await models.Tag.add( + { name: 'Stored normalized relation', slug: 'csv-normalized-relation' }, + tagOptions, + ); + const relationsCsvPath = await csvFile( + 'posts-import-existing-relations.csv', + 'Headline,Bylines,Emails,Topics\n' + + `CSV existing relations,"${emailAuthor.name}, ${nameAuthor.name}, ${emailAuthor.name}","${emailAuthor.email.toUpperCase()}, , ${emailAuthor.email}","CSV exact relation,csv-explicit-relation,CSV normalized relation,CSV exact relation"\n`, + ); + const form = new FormData(); + for (const [header, field] of Object.entries({ + Headline: 'title', + Bylines: 'authors', + Emails: 'author_emails', + Topics: 'tags', + })) { + form.append(`mapping[${header}]`, field); + } + form.append('postsfile', await fs.readFile(relationsCsvPath), { + filename: path.basename(relationsCsvPath), + contentType: 'text/csv', + }); + + await agent.post('posts/upload/').body(form).expectStatus(202); + await jobsService.allSettled(); + + const post = await models.Post.findOne( + { title: 'CSV existing relations', status: 'all' }, + { withRelated: ['authors', 'tags'] }, + ); + assert.ok(post); + assert.deepEqual( + post.related('authors').map((author) => author.id), + [emailAuthor.id, nameAuthor.id], + 'email and name-only matches retain source order and remove duplicates', + ); + const importedTags = post.related('tags').models; + assert.deepEqual( + importedTags.slice(0, 3).map((tag) => tag.id), + [exactTag.id, explicitSlugTag.id, normalizedSlugTag.id], + 'exact names, explicit slugs, and normalized slugs retain source order', + ); + assert.equal(importedTags.length, 5); + assert.match(importedTags[3].get('name'), /^#Import /); + assert.match(importedTags[4].get('name'), /^#Import Run /); + }); + + it('Creates missing CSV authors as locked Contributors and falls back to Owner', async function () { + await agent.loginAsOwner(); + + const authorsCsvPath = await csvFile( + 'posts-import-new-authors.csv', + 'title,authors,author_emails\n' + + 'CSV created contributor,"New CSV Contributor, New CSV Contributor","new-csv-contributor@example.com, new-csv-contributor@example.com"\n' + + 'CSV missing author email,Missing CSV Email,\n' + + 'CSV invalid author email,Invalid CSV Email,not-an-email\n', + ); + + await agent.post('posts/upload/').attach('postsfile', authorsCsvPath).expectStatus(202); + await jobsService.allSettled(); + + const contributor = await models.User.findOne( + { email: 'new-csv-contributor@example.com', status: 'all' }, + { withRelated: ['roles'] }, + ); + assert.ok(contributor); + assert.equal(contributor.get('status'), 'locked'); + assert.deepEqual( + contributor.related('roles').map((role) => role.get('name')), + ['Contributor'], + ); + + const owner = await models.User.getOwnerUser(); + const createdPost = await models.Post.findOne( + { title: 'CSV created contributor', status: 'all' }, + { withRelated: ['authors'] }, + ); + const missingEmailPost = await models.Post.findOne( + { title: 'CSV missing author email', status: 'all' }, + { withRelated: ['authors'] }, + ); + const invalidEmailPost = await models.Post.findOne( + { title: 'CSV invalid author email', status: 'all' }, + { withRelated: ['authors'] }, + ); + assert.deepEqual( + createdPost.related('authors').map((author) => author.id), + [contributor.id], + 'duplicate author inputs create and attach one Contributor', + ); + for (const post of [missingEmailPost, invalidEmailPost]) { + assert.deepEqual( + post.related('authors').map((author) => author.id), + [owner.id], + ); + } + assert.equal(await models.User.findOne({ slug: 'missing-csv-email', status: 'all' }), null); + assert.equal(await models.User.findOne({ slug: 'invalid-csv-email', status: 'all' }), null); + }); + + it('Rolls back a newly created Contributor when the post model fails', async function () { + await agent.loginAsOwner(); + sinon.stub(models.Post, 'add').rejects(new Error('post model failed')); + const authorsCsvPath = await csvFile( + 'posts-import-author-rollback.csv', + 'title,authors,author_emails\n' + + 'CSV contributor rollback,Rollback Contributor,csv-rollback-contributor@example.com\n', + ); + + await agent.post('posts/upload/').attach('postsfile', authorsCsvPath).expectStatus(202); + await jobsService.allSettled(); + + assert.equal( + await models.User.findOne({ email: 'csv-rollback-contributor@example.com', status: 'all' }), + null, + ); + assert.equal( + await models.Post.findOne({ title: 'CSV contributor rollback', status: 'all' }), + null, + ); + }); + + it('Creates missing CSV tags once and preserves their order and visibility', async function () { + await agent.loginAsOwner(); + + const firstCsvPath = await csvFile( + 'posts-import-new-tags.csv', + 'title,tags\n' + + 'CSV created tags one,"New CSV Tag,#CSV Internal Tag,New CSV Tag"\n' + + 'CSV created tags two,"#CSV Internal Tag,New CSV Tag"\n', + ); + await agent.post('posts/upload/').attach('postsfile', firstCsvPath).expectStatus(202); + await jobsService.allSettled(); + + const secondCsvPath = await csvFile( + 'posts-import-reused-tags.csv', + 'title,tags\nCSV reused tags,New CSV Tag\n', + ); + await agent.post('posts/upload/').attach('postsfile', secondCsvPath).expectStatus(202); + await jobsService.allSettled(); + + const publicTags = await models.Tag.findAll({ filter: "name:'New CSV Tag'" }); + const internalTags = await models.Tag.findAll({ filter: "name:'#CSV Internal Tag'" }); + assert.equal(publicTags.length, 1, 'later rows and imports reuse the created public tag'); + assert.equal(internalTags.length, 1, 'duplicate inputs create one internal tag'); + const publicTag = publicTags.at(0); + const internalTag = internalTags.at(0); + assert.equal(publicTag.get('visibility'), 'public'); + assert.equal(internalTag.get('visibility'), 'internal'); + + const firstPost = await models.Post.findOne( + { title: 'CSV created tags one', status: 'all' }, + { withRelated: ['tags'] }, + ); + const secondPost = await models.Post.findOne( + { title: 'CSV created tags two', status: 'all' }, + { withRelated: ['tags'] }, + ); + const reusedPost = await models.Post.findOne( + { title: 'CSV reused tags', status: 'all' }, + { withRelated: ['tags'] }, + ); + assert.deepEqual( + firstPost + .related('tags') + .models.slice(0, 2) + .map((tag) => tag.id), + [publicTag.id, internalTag.id], + ); + assert.deepEqual( + secondPost + .related('tags') + .models.slice(0, 2) + .map((tag) => tag.id), + [internalTag.id, publicTag.id], + ); + assert.equal(firstPost.related('tags').length, 4, 'the two batch tags remain attached'); + assert.equal(secondPost.related('tags').length, 4, 'the two batch tags remain attached'); + assert.deepEqual( + reusedPost + .related('tags') + .models.slice(0, 1) + .map((tag) => tag.id), + [publicTag.id], + ); + assert.equal(reusedPost.related('tags').length, 3, 'a later import gets its own batch tags'); + }); + + it('Rolls back a newly created tag when the post model fails', async function () { + await agent.loginAsOwner(); + sinon.stub(models.Post, 'add').rejects(new Error('post model failed')); + const tagsCsvPath = await csvFile( + 'posts-import-tag-rollback.csv', + 'title,tags\nCSV tag rollback,CSV Rollback Tag\n', + ); + + await agent.post('posts/upload/').attach('postsfile', tagsCsvPath).expectStatus(202); + await jobsService.allSettled(); + + assert.equal(await models.Tag.findOne({ name: 'CSV Rollback Tag' }), null); + assert.equal(await models.Post.findOne({ title: 'CSV tag rollback', status: 'all' }), null); + }); + it('Renders a mapped Markdown column through the post content converter', async function () { await agent.loginAsOwner(); diff --git a/ghost/core/test/e2e-webhooks/posts-importer.test.ts b/ghost/core/test/e2e-webhooks/posts-importer.test.ts index b196365a917..32cb70aca79 100644 --- a/ghost/core/test/e2e-webhooks/posts-importer.test.ts +++ b/ghost/core/test/e2e-webhooks/posts-importer.test.ts @@ -82,20 +82,32 @@ describe('CSV content import side-effects', function () { // published, so both events are armed. const publishedURL = 'https://test-webhook-receiver.com/post-published-import/'; const addedURL = 'https://test-webhook-receiver.com/post-added-import/'; + const editedURL = 'https://test-webhook-receiver.com/post-edited-import/'; await webhookMockReceiver.mock(publishedURL); await webhookMockReceiver.mock(addedURL); + await webhookMockReceiver.mock(editedURL); await fixtureManager.insertWebhook({ event: 'post.published', url: publishedURL }); await fixtureManager.insertWebhook({ event: 'post.added', url: addedURL }); + await fixtureManager.insertWebhook({ event: 'post.edited', url: editedURL }); const csvPath = path.join(tmpDir, 'posts-import-side-effects.csv'); await fs.writeFile( csvPath, - 'title,html,published_at\n' + - 'Side effect check one,

First

,2024-07-01T00:00:00.000Z\n' + - 'Side effect check two,

Second

,2024-07-02T00:00:00.000Z\n', + 'title,html,published_at,updated_at,comment_id\n' + + 'Side effect check one,

First

,2024-07-01T00:00:00.000Z,2024-07-01T00:00:00.000Z,side-effect-one\n' + + 'Side effect check two,

Second

,2024-07-02T00:00:00.000Z,2024-07-02T00:00:00.000Z,side-effect-two\n', ); await adminAPIAgent.post('posts/upload/').attach('postsfile', csvPath).expectStatus(202); + await jobsService.allSettled(); + + const updateCsvPath = path.join(tmpDir, 'posts-import-update-side-effects.csv'); + await fs.writeFile( + updateCsvPath, + 'title,slug,html,updated_at,comment_id\n' + + 'Side effect check one updated,side-effect-check-one-updated,

Updated

,2024-08-01T00:00:00.000Z,side-effect-one\n', + ); + await adminAPIAgent.post('posts/upload/').attach('postsfile', updateCsvPath).expectStatus(202); await jobsService.allSettled(); await DomainEvents.allSettled(); @@ -113,6 +125,10 @@ describe('CSV content import side-effects', function () { }); assert.equal(posts.length, 2, 'both rows imported'); assert.equal(posts[0].get('status'), 'published'); + assert.ok( + posts.some((post: any) => post.get('title') === 'Side effect check one updated'), + 'the newer row updated its matching post', + ); // Zero webhooks: the receiver never recorded a request assert.equal(webhookMockReceiver.body, undefined, 'no webhook fired for the imported posts'); diff --git a/ghost/core/test/unit/server/services/content-import/import/importer.test.ts b/ghost/core/test/unit/server/services/content-import/import/importer.test.ts index d08124bf46d..15b1eb73cec 100644 --- a/ghost/core/test/unit/server/services/content-import/import/importer.test.ts +++ b/ghost/core/test/unit/server/services/content-import/import/importer.test.ts @@ -5,6 +5,7 @@ import ContentCSVImporter from '../../../../../../core/server/services/content-i import { ImportRunStore } from '../../../../../../core/server/services/content-import/import/store'; import type { PostImportRow } from '../../../../../../core/server/services/content-import/import/row'; import type { PostData } from '../../../../../../core/server/services/content-import/import/post-data'; +import type { PostWriteMetadata } from '../../../../../../core/server/services/content-import/import/post-repository'; const row = (title: string, html = `

${title}

`): PostImportRow => ({ title, @@ -15,10 +16,13 @@ const row = (title: string, html = `

${title}

`): PostImportRow => ({ // Collaborators are handed back as `deps` so a test can repoint the seam it is breaking. function harness(rows: PostImportRow[] = [row('First'), row('Second')]) { - const created: Array<{ data: PostData; options: object }> = []; + const created: Array<{ data: PostData; options: object; metadata?: PostWriteMetadata }> = []; const reported: unknown[] = []; const jobs: Array<{ name: string; offloaded: boolean; job: () => Promise }> = []; const createFailures = new Map(); + const duplicateSlugs = new Set(); + const updatedTitles = new Set(); + const warningsByTitle = new Map(); const urlFailures = new Map(); const store = new ImportRunStore(); let converterResolutions = 0; @@ -41,14 +45,24 @@ function harness(rows: PostImportRow[] = [row('First'), row('Second')]) { const deps = { readRows: async () => rows, posts: { - create: async (data: PostData, options: object) => { + write: async (data: PostData, options: object, metadata?: PostWriteMetadata) => { const failure = createFailures.get(data.title); if (failure) { throw failure; } - created.push({ data, options }); + if (duplicateSlugs.has(data.slug)) { + return { + status: 'skipped' as const, + reason: `A post with the slug "${data.slug}" already exists.`, + }; + } + created.push({ data, options, metadata }); const id = `post_${created.length}`; - return { id, toJSON: () => ({ id, slug: `slug-${created.length}` }) }; + return { + status: updatedTitles.has(data.title) ? ('updated' as const) : ('created' as const), + post: { id, toJSON: () => ({ id, slug: `slug-${created.length}` }) }, + warnings: warningsByTitle.get(data.title) ?? [], + }; }, }, getHtmlToLexical: () => htmlToLexicalFactory(), @@ -107,6 +121,9 @@ function harness(rows: PostImportRow[] = [row('First'), row('Second')]) { reported, jobs, createFailures, + duplicateSlugs, + updatedTitles, + warningsByTitle, urlFailures, store, setHtmlToLexicalFactory, @@ -306,9 +323,35 @@ describe('ContentCSVImporter', function () { ); for (const call of h.created) { assert.deepEqual(call.options, { importing: true, context: { internal: true } }); + assert.deepEqual(call.metadata, { + sourceUpdatedAt: undefined, + authorNames: undefined, + authorEmails: undefined, + tagNames: undefined, + }); } }); + it('forwards author and tag cells to the transactional write seam', async function () { + const h = harness([ + { + ...row('Related post'), + authors: 'Alice, Bob', + author_emails: 'alice@example.com, bob@example.com', + tags: 'News, Features', + }, + ]); + + await h.run(); + + assert.deepEqual(h.created[0].metadata, { + sourceUpdatedAt: undefined, + authorNames: 'Alice, Bob', + authorEmails: 'alice@example.com, bob@example.com', + tagNames: 'News, Features', + }); + }); + it('files every row of a run under a date-stamped tag and a unique run tag', async function () { const h = harness(); @@ -481,6 +524,93 @@ describe('ContentCSVImporter', function () { }); }); + it('records an existing slug as skipped without treating it as a failed write', async function () { + const h = harness([row('Duplicate'), row('Created')]); + h.duplicateSlugs.add('duplicate'); + + await h.run(); + + assert.deepEqual( + h.created.map((call) => call.data.title), + ['Created'], + ); + assert.deepEqual(h.reported, []); + assert.deepEqual(h.store.get('run_test')?.rows, [ + { + line: 2, + title: 'Duplicate', + status: 'skipped', + reason: 'A post with the slug "duplicate" already exists.', + }, + { + line: 3, + title: 'Created', + status: 'created', + postId: 'post_1', + url: 'https://example.com/post_1/', + }, + ]); + }); + + it('records an updated post and forwards only its explicit source timestamp', async function () { + const updatedRow = { ...row('Updated'), updated_at: '2025-02-01T00:00:00.000Z' }; + const h = harness([updatedRow]); + h.updatedTitles.add('Updated'); + + await h.run(); + + assert.deepEqual(h.created[0].metadata, { + sourceUpdatedAt: '2025-02-01T00:00:00.000Z', + authorNames: undefined, + authorEmails: undefined, + tagNames: undefined, + }); + assert.deepEqual(h.store.get('run_test')?.rows, [ + { + line: 2, + title: 'Updated', + status: 'updated', + postId: 'post_1', + url: 'https://example.com/post_1/', + }, + ]); + }); + + it('records a failed update against its row and continues importing', async function () { + const h = harness([row('Update failure'), row('Created')]); + h.updatedTitles.add('Update failure'); + h.createFailures.set('Update failure', new Error('update failed')); + + await h.run(); + + assert.deepEqual(h.store.get('run_test')?.rows[0], { + line: 2, + title: 'Update failure', + status: 'failed', + reason: 'update failed', + }); + assert.deepEqual( + h.created.map((call) => call.data.title), + ['Created'], + ); + assert.deepEqual(h.reported, []); + }); + + it('completes without reporting when every row is an existing slug', async function () { + const h = harness([row('First'), row('Second')]); + h.duplicateSlugs.add('first'); + h.duplicateSlugs.add('second'); + + await h.run(); + + assert.equal(h.created.length, 0); + assert.deepEqual(h.reported, []); + assert.deepEqual( + h.store.get('run_test')?.rows.map((outcome) => outcome.status), + ['skipped', 'skipped'], + ); + }); + it('reports once when every attempted write fails', async function () { const h = harness([row('First'), row('Second')]); h.createFailures.set('First', new Error('first insert failed')); @@ -507,6 +637,19 @@ describe('ContentCSVImporter', function () { ); }); + it('uses the singular post noun when the only attempted write fails', async function () { + const h = harness([row('Only')]); + h.createFailures.set('Only', new Error('only insert failed')); + + await h.run(); + + assert.equal(h.reported.length, 1); + assert.equal( + (h.reported[0] as Error).message, + 'Content import failed to write all 1 attempted post.', + ); + }); + it('skips a malformed row on its own and imports the rest', async function () { const h = harness([ row('First'), @@ -645,15 +788,34 @@ describe('ContentCSVImporter', function () { assert.equal(h.store.get('run_test')?.failureReason, 'Unknown error'); }); - it('keeps a successfully written post created when its URL cannot be resolved', async function () { + it('records relation warnings on successful row outcomes', async function () { + const h = harness([row('Owner fallback')]); + h.warningsByTitle.set('Owner fallback', [ + 'Author "Missing Author" has no email; assigned Owner instead.', + ]); + + await h.run(); + + assert.deepEqual(h.store.get('run_test')?.rows[0], { + line: 2, + title: 'Owner fallback', + status: 'created', + postId: 'post_1', + url: 'https://example.com/post_1/', + warnings: ['Author "Missing Author" has no email; assigned Owner instead.'], + }); + }); + + it('keeps a successfully written post when its URL cannot be resolved', async function () { const h = harness(); + h.updatedTitles.add('First'); h.urlFailures.set('post_1', new Error('URL service unavailable')); await h.run(); assert.equal(h.created.length, 2); assert.deepEqual(h.store.get('run_test')?.rows, [ - { line: 2, title: 'First', status: 'created', postId: 'post_1' }, + { line: 2, title: 'First', status: 'updated', postId: 'post_1' }, { line: 3, title: 'Second', @@ -665,7 +827,7 @@ describe('ContentCSVImporter', function () { assert.equal(h.reported.length, 1); assert.equal( (h.reported[0] as Error).message, - 'Content import could not resolve a URL for 1 created post.', + 'Content import could not resolve a URL for 1 imported post.', ); assert.match((h.reported[0] as Error).stack ?? '', /URL service unavailable/); }); @@ -684,7 +846,7 @@ describe('ContentCSVImporter', function () { assert.equal(h.reported.length, 1); assert.equal( (h.reported[0] as Error).message, - 'Content import could not resolve a URL for 2 created posts.', + 'Content import could not resolve a URL for 2 imported posts.', ); assert.match( (h.reported[0] as Error).stack ?? '', diff --git a/ghost/core/test/unit/server/services/content-import/import/post-data.test.ts b/ghost/core/test/unit/server/services/content-import/import/post-data.test.ts index a47f7b18a88..c6272df68cb 100644 --- a/ghost/core/test/unit/server/services/content-import/import/post-data.test.ts +++ b/ghost/core/test/unit/server/services/content-import/import/post-data.test.ts @@ -88,6 +88,17 @@ describe('buildPostData', function () { assert.equal(data.lexical, JSON.stringify({ converted: '

Hello

' })); }); + it('skips markdown when no renderer is available', function () { + assert.throws( + () => buildPostData(row({ title: 'T', markdown: '# Hello' }), htmlToLexical, TAGS), + (error: unknown) => { + assert.ok(error instanceof RowSkipped); + assert.equal(error.message, 'markdown could not be converted'); + return true; + }, + ); + }); + it('cleans markdown-rendered HTML before converting it to lexical', function () { const styledMarkdownRenderer = () => '

Hello

'; const data = buildPostData( @@ -160,6 +171,7 @@ describe('buildPostData', function () { data.tags, TAGS.map((name) => ({ name })), ); + assert.equal('comment_id' in data, false); assert.equal('authors' in data, false); }); @@ -178,6 +190,7 @@ describe('buildPostData', function () { custom_template: 'wide', codeinjection_head: '', codeinjection_foot: '', + comment_id: 'legacy-source-123', }), htmlToLexical, TAGS, @@ -194,6 +207,7 @@ describe('buildPostData', function () { assert.equal(data.custom_template, 'wide'); assert.equal(data.codeinjection_head, ''); assert.equal(data.codeinjection_foot, ''); + assert.equal(data.comment_id, 'legacy-source-123'); }); it('puts feature metadata, SEO, social fields, and frontmatter in posts_meta', function () { @@ -289,6 +303,23 @@ describe('buildPostData', function () { assert.equal(data.title.length, 255); }); + it('accepts a source ID of exactly 50 characters', function () { + const data = buildPostData( + row({ title: 'T', comment_id: 'x'.repeat(50) }), + htmlToLexical, + TAGS, + ); + + assert.equal(data.comment_id, 'x'.repeat(50)); + }); + + it('skips a row whose source ID is longer than 50 characters', function () { + skipsWith( + { title: 'T', comment_id: 'x'.repeat(51) }, + 'comment_id must be 50 characters or fewer', + ); + }); + it('skips a row whose published_at is not a date, quoting the cell', function () { skipsWith( { title: 'T', published_at: 'not-a-date' }, @@ -296,6 +327,13 @@ describe('buildPostData', function () { ); }); + it('skips a row whose explicit updated_at is not a date', function () { + skipsWith( + { title: 'T', updated_at: 'not-an-update-date' }, + 'updated_at is not a valid date: "not-an-update-date"', + ); + }); + it('skips a row whose published_at is a rolled-over calendar date', function () { // new Date() would normalize this to March 2 and quietly mis-date the post skipsWith( diff --git a/ghost/core/test/unit/server/services/content-import/import/post-repository.test.ts b/ghost/core/test/unit/server/services/content-import/import/post-repository.test.ts new file mode 100644 index 00000000000..298781866a6 --- /dev/null +++ b/ghost/core/test/unit/server/services/content-import/import/post-repository.test.ts @@ -0,0 +1,562 @@ +import assert from 'node:assert/strict'; +import sinon from 'sinon'; +import { BookshelfPostsRepository } from '../../../../../../core/server/services/content-import/import/post-repository'; +import type { PostData } from '../../../../../../core/server/services/content-import/import/post-data'; + +const data: PostData = { + title: 'Imported post', + slug: 'imported-post', + status: 'published', + type: 'post', + visibility: 'public', + tags: [], +}; + +function harness() { + const transacting = { transaction: true }; + const existing = { + id: 'existing', + toJSON: sinon.stub().returns({ id: 'existing', updated_at: new Date('2025-01-01T00:00:00Z') }), + }; + const created = { id: 'created', toJSON: () => ({ id: 'created' }) }; + const updated = { id: 'existing', toJSON: () => ({ id: 'existing' }) }; + const findOne = sinon.stub().resolves(null); + const findUser = sinon.stub().resolves(null); + const getUserByEmail = sinon.stub().resolves(undefined); + const addUser = sinon.stub().resolves({ id: 'author-created' }); + const getOwnerUser = sinon.stub().resolves({ id: 'owner' }); + const findTag = sinon.stub().resolves(null); + const addTag = sinon.stub().resolves({ id: 'tag-created' }); + const add = sinon.stub().resolves(created); + const edit = sinon.stub().resolves(updated); + const transaction = sinon.stub().callsFake(async (callback) => callback(transacting)); + const repository = new BookshelfPostsRepository({ + Base: { transaction }, + Post: { findOne, add, edit }, + User: { findOne: findUser, getByEmail: getUserByEmail, add: addUser, getOwnerUser }, + Tag: { findOne: findTag, add: addTag }, + }); + + return { + repository, + transaction, + transacting, + findOne, + findUser, + getUserByEmail, + addUser, + getOwnerUser, + findTag, + addTag, + add, + edit, + existing, + created, + updated, + }; +} + +describe('BookshelfPostsRepository', function () { + afterEach(function () { + sinon.restore(); + }); + + it('locks the matching slug and creates the post in one transaction', async function () { + const h = harness(); + const options = { importing: true, context: { internal: true } }; + + const result = await h.repository.write(data, options); + + assert.deepEqual(result, { status: 'created', post: h.created, warnings: [] }); + sinon.assert.calledOnce(h.transaction); + sinon.assert.calledWithExactly( + h.findOne, + { slug: 'imported-post', status: 'all' }, + { ...options, transacting: h.transacting, forUpdate: true }, + ); + sinon.assert.calledWithExactly(h.add, data, { ...options, transacting: h.transacting }); + assert.equal('transacting' in options, false, 'caller options are not mutated'); + }); + + it('skips an existing slug without creating another post', async function () { + const h = harness(); + h.findOne.resolves(h.existing); + + const result = await h.repository.write(data, { + importing: true, + context: { internal: true }, + }); + + assert.deepEqual(result, { + status: 'skipped', + reason: 'A post with the slug "imported-post" already exists.', + }); + sinon.assert.notCalled(h.add); + sinon.assert.notCalled(h.edit); + }); + + it('matches an explicit source ID before considering the slug', async function () { + const h = harness(); + h.findOne.resolves(h.existing); + + const result = await h.repository.write( + { ...data, comment_id: 'source-123', slug: 'a-different-slug' }, + { importing: true, context: { internal: true } }, + ); + + assert.deepEqual(result, { + status: 'skipped', + reason: 'A post with the source ID "source-123" already exists.', + }); + sinon.assert.calledOnce(h.findOne); + sinon.assert.calledWithMatch(h.findOne, { + comment_id: 'source-123', + status: 'all', + }); + sinon.assert.notCalled(h.add); + sinon.assert.notCalled(h.edit); + }); + + it('falls back to the slug when an explicit source ID does not match', async function () { + const h = harness(); + h.findOne.onFirstCall().resolves(null); + h.findOne.onSecondCall().resolves(h.existing); + + const result = await h.repository.write( + { ...data, comment_id: 'new-source' }, + { importing: true, context: { internal: true } }, + ); + + assert.deepEqual(result, { + status: 'skipped', + reason: 'A post with the slug "imported-post" already exists.', + }); + assert.deepEqual(h.findOne.firstCall.args[0], { + comment_id: 'new-source', + status: 'all', + }); + assert.deepEqual(h.findOne.secondCall.args[0], { + slug: 'imported-post', + status: 'all', + }); + sinon.assert.notCalled(h.add); + sinon.assert.notCalled(h.edit); + }); + + it('creates with an explicit source ID when neither source ID nor slug matches', async function () { + const h = harness(); + const sourceData = { ...data, comment_id: 'new-source' }; + + const result = await h.repository.write(sourceData, { + importing: true, + context: { internal: true }, + }); + + assert.deepEqual(result, { status: 'created', post: h.created, warnings: [] }); + sinon.assert.calledTwice(h.findOne); + sinon.assert.calledWithExactly( + h.add, + sourceData, + sinon.match({ importing: true, transacting: h.transacting }), + ); + sinon.assert.notCalled(h.edit); + }); + + it('reconciles existing authors and tags before creating the post', async function () { + const h = harness(); + h.getUserByEmail.resolves({ id: 'author-existing' }); + h.findTag.resolves({ id: 'tag-existing' }); + const options = { importing: true, context: { internal: true } }; + + await h.repository.write(data, options, { + authorNames: 'Existing Author', + authorEmails: 'author@example.com', + tagNames: 'Existing Tag', + }); + + sinon.assert.calledWithExactly( + h.add, + { + ...data, + authors: [{ id: 'author-existing' }], + tags: [{ id: 'tag-existing' }, ...data.tags], + }, + { ...options, transacting: h.transacting }, + ); + sinon.assert.calledWithExactly(h.getUserByEmail, 'author@example.com', { + ...options, + transacting: h.transacting, + }); + sinon.assert.calledWithExactly( + h.findTag, + { name: 'Existing Tag' }, + { ...options, transacting: h.transacting }, + ); + }); + + it('creates missing tags inside the post transaction and attaches their IDs', async function () { + const h = harness(); + const options = { importing: true, context: { internal: true } }; + + await h.repository.write(data, options, { tagNames: 'New Tag' }); + + sinon.assert.calledWithExactly( + h.addTag, + { name: 'New Tag' }, + { ...options, transacting: h.transacting }, + ); + sinon.assert.calledWithExactly( + h.add, + { ...data, tags: [{ id: 'tag-created' }, ...data.tags] }, + { ...options, transacting: h.transacting }, + ); + }); + + it('creates missing contributors and returns Owner fallback warnings with the post', async function () { + const h = harness(); + const options = { importing: true, context: { internal: true } }; + + const result = await h.repository.write(data, options, { + authorNames: 'New Contributor,Missing Email', + authorEmails: 'new@example.com,', + }); + + assert.deepEqual(result, { + status: 'created', + post: h.created, + warnings: ['Author "Missing Email" has no email; assigned Owner instead.'], + }); + sinon.assert.calledWithExactly( + h.addUser, + { + name: 'New Contributor', + email: 'new@example.com', + roles: ['Contributor'], + }, + { ...options, transacting: h.transacting }, + ); + sinon.assert.calledWithExactly(h.getOwnerUser, { ...options, transacting: h.transacting }); + assert.deepEqual(h.add.firstCall.args[0].authors, [{ id: 'author-created' }, { id: 'owner' }]); + }); + + it('does not reconcile relations for a skipped duplicate', async function () { + const h = harness(); + h.findOne.resolves(h.existing); + + await h.repository.write( + data, + {}, + { + authorEmails: 'author@example.com', + tagNames: 'Existing Tag', + }, + ); + + sinon.assert.notCalled(h.findUser); + sinon.assert.notCalled(h.getUserByEmail); + sinon.assert.notCalled(h.findTag); + sinon.assert.notCalled(h.add); + sinon.assert.notCalled(h.edit); + }); + + it('updates a matching post when the explicit incoming timestamp is newer', async function () { + const h = harness(); + const options = { importing: true, context: { internal: true } }; + const updatedData = { ...data, updated_at: '2025-02-01T00:00:00.000Z' }; + h.findOne.resolves(h.existing); + + const result = await h.repository.write(updatedData, options, { + sourceUpdatedAt: '2025-02-01T00:00:00.000Z', + }); + + assert.deepEqual(result, { status: 'updated', post: h.updated, warnings: [] }); + sinon.assert.calledTwice(h.edit); + sinon.assert.calledWithExactly( + h.edit.firstCall, + { ...updatedData, updated_at: new Date('2025-01-01T00:00:00Z') }, + { + ...options, + transacting: h.transacting, + id: 'existing', + }, + ); + sinon.assert.calledWithExactly( + h.edit.secondCall, + { updated_at: '2025-02-01T00:00:00.000Z' }, + { + ...options, + transacting: h.transacting, + id: 'existing', + }, + ); + sinon.assert.notCalled(h.add); + }); + + it('compares equal timestamps as instants and skips the update', async function () { + const h = harness(); + h.findOne.resolves(h.existing); + + const result = await h.repository.write( + data, + {}, + { + sourceUpdatedAt: '2025-01-01T01:00:00+01:00', + }, + ); + + assert.deepEqual(result, { + status: 'skipped', + reason: 'The existing post is newer than or as recent as the imported row.', + }); + sinon.assert.notCalled(h.edit); + }); + + it('skips an older incoming timestamp', async function () { + const h = harness(); + h.findOne.resolves(h.existing); + + const result = await h.repository.write( + data, + {}, + { + sourceUpdatedAt: '2024-12-31T23:59:59.999Z', + }, + ); + + assert.deepEqual(result.status, 'skipped'); + sinon.assert.notCalled(h.edit); + }); + + it('treats a missing stored timestamp as older', async function () { + const h = harness(); + h.existing.toJSON.returns({ id: 'existing', updated_at: null }); + h.findOne.resolves(h.existing); + + const result = await h.repository.write( + { ...data, updated_at: '2025-01-01T00:00:00.000Z' }, + { importing: true }, + { sourceUpdatedAt: '2025-01-01T00:00:00.000Z' }, + ); + + assert.deepEqual(result, { status: 'updated', post: h.updated, warnings: [] }); + sinon.assert.calledTwice(h.edit); + assert.equal('updated_at' in h.edit.firstCall.args[0], false); + }); + + it('reconciles relations before updating a newer post', async function () { + const h = harness(); + h.findOne.resolves(h.existing); + h.findUser.resolves({ id: 'author-existing' }); + h.findTag.resolves({ id: 'tag-existing' }); + + await h.repository.write( + { ...data, updated_at: '2025-02-01T00:00:00.000Z' }, + { importing: true }, + { + sourceUpdatedAt: '2025-02-01T00:00:00.000Z', + authorNames: 'Existing Author', + tagNames: 'Existing Tag', + }, + ); + + assert.deepEqual(h.edit.firstCall.args[0].authors, [{ id: 'author-existing' }]); + assert.deepEqual(h.edit.firstCall.args[0].tags, [{ id: 'tag-existing' }]); + }); + + it('does not update for an invalid incoming timestamp', async function () { + const h = harness(); + h.findOne.resolves(h.existing); + + const result = await h.repository.write(data, {}, { sourceUpdatedAt: 'not-a-date' }); + + assert.deepEqual(result.status, 'skipped'); + sinon.assert.notCalled(h.edit); + }); + + it('propagates lookup failures and never attempts the insert', async function () { + const h = harness(); + const failure = new Error('lookup failed'); + h.findOne.rejects(failure); + + await assert.rejects( + h.repository.write(data, { importing: true, context: { internal: true } }), + failure, + ); + + sinon.assert.notCalled(h.add); + }); + + it('propagates source ID lookup failures without falling back to the slug', async function () { + const h = harness(); + const failure = new Error('source lookup failed'); + h.findOne.rejects(failure); + + await assert.rejects( + h.repository.write( + { ...data, comment_id: 'source-123' }, + { importing: true, context: { internal: true } }, + ), + failure, + ); + + sinon.assert.calledOnce(h.findOne); + sinon.assert.notCalled(h.add); + }); + + it('propagates insert failures through the transaction', async function () { + const h = harness(); + const failure = new Error('insert failed'); + h.add.rejects(failure); + + await assert.rejects( + h.repository.write(data, { importing: true, context: { internal: true } }), + failure, + ); + }); + + it('keeps contributor creation in the post transaction so insert failures roll it back', async function () { + const h = harness(); + const failure = new Error('post insert failed'); + h.add.rejects(failure); + const options = { importing: true, context: { internal: true } }; + + await assert.rejects( + h.repository.write(data, options, { + authorNames: 'New Contributor', + authorEmails: 'new@example.com', + }), + failure, + ); + + sinon.assert.calledWithExactly( + h.addUser, + { + name: 'New Contributor', + email: 'new@example.com', + roles: ['Contributor'], + }, + { ...options, transacting: h.transacting }, + ); + sinon.assert.calledWithExactly(h.add, sinon.match.object, { + ...options, + transacting: h.transacting, + }); + }); + + it('keeps tag creation in the post transaction so insert failures roll it back', async function () { + const h = harness(); + const failure = new Error('post insert failed'); + h.add.rejects(failure); + const options = { importing: true, context: { internal: true } }; + + await assert.rejects(h.repository.write(data, options, { tagNames: 'New Tag' }), failure); + + sinon.assert.calledWithExactly( + h.addTag, + { name: 'New Tag' }, + { ...options, transacting: h.transacting }, + ); + sinon.assert.calledWithExactly(h.add, sinon.match.object, { + ...options, + transacting: h.transacting, + }); + }); + + it('propagates tag model failures without writing the post', async function () { + const h = harness(); + const failure = new Error('tag insert failed'); + h.addTag.rejects(failure); + + await assert.rejects( + h.repository.write(data, { importing: true }, { tagNames: 'New Tag' }), + failure, + ); + + sinon.assert.notCalled(h.add); + sinon.assert.notCalled(h.edit); + }); + + it('propagates contributor model failures without writing the post', async function () { + const h = harness(); + const failure = new Error('contributor insert failed'); + h.addUser.rejects(failure); + + await assert.rejects( + h.repository.write( + data, + { importing: true }, + { authorNames: 'New Contributor', authorEmails: 'new@example.com' }, + ), + failure, + ); + + sinon.assert.notCalled(h.add); + sinon.assert.notCalled(h.edit); + }); + + it('propagates relation lookup failures without writing the post', async function () { + const h = harness(); + const failure = new Error('relation lookup failed'); + h.getUserByEmail.rejects(failure); + + await assert.rejects( + h.repository.write(data, { importing: true }, { authorEmails: 'author@example.com' }), + failure, + ); + + sinon.assert.notCalled(h.add); + sinon.assert.notCalled(h.edit); + }); + + it('propagates update failures through the transaction', async function () { + const h = harness(); + const failure = new Error('update failed'); + h.findOne.resolves(h.existing); + h.edit.rejects(failure); + + await assert.rejects( + h.repository.write( + { ...data, updated_at: '2025-02-01T00:00:00.000Z' }, + { importing: true }, + { sourceUpdatedAt: '2025-02-01T00:00:00.000Z' }, + ), + failure, + ); + + sinon.assert.notCalled(h.add); + }); + + it('rolls back when persisting the incoming update timestamp fails', async function () { + const h = harness(); + const failure = new Error('timestamp update failed'); + h.findOne.resolves(h.existing); + h.edit.onFirstCall().resolves(h.updated); + h.edit.onSecondCall().rejects(failure); + + await assert.rejects( + h.repository.write( + { ...data, updated_at: '2025-02-01T00:00:00.000Z' }, + { importing: true }, + { sourceUpdatedAt: '2025-02-01T00:00:00.000Z' }, + ), + failure, + ); + + sinon.assert.calledTwice(h.edit); + sinon.assert.notCalled(h.add); + }); + + it('propagates a failure to open the transaction without querying posts', async function () { + const h = harness(); + const failure = new Error('transaction failed'); + h.transaction.rejects(failure); + + await assert.rejects( + h.repository.write(data, { importing: true, context: { internal: true } }), + failure, + ); + + sinon.assert.notCalled(h.findOne); + sinon.assert.notCalled(h.add); + }); +}); diff --git a/ghost/core/test/unit/server/services/content-import/import/reader.test.ts b/ghost/core/test/unit/server/services/content-import/import/reader.test.ts index bd481ae7edc..b2ebc649271 100644 --- a/ghost/core/test/unit/server/services/content-import/import/reader.test.ts +++ b/ghost/core/test/unit/server/services/content-import/import/reader.test.ts @@ -41,7 +41,10 @@ describe('content import reader', function () { it('keeps full editorial identity headers for direct API clients', async function () { const file = path.join(directory, 'full-post.csv'); - await fs.writeFile(file, 'title,slug,featured,meta_title\nHello,custom,1,Search title\n'); + await fs.writeFile( + file, + 'title,slug,featured,meta_title,comment_id,authors,author_emails,tags\nHello,custom,1,Search title,source-123,"Alice, Bob","alice@example.com, bob@example.com","News, Features"\n', + ); const rows = await readPostRows(file); @@ -51,6 +54,10 @@ describe('content import reader', function () { slug: 'custom', featured: '1', meta_title: 'Search title', + comment_id: 'source-123', + authors: 'Alice, Bob', + author_emails: 'alice@example.com, bob@example.com', + tags: 'News, Features', html: '', markdown: '', }, diff --git a/ghost/core/test/unit/server/services/content-import/import/relations.test.ts b/ghost/core/test/unit/server/services/content-import/import/relations.test.ts new file mode 100644 index 00000000000..7755fe318ab --- /dev/null +++ b/ghost/core/test/unit/server/services/content-import/import/relations.test.ts @@ -0,0 +1,483 @@ +import assert from 'node:assert/strict'; +import sinon from 'sinon'; +import { + BookshelfPostRelationsResolver, + parseAuthorReferences, + parseTagReferences, +} from '../../../../../../core/server/services/content-import/import/relations'; +import type { PostData } from '../../../../../../core/server/services/content-import/import/post-data'; + +const data: PostData = { + title: 'Imported post', + slug: 'imported-post', + status: 'published', + type: 'post', + visibility: 'public', + tags: [{ name: '#Import batch' }], +}; + +const relation = (id: string) => ({ id }); +const userModels = ( + findOne: sinon.SinonStub = sinon.stub().resolves(null), + add: sinon.SinonStub = sinon.stub().resolves(relation('author-created')), + getOwnerUser: sinon.SinonStub = sinon.stub().resolves(relation('owner')), + getByEmail: sinon.SinonStub = sinon.stub().resolves(undefined), +) => ({ findOne, getByEmail, add, getOwnerUser }); +const tagModels = ( + findOne: sinon.SinonStub = sinon.stub().resolves(null), + add: sinon.SinonStub = sinon.stub().resolves(relation('tag-created')), +) => ({ findOne, add }); + +describe('CSV post relation parsing', function () { + it('pairs author names and emails positionally to the longest list', function () { + assert.deepEqual( + parseAuthorReferences( + ' Alice Example, , Charlie Example ', + 'alice@example.com,bob@example.com,,fourth@example.com', + ), + [ + { name: 'Alice Example', email: 'alice@example.com' }, + { email: 'bob@example.com' }, + { name: 'Charlie Example' }, + { email: 'fourth@example.com' }, + ], + ); + assert.deepEqual(parseAuthorReferences(',', ','), [{}, {}]); + }); + + it('preserves tag order while trimming and dropping empty positions', function () { + assert.deepEqual(parseTagReferences(' First, ,Second,First '), ['First', 'Second', 'First']); + assert.deepEqual(parseTagReferences(), []); + }); +}); + +describe('BookshelfPostRelationsResolver', function () { + afterEach(function () { + sinon.restore(); + }); + + it('matches authors by email or a name-only slug, preserving order and uniqueness', async function () { + const getUserByEmail = sinon.stub().callsFake(async (email: string) => { + if (email === 'alice@example.com') { + return relation('author-alice'); + } + return undefined; + }); + const findUser = sinon.stub().callsFake(async (lookup: { slug?: string }) => { + if (lookup.slug === 'charlie-example') { + return relation('author-charlie'); + } + return null; + }); + const findTag = sinon.stub().resolves(null); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(findUser, undefined, undefined, getUserByEmail), + Tag: tagModels(findTag), + }); + const transacting = { transaction: true }; + + const resolved = await resolver.resolve( + data, + { + authorNames: 'Alice Example,No name fallback,Charlie Example,Alice again', + authorEmails: 'alice@example.com,missing@example.com,,alice@example.com', + }, + { importing: true, transacting }, + ); + + assert.deepEqual(resolved.data.authors, [ + { id: 'author-alice' }, + { id: 'author-created' }, + { id: 'author-charlie' }, + ]); + assert.deepEqual(resolved.warnings, []); + assert.deepEqual( + getUserByEmail.getCalls().map((call) => call.args[0]), + ['alice@example.com', 'missing@example.com'], + 'duplicate emails reuse the first match', + ); + sinon.assert.calledOnceWithExactly( + findUser, + { slug: 'charlie-example' }, + { + importing: true, + transacting, + }, + ); + for (const call of getUserByEmail.getCalls()) { + assert.deepEqual(call.args[1], { importing: true, transacting }); + } + sinon.assert.notCalled(findTag); + }); + + it('falls back to Owner with a warning when a name-only author does not match', async function () { + const findUser = sinon.stub().resolves(null); + const getOwnerUser = sinon.stub().resolves(relation('owner')); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(findUser, undefined, getOwnerUser), + Tag: tagModels(), + }); + + const resolved = await resolver.resolve( + data, + { authorNames: 'Missing Author' }, + { importing: true }, + ); + + assert.deepEqual(resolved.data.authors, [{ id: 'owner' }]); + assert.deepEqual(resolved.warnings, [ + 'Author "Missing Author" has no email; assigned Owner instead.', + ]); + sinon.assert.calledWithMatch(findUser, { slug: 'missing-author' }); + sinon.assert.calledOnce(getOwnerUser); + }); + + it('creates an unmatched named author as a locked Contributor through the importing path', async function () { + const findUser = sinon.stub().resolves(null); + const addUser = sinon.stub().resolves(relation('author-created')); + const getOwnerUser = sinon.stub().resolves(relation('owner')); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(findUser, addUser, getOwnerUser, sinon.stub().resolves(undefined)), + Tag: tagModels(), + }); + const options = { importing: true, context: { internal: true }, transacting: {} }; + + const resolved = await resolver.resolve( + data, + { authorNames: 'New Contributor', authorEmails: 'new@example.com' }, + options, + ); + + assert.deepEqual(resolved.data.authors, [{ id: 'author-created' }]); + assert.deepEqual(resolved.warnings, []); + sinon.assert.notCalled(findUser); + sinon.assert.calledWithExactly( + addUser, + { + name: 'New Contributor', + email: 'new@example.com', + roles: ['Contributor'], + }, + options, + ); + sinon.assert.notCalled(getOwnerUser); + }); + + it('matches an existing author by email without requiring a supplied name', async function () { + const findUser = sinon.stub().resolves(null); + const getUserByEmail = sinon.stub().resolves(relation('author-existing')); + const addUser = sinon.stub().resolves(relation('author-created')); + const getOwnerUser = sinon.stub().resolves(relation('owner')); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(findUser, addUser, getOwnerUser, getUserByEmail), + Tag: tagModels(), + }); + + const resolved = await resolver.resolve( + data, + { authorEmails: 'existing@example.com' }, + { importing: true }, + ); + + assert.deepEqual(resolved.data.authors, [{ id: 'author-existing' }]); + assert.deepEqual(resolved.warnings, []); + sinon.assert.calledOnceWithExactly(getUserByEmail, 'existing@example.com', { + importing: true, + }); + sinon.assert.notCalled(findUser); + sinon.assert.notCalled(addUser); + sinon.assert.notCalled(getOwnerUser); + }); + + it('reuses a contributor created for a duplicate email in the same row', async function () { + const findUser = sinon.stub().resolves(null); + const getUserByEmail = sinon.stub().resolves(undefined); + const addUser = sinon.stub().resolves(relation('author-created')); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(findUser, addUser, undefined, getUserByEmail), + Tag: tagModels(), + }); + + const resolved = await resolver.resolve( + data, + { + authorNames: 'New Contributor,Duplicate Name', + authorEmails: 'NEW@example.com,new@example.com', + }, + { importing: true }, + ); + + assert.deepEqual(resolved.data.authors, [{ id: 'author-created' }]); + sinon.assert.calledOnceWithExactly(getUserByEmail, 'new@example.com', { importing: true }); + sinon.assert.notCalled(findUser); + sinon.assert.calledOnce(addUser); + sinon.assert.calledWithMatch(addUser, { email: 'new@example.com' }); + }); + + it('falls back to Owner for email-only, invalid, and empty entries with clear warnings', async function () { + const findUser = sinon.stub().resolves(null); + const getUserByEmail = sinon.stub().resolves(undefined); + const addUser = sinon.stub().resolves(relation('author-created')); + const getOwnerUser = sinon.stub().resolves(relation('owner')); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(findUser, addUser, getOwnerUser, getUserByEmail), + Tag: tagModels(), + }); + + const resolved = await resolver.resolve( + data, + { + authorNames: ',Invalid Author,', + authorEmails: 'missing@example.com,not-an-email,', + }, + { importing: true }, + ); + + assert.deepEqual(resolved.data.authors, [{ id: 'owner' }]); + assert.deepEqual(resolved.warnings, [ + 'Author email "missing@example.com" has no name; assigned Owner instead.', + 'Author email "not-an-email" is invalid; assigned Owner instead.', + 'An empty author entry was assigned to Owner instead.', + ]); + sinon.assert.calledOnceWithExactly(getUserByEmail, 'missing@example.com', { + importing: true, + }); + sinon.assert.notCalled(findUser); + sinon.assert.notCalled(addUser); + sinon.assert.calledOnce(getOwnerUser); + }); + + it('does not duplicate Owner when an existing match is followed by an Owner fallback', async function () { + const owner = relation('owner'); + const getUserByEmail = sinon.stub().callsFake(async (email: string) => { + return email === 'owner@example.com' ? owner : undefined; + }); + const getOwnerUser = sinon.stub().resolves(owner); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(undefined, undefined, getOwnerUser, getUserByEmail), + Tag: tagModels(), + }); + + const resolved = await resolver.resolve( + data, + { + authorNames: 'Site Owner,Missing Author', + authorEmails: 'owner@example.com,', + }, + { importing: true }, + ); + + assert.deepEqual(resolved.data.authors, [{ id: 'owner' }]); + assert.deepEqual(resolved.warnings, [ + 'Author "Missing Author" has no email; assigned Owner instead.', + ]); + sinon.assert.calledOnce(getOwnerUser); + }); + + it('propagates author creation failures so the row transaction can roll back', async function () { + const failure = new Error('user creation failed'); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(sinon.stub().resolves(null), sinon.stub().rejects(failure)), + Tag: tagModels(), + }); + + await assert.rejects( + resolver.resolve( + data, + { authorNames: 'New Contributor', authorEmails: 'new@example.com' }, + { importing: true }, + ), + failure, + ); + }); + + it('propagates Owner lookup failures so the row transaction can roll back', async function () { + const failure = new Error('owner lookup failed'); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(sinon.stub().resolves(null), undefined, sinon.stub().rejects(failure)), + Tag: tagModels(), + }); + + await assert.rejects( + resolver.resolve(data, { authorNames: 'Missing Author' }, { importing: true }), + failure, + ); + }); + + it('matches tags by exact name, explicit slug, and normalized slug before creating the rest', async function () { + const findTag = sinon.stub().callsFake(async (lookup: { name?: string; slug?: string }) => { + if (lookup.name === 'Exact Name') { + return relation('tag-exact'); + } + if (lookup.slug === 'explicit-slug') { + return relation('tag-explicit-slug'); + } + if (lookup.slug === 'needs-normalizing') { + return relation('tag-normalized'); + } + return null; + }); + const addTag = sinon.stub().resolves(relation('tag-created')); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(), + Tag: tagModels(findTag, addTag), + }); + + const resolved = await resolver.resolve( + data, + { + tagNames: 'Exact Name,explicit-slug,Needs Normalizing,Missing Tag,Exact Name', + }, + { importing: true }, + ); + + assert.deepEqual(resolved.data.tags, [ + { id: 'tag-exact' }, + { id: 'tag-explicit-slug' }, + { id: 'tag-normalized' }, + { id: 'tag-created' }, + { name: '#Import batch' }, + ]); + assert.deepEqual( + findTag.getCalls().map((call) => call.args[0]), + [ + { name: 'Exact Name' }, + { name: 'explicit-slug' }, + { slug: 'explicit-slug' }, + { name: 'Needs Normalizing' }, + { slug: 'Needs Normalizing' }, + { slug: 'needs-normalizing' }, + { name: 'Missing Tag' }, + { slug: 'Missing Tag' }, + { slug: 'missing-tag' }, + { name: 'Exact Name' }, + ], + ); + sinon.assert.calledOnceWithExactly(addTag, { name: 'Missing Tag' }, { importing: true }); + }); + + it('reuses a tag created for duplicate inputs while preserving source order', async function () { + let created: { id: string } | null = null; + const findTag = sinon.stub().callsFake(async (lookup: { name?: string }) => { + return lookup.name === 'New Tag' ? created : null; + }); + const addTag = sinon.stub().callsFake(async () => { + created = relation('tag-created'); + return created; + }); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(), + Tag: tagModels(findTag, addTag), + }); + const options = { importing: true, transacting: {} }; + + const resolved = await resolver.resolve(data, { tagNames: 'New Tag,New Tag' }, options); + + assert.deepEqual(resolved.data.tags, [{ id: 'tag-created' }, { name: '#Import batch' }]); + sinon.assert.calledOnceWithExactly(addTag, { name: 'New Tag' }, options); + }); + + for (const code of ['ER_DUP_ENTRY', 'SQLITE_CONSTRAINT_UNIQUE']) { + it(`refetches a concurrently created tag after ${code}`, async function () { + const duplicate = Object.assign(new Error('duplicate tag'), { code }); + const findTag = sinon.stub().callsFake(async (_lookup: object, options: object) => { + return 'forUpdate' in options ? relation('tag-concurrent') : null; + }); + const addTag = sinon.stub().rejects(duplicate); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(), + Tag: tagModels(findTag, addTag), + }); + const options = { importing: true, transacting: {} }; + + const resolved = await resolver.resolve(data, { tagNames: 'Concurrent Tag' }, options); + + assert.deepEqual(resolved.data.tags, [{ id: 'tag-concurrent' }, { name: '#Import batch' }]); + sinon.assert.calledOnceWithExactly(addTag, { name: 'Concurrent Tag' }, options); + sinon.assert.calledWithExactly( + findTag.lastCall, + { name: 'Concurrent Tag' }, + { + ...options, + forUpdate: true, + }, + ); + }); + } + + it('rethrows a duplicate error when the concurrent tag cannot be refetched', async function () { + const duplicate = Object.assign(new Error('duplicate tag'), { code: 'ER_DUP_ENTRY' }); + const findTag = sinon.stub().resolves(null); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(), + Tag: tagModels(findTag, sinon.stub().rejects(duplicate)), + }); + + await assert.rejects( + resolver.resolve(data, { tagNames: 'Missing Concurrent Tag' }, { importing: true }), + duplicate, + ); + assert.equal( + findTag.getCalls().filter((call) => call.args[1].forUpdate).length, + 3, + 'every supported lookup is retried with a locking read', + ); + }); + + it('propagates non-duplicate tag creation failures without refetching', async function () { + const failure = Object.assign(new Error('tag creation failed'), { code: 'ECONNRESET' }); + const findTag = sinon.stub().resolves(null); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(), + Tag: tagModels(findTag, sinon.stub().rejects(failure)), + }); + + await assert.rejects( + resolver.resolve(data, { tagNames: 'Broken Tag' }, { importing: true }), + failure, + ); + assert.equal(findTag.getCalls().filter((call) => call.args[1].forUpdate).length, 0); + }); + + it('propagates tag creation failures without an error code', async function () { + const failure = new Error('tag creation failed'); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(), + Tag: tagModels(sinon.stub().resolves(null), sinon.stub().rejects(failure)), + }); + + await assert.rejects( + resolver.resolve(data, { tagNames: 'Broken Tag' }, { importing: true }), + failure, + ); + }); + + it('does no lookups when relation cells are absent', async function () { + const findUser = sinon.stub().resolves(null); + const findTag = sinon.stub().resolves(null); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(findUser), + Tag: tagModels(findTag), + }); + + const resolved = await resolver.resolve(data, {}, { importing: true }); + + assert.deepEqual(resolved, { data, warnings: [] }); + assert.notEqual(resolved.data, data, 'callers receive data they can safely change'); + sinon.assert.notCalled(findUser); + sinon.assert.notCalled(findTag); + }); + + it('propagates lookup failures to roll back the row transaction', async function () { + const failure = new Error('author lookup failed'); + const resolver = new BookshelfPostRelationsResolver({ + User: userModels(undefined, undefined, undefined, sinon.stub().rejects(failure)), + Tag: tagModels(), + }); + + await assert.rejects( + resolver.resolve(data, { authorEmails: 'author@example.com' }, { importing: true }), + failure, + ); + }); +}); diff --git a/ghost/core/test/unit/server/services/content-import/import/row.test.ts b/ghost/core/test/unit/server/services/content-import/import/row.test.ts index 9c9f9b367b6..a89c0db46c5 100644 --- a/ghost/core/test/unit/server/services/content-import/import/row.test.ts +++ b/ghost/core/test/unit/server/services/content-import/import/row.test.ts @@ -39,12 +39,21 @@ describe('post import row schema', function () { feature_image: 'undefined', meta_title: '', frontmatter: '', + comment_id: '', + authors: '', + author_emails: 'undefined', + tags: '', }); assert.equal(parsed.slug, undefined); assert.equal(parsed.feature_image, undefined); assert.equal(parsed.meta_title, undefined); assert.equal(parsed.frontmatter, undefined); + assert.equal(parsed.comment_id, undefined); + assert.equal(parsed.authors, undefined); + assert.equal(parsed.author_emails, undefined); + assert.equal(parsed.tags, undefined); + assert.equal(postImportRowSchema.parse({ comment_id: 'undefined' }).comment_id, undefined); }); it('passes unknown columns through for later milestones to consume', function () { diff --git a/ghost/core/test/unit/server/services/content-import/import/schema.test.ts b/ghost/core/test/unit/server/services/content-import/import/schema.test.ts index d08044a7def..6d869c88436 100644 --- a/ghost/core/test/unit/server/services/content-import/import/schema.test.ts +++ b/ghost/core/test/unit/server/services/content-import/import/schema.test.ts @@ -21,6 +21,10 @@ describe('content import schema', function () { mapping: { Headline: 'title', Body: 'html', + Source: 'comment_id', + Bylines: 'authors', + Emails: 'author_emails', + Topics: 'tags', Notes: '', }, }), @@ -30,6 +34,10 @@ describe('content import schema', function () { mapping: { Headline: 'title', Body: 'html', + Source: 'comment_id', + Bylines: 'authors', + Emails: 'author_emails', + Topics: 'tags', Notes: '', }, }, diff --git a/ghost/core/test/unit/server/services/content-import/import/store.test.ts b/ghost/core/test/unit/server/services/content-import/import/store.test.ts index c5654dfb0c6..411176115ef 100644 --- a/ghost/core/test/unit/server/services/content-import/import/store.test.ts +++ b/ghost/core/test/unit/server/services/content-import/import/store.test.ts @@ -36,6 +36,20 @@ describe('ImportRunStore', function () { assert.ok(finished?.finishedAt instanceof Date); }); + it('records updated rows as successful outcomes', function () { + const store = new ImportRunStore(); + store.create('run_updated', 1); + + store.record('run_updated', { + line: 2, + title: 'Updated post', + status: 'updated', + postId: 'post_updated', + }); + + assert.equal(store.get('run_updated')?.rows[0].status, 'updated'); + }); + it('ignores writes against an unknown run rather than throwing', function () { const store = new ImportRunStore(); From 7b638f36d3edcaee1b65f7118fd9f016263bd8bc Mon Sep 17 00:00:00 2001 From: Rob Lester Date: Wed, 26 Aug 2026 13:35:34 +0100 Subject: [PATCH 02/11] Changed database type codecs to sit together (#30302) `db-date` held the one codec that turns a database value into something the domain can use, and a second was about to join it for booleans, where SQLite answers 0 or 1 and MySQL answers true or false. Two of them loose in lib is how lib became a drawer of unrelated things, so they sit in a folder that names what they are and gives the next one somewhere obvious to go. The boolean codec arrived without tests, and the interesting part of it is not that 0 and 1 become false and true but what it does with the rest of the range: a boolean column is a tinyint underneath, and both engines read every non-zero value in it as true, so the codec does too rather than failing a read over a value the database itself is happy with. ref https://linear.app/ghost/issue/BER-3872 Claude-Session: https://claude.ai/code/session_018jPEJMYLoUzNvYbdxWb3dY --- ghost/core/content/themes/casper | 2 +- ghost/core/content/themes/source | 2 +- .../automation-action-revisions-importer.ts | 2 +- .../importers/automation-actions-importer.ts | 2 +- .../automation-run-steps-importer.ts | 2 +- .../importers/automation-runs-importer.ts | 2 +- .../seeders/importers/automations-importer.ts | 2 +- .../importers/comment-reports-importer.js | 2 +- .../seeders/importers/comments-importer.js | 2 +- .../importers/email-batches-importer.js | 2 +- .../importers/email-recipients-importer.js | 2 +- .../data/seeders/importers/emails-importer.js | 2 +- .../data/seeders/importers/labels-importer.js | 2 +- .../members-click-events-importer.js | 2 +- .../members-created-events-importer.js | 2 +- .../importers/members-feedback-importer.js | 2 +- .../seeders/importers/members-importer.js | 2 +- .../members-login-events-importer.js | 2 +- .../importers/members-products-importer.js | 2 +- .../members-status-events-importer.js | 2 +- .../members-stripe-customers-importer.js | 2 +- ...stripe-customers-subscriptions-importer.js | 2 +- .../members-subscribe-events-importer.js | 2 +- ...rs-subscription-created-events-importer.js | 2 +- .../importers/offer-redemptions-importer.js | 2 +- .../data/seeders/importers/offers-importer.js | 2 +- .../data/seeders/importers/posts-importer.js | 2 +- .../data/seeders/importers/table-importer.ts | 2 +- .../data/seeders/importers/tags-importer.js | 2 +- .../data/seeders/importers/users-importer.js | 2 +- .../importers/web-mentions-importer.js | 2 +- .../core/server/data/seeders/utils/random.ts | 2 +- .../core/core/server/lib/db-types/boolean.ts | 16 ++++++++ .../lib/{db-date.ts => db-types/date.ts} | 0 .../database-automations-repository.ts | 2 +- .../core/server/services/gift-links/schema.ts | 2 +- .../gifts/gift-bookshelf-repository.ts | 2 +- .../gift-delivery-bookshelf-repository.ts | 2 +- .../services/gifts/gift-delivery-schema.ts | 2 +- .../core/server/services/gifts/gift-schema.ts | 2 +- .../services/members-custom-fields/schema.ts | 2 +- .../unit/server/lib/db-types/boolean.test.ts | 38 +++++++++++++++++++ .../date.test.ts} | 8 +++- .../automations-repository.test.ts | 2 +- 44 files changed, 100 insertions(+), 42 deletions(-) create mode 100644 ghost/core/core/server/lib/db-types/boolean.ts rename ghost/core/core/server/lib/{db-date.ts => db-types/date.ts} (100%) create mode 100644 ghost/core/test/unit/server/lib/db-types/boolean.test.ts rename ghost/core/test/unit/server/lib/{db-date.test.ts => db-types/date.test.ts} (96%) diff --git a/ghost/core/content/themes/casper b/ghost/core/content/themes/casper index f968f19dc9b..bb3d0de3c70 160000 --- a/ghost/core/content/themes/casper +++ b/ghost/core/content/themes/casper @@ -1 +1 @@ -Subproject commit f968f19dc9b2fa8cda4508c40d82d86c03bb6c2f +Subproject commit bb3d0de3c7065677bb18b38d8185a8a980b163bd diff --git a/ghost/core/content/themes/source b/ghost/core/content/themes/source index fb629deb392..3a5643b42a4 160000 --- a/ghost/core/content/themes/source +++ b/ghost/core/content/themes/source @@ -1 +1 @@ -Subproject commit fb629deb3922e24bc27ab8aea566ddc76db93ceb +Subproject commit 3a5643b42a4700e961e65ebda92f6a6c56bf8c86 diff --git a/ghost/core/core/server/data/seeders/importers/automation-action-revisions-importer.ts b/ghost/core/core/server/data/seeders/importers/automation-action-revisions-importer.ts index fa6674e3f41..8e64429c5f0 100644 --- a/ghost/core/core/server/data/seeders/importers/automation-action-revisions-importer.ts +++ b/ghost/core/core/server/data/seeders/importers/automation-action-revisions-importer.ts @@ -2,7 +2,7 @@ import { faker } from '@faker-js/faker'; import errors from '@tryghost/errors'; import type { Knex } from 'knex'; import { TableImporter } from './table-importer'; -import { fromDatabaseDate, toDatabaseDate } from '../../../lib/db-date'; +import { fromDatabaseDate, toDatabaseDate } from '../../../lib/db-types/date'; import { DEFAULT_EMAIL_DESIGN_SETTING_SLUG } from '../../../services/member-welcome-emails/constants'; type AutomationAction = { diff --git a/ghost/core/core/server/data/seeders/importers/automation-actions-importer.ts b/ghost/core/core/server/data/seeders/importers/automation-actions-importer.ts index 9fe4265ee79..18f95159d77 100644 --- a/ghost/core/core/server/data/seeders/importers/automation-actions-importer.ts +++ b/ghost/core/core/server/data/seeders/importers/automation-actions-importer.ts @@ -1,7 +1,7 @@ import errors from '@tryghost/errors'; import type { Knex } from 'knex'; import { TableImporter } from './table-importer'; -import { toDatabaseDate } from '../../../lib/db-date'; +import { toDatabaseDate } from '../../../lib/db-types/date'; import { randomDateBetween } from '../utils/random'; type Automation = { diff --git a/ghost/core/core/server/data/seeders/importers/automation-run-steps-importer.ts b/ghost/core/core/server/data/seeders/importers/automation-run-steps-importer.ts index a4734e06e18..620fb4a623b 100644 --- a/ghost/core/core/server/data/seeders/importers/automation-run-steps-importer.ts +++ b/ghost/core/core/server/data/seeders/importers/automation-run-steps-importer.ts @@ -3,7 +3,7 @@ import errors from '@tryghost/errors'; import { clamp } from 'lodash'; import type { Knex } from 'knex'; import { TableImporter } from './table-importer'; -import { fromDatabaseDate, toDatabaseDate } from '../../../lib/db-date'; +import { fromDatabaseDate, toDatabaseDate } from '../../../lib/db-types/date'; type AutomationRun = { id: string; diff --git a/ghost/core/core/server/data/seeders/importers/automation-runs-importer.ts b/ghost/core/core/server/data/seeders/importers/automation-runs-importer.ts index 64663b389f8..3e871938543 100644 --- a/ghost/core/core/server/data/seeders/importers/automation-runs-importer.ts +++ b/ghost/core/core/server/data/seeders/importers/automation-runs-importer.ts @@ -4,7 +4,7 @@ import assert from 'node:assert/strict'; import type { Knex } from 'knex'; import { TableImporter } from './table-importer'; import { parseEmailAddress } from '@tryghost/parse-email-address'; -import { fromDatabaseDate, toDatabaseDate } from '../../../lib/db-date'; +import { fromDatabaseDate, toDatabaseDate } from '../../../lib/db-types/date'; import { randomDateBetween } from '../utils/random'; type Automation = { diff --git a/ghost/core/core/server/data/seeders/importers/automations-importer.ts b/ghost/core/core/server/data/seeders/importers/automations-importer.ts index 3799647d290..925db92a5ac 100644 --- a/ghost/core/core/server/data/seeders/importers/automations-importer.ts +++ b/ghost/core/core/server/data/seeders/importers/automations-importer.ts @@ -5,7 +5,7 @@ import type { Knex } from 'knex'; import { TableImporter } from './table-importer'; // @ts-expect-error This module currently lacks type definitions. import { blogStartDate } from '../utils/blog-info'; -import { toDatabaseDate } from '../../../lib/db-date'; +import { toDatabaseDate } from '../../../lib/db-types/date'; import { MEMBER_WELCOME_EMAIL_SLUGS } from '../../../services/member-welcome-emails/constants'; type Automation = { diff --git a/ghost/core/core/server/data/seeders/importers/comment-reports-importer.js b/ghost/core/core/server/data/seeders/importers/comment-reports-importer.js index 1924b8fa176..57e6d3fa7c6 100644 --- a/ghost/core/core/server/data/seeders/importers/comment-reports-importer.js +++ b/ghost/core/core/server/data/seeders/importers/comment-reports-importer.js @@ -1,7 +1,7 @@ const { TableImporter } = require('./table-importer'); const { faker } = require('@faker-js/faker'); const { luck, randomDateBetween } = require('../utils/random'); -const { toDatabaseDate } = require('../../../lib/db-date'); +const { toDatabaseDate } = require('../../../lib/db-types/date'); class CommentReportsImporter extends TableImporter { static table = 'comment_reports'; diff --git a/ghost/core/core/server/data/seeders/importers/comments-importer.js b/ghost/core/core/server/data/seeders/importers/comments-importer.js index 533ab3eee29..3b71fbe62df 100644 --- a/ghost/core/core/server/data/seeders/importers/comments-importer.js +++ b/ghost/core/core/server/data/seeders/importers/comments-importer.js @@ -2,7 +2,7 @@ const { faker } = require('@faker-js/faker'); const { TableImporter } = require('./table-importer'); const { luck } = require('../utils/random'); const generateEvents = require('../utils/event-generator'); -const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-date'); +const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-types/date'); class CommentsImporter extends TableImporter { static table = 'comments'; diff --git a/ghost/core/core/server/data/seeders/importers/email-batches-importer.js b/ghost/core/core/server/data/seeders/importers/email-batches-importer.js index 76e2c7e8137..5c4e09b8d26 100644 --- a/ghost/core/core/server/data/seeders/importers/email-batches-importer.js +++ b/ghost/core/core/server/data/seeders/importers/email-batches-importer.js @@ -1,7 +1,7 @@ const { TableImporter } = require('./table-importer'); const { faker } = require('@faker-js/faker'); const { randomDateBetween } = require('../utils/random'); -const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-date'); +const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-types/date'); class EmailBatchesImporter extends TableImporter { static table = 'email_batches'; diff --git a/ghost/core/core/server/data/seeders/importers/email-recipients-importer.js b/ghost/core/core/server/data/seeders/importers/email-recipients-importer.js index 89c51d7c9ac..6ec8e3dc745 100644 --- a/ghost/core/core/server/data/seeders/importers/email-recipients-importer.js +++ b/ghost/core/core/server/data/seeders/importers/email-recipients-importer.js @@ -2,7 +2,7 @@ const { TableImporter } = require('./table-importer'); const { faker } = require('@faker-js/faker'); const generateEvents = require('../utils/event-generator'); const { randomDateBetween } = require('../utils/random'); -const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-date'); +const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-types/date'); const debug = require('@tryghost/debug')('EmailRecipientsImporter'); const emailStatus = { diff --git a/ghost/core/core/server/data/seeders/importers/emails-importer.js b/ghost/core/core/server/data/seeders/importers/emails-importer.js index 03689c80bef..043bd904ecf 100644 --- a/ghost/core/core/server/data/seeders/importers/emails-importer.js +++ b/ghost/core/core/server/data/seeders/importers/emails-importer.js @@ -2,7 +2,7 @@ const { TableImporter } = require('./table-importer'); const { faker } = require('@faker-js/faker'); const generateEvents = require('../utils/event-generator'); const { luck } = require('../utils/random'); -const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-date'); +const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-types/date'); class EmailsImporter extends TableImporter { static table = 'emails'; diff --git a/ghost/core/core/server/data/seeders/importers/labels-importer.js b/ghost/core/core/server/data/seeders/importers/labels-importer.js index 185dc96fe22..f2e028afab5 100644 --- a/ghost/core/core/server/data/seeders/importers/labels-importer.js +++ b/ghost/core/core/server/data/seeders/importers/labels-importer.js @@ -2,7 +2,7 @@ const { TableImporter } = require('./table-importer'); const { faker } = require('@faker-js/faker'); const { slugify } = require('@tryghost/string'); const { blogStartDate } = require('../utils/blog-info'); -const { toDatabaseDate } = require('../../../lib/db-date'); +const { toDatabaseDate } = require('../../../lib/db-types/date'); class LabelsImporter extends TableImporter { static table = 'labels'; diff --git a/ghost/core/core/server/data/seeders/importers/members-click-events-importer.js b/ghost/core/core/server/data/seeders/importers/members-click-events-importer.js index da3a60f0713..8e31f781990 100644 --- a/ghost/core/core/server/data/seeders/importers/members-click-events-importer.js +++ b/ghost/core/core/server/data/seeders/importers/members-click-events-importer.js @@ -1,7 +1,7 @@ const { TableImporter } = require('./table-importer'); const { faker } = require('@faker-js/faker'); const { luck, randomDateBetween } = require('../utils/random'); -const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-date'); +const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-types/date'); class MembersClickEventsImporter extends TableImporter { static table = 'members_click_events'; diff --git a/ghost/core/core/server/data/seeders/importers/members-created-events-importer.js b/ghost/core/core/server/data/seeders/importers/members-created-events-importer.js index 548ee0c0612..75a86f53a62 100644 --- a/ghost/core/core/server/data/seeders/importers/members-created-events-importer.js +++ b/ghost/core/core/server/data/seeders/importers/members-created-events-importer.js @@ -1,7 +1,7 @@ const { TableImporter } = require('./table-importer'); const { faker } = require('@faker-js/faker'); const { luck } = require('../utils/random'); -const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-date'); +const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-types/date'); class MembersCreatedEventsImporter extends TableImporter { static table = 'members_created_events'; diff --git a/ghost/core/core/server/data/seeders/importers/members-feedback-importer.js b/ghost/core/core/server/data/seeders/importers/members-feedback-importer.js index f5b533228e8..ce861a20e17 100644 --- a/ghost/core/core/server/data/seeders/importers/members-feedback-importer.js +++ b/ghost/core/core/server/data/seeders/importers/members-feedback-importer.js @@ -1,6 +1,6 @@ const { TableImporter } = require('./table-importer'); const { luck, randomDateBetween } = require('../utils/random'); -const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-date'); +const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-types/date'); class MembersFeedbackImporter extends TableImporter { static table = 'members_feedback'; diff --git a/ghost/core/core/server/data/seeders/importers/members-importer.js b/ghost/core/core/server/data/seeders/importers/members-importer.js index b2b5942b33a..c00e6e8d56d 100644 --- a/ghost/core/core/server/data/seeders/importers/members-importer.js +++ b/ghost/core/core/server/data/seeders/importers/members-importer.js @@ -4,7 +4,7 @@ const { faker: americanFaker } = require('@faker-js/faker/locale/en_US'); const { blogStartDate: startTime } = require('../utils/blog-info'); const generateEvents = require('../utils/event-generator'); const { luck } = require('../utils/random'); -const { toDatabaseDate } = require('../../../lib/db-date'); +const { toDatabaseDate } = require('../../../lib/db-types/date'); const debug = require('@tryghost/debug')('MembersImporter'); class MembersImporter extends TableImporter { diff --git a/ghost/core/core/server/data/seeders/importers/members-login-events-importer.js b/ghost/core/core/server/data/seeders/importers/members-login-events-importer.js index 394be31bf88..2e0c7403af2 100644 --- a/ghost/core/core/server/data/seeders/importers/members-login-events-importer.js +++ b/ghost/core/core/server/data/seeders/importers/members-login-events-importer.js @@ -1,7 +1,7 @@ const { TableImporter } = require('./table-importer'); const { luck } = require('../utils/random'); const generateEvents = require('../utils/event-generator'); -const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-date'); +const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-types/date'); class MembersLoginEventsImporter extends TableImporter { static table = 'members_login_events'; diff --git a/ghost/core/core/server/data/seeders/importers/members-products-importer.js b/ghost/core/core/server/data/seeders/importers/members-products-importer.js index 13ea81080fa..48dadd82a03 100644 --- a/ghost/core/core/server/data/seeders/importers/members-products-importer.js +++ b/ghost/core/core/server/data/seeders/importers/members-products-importer.js @@ -1,7 +1,7 @@ const { faker } = require('@faker-js/faker'); const { TableImporter } = require('./table-importer'); const { luck } = require('../utils/random'); -const { toDatabaseDate } = require('../../../lib/db-date'); +const { toDatabaseDate } = require('../../../lib/db-types/date'); class MembersProductsImporter extends TableImporter { static table = 'members_products'; diff --git a/ghost/core/core/server/data/seeders/importers/members-status-events-importer.js b/ghost/core/core/server/data/seeders/importers/members-status-events-importer.js index 91abda2acd3..fa94b04bd8a 100644 --- a/ghost/core/core/server/data/seeders/importers/members-status-events-importer.js +++ b/ghost/core/core/server/data/seeders/importers/members-status-events-importer.js @@ -1,6 +1,6 @@ const { TableImporter } = require('./table-importer'); const { randomDateBetween } = require('../utils/random'); -const { toDatabaseDate } = require('../../../lib/db-date'); +const { toDatabaseDate } = require('../../../lib/db-types/date'); class MembersStatusEventsImporter extends TableImporter { static table = 'members_status_events'; diff --git a/ghost/core/core/server/data/seeders/importers/members-stripe-customers-importer.js b/ghost/core/core/server/data/seeders/importers/members-stripe-customers-importer.js index 746c519f99b..132caa85d5e 100644 --- a/ghost/core/core/server/data/seeders/importers/members-stripe-customers-importer.js +++ b/ghost/core/core/server/data/seeders/importers/members-stripe-customers-importer.js @@ -1,6 +1,6 @@ const { faker } = require('@faker-js/faker'); const { TableImporter } = require('./table-importer'); -const { fromDatabaseDate } = require('../../../lib/db-date'); +const { fromDatabaseDate } = require('../../../lib/db-types/date'); class MembersStripeCustomersImporter extends TableImporter { static table = 'members_stripe_customers'; diff --git a/ghost/core/core/server/data/seeders/importers/members-stripe-customers-subscriptions-importer.js b/ghost/core/core/server/data/seeders/importers/members-stripe-customers-subscriptions-importer.js index ad6943c891f..d898d9067fd 100644 --- a/ghost/core/core/server/data/seeders/importers/members-stripe-customers-subscriptions-importer.js +++ b/ghost/core/core/server/data/seeders/importers/members-stripe-customers-subscriptions-importer.js @@ -2,7 +2,7 @@ const { faker } = require('@faker-js/faker'); const { TableImporter } = require('./table-importer'); const generateEvents = require('../utils/event-generator'); const { luck } = require('../utils/random'); -const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-date'); +const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-types/date'); class MembersStripeCustomersSubscriptionsImporter extends TableImporter { static table = 'members_stripe_customers_subscriptions'; diff --git a/ghost/core/core/server/data/seeders/importers/members-subscribe-events-importer.js b/ghost/core/core/server/data/seeders/importers/members-subscribe-events-importer.js index 807679b80ed..58ee7f187c2 100644 --- a/ghost/core/core/server/data/seeders/importers/members-subscribe-events-importer.js +++ b/ghost/core/core/server/data/seeders/importers/members-subscribe-events-importer.js @@ -1,6 +1,6 @@ const { TableImporter } = require('./table-importer'); const { luck, randomDateBetween } = require('../utils/random'); -const { toDatabaseDate } = require('../../../lib/db-date'); +const { toDatabaseDate } = require('../../../lib/db-types/date'); class MembersSubscribeEventsImporter extends TableImporter { static table = 'members_subscribe_events'; diff --git a/ghost/core/core/server/data/seeders/importers/members-subscription-created-events-importer.js b/ghost/core/core/server/data/seeders/importers/members-subscription-created-events-importer.js index c9cab3d714b..a422a14e2be 100644 --- a/ghost/core/core/server/data/seeders/importers/members-subscription-created-events-importer.js +++ b/ghost/core/core/server/data/seeders/importers/members-subscription-created-events-importer.js @@ -1,7 +1,7 @@ const { TableImporter } = require('./table-importer'); const { faker } = require('@faker-js/faker'); const { luck } = require('../utils/random'); -const { fromDatabaseDate } = require('../../../lib/db-date'); +const { fromDatabaseDate } = require('../../../lib/db-types/date'); class MembersSubscriptionCreatedEventsImporter extends TableImporter { static table = 'members_subscription_created_events'; diff --git a/ghost/core/core/server/data/seeders/importers/offer-redemptions-importer.js b/ghost/core/core/server/data/seeders/importers/offer-redemptions-importer.js index 2fdf72b4f73..4b9b88aad26 100644 --- a/ghost/core/core/server/data/seeders/importers/offer-redemptions-importer.js +++ b/ghost/core/core/server/data/seeders/importers/offer-redemptions-importer.js @@ -2,7 +2,7 @@ const { faker } = require('@faker-js/faker'); const errors = require('@tryghost/errors'); const { TableImporter } = require('./table-importer'); const { randomDateBetween } = require('../utils/random'); -const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-date'); +const { fromDatabaseDate, toDatabaseDate } = require('../../../lib/db-types/date'); class OfferRedemptionsImporter extends TableImporter { static table = 'offer_redemptions'; diff --git a/ghost/core/core/server/data/seeders/importers/offers-importer.js b/ghost/core/core/server/data/seeders/importers/offers-importer.js index bd3740569e2..99c655d6f77 100644 --- a/ghost/core/core/server/data/seeders/importers/offers-importer.js +++ b/ghost/core/core/server/data/seeders/importers/offers-importer.js @@ -1,7 +1,7 @@ const { TableImporter } = require('./table-importer'); const { slugify } = require('@tryghost/string'); const { blogStartDate } = require('../utils/blog-info'); -const { toDatabaseDate } = require('../../../lib/db-date'); +const { toDatabaseDate } = require('../../../lib/db-types/date'); const offerTemplates = [ { diff --git a/ghost/core/core/server/data/seeders/importers/posts-importer.js b/ghost/core/core/server/data/seeders/importers/posts-importer.js index 89c6859f4b0..031f0c9aa62 100644 --- a/ghost/core/core/server/data/seeders/importers/posts-importer.js +++ b/ghost/core/core/server/data/seeders/importers/posts-importer.js @@ -2,7 +2,7 @@ const { faker } = require('@faker-js/faker'); const { slugify } = require('@tryghost/string'); const { luck } = require('../utils/random'); const { TableImporter } = require('./table-importer'); -const { toDatabaseDate } = require('../../../lib/db-date'); +const { toDatabaseDate } = require('../../../lib/db-types/date'); class PostsImporter extends TableImporter { static table = 'posts'; diff --git a/ghost/core/core/server/data/seeders/importers/table-importer.ts b/ghost/core/core/server/data/seeders/importers/table-importer.ts index dab966a6f86..c336f02aab6 100644 --- a/ghost/core/core/server/data/seeders/importers/table-importer.ts +++ b/ghost/core/core/server/data/seeders/importers/table-importer.ts @@ -1,5 +1,5 @@ import debugFactory from '@tryghost/debug'; -import { toDatabaseDate } from '../../../lib/db-date'; +import { toDatabaseDate } from '../../../lib/db-types/date'; import path from 'node:path'; import fs from 'node:fs'; import papaparse from 'papaparse'; diff --git a/ghost/core/core/server/data/seeders/importers/tags-importer.js b/ghost/core/core/server/data/seeders/importers/tags-importer.js index 78fb2152cb2..b223e43cbec 100644 --- a/ghost/core/core/server/data/seeders/importers/tags-importer.js +++ b/ghost/core/core/server/data/seeders/importers/tags-importer.js @@ -1,7 +1,7 @@ const { faker } = require('@faker-js/faker'); const { slugify } = require('@tryghost/string'); const { TableImporter } = require('./table-importer'); -const { toDatabaseDate } = require('../../../lib/db-date'); +const { toDatabaseDate } = require('../../../lib/db-types/date'); class TagsImporter extends TableImporter { static table = 'tags'; diff --git a/ghost/core/core/server/data/seeders/importers/users-importer.js b/ghost/core/core/server/data/seeders/importers/users-importer.js index b7e2392623d..d65e063a29a 100644 --- a/ghost/core/core/server/data/seeders/importers/users-importer.js +++ b/ghost/core/core/server/data/seeders/importers/users-importer.js @@ -2,7 +2,7 @@ const { TableImporter } = require('./table-importer'); const { faker } = require('@faker-js/faker'); const { slugify } = require('@tryghost/string'); const security = require('@tryghost/security'); -const { toDatabaseDate } = require('../../../lib/db-date'); +const { toDatabaseDate } = require('../../../lib/db-types/date'); class UsersImporter extends TableImporter { static table = 'users'; diff --git a/ghost/core/core/server/data/seeders/importers/web-mentions-importer.js b/ghost/core/core/server/data/seeders/importers/web-mentions-importer.js index e8dfbc297bd..d9f7404fca2 100644 --- a/ghost/core/core/server/data/seeders/importers/web-mentions-importer.js +++ b/ghost/core/core/server/data/seeders/importers/web-mentions-importer.js @@ -1,6 +1,6 @@ const { TableImporter } = require('./table-importer'); const { faker } = require('@faker-js/faker'); -const { toDatabaseDate } = require('../../../lib/db-date'); +const { toDatabaseDate } = require('../../../lib/db-types/date'); class WebMentionsImporter extends TableImporter { static table = 'mentions'; diff --git a/ghost/core/core/server/data/seeders/utils/random.ts b/ghost/core/core/server/data/seeders/utils/random.ts index 3a92cefd353..e73fc361c74 100644 --- a/ghost/core/core/server/data/seeders/utils/random.ts +++ b/ghost/core/core/server/data/seeders/utils/random.ts @@ -1,5 +1,5 @@ import { faker } from '@faker-js/faker'; -import { fromDatabaseDate, type DatabaseDate } from '../../../lib/db-date'; +import { fromDatabaseDate, type DatabaseDate } from '../../../lib/db-types/date'; /** * Adds another degree of randomness into some decisions diff --git a/ghost/core/core/server/lib/db-types/boolean.ts b/ghost/core/core/server/lib/db-types/boolean.ts new file mode 100644 index 00000000000..4845ac72b4a --- /dev/null +++ b/ghost/core/core/server/lib/db-types/boolean.ts @@ -0,0 +1,16 @@ +import { z } from 'zod'; + +/** + * A boolean column, whichever engine handed it back. + * + * SQLite has no boolean type and answers with 0 or 1; MySQL answers with a boolean. Every + * read normalises so nothing downstream has to know which database it is talking to, and + * so a falsy check never has to reason about the number zero. + * + * Encoding leaves the value alone: a boolean column takes a boolean, and takes the 0 or 1 + * it gave us just as happily. + */ +export const DbBoolean = z.codec(z.union([z.boolean(), z.number()]), z.boolean(), { + decode: (stored) => Boolean(stored), + encode: (value) => value, +}); diff --git a/ghost/core/core/server/lib/db-date.ts b/ghost/core/core/server/lib/db-types/date.ts similarity index 100% rename from ghost/core/core/server/lib/db-date.ts rename to ghost/core/core/server/lib/db-types/date.ts diff --git a/ghost/core/core/server/services/automations/database-automations-repository.ts b/ghost/core/core/server/services/automations/database-automations-repository.ts index f40021c8c28..04ac4c7fb79 100644 --- a/ghost/core/core/server/services/automations/database-automations-repository.ts +++ b/ghost/core/core/server/services/automations/database-automations-repository.ts @@ -25,7 +25,7 @@ import type { EditAutomationData, Page, } from './automations-repository'; -import { fromDatabaseDate, toDatabaseDate, type DatabaseDate } from '../../lib/db-date'; +import { fromDatabaseDate, toDatabaseDate, type DatabaseDate } from '../../lib/db-types/date'; import { getStaleLockCutoff } from './stale-lock-cutoff'; import type { ExclusifyUnion, ReadonlyDeep } from 'type-fest'; diff --git a/ghost/core/core/server/services/gift-links/schema.ts b/ghost/core/core/server/services/gift-links/schema.ts index ee43a292ca4..7db3f08bf59 100644 --- a/ghost/core/core/server/services/gift-links/schema.ts +++ b/ghost/core/core/server/services/gift-links/schema.ts @@ -1,6 +1,6 @@ import { z } from 'zod'; import type { Knex } from 'knex'; -import { DbDate } from '../../lib/db-date'; +import { DbDate } from '../../lib/db-types/date'; export const DbGiftLink = z.object({ token: z.string(), diff --git a/ghost/core/core/server/services/gifts/gift-bookshelf-repository.ts b/ghost/core/core/server/services/gifts/gift-bookshelf-repository.ts index a843e34ef6c..d37f7e11819 100644 --- a/ghost/core/core/server/services/gifts/gift-bookshelf-repository.ts +++ b/ghost/core/core/server/services/gifts/gift-bookshelf-repository.ts @@ -4,7 +4,7 @@ import type { Knex } from 'knex'; import { Gift } from './gift'; import { decodeGiftRow, encodeGift } from './gift-codec'; import type { GiftCadence, GiftRow } from './gift-schema'; -import { toDatabaseDate } from '../../lib/db-date'; +import { toDatabaseDate } from '../../lib/db-types/date'; type ParsedNqlFilter = unknown; diff --git a/ghost/core/core/server/services/gifts/gift-delivery-bookshelf-repository.ts b/ghost/core/core/server/services/gifts/gift-delivery-bookshelf-repository.ts index 0b569901d42..331e2ea2ae2 100644 --- a/ghost/core/core/server/services/gifts/gift-delivery-bookshelf-repository.ts +++ b/ghost/core/core/server/services/gifts/gift-delivery-bookshelf-repository.ts @@ -1,6 +1,6 @@ import errors from '@tryghost/errors'; import type { Knex } from 'knex'; -import { fromDatabaseDate, toDatabaseDate } from '../../lib/db-date'; +import { fromDatabaseDate, toDatabaseDate } from '../../lib/db-types/date'; import { decodeGiftRow } from './gift-codec'; import { decodeGiftDeliveryRow, encodeGiftDelivery } from './gift-delivery-codec'; import type { Gift } from './gift'; diff --git a/ghost/core/core/server/services/gifts/gift-delivery-schema.ts b/ghost/core/core/server/services/gifts/gift-delivery-schema.ts index abe3d034f08..26b1785f914 100644 --- a/ghost/core/core/server/services/gifts/gift-delivery-schema.ts +++ b/ghost/core/core/server/services/gifts/gift-delivery-schema.ts @@ -1,5 +1,5 @@ import { z } from 'zod'; -import { DbDate } from '../../lib/db-date'; +import { DbDate } from '../../lib/db-types/date'; import type { CamelKeys } from '../../lib/case-keys'; export const GiftDeliveryStatusSchema = z.enum([ diff --git a/ghost/core/core/server/services/gifts/gift-schema.ts b/ghost/core/core/server/services/gifts/gift-schema.ts index 95fb483ceef..5fb65f2b642 100644 --- a/ghost/core/core/server/services/gifts/gift-schema.ts +++ b/ghost/core/core/server/services/gifts/gift-schema.ts @@ -1,6 +1,6 @@ import type { OptionalKeysOf, SetOptional } from 'type-fest'; import { z } from 'zod'; -import { DbDate } from '../../lib/db-date'; +import { DbDate } from '../../lib/db-types/date'; import type { CamelKeys } from '../../lib/case-keys'; export const GiftCadenceSchema = z.enum(['month', 'year']); diff --git a/ghost/core/core/server/services/members-custom-fields/schema.ts b/ghost/core/core/server/services/members-custom-fields/schema.ts index 8f29cef620e..12b2c2baa71 100644 --- a/ghost/core/core/server/services/members-custom-fields/schema.ts +++ b/ghost/core/core/server/services/members-custom-fields/schema.ts @@ -1,7 +1,7 @@ import { z } from 'zod'; import type { Knex } from 'knex'; import { FieldTypeSchema } from '@tryghost/custom-field-types'; -import { DbDate } from '../../lib/db-date'; +import { DbDate } from '../../lib/db-types/date'; // `archived` is soft: the field drops out of the values path but stays in the definition // list so it can be renamed, restored or deleted. Mirrors schema.js's `isIn` on the diff --git a/ghost/core/test/unit/server/lib/db-types/boolean.test.ts b/ghost/core/test/unit/server/lib/db-types/boolean.test.ts new file mode 100644 index 00000000000..d1639105512 --- /dev/null +++ b/ghost/core/test/unit/server/lib/db-types/boolean.test.ts @@ -0,0 +1,38 @@ +import assert from 'node:assert/strict'; +import { DbBoolean } from '../../../../../core/server/lib/db-types/boolean'; + +describe('DbBoolean', function () { + describe('decode', function () { + it('reads the 0 and 1 SQLite stores', function () { + assert.strictEqual(DbBoolean.decode(0), false); + assert.strictEqual(DbBoolean.decode(1), true); + }); + + it('reads the booleans MySQL returns', function () { + assert.strictEqual(DbBoolean.decode(false), false); + assert.strictEqual(DbBoolean.decode(true), true); + }); + + // A boolean column is a tinyint underneath, so it can hold a number neither + // engine considers out of range. Both of them treat every non-zero value as + // true, and so do we: a read is a worse place to discover the surprise than + // wherever the value came from. + it('reads any other number the column can hold as true', function () { + assert.strictEqual(DbBoolean.decode(2), true); + assert.strictEqual(DbBoolean.decode(-1), true); + }); + + it('rejects anything a boolean column cannot hold', function () { + for (const stored of ['1', null, undefined, Number.NaN, {}]) { + assert.throws(() => DbBoolean.decode(stored as never)); + } + }); + }); + + describe('encode', function () { + it('writes booleans unchanged', function () { + assert.strictEqual(DbBoolean.encode(false), false); + assert.strictEqual(DbBoolean.encode(true), true); + }); + }); +}); diff --git a/ghost/core/test/unit/server/lib/db-date.test.ts b/ghost/core/test/unit/server/lib/db-types/date.test.ts similarity index 96% rename from ghost/core/test/unit/server/lib/db-date.test.ts rename to ghost/core/test/unit/server/lib/db-types/date.test.ts index 80cdd401461..2047d430c36 100644 --- a/ghost/core/test/unit/server/lib/db-date.test.ts +++ b/ghost/core/test/unit/server/lib/db-types/date.test.ts @@ -2,7 +2,11 @@ import assert from 'node:assert/strict'; import { spawn } from 'node:child_process'; import { once } from 'node:events'; import * as errors from '@tryghost/errors'; -import { DbDate, fromDatabaseDate, toDatabaseDate } from '../../../../core/server/lib/db-date'; +import { + DbDate, + fromDatabaseDate, + toDatabaseDate, +} from '../../../../../core/server/lib/db-types/date'; describe('database date utilities', function () { const timezones = [ @@ -20,7 +24,7 @@ describe('database date utilities', function () { const runInOtherTimezones = async (toRun: string) => { // JSON.stringify does a good job wrapping strings in quotes and escaping. const s = JSON.stringify; - const modulePath = require.resolve('../../../../core/server/lib/db-date'); + const modulePath = require.resolve('../../../../../core/server/lib/db-types/date'); await Promise.all( timezones.map(async ({ tz, expectedNaive }) => { diff --git a/ghost/core/test/unit/server/services/automations/automations-repository.test.ts b/ghost/core/test/unit/server/services/automations/automations-repository.test.ts index 575d0cc823f..e18153d0e71 100644 --- a/ghost/core/test/unit/server/services/automations/automations-repository.test.ts +++ b/ghost/core/test/unit/server/services/automations/automations-repository.test.ts @@ -13,7 +13,7 @@ import type { AutomationsRepository, AutomationStepToRun, } from '../../../../../core/server/services/automations/automations-repository'; -import { fromDatabaseDate, toDatabaseDate } from '../../../../../core/server/lib/db-date'; +import { fromDatabaseDate, toDatabaseDate } from '../../../../../core/server/lib/db-types/date'; const HOUR_MS = 60 * 60 * 1000; const FAKE_WAIT_HOURS_MULTIPLIER = 2500; From 87ffcfd17caf4c07a19c1ed8f349dc6aea97c9cd Mon Sep 17 00:00:00 2001 From: Rob Lester Date: Tue, 25 Aug 2026 21:11:58 +0100 Subject: [PATCH 03/11] Changed a composite field's summary to come from its declared parts ref https://linear.app/ghost/issue/BER-3872 Storage, validation, CSV columns, the import mapping, the member form and filters all read the parts a composite type declares, and the part labels fail the build until they name a new one. The one-line summary did not: it was written out per type, so a part left out of it was collected, stored, exported and filtered on while being invisible in every list cell and detail row. It now comes from declaration order, and the only thing stated by hand is which parts read as one run, so that a state and a postal code still read "NY 00001". That is typed against the parts each type declares, which makes a part added upstream appear on its own and a part renamed or removed upstream fail the build here rather than going quietly missing from what a publisher sees. --- .../src/api/member-custom-fields.ts | 94 ++++++++++++++----- .../unit/api/member-custom-fields.test.ts | 45 ++++++++- packages/custom-field-types/src/index.ts | 6 ++ packages/custom-field-types/test/csv.test.ts | 14 +++ .../custom-field-types/test/index.test.ts | 7 ++ 5 files changed, 139 insertions(+), 27 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 36a68ba9480..22741bb9107 100644 --- a/apps/admin-x-framework/src/api/member-custom-fields.ts +++ b/apps/admin-x-framework/src/api/member-custom-fields.ts @@ -191,31 +191,63 @@ const isPartRecord = (value: unknown): value is Record => typeof value === 'object' && value !== null && !Array.isArray(value); /** - * How each composite type reads as one line. Written per type rather than walked from - * `subFieldsOf`, because where a part sits in the sentence is a fact about how the value - * reads, not one the value schema can supply — an address fuses state and postal code the - * way people write them. A part added upstream stays out of the line until someone decides - * where it belongs. + * Parts that read as one run rather than as separate items — "NY 00001", not "NY, 00001". * - * Total over the field types, the way the presentation catalog above is: a type added - * upstream fails to compile here until someone has decided how its value reads, rather - * than reaching every surface as a blank cell. A scalar declares `undefined`, which is - * how "its value is already a line" is said. + * This is the whole of what a composite's one-line form needs stated. Everything else + * comes from the value schema's declaration order, so a part added to a type upstream + * appears in the line on its own, without anyone knowing to come here. That was the point: + * the previous version wrote each type's line out by hand, and a part left out of it was + * collected, stored, exported and filtered on while being invisible in every summary — + * a silent omission, which is the worst way for this to fail. + * + * Typed against the parts each type declares, so renaming or removing one upstream fails + * the build here. Deliberately not exhaustive: a part nobody mentions is one that reads + * perfectly well on its own, and requiring an entry for each would put the omission + * problem straight back. */ -const compositeValueFormatters: { - [T in FieldType]: [PartsOf] extends [never] - ? undefined - : (value: Record) => string; -} = { - short_text: undefined, - long_text: undefined, - address: (value) => { - const { line1, line2, city, state, postal_code: postalCode, country } = value; - const statePostal = [state, postalCode].filter(Boolean).join(' '); - return [line1, line2, city, statePostal, country].filter(Boolean).join(', '); - }, +export type CompositePartRuns = { [T in FieldType]?: ReadonlyArray[]> }; + +const fusedParts: CompositePartRuns = { + address: [['state', 'postal_code']], }; +/** + * A composite type's parts grouped into the runs its line is built from: declaration + * order, with anything fused above kept together. + * + * A fused pair that is not adjacent in declaration order simply reads as two runs, so + * reordering a type upstream costs a comma rather than a wrong sentence. + */ +function partRunsFor(type: FieldType): string[][] { + const parts: string[] | null = subFieldsOf(type); + if (!parts) { + return []; + } + + const runOf = new Map(); + ((fusedParts[type] ?? []) as ReadonlyArray).forEach((group, index) => { + group.forEach((part) => runOf.set(part, index)); + }); + + const runs: string[][] = []; + let openRun: number | undefined; + for (const part of parts) { + const run = runOf.get(part); + if (run !== undefined && run === openRun) { + runs[runs.length - 1].push(part); + continue; + } + runs.push([part]); + openRun = run; + } + return runs; +} + +// Resolved once: the catalog is static, and this is read for every row of a member list. +const partRuns = Object.fromEntries( + FIELD_TYPE_IDS.map((type) => [type, partRunsFor(type)]), +) as Record; + /** * A member's value for one field as a single readable line: the string itself for a * scalar, and for a composite its parts joined the way that type reads — e.g. @@ -228,13 +260,25 @@ const compositeValueFormatters: { * table cell than in a detail row. */ export const formatMemberCustomFieldValue = (type: FieldType, value: unknown): string => { - const formatComposite = compositeValueFormatters[type]; + // Null for a scalar, and for a type this build has never heard of — both of which read + // as text or as nothing. + if (subFieldsOf(type) === null) { + return typeof value === 'string' ? value : ''; + } - if (formatComposite) { - return isPartRecord(value) ? formatComposite(value) : ''; + if (!isPartRecord(value)) { + return ''; } - return typeof value === 'string' ? value : ''; + return (partRuns[type] ?? []) + .map((run) => + run + .map((part) => value[part]) + .filter((part): part is string => typeof part === 'string' && part !== '') + .join(' '), + ) + .filter(Boolean) + .join(', '); }; export interface MemberCustomFieldsResponseType { 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 4c1a21db578..bdc22cd0c0f 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,5 @@ import { + type CompositePartRuns, type FieldTypePresentation, type MemberCustomField, formatMemberCustomFieldValue, @@ -41,7 +42,27 @@ const scalarWithParts: FieldTypePresentation<'short_text'> = { subFields: { line1: 'a' }, }; -export { labelled, missingPart, unknownPart, unlabelled, scalarWithParts }; +// A run is how the one-line form is told which parts read together. It names parts rather +// than listing them all, so a part *added* upstream needs no entry — but a part renamed or +// removed upstream has to be caught, or the run would silently stop fusing anything. +const fusedRun: CompositePartRuns = { address: [['state', 'postal_code']] }; + +// @ts-expect-error a run naming a part its value schema does not declare +const unknownFusedPart: CompositePartRuns = { address: [['state', 'postcode']] }; + +// @ts-expect-error a scalar type has no parts to run together +const scalarWithRun: CompositePartRuns = { short_text: [['line1']] }; + +export { + labelled, + missingPart, + unknownPart, + unlabelled, + scalarWithParts, + fusedRun, + unknownFusedPart, + scalarWithRun, +}; const field = (overrides: Partial): MemberCustomField => ({ key: 'nickname', @@ -122,7 +143,9 @@ describe('member custom fields api helpers', () => { field({ key: 'address_home', name: 'Address (Home)', type: 'address' }), ]); - expect(columns[2]).toEqual({ + // Found rather than indexed: what this is about is the label, not the position + // of a part in the address. + expect(columns.find((column) => column.partLabel === 'City')).toEqual({ label: 'Address (Home) (City)', fieldName: 'Address (Home)', partLabel: 'City', @@ -194,6 +217,24 @@ describe('member custom fields api helpers', () => { ).toBe('1 Main St, 12 apt B, New York, NY 00001, US'); }); + // The property this is built for. A part added to a type upstream has to appear in + // the line on its own, because the alternative is a value that is collected, stored, + // exported and filtered on while being invisible in every summary. Asserted through + // the catalog rather than against a hardcoded list, so it keeps holding as the + // catalog grows. + it('includes every part a type declares, in the order it declares them', () => { + const parts = memberCustomFieldParts('address')!; + const value = Object.fromEntries(parts.map(({ key }) => [key, key])); + + const line = formatMemberCustomFieldValue('address', value); + + for (const { key } of parts) { + expect(line, `${key} is missing from the line`).toContain(key); + } + // Separators aside, the parts read in the order the value schema declares them. + expect(line.split(/,\s|\s/)).toEqual(parts.map(({ key }) => key)); + }); + it('pairs state and postal code, and drops missing parts cleanly', () => { expect( formatMemberCustomFieldValue('address', { diff --git a/packages/custom-field-types/src/index.ts b/packages/custom-field-types/src/index.ts index ab6faa5e17a..54efe50c3fc 100644 --- a/packages/custom-field-types/src/index.ts +++ b/packages/custom-field-types/src/index.ts @@ -187,6 +187,12 @@ export const FIELD_TYPES = defineFieldTypes({ long_text: longText(), // An address is a delivery address, so its bounds are what a courier will accept // rather than what the column could hold. Modelled on Stripe's Address object. + // + // Who the parcel is addressed to is not here. A parcel needs a name as well as an + // address, but that is a fact about posting parcels rather than about either type, + // and the name is often not the account name — gift subscriptions, workplace + // deliveries, c/o. A site that needs one keeps it in a field of its own, which is + // also how Stripe hands it back: beside the address rather than inside it. address: record( { line1: shortText(), diff --git a/packages/custom-field-types/test/csv.test.ts b/packages/custom-field-types/test/csv.test.ts index 4f6654c8e1f..1dafbb998a1 100644 --- a/packages/custom-field-types/test/csv.test.ts +++ b/packages/custom-field-types/test/csv.test.ts @@ -197,6 +197,20 @@ describe('reading custom field values from a CSV row', function () { ); }); + // The sub-cells a composite occupies are the parts its value schema declares, so a + // column naming anything else is dropped the way a column naming no field is. A + // recipient's name is the one that gets written by hand: a parcel needs one, but it + // belongs to a field of its own rather than to the address. + it('drops a sub-cell that names no part of the composite', function () { + assert.deepEqual( + fieldValuesFromCsvRow([address], { + 'custom_fields.shipping_address.name': 'Bex Jones', + 'custom_fields.shipping_address.city': 'London', + }), + { shipping_address: { city: 'London' } }, + ); + }); + // A partial composite is read as a value (validation, run by the caller, is what // rejects it) rather than silently dropped like an all-blank one. it('reads a partial composite so its validation can fail the row', function () { diff --git a/packages/custom-field-types/test/index.test.ts b/packages/custom-field-types/test/index.test.ts index 0c409e09617..9530c38fc0b 100644 --- a/packages/custom-field-types/test/index.test.ts +++ b/packages/custom-field-types/test/index.test.ts @@ -97,6 +97,13 @@ describe('custom-field-types catalog', function () { assert.equal(parse({ line1: 'Cloonlara', state: 'Co. Clare', country: 'IE' }), true); }); + // A recipient's name is not part of an address. A parcel needs both, which is a + // fact about posting parcels rather than about either type, so a site that collects + // a name keeps it in a field of its own. + it('is not where a recipient name lives', function () { + assert.equal(parse({ name: 'Bex Jones' }), false); + }); + it('rejects an address that names nothing', function () { assert.equal(parse({}), false); // A key present but explicitly undefined names nothing either: undefined is From 9a33b01b0f93cdb5238f910757a4e48734c58571 Mon Sep 17 00:00:00 2001 From: Rob Lester Date: Wed, 26 Aug 2026 13:36:10 +0100 Subject: [PATCH 04/11] Added checkout collection to the fake Stripe server (#30304) Stripe Checkout can be asked to collect a shipping address, a phone number or a tax number alongside the payment, and the fake server the end-to-end tests run against knew none of it. Anything built on top would have been tested against a server that accepted whatever it was sent, which is the opposite of what a fake is for. The rules it now enforces were measured against the live API at the version Ghost pins rather than read from the reference, which disagreed with the API in three of five probes, so a request the real Stripe would refuse now fails a test here first, including the one refusal that is easy to miss: a tax number cannot be collected for a customer Stripe may not rename. Modelling those rules turned up one of its own. Stripe's SDK form-encodes its request bodies, so a checkout that switched tax collection off arrives carrying the string "false" rather than the boolean, and reading that flag for truthiness refused a checkout that had asked for no tax number at all. The existing tests missed it because they post JSON, where the flag is a real boolean, so the refusal only appeared under the encoding Ghost actually uses. A captured fixture also carried the real street and postcode of a payment made while capturing it, and carries invented ones instead now that nothing asserts on their contents. ref https://linear.app/ghost/issue/BER-3872 Claude-Session: https://claude.ai/code/session_018jPEJMYLoUzNvYbdxWb3dY --- e2e/helpers/services/stripe/builders.ts | 6 + .../services/stripe/completed-checkout.ts | 210 ++++++++++++++++++ .../services/stripe/fake-stripe-server.ts | 19 ++ .../fixtures/checkout_session.collection.json | 124 +++++++++++ .../fixtures/checkout_session.completed.json | 30 +-- .../fixtures/checkout_session.donation.json | 8 +- .../checkout_session.subscription.json | 6 +- .../services/stripe/fixtures/customer.json | 8 +- .../services/stripe/fixtures/manifest.json | 2 +- .../stripe/fixtures/payment_method.json | 6 +- .../fixtures/subscription.complimentary.json | 28 +-- .../stripe/fixtures/subscription.paid.json | 30 +-- .../services/stripe/request-schemas.ts | 18 +- e2e/helpers/services/stripe/stripe-service.ts | 35 ++- e2e/tests/stripe-fixtures/constraints.test.ts | 42 ++++ 15 files changed, 509 insertions(+), 63 deletions(-) create mode 100644 e2e/helpers/services/stripe/completed-checkout.ts create mode 100644 e2e/helpers/services/stripe/fixtures/checkout_session.collection.json diff --git a/e2e/helpers/services/stripe/builders.ts b/e2e/helpers/services/stripe/builders.ts index aef19bbbf55..73ba4b10cbe 100644 --- a/e2e/helpers/services/stripe/builders.ts +++ b/e2e/helpers/services/stripe/builders.ts @@ -205,6 +205,12 @@ export interface StripeCheckoutSessionRequest { }; }; submit_type?: 'auto' | 'book' | 'donate' | 'pay' | 'send'; + // What the page collects for itself, as opposed to the questions above. Recorded so a + // test can assert what a publisher's configuration actually asked Stripe for, and so a + // completion can only carry data the checkout collected. + shipping_address_collection?: { allowed_countries: string[] }; + tax_id_collection?: { enabled: boolean }; + phone_number_collection?: { enabled: boolean }; } export interface RecordedStripeCheckoutSession { diff --git a/e2e/helpers/services/stripe/completed-checkout.ts b/e2e/helpers/services/stripe/completed-checkout.ts new file mode 100644 index 00000000000..2aa2547b667 --- /dev/null +++ b/e2e/helpers/services/stripe/completed-checkout.ts @@ -0,0 +1,210 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import type { RecordedStripeCheckoutSession, StripeEvent } from './builders'; + +/** + * Completing a checkout the way Stripe completes one. + * + * ## Why this reads a fixture instead of building an object + * + * The completed session is the only Stripe payload Ghost reads publisher data out of, + * and its shape is version-dependent in a way nothing in the payload would warn us + * about — at our pinned API version the address is `shipping`, and later versions move + * it to `collected_information.shipping_details`. A hand-built event would let a test + * pass against a shape Stripe stopped sending, which is the failure this whole fixture + * suite exists to prevent. So the event is a real captured session with the parts a + * test cares about substituted in, and `drift.test.ts` keeps the capture honest. + * + * ## Why it reads the request too + * + * A member can only answer a question the checkout page asked. Building the completion + * from the session Ghost actually created means a test cannot answer a question that + * was never rendered, or supply an address for a checkout that never asked for one — + * both of which would otherwise pass while proving nothing. Those are errors here, + * named, rather than silently accepted. + */ + +const fixtureDir = path.resolve(path.dirname(fileURLToPath(import.meta.url)), 'fixtures'); + +function capturedSession(): Record { + return JSON.parse( + fs.readFileSync(path.resolve(fixtureDir, 'checkout_session.completed.json'), 'utf8'), + ); +} + +/** An address as a member would type it on the checkout page. */ +export interface CheckoutShippingInput { + name?: string; + line1?: string; + line2?: string; + city?: string; + state?: string; + postal_code?: string; + country?: string; +} + +/** What a member filled in on the checkout page. Everything is optional; nothing is invented. */ +export interface CollectedCheckoutInput { + /** Answers to the publisher's questions, keyed by the custom field key Ghost asked under. */ + answers?: Record; + /** + * The delivery address. Omitted, a checkout that asked for one gets the captured + * fixture's address, so a test that does not care about the values stays short. + * `null` is a checkout that asked and collected nothing. + */ + shipping?: CheckoutShippingInput | null; + taxId?: string | null; + phone?: string | null; +} + +function askedQuestions(session: RecordedStripeCheckoutSession): string[] { + const asked = (session.request as { custom_fields?: Array<{ key: string }> }).custom_fields ?? []; + return asked.map((field) => field.key); +} + +/** + * Whether the checkout asked for something. Shipping says so by carrying a list of + * countries it will deliver to; tax and phone carry an `enabled` flag instead, and a flag + * turned off is a question the page never rendered. + */ +function asks(session: RecordedStripeCheckoutSession, parameter: string): boolean { + const asked = (session.request as Record)[parameter]; + + if (asked === undefined || asked === null) { + return false; + } + if (typeof asked === 'object' && 'enabled' in asked) { + return (asked as { enabled?: unknown }).enabled === true; + } + return true; +} + +/** + * The `custom_fields` a completed session carries: every question the page asked, each + * holding the answer a test says the member gave, or none. + * + * Built from the request rather than from the answers, because that is the direction the + * real thing works in — Stripe returns every field it rendered, answered or not. + */ +function answeredQuestions( + session: RecordedStripeCheckoutSession, + answers: Record, +) { + const asked = askedQuestions(session); + + for (const key of Object.keys(answers)) { + if (!asked.includes(key)) { + throw new Error( + `The checkout never asked for "${key}", so a member could not have answered it. ` + + `It asked for: ${asked.length > 0 ? asked.join(', ') : 'nothing'}.`, + ); + } + } + + const requested = + ( + session.request as { + custom_fields?: Array<{ key: string; label?: { custom?: string }; optional?: boolean }>; + } + ).custom_fields ?? []; + return requested.map((field) => ({ + key: field.key, + type: 'text', + optional: field.optional ?? true, + label: { type: 'custom', custom: field.label?.custom ?? field.key }, + // Stripe returns the whole `text` object whether or not it was filled in. + text: { + value: answers[field.key] ?? null, + default_value: null, + maximum_length: null, + minimum_length: null, + }, + })); +} + +function shippingBlock( + session: RecordedStripeCheckoutSession, + given: CollectedCheckoutInput['shipping'], + captured: Record, +) { + const collecting = asks(session, 'shipping_address_collection'); + + if (given !== undefined && given !== null && !collecting) { + throw new Error( + 'The checkout never asked for a shipping address, so a member could not have given one.', + ); + } + if (!collecting || given === null) { + return null; + } + if (given === undefined) { + return captured.shipping; + } + + const { name, ...address } = given; + const capturedShipping = captured.shipping as { address: Record }; + return { + name: name ?? null, + // Every part Stripe returns is present, and one the test left out comes back null + // rather than absent — which is what makes "an address with holes in it" testable. + address: Object.fromEntries( + Object.keys(capturedShipping.address).map((part) => [ + part, + address[part as keyof typeof address] ?? null, + ]), + ), + carrier: null, + phone: null, + tracking_number: null, + }; +} + +/** + * A `checkout.session.completed` event for a session Ghost created, carrying what a + * member filled in on the page. + */ +export function buildCollectedCheckoutCompletedEvent(opts: { + session: RecordedStripeCheckoutSession; + customerId: string; + collected?: CollectedCheckoutInput; +}): StripeEvent { + const { session, customerId, collected = {} } = opts; + const captured = capturedSession(); + + if (collected.taxId && !asks(session, 'tax_id_collection')) { + throw new Error( + 'The checkout never asked for a tax number, so a member could not have given one.', + ); + } + if (collected.phone && !asks(session, 'phone_number_collection')) { + throw new Error( + 'The checkout never asked for a phone number, so a member could not have given one.', + ); + } + + const customerDetails = captured.customer_details as Record; + + return { + id: `evt_${session.response.id}`, + object: 'event', + type: 'checkout.session.completed', + data: { + object: { + ...captured, + id: session.response.id, + mode: 'subscription', + customer: customerId, + metadata: { checkoutType: 'signup', ...session.response.metadata }, + custom_fields: answeredQuestions(session, collected.answers ?? {}), + shipping: shippingBlock(session, collected.shipping, captured), + customer_details: { + ...customerDetails, + phone: collected.phone ?? null, + // A typed pair, of which only the value ever crosses into a field. + tax_ids: collected.taxId ? [{ type: 'gb_vat', value: collected.taxId }] : [], + }, + }, + }, + }; +} diff --git a/e2e/helpers/services/stripe/fake-stripe-server.ts b/e2e/helpers/services/stripe/fake-stripe-server.ts index bede0a9cde8..090c04f0b21 100644 --- a/e2e/helpers/services/stripe/fake-stripe-server.ts +++ b/e2e/helpers/services/stripe/fake-stripe-server.ts @@ -507,6 +507,9 @@ export class FakeStripeServer extends FakeServer { submit_type: body.submit_type, subscription_data: body.subscription_data, line_items: body.line_items, + shipping_address_collection: body.shipping_address_collection, + tax_id_collection: body.tax_id_collection, + phone_number_collection: body.phone_number_collection, }, response: { mode, @@ -729,6 +732,22 @@ export class FakeStripeServer extends FakeServer { } } + // Stripe will not collect a tax id for a customer it may not rename. Measured, like + // everything else here: shipping and phone collection carry no such requirement, so + // this is specific to tax rather than a rule about collecting from a customer at all. + // Read against `true` rather than for truthiness: form decoding delivers the flag as + // the string `"false"`, which is truthy, and refusing on that would refuse a checkout + // that had switched tax collection off. + const taxIdFlag = (body.tax_id_collection as { enabled?: unknown })?.enabled; + const collectsTaxId = taxIdFlag === true || taxIdFlag === 'true'; + const mayRename = (body.customer_update as { name?: unknown })?.name === 'auto'; + if (collectsTaxId && body.customer && !mayRename) { + return ( + 'Tax ID collection requires updating business name on the customer. To enable tax ID ' + + 'collection for an existing customer, please set `customer_update[name]` to `auto`.' + ); + } + return null; } diff --git a/e2e/helpers/services/stripe/fixtures/checkout_session.collection.json b/e2e/helpers/services/stripe/fixtures/checkout_session.collection.json new file mode 100644 index 00000000000..a8ee043fdce --- /dev/null +++ b/e2e/helpers/services/stripe/fixtures/checkout_session.collection.json @@ -0,0 +1,124 @@ +{ + "id": "cs_test_a1x0bgEIkVCwnGDl3sbDcHplHRhK4DGw3GtdMBiUkeqoMBQ1GFkFPKTgtV", + "object": "checkout.session", + "adaptive_pricing": { + "enabled": true + }, + "after_expiration": null, + "allow_promotion_codes": null, + "amount_subtotal": 500, + "amount_total": 500, + "automatic_tax": { + "enabled": false, + "liability": null, + "provider": null, + "status": null + }, + "billing_address_collection": null, + "branding_settings": { + "background_color": "#ffffff", + "border_style": "rounded", + "button_color": "#0074d4", + "display_name": "Counterproof Collective", + "font_family": "default", + "icon": null, + "logo": null + }, + "cancel_url": "https://example.com/cancel", + "client_reference_id": null, + "client_secret": null, + "collected_information": null, + "consent": null, + "consent_collection": null, + "created": 1787267115, + "currency": "usd", + "currency_conversion": null, + "custom_fields": [ + { + "key": "delivery_notes", + "label": { + "custom": "Delivery notes", + "type": "custom" + }, + "optional": true, + "text": { + "default_value": null, + "maximum_length": null, + "minimum_length": null, + "value": null + }, + "type": "text" + } + ], + "custom_text": { + "after_submit": null, + "shipping_address": null, + "submit": null, + "terms_of_service_acceptance": null + }, + "customer": null, + "customer_account": null, + "customer_creation": "always", + "customer_details": null, + "customer_email": null, + "discounts": [], + "expires_at": 1787353515, + "integration_identifier": null, + "invoice": null, + "invoice_creation": null, + "livemode": false, + "locale": null, + "managed_payments": { + "enabled": false + }, + "metadata": {}, + "mode": "subscription", + "origin_context": null, + "payment_intent": null, + "payment_link": null, + "payment_method_collection": "always", + "payment_method_configuration_details": { + "id": "pmc_1SAoFrCvhsRvBqJ0uYiAIPaH", + "parent": null + }, + "payment_method_options": { + "card": { + "request_three_d_secure": "automatic" + } + }, + "payment_method_types": ["card", "link", "amazon_pay"], + "payment_status": "unpaid", + "permissions": null, + "phone_number_collection": { + "enabled": false + }, + "recovered_from": null, + "saved_payment_method_options": { + "allow_redisplay_filters": ["always"], + "payment_method_remove": "disabled", + "payment_method_save": null + }, + "setup_intent": null, + "shipping": null, + "shipping_address_collection": { + "allowed_countries": ["GB", "US"] + }, + "shipping_options": [], + "shipping_rate": null, + "status": "open", + "submit_type": null, + "subscription": null, + "success_url": "https://example.com/success", + "tax_id_collection": { + "enabled": true, + "required": "never" + }, + "total_details": { + "amount_discount": 0, + "amount_shipping": 0, + "amount_tax": 0 + }, + "ui_mode": "hosted", + "url": "https://checkout.stripe.com/c/pay/cs_test_redacted", + "wallet_options": null +} diff --git a/e2e/helpers/services/stripe/fixtures/checkout_session.completed.json b/e2e/helpers/services/stripe/fixtures/checkout_session.completed.json index f175b16316c..82e89d18f37 100644 --- a/e2e/helpers/services/stripe/fixtures/checkout_session.completed.json +++ b/e2e/helpers/services/stripe/fixtures/checkout_session.completed.json @@ -1,5 +1,5 @@ { - "id": "cs_test_a1YnnBBjnCdOg0U3Vsen6LEWYp9JkPdeTqBnP0ZriFhZsoNpAufR9uaUmZ", + "id": "cs_test_a1BZS9tua8DNQPNiyWU0vOOHJgrC3CteT1RkZlNO1knK0Ez7KiZ2phprgY", "object": "checkout.session", "adaptive_pricing": { "enabled": true @@ -19,7 +19,7 @@ "background_color": "#ffffff", "border_style": "rounded", "button_color": "#0074d4", - "display_name": "Example Publication", + "display_name": "Counterproof Collective", "font_family": "default", "icon": null, "logo": null @@ -28,7 +28,7 @@ "client_reference_id": null, "client_secret": null, "collected_information": { - "business_name": "Example Company Ltd", + "business_name": "Testing Co", "individual_name": null, "shipping_details": { "address": { @@ -39,12 +39,12 @@ "postal_code": "EC1A 1AA", "state": null }, - "name": "Jamie Example" + "name": "Test Tester" } }, "consent": null, "consent_collection": null, - "created": 1787091163, + "created": 1787267225, "currency": "usd", "currency_conversion": null, "custom_fields": [ @@ -59,7 +59,7 @@ "default_value": null, "maximum_length": null, "minimum_length": null, - "value": "These are delivery notes" + "value": "Some example delivery notes." }, "type": "text" } @@ -70,7 +70,7 @@ "submit": null, "terms_of_service_acceptance": null }, - "customer": "cus_V67VaSsCShqIuI", + "customer": "cus_V6spNElFR59wgr", "customer_account": null, "customer_creation": "always", "customer_details": { @@ -82,10 +82,10 @@ "postal_code": "EC1A 1AA", "state": null }, - "business_name": "Example Company Ltd", - "email": "member@example.com", + "business_name": "Testing Co", + "email": "test@example.com", "individual_name": null, - "name": "Example Company Ltd", + "name": "Testing Co", "phone": null, "tax_exempt": "none", "tax_ids": [ @@ -97,9 +97,9 @@ }, "customer_email": null, "discounts": [], - "expires_at": 1787177562, + "expires_at": 1787353625, "integration_identifier": null, - "invoice": "in_1U5vGmCvhsRvBqJ0YdYrEKnB", + "invoice": "in_1U6f47CvhsRvBqJ0cSDraiS4", "invoice_creation": null, "livemode": false, "locale": null, @@ -113,7 +113,7 @@ "payment_link": null, "payment_method_collection": "always", "payment_method_configuration_details": { - "id": "pmc_000000000000000000000000", + "id": "pmc_1SAoFrCvhsRvBqJ0uYiAIPaH", "parent": null }, "payment_method_options": { @@ -144,7 +144,7 @@ "state": null }, "carrier": null, - "name": "Jamie Example", + "name": "Test Tester", "phone": null, "tracking_number": null }, @@ -155,7 +155,7 @@ "shipping_rate": null, "status": "complete", "submit_type": null, - "subscription": "sub_1U5vGmCvhsRvBqJ0Asiwj5Es", + "subscription": "sub_1U6f47CvhsRvBqJ0RXvtkYnc", "success_url": "https://example.com/success", "tax_id_collection": { "enabled": true, diff --git a/e2e/helpers/services/stripe/fixtures/checkout_session.donation.json b/e2e/helpers/services/stripe/fixtures/checkout_session.donation.json index c90a1499f0a..af042b606c2 100644 --- a/e2e/helpers/services/stripe/fixtures/checkout_session.donation.json +++ b/e2e/helpers/services/stripe/fixtures/checkout_session.donation.json @@ -1,5 +1,5 @@ { - "id": "cs_test_a1JFU1pkfvMURRP8L50qzu83Xp0qpKB2QZid0dEoVANrxtQaorTTmNcHG3", + "id": "cs_test_a1l3zDI0UWbKmRw4oGRm0NuyhQzkGwv6gvOT20lZTdKFUHNxpOEgGzg5Yf", "object": "checkout.session", "adaptive_pricing": { "enabled": true @@ -30,7 +30,7 @@ "collected_information": null, "consent": null, "consent_collection": null, - "created": 1787146483, + "created": 1787267116, "currency": "usd", "currency_conversion": null, "custom_fields": [ @@ -62,7 +62,7 @@ "customer_details": null, "customer_email": null, "discounts": [], - "expires_at": 1787232882, + "expires_at": 1787353516, "integration_identifier": null, "invoice": null, "invoice_creation": { @@ -85,7 +85,7 @@ "metadata": {}, "mode": "payment", "origin_context": null, - "payment_intent": "pi_3U69dfCvhsRvBqJ01yIXNFSb", + "payment_intent": "pi_3U6f1MCvhsRvBqJ01a44znVT", "payment_link": null, "payment_method_collection": "always", "payment_method_configuration_details": { diff --git a/e2e/helpers/services/stripe/fixtures/checkout_session.subscription.json b/e2e/helpers/services/stripe/fixtures/checkout_session.subscription.json index 405fd5d6f24..024dcb7bb86 100644 --- a/e2e/helpers/services/stripe/fixtures/checkout_session.subscription.json +++ b/e2e/helpers/services/stripe/fixtures/checkout_session.subscription.json @@ -1,5 +1,5 @@ { - "id": "cs_test_a1Idpdr8cvKb63vogqazoFD99ykbrxo3gH084qaMmGngOeWAka9zNaKbzw", + "id": "cs_test_a12XfPsq3yC6xEIbQiHiGDdhiObvgsOi9fomZrYakQ7TFiWEpsmdXoqy51", "object": "checkout.session", "adaptive_pricing": { "enabled": true @@ -30,7 +30,7 @@ "collected_information": null, "consent": null, "consent_collection": null, - "created": 1787146482, + "created": 1787267115, "currency": "usd", "currency_conversion": null, "custom_fields": [], @@ -46,7 +46,7 @@ "customer_details": null, "customer_email": null, "discounts": [], - "expires_at": 1787232882, + "expires_at": 1787353515, "integration_identifier": null, "invoice": null, "invoice_creation": null, diff --git a/e2e/helpers/services/stripe/fixtures/customer.json b/e2e/helpers/services/stripe/fixtures/customer.json index df319feb581..ea07031c547 100644 --- a/e2e/helpers/services/stripe/fixtures/customer.json +++ b/e2e/helpers/services/stripe/fixtures/customer.json @@ -1,9 +1,9 @@ { - "id": "cus_V6MM2ouqpuTHrR", + "id": "cus_V6smdPRjHxGUCZ", "object": "customer", "address": null, "balance": 0, - "created": 1787146476, + "created": 1787267109, "currency": null, "customer_account": null, "default_currency": null, @@ -11,8 +11,8 @@ "delinquent": false, "description": null, "discount": null, - "email": "fixture-1787146475935@example.com", - "invoice_prefix": "IMQPIQTM", + "email": "fixture-1787267109071@example.com", + "invoice_prefix": "6YHAPGHM", "invoice_settings": { "custom_fields": null, "default_payment_method": null, diff --git a/e2e/helpers/services/stripe/fixtures/manifest.json b/e2e/helpers/services/stripe/fixtures/manifest.json index ed54752311a..ecb6cb2ac83 100644 --- a/e2e/helpers/services/stripe/fixtures/manifest.json +++ b/e2e/helpers/services/stripe/fixtures/manifest.json @@ -1,5 +1,5 @@ { - "captured_at": "2026-08-19T13:34:43.264Z", + "captured_at": "2026-08-20T23:05:16.663Z", "api_version": "2020-08-27", "stripe_node": "see e2e/package.json", "note": "Regenerate with `pnpm stripe:fixtures`. Completed checkout is captured separately by hand." diff --git a/e2e/helpers/services/stripe/fixtures/payment_method.json b/e2e/helpers/services/stripe/fixtures/payment_method.json index bcc580783df..22da4d1b757 100644 --- a/e2e/helpers/services/stripe/fixtures/payment_method.json +++ b/e2e/helpers/services/stripe/fixtures/payment_method.json @@ -1,5 +1,5 @@ { - "id": "pm_1U69dYCvhsRvBqJ0aW3bUj9N", + "id": "pm_1U6f1FCvhsRvBqJ0Tn5Goz44", "object": "payment_method", "allow_redisplay": "unspecified", "billing_details": { @@ -41,8 +41,8 @@ }, "wallet": null }, - "created": 1787146476, - "customer": "cus_V6MM2ouqpuTHrR", + "created": 1787267109, + "customer": "cus_V6smdPRjHxGUCZ", "customer_account": null, "livemode": false, "metadata": {}, diff --git a/e2e/helpers/services/stripe/fixtures/subscription.complimentary.json b/e2e/helpers/services/stripe/fixtures/subscription.complimentary.json index feb2841ba7b..de7569b9f0b 100644 --- a/e2e/helpers/services/stripe/fixtures/subscription.complimentary.json +++ b/e2e/helpers/services/stripe/fixtures/subscription.complimentary.json @@ -1,5 +1,5 @@ { - "id": "sub_1U69dcCvhsRvBqJ0IolEWGOg", + "id": "sub_1U6f1KCvhsRvBqJ0wp8NMiMG", "object": "subscription", "application": null, "application_fee_percent": null, @@ -8,7 +8,7 @@ "enabled": false, "liability": null }, - "billing_cycle_anchor": 1787146480, + "billing_cycle_anchor": 1787267114, "billing_cycle_anchor_config": null, "billing_mode": { "flexible": null, @@ -25,11 +25,11 @@ "reason": null }, "collection_method": "charge_automatically", - "created": 1787146480, + "created": 1787267114, "currency": "usd", - "current_period_end": 1818682480, - "current_period_start": 1787146480, - "customer": "cus_V6MM2ouqpuTHrR", + "current_period_end": 1818803114, + "current_period_start": 1787267114, + "customer": "cus_V6smdPRjHxGUCZ", "customer_account": null, "days_until_due": null, "default_payment_method": null, @@ -52,12 +52,12 @@ "object": "list", "data": [ { - "id": "si_V6MM1T6OLQ9rq3", + "id": "si_V6smCDciyDMUfh", "object": "subscription_item", "billing_thresholds": null, - "created": 1787146481, - "current_period_end": 1818682480, - "current_period_start": 1787146480, + "created": 1787267114, + "current_period_end": 1818803114, + "current_period_start": 1787267114, "discounts": [], "metadata": {}, "plan": { @@ -111,15 +111,15 @@ "unit_amount_decimal": "0" }, "quantity": 1, - "subscription": "sub_1U69dcCvhsRvBqJ0IolEWGOg", + "subscription": "sub_1U6f1KCvhsRvBqJ0wp8NMiMG", "tax_rates": [] } ], "has_more": false, "total_count": 1, - "url": "/v1/subscription_items?subscription=sub_1U69dcCvhsRvBqJ0IolEWGOg" + "url": "/v1/subscription_items?subscription=sub_1U6f1KCvhsRvBqJ0wp8NMiMG" }, - "latest_invoice": "in_1U69dcCvhsRvBqJ08yHXCeax", + "latest_invoice": "in_1U6f1KCvhsRvBqJ0XciUTJZX", "livemode": false, "managed_payments": { "enabled": false @@ -160,7 +160,7 @@ }, "quantity": 1, "schedule": null, - "start_date": 1787146480, + "start_date": 1787267114, "status": "active", "test_clock": null, "transfer_data": null, diff --git a/e2e/helpers/services/stripe/fixtures/subscription.paid.json b/e2e/helpers/services/stripe/fixtures/subscription.paid.json index 78c563dcad2..de6241c2d26 100644 --- a/e2e/helpers/services/stripe/fixtures/subscription.paid.json +++ b/e2e/helpers/services/stripe/fixtures/subscription.paid.json @@ -1,5 +1,5 @@ { - "id": "sub_1U69dZCvhsRvBqJ0U6OTSuAi", + "id": "sub_1U6f1GCvhsRvBqJ0tLIMH2xZ", "object": "subscription", "application": null, "application_fee_percent": null, @@ -8,7 +8,7 @@ "enabled": false, "liability": null }, - "billing_cycle_anchor": 1787146477, + "billing_cycle_anchor": 1787267110, "billing_cycle_anchor_config": null, "billing_mode": { "flexible": null, @@ -25,14 +25,14 @@ "reason": null }, "collection_method": "charge_automatically", - "created": 1787146477, + "created": 1787267110, "currency": "usd", - "current_period_end": 1789824877, - "current_period_start": 1787146477, - "customer": "cus_V6MM2ouqpuTHrR", + "current_period_end": 1789945510, + "current_period_start": 1787267110, + "customer": "cus_V6smdPRjHxGUCZ", "customer_account": null, "days_until_due": null, - "default_payment_method": "pm_1U69dYCvhsRvBqJ0aW3bUj9N", + "default_payment_method": "pm_1U6f1FCvhsRvBqJ0Tn5Goz44", "default_source": null, "default_tax_rates": [], "description": null, @@ -52,12 +52,12 @@ "object": "list", "data": [ { - "id": "si_V6MMm5uCeFqpp0", + "id": "si_V6smIOddNd0TTs", "object": "subscription_item", "billing_thresholds": null, - "created": 1787146477, - "current_period_end": 1789824877, - "current_period_start": 1787146477, + "created": 1787267111, + "current_period_end": 1789945510, + "current_period_start": 1787267110, "discounts": [], "metadata": {}, "plan": { @@ -111,15 +111,15 @@ "unit_amount_decimal": "500" }, "quantity": 1, - "subscription": "sub_1U69dZCvhsRvBqJ0U6OTSuAi", + "subscription": "sub_1U6f1GCvhsRvBqJ0tLIMH2xZ", "tax_rates": [] } ], "has_more": false, "total_count": 1, - "url": "/v1/subscription_items?subscription=sub_1U69dZCvhsRvBqJ0U6OTSuAi" + "url": "/v1/subscription_items?subscription=sub_1U6f1GCvhsRvBqJ0tLIMH2xZ" }, - "latest_invoice": "in_1U69dZCvhsRvBqJ0hbWczbvu", + "latest_invoice": "in_1U6f1GCvhsRvBqJ07l7xs13W", "livemode": false, "managed_payments": { "enabled": false @@ -160,7 +160,7 @@ }, "quantity": 1, "schedule": null, - "start_date": 1787146477, + "start_date": 1787267110, "status": "active", "test_clock": null, "transfer_data": null, diff --git a/e2e/helpers/services/stripe/request-schemas.ts b/e2e/helpers/services/stripe/request-schemas.ts index 9f901eb05ea..f827ad2ca8e 100644 --- a/e2e/helpers/services/stripe/request-schemas.ts +++ b/e2e/helpers/services/stripe/request-schemas.ts @@ -205,8 +205,22 @@ export const CreateCheckoutSessionSchema = z.object({ .optional() .catch(undefined), customer_update: z.unknown().optional(), - shipping_address_collection: z.unknown().optional(), - tax_id_collection: z.unknown().optional(), + // Read rather than passed through, so a test can assert which countries a publisher's + // checkout offers. The list is what makes this parameter exist at all: an empty + // collection object form-encodes to nothing, so a request built that way carries no + // parameter and Stripe accepts it having been asked to collect nothing. + shipping_address_collection: z + .object({ allowed_countries: list(z.string()) }) + .optional() + .catch(undefined), + tax_id_collection: z + .object({ enabled: bool(false) }) + .optional() + .catch(undefined), + phone_number_collection: z + .object({ enabled: bool(false) }) + .optional() + .catch(undefined), }); export const UpdateSubscriptionSchema = z.object({ diff --git a/e2e/helpers/services/stripe/stripe-service.ts b/e2e/helpers/services/stripe/stripe-service.ts index a17b786e361..3fa2505a5cd 100644 --- a/e2e/helpers/services/stripe/stripe-service.ts +++ b/e2e/helpers/services/stripe/stripe-service.ts @@ -1,4 +1,8 @@ import baseDebug from '@tryghost/debug'; +import { + type CollectedCheckoutInput, + buildCollectedCheckoutCompletedEvent, +} from './completed-checkout'; import { FakeStripeServer } from './fake-stripe-server'; import { WebhookClient } from './webhook-client'; import { @@ -68,8 +72,15 @@ export class StripeTestService { return this.server.getCheckoutSessions(); } + /** + * Complete the checkout Ghost most recently created, as a member would. + * + * `collected` is what the member filled in on the page — answers to the publisher's + * questions, a delivery address, a tax number. It is checked against the session Ghost + * actually created, so a test cannot answer a question the checkout never asked. + */ async completeLatestSubscriptionCheckout( - opts: { name?: string } = {}, + opts: { name?: string; collected?: CollectedCheckoutInput } = {}, ): Promise { const session = this.getCheckoutSessions().at(-1); @@ -80,6 +91,7 @@ export class StripeTestService { return await this.completeSubscriptionCheckout({ sessionId: session.response.id, name: opts.name, + collected: opts.collected, }); } @@ -249,6 +261,7 @@ export class StripeTestService { private async completeSubscriptionCheckout(opts: { sessionId: string; name?: string; + collected?: CollectedCheckoutInput; }): Promise { const session = this.getCheckoutSessions().find((item) => item.response.id === opts.sessionId); @@ -286,7 +299,7 @@ export class StripeTestService { this.server.upsertSubscription(subscription); this.server.upsertCheckoutSession(session); - await this.sendCheckoutSessionCompletedWebhook(customer.id, session.response.metadata); + await this.sendCollectedCheckoutCompletedWebhook(session, customer.id, opts.collected); await this.sendSubscriptionCreatedWebhook(subscription); return { customer, subscription, price, paymentMethod }; @@ -386,6 +399,24 @@ export class StripeTestService { return customer; } + /** + * The completion for a subscription checkout, built from a real captured session so a + * test cannot pass against a payload shape Stripe has stopped sending. + */ + private async sendCollectedCheckoutCompletedWebhook( + session: RecordedStripeCheckoutSession, + customerId: string, + collected?: CollectedCheckoutInput, + ): Promise { + const event = buildCollectedCheckoutCompletedEvent({ session, customerId, collected }); + const response = await this.webhookClient.sendWebhook(event); + debug('checkout.session.completed webhook response: %d', response.status); + if (!response.ok) { + const body = await response.text(); + throw new Error(`checkout.session.completed webhook failed (${response.status}): ${body}`); + } + } + private async sendCheckoutSessionCompletedWebhook( customerId: string, metadata?: Record, diff --git a/e2e/tests/stripe-fixtures/constraints.test.ts b/e2e/tests/stripe-fixtures/constraints.test.ts index 7140bdc1b7e..f451d716e1b 100644 --- a/e2e/tests/stripe-fixtures/constraints.test.ts +++ b/e2e/tests/stripe-fixtures/constraints.test.ts @@ -97,4 +97,46 @@ test.describe('Fake Stripe - rejects what Stripe rejects', () => { expect(status).toBe(200); }); + + // Every signed-in checkout carries a customer, so a tier collecting a tax number could + // not be bought by an existing member until Ghost sent the pair. Shipping and phone + // collection were probed alongside it and need nothing, so this is a rule about tax + // rather than about collecting from a customer. + test('tax_id_collection for an existing customer needs customer_update[name]', async () => { + const { status, body } = await createSession({ + customer: 'cus_probe', + tax_id_collection: { enabled: true }, + }); + + expect(status).toBe(400); + expect(body.error.message).toContain('please set `customer_update[name]`'); + }); + + test('tax_id_collection for an existing customer is accepted with it', async () => { + const { status } = await createSession({ + customer: 'cus_probe', + tax_id_collection: { enabled: true }, + customer_update: { name: 'auto' }, + }); + + expect(status).toBe(200); + }); + + test('tax_id_collection without a customer needs nothing', async () => { + const { status } = await createSession({ tax_id_collection: { enabled: true } }); + + expect(status).toBe(200); + }); + + // Form encoding delivers the flag as a string, and `"false"` is truthy. Reading it for + // truthiness would refuse a checkout that asks for no tax number at all, which is the + // fake being stricter than Stripe rather than matching it. + test('tax_id_collection switched off needs nothing', async () => { + const { status } = await createSession({ + customer: 'cus_probe', + tax_id_collection: { enabled: 'false' }, + }); + + expect(status).toBe(200); + }); }); From aff61d4b5622a77258a7e3e7f4afb0bc265c3d77 Mon Sep 17 00:00:00 2001 From: Jonatan Svennberg Date: Wed, 26 Aug 2026 14:43:37 +0200 Subject: [PATCH 05/11] Refactor gift purchase form (#30278) ref https://linear.app/ghost/issue/BER-3890 The personalised gift purchase form has grown out of bounds. Splitting it into smaller components and porting to TS in preparation for reworking the state management. --- apps/portal/src/components/frame.styles.js | 2 +- ...gift-page.jsx => beta-gift-page.styles.js} | 705 ------------------ .../src/components/pages/beta-gift-page.tsx | 441 +++++++++++ .../pages/beta-gift/delivery-step.tsx | 167 +++++ .../components/pages/beta-gift/plan-step.tsx | 213 ++++++ .../pages/beta-gift/preview-panel.tsx | 123 +++ .../src/components/pages/beta-gift/types.ts | 27 + apps/portal/src/utils/gift-subscriptions.ts | 24 +- .../components/pages/beta-gift-page.test.tsx | 13 + .../core/server/services/gifts/constants.ts | 2 +- 10 files changed, 1004 insertions(+), 713 deletions(-) rename apps/portal/src/components/pages/{beta-gift-page.jsx => beta-gift-page.styles.js} (55%) create mode 100644 apps/portal/src/components/pages/beta-gift-page.tsx create mode 100644 apps/portal/src/components/pages/beta-gift/delivery-step.tsx create mode 100644 apps/portal/src/components/pages/beta-gift/plan-step.tsx create mode 100644 apps/portal/src/components/pages/beta-gift/preview-panel.tsx create mode 100644 apps/portal/src/components/pages/beta-gift/types.ts diff --git a/apps/portal/src/components/frame.styles.js b/apps/portal/src/components/frame.styles.js index f393edda0c2..584f7530147 100644 --- a/apps/portal/src/components/frame.styles.js +++ b/apps/portal/src/components/frame.styles.js @@ -25,7 +25,7 @@ import { TipsAndDonationsSuccessStyle } from './pages/support-success'; import { GiftRedemptionStyles } from './pages/gift-redemption-page'; import { BetaGiftRedemptionStyles } from './pages/beta-gift-redemption-page'; import { GiftPageStyles } from './pages/gift-page'; -import { BetaGiftPageStyles } from './pages/beta-gift-page'; +import { BetaGiftPageStyles } from './pages/beta-gift-page.styles'; import { GiftSuccessStyle } from './pages/gift-success-page'; import { BetaGiftSuccessStyle } from './pages/beta-gift-success-page'; import { TipsAndDonationsErrorStyle } from './pages/support-error'; diff --git a/apps/portal/src/components/pages/beta-gift-page.jsx b/apps/portal/src/components/pages/beta-gift-page.styles.js similarity index 55% rename from apps/portal/src/components/pages/beta-gift-page.jsx rename to apps/portal/src/components/pages/beta-gift-page.styles.js index 375626f6a68..28696ff6d12 100644 --- a/apps/portal/src/components/pages/beta-gift-page.jsx +++ b/apps/portal/src/components/pages/beta-gift-page.styles.js @@ -1,32 +1,5 @@ -import { useContext, useEffect, useRef, useState } from 'react'; -import AppContext from '../../app-context'; -import CloseButton from '../common/close-button'; -import DatePicker from '../common/date-picker'; -import SiteTitleBackButton from '../common/site-title-back-button'; -import ActionButton from '../common/action-button'; -import GiftCard from '../common/gift-card'; -import GiftEmailPreview from '../common/gift-email-preview'; -import InputField from '../common/input-field'; -import LoadingPage from './loading-page'; -import CheckmarkIcon from '../../images/icons/checkmark.svg?react'; import giftCardNoiseUrl from '../../images/gift-card-noise.webp'; import giftCardOrbUrl from '../../images/gift-card-orb.webp'; -import { isCookiesDisabled } from '../../utils/helpers'; -import { addCalendarDays, getDateInputValue } from '../../utils/date-time'; -import { - getActiveGiftDuration, - getAvailableGiftDurations, - getGiftPrice, - getGiftProducts, -} from '../../utils/gift-subscriptions'; -import { - getGiftDurationAttributiveLabel, - getGiftDurationLabel, -} from '../../utils/gift-redemption-notification'; -import { ValidateInputForm } from '../../utils/form'; -import { t } from '../../utils/i18n'; -import useCardTilt from '../../utils/use-card-tilt'; -import { formatGiftValue } from './gift-page'; export const BetaGiftPageStyles = ` @property --shine-angle { @@ -1331,681 +1304,3 @@ html[dir="rtl"] .gh-portal-content.gift .gh-portal-btn-site-title-back { } } `; - -function GiftDurationSwitch({ offeredDurations, activeDuration, setSelectedDuration }) { - if (offeredDurations.length < 2) { - return null; - } - - return ( -
- {offeredDurations.map((months) => { - const isActive = months === activeDuration; - return ( - - ); - })} -
- ); -} - -const GIFT_EMAIL_MAX_LENGTH = 191; -const GIFT_NAME_MAX_LENGTH = 191; -const GIFT_MESSAGE_MAX_LENGTH = 250; -// Mirrors GIFT_MAX_SCHEDULE_DAYS in ghost/core's gifts constants — change -// them together. -const GIFT_MAX_SCHEDULE_DAYS = 365; - -function getTierPriceLabel(product, months) { - return formatGiftValue(getGiftPrice(product, months)); -} - -const BetaGiftPage = () => { - const { site, member, brandColor, action, doAction, lastPage } = useContext(AppContext); - const [step, setStep] = useState('plan'); - const [selectedDuration, setSelectedDuration] = useState(null); - const [selectedProductId, setSelectedProductId] = useState(null); - const [email, setEmail] = useState(''); - const [recipientEmail, setRecipientEmail] = useState(''); - const [recipientName, setRecipientName] = useState(''); - const [buyerName, setBuyerName] = useState(member?.name || ''); - const [giftMessage, setGiftMessage] = useState(''); - const [deliveryMethod, setDeliveryMethod] = useState('email'); - // null means untouched: the effective date then tracks "today" in the - // site's timezone on every render, so an untouched form still means - // "send now" after the page sits open across site-midnight. - const [deliveryDate, setDeliveryDate] = useState(null); - const [errors, setErrors] = useState({}); - const { cardRef, containerProps: cardTiltProps } = useCardTilt(); - - // Prefill the "from" name once the logged-in member loads, without - // clobbering anything the buyer has already typed - useEffect(() => { - setBuyerName((current) => current || member?.name || ''); - }, [member?.name]); - - // Anchors us to the popup's real (iframe) document for scroll control. - const contentRef = useRef(null); - - // Moving between the plan and delivery steps swaps a full screen of content, - // so reset the popup scroll to the top — otherwise the buyer can land partway - // down the next step. Portal renders inside a react-frame-component iframe, so - // the global `document` here is the parent; reach the popup via the rendered - // node's own document instead. Depending on the embed the actual scroller is - // the wrapper (live portal) or the container, so reset both — and any other - // scrollable ancestor — since only the one that overflows will move. Deferred - // a frame so it runs after the browser's scroll anchoring. - useEffect(() => { - const raf = requestAnimationFrame(() => { - const node = contentRef.current; - const doc = node?.ownerDocument; - if (!doc) { - return; - } - const view = doc.defaultView; - doc.querySelectorAll('.gh-portal-popup-wrapper, .gh-portal-popup-container').forEach((el) => { - el.scrollTop = 0; - }); - // Fallback: walk ancestors and reset whichever one actually scrolls. - for (let el = node.parentElement; el; el = el.parentElement) { - const overflowY = view?.getComputedStyle(el).overflowY; - if ((overflowY === 'auto' || overflowY === 'scroll') && el.scrollHeight > el.clientHeight) { - el.scrollTop = 0; - } - } - }); - return () => cancelAnimationFrame(raf); - }, [step]); - - if (!site) { - return ; - } - - const { portal_default_plan: portalDefaultPlan } = site; - const offeredDurations = getAvailableGiftDurations({ site }); - const activeDuration = getActiveGiftDuration({ - availableDurations: offeredDurations, - portalDefaultPlan, - selectedDuration, - }); - const products = getGiftProducts({ site, duration: activeDuration }); - - const siteIcon = site.icon; - const siteTitle = site.title || ''; - if (products.length === 0) { - return ( - <> -
- -
-
- - -
- - ); - } - - const activeProduct = products.find((p) => p.id === selectedProductId) || products[0]; - const isSingleTier = products.length === 1; - const emailDuration = - activeDuration === 12 - ? { cadence: 'year', duration: 1 } - : { cadence: 'month', duration: activeDuration }; - // This use sits in front of a noun ("6 month membership"), so this is the - // attributive form. The picker and the gift card face use the standalone - // one ("6 months"). - const activeDurationLabel = getGiftDurationAttributiveLabel(emailDuration); - const isPurchasing = action === 'checkoutGift:running'; - const hasErrors = - step === 'plan' - ? !!(errors.email || errors.buyerName) - : !!(errors.recipientEmail || errors.deliveryDate); - const isDisabled = isCookiesDisabled() || isPurchasing || hasErrors; - const isLoggedIn = !!member; - const showBuyerName = !(member?.name || '').trim(); - const showBuyerEmail = !isLoggedIn; - // On the delivery step the email being composed is the more useful thing to - // show than the gift card — it's what the recipient actually opens. The card - // stays for the plan step and for "I'll share it myself", where no email is - // sent and the card is what the buyer passes on. - const showEmailPreview = step === 'delivery' && deliveryMethod === 'email'; - const minDeliveryDate = getDateInputValue(new Date(), site.timezone); - const maxDeliveryDate = addCalendarDays(minDeliveryDate, GIFT_MAX_SCHEDULE_DAYS); - const effectiveDeliveryDate = deliveryDate ?? minDeliveryDate; - - const emailField = { - type: 'email', - value: email, - placeholder: t('jamie@example.com'), - label: t('Your email'), - name: 'email', - required: true, - maxLength: GIFT_EMAIL_MAX_LENGTH, - errorMessage: errors.email || '', - }; - - const recipientEmailField = { - type: 'email', - value: recipientEmail, - placeholder: t('taylor@example.com'), - label: t("Recipient's email"), - name: 'recipientEmail', - required: false, - maxLength: GIFT_EMAIL_MAX_LENGTH, - errorMessage: errors.recipientEmail || '', - }; - - const buyerNameField = { - type: 'text', - value: buyerName, - placeholder: t('Jamie Larson'), - label: t('Your name'), - name: 'buyerName', - required: false, - maxLength: GIFT_NAME_MAX_LENGTH, - errorMessage: errors.buyerName || '', - }; - - const recipientNameField = { - type: 'text', - value: recipientName, - placeholder: t('Taylor Reid'), - label: t("Recipient's name"), - name: 'recipientName', - required: false, - maxLength: GIFT_NAME_MAX_LENGTH, - errorMessage: '', - }; - - const handleEmailChange = (event) => { - setErrors((currentErrors) => ({ - ...currentErrors, - email: '', - })); - setEmail(event.target.value); - }; - - const handleRecipientEmailChange = (event) => { - setErrors((currentErrors) => ({ - ...currentErrors, - recipientEmail: '', - deliveryDate: '', - })); - setRecipientEmail(event.target.value); - }; - - const handleDeliveryMethodChange = (method) => { - setErrors((currentErrors) => ({ - ...currentErrors, - recipientEmail: '', - deliveryDate: '', - })); - setDeliveryMethod(method); - }; - - const handleDeliveryDateChange = (nextDate) => { - setErrors((currentErrors) => ({ - ...currentErrors, - deliveryDate: '', - })); - // Store today as null so "send now" keeps tracking the site day across - // midnight; a typed past date stays put for validation to call out. - setDeliveryDate(nextDate === minDeliveryDate ? null : nextDate); - }; - - const handleContinueToDelivery = (e) => { - e.preventDefault(); - if (!isLoggedIn) { - const formErrors = ValidateInputForm({ fields: [{ ...emailField, value: email.trim() }] }); - const formHasErrors = Object.values(formErrors).some((errorMessage) => !!errorMessage); - - setErrors(formErrors); - - if (formHasErrors) { - return; - } - } - setStep('delivery'); - }; - - const handleBackToPlan = () => { - setErrors({}); - setStep('plan'); - }; - - const handlePurchase = (e) => { - e.preventDefault(); - - if (isPurchasing) { - return; - } - - const customerEmail = email.trim(); - const trimmedRecipientEmail = recipientEmail.trim(); - const trimmedRecipientName = recipientName.trim(); - const trimmedBuyerName = buyerName.trim(); - const trimmedGiftMessage = giftMessage.trim(); - const isEmailDelivery = deliveryMethod === 'email'; - const isScheduled = isEmailDelivery && effectiveDeliveryDate > minDeliveryDate; - - const fieldsToValidate = []; - if (!isLoggedIn) { - fieldsToValidate.push({ ...emailField, value: customerEmail }); - } - if (isEmailDelivery && trimmedRecipientEmail) { - fieldsToValidate.push({ ...recipientEmailField, value: trimmedRecipientEmail }); - } - - const formErrors = ValidateInputForm({ fields: fieldsToValidate }); - - if (isEmailDelivery && !trimmedBuyerName) { - formErrors.buyerName = t('Enter your name'); - } - - // No confirm-email field: the buyer gets a confirmation copy, which - // covers the (unlikely) mistyped-recipient case. - if (isEmailDelivery && !trimmedRecipientEmail) { - formErrors.recipientEmail = t("Enter the recipient's email address"); - } - - if (isEmailDelivery) { - if (!effectiveDeliveryDate) { - formErrors.deliveryDate = t('Choose a delivery date'); - } else if (effectiveDeliveryDate < minDeliveryDate) { - formErrors.deliveryDate = t('Choose a date from today onwards'); - } else if (effectiveDeliveryDate > maxDeliveryDate) { - formErrors.deliveryDate = t('Choose a date within the next year'); - } - } - - const formHasErrors = Object.values(formErrors).some((errorMessage) => !!errorMessage); - - setErrors(formErrors); - - if (formHasErrors) { - if (formErrors.buyerName) { - setStep('plan'); - } - return; - } - - doAction('checkoutGift', { - tierId: activeProduct.id, - duration: activeDuration, - ...(!isLoggedIn ? { email: customerEmail } : {}), - deliveryMethod, - ...(isEmailDelivery ? { recipientEmail: trimmedRecipientEmail } : {}), - ...(isEmailDelivery && trimmedRecipientName ? { recipientName: trimmedRecipientName } : {}), - ...(trimmedBuyerName ? { buyerName: trimmedBuyerName } : {}), - ...(isEmailDelivery && trimmedGiftMessage ? { personalMessage: trimmedGiftMessage } : {}), - ...(isScheduled ? { deliveryDate: effectiveDeliveryDate } : {}), - }); - }; - - return ( - <> -
- -
-
-