diff --git a/.agents/skills/format-number/SKILL.md b/.agents/skills/format-number/SKILL.md index 81c86358f8c..ab94c9db5de 100644 --- a/.agents/skills/format-number/SKILL.md +++ b/.agents/skills/format-number/SKILL.md @@ -7,12 +7,12 @@ autoTrigger: # Format Numbers -When editing `.tsx` files, ensure all user-facing numbers are formatted using the `formatNumber` utility from `@tryghost/shade`. +When editing `.tsx` files, ensure all user-facing numbers are formatted using the `formatNumber` utility from `@tryghost/shade/utils`. ## Import ```typescript -import {formatNumber} from '@tryghost/shade'; +import { formatNumber } from '@tryghost/shade/utils'; ``` ## When to use formatNumber @@ -58,4 +58,4 @@ Do NOT use any of these patterns for formatting numbers in TSX files: - `abbreviateNumber()` - for compact notation (e.g., 1.2M, 50k) - `centsToDollars()` - convert cents to dollars before passing to `formatNumber` -All are imported from `@tryghost/shade`. +All are imported from `@tryghost/shade/utils`. diff --git a/apps/admin/src/analytics/analytics.acceptance.test.tsx b/apps/admin/src/analytics/analytics.acceptance.test.tsx index 7e3ae2d8738..6be77376850 100644 --- a/apps/admin/src/analytics/analytics.acceptance.test.tsx +++ b/apps/admin/src/analytics/analytics.acceptance.test.tsx @@ -3,6 +3,7 @@ import { describe, expect, it } from 'vitest'; import { TINYBIRD_SITE_UUID, currentRoute, + fakeAdminEndpoint, fakeAdminStats, fakeAnalyticsOverview, fakeNewsletters, @@ -88,6 +89,40 @@ function seedTopPostsViews() { ]); } +/** + * The stats views' world with web analytics on but nothing recorded yet: a + * site with no members, no MRR, no posts and no visits — the empty states. + */ +function seedEmptyAnalyticsWorld() { + fakeAdminStats.memberCount(); + fakeAdminStats.mrr(); + fakeAdminStats.subscriptions(); + fakeAdminStats.topPostViews(); + fakePosts([]); + fakeTinybirdToken(); + fakeTinybirdPipe('api_active_visitors', []); + fakeTinybirdPipe('api_kpis', []); +} + +/** The web traffic view's own cards, all empty. */ +function seedEmptyWebTraffic() { + fakeAdminStats.topContent([]); + fakeTinybirdPipe('api_top_sources', []); + fakeTinybirdPipe('api_top_locations', []); +} + +/** + * A member-count history of zero rows, the shape the server serves for a + * site with no members — a fully empty history would collapse the growth + * view into its no-stats state instead of rendering the empty cards. + */ +function seedZeroMemberHistory() { + fakeAdminStats.memberCount({ + stats: [{ date: daysAgo(1), free: 0, paid: 0, comped: 0 }], + totals: { free: 0, paid: 0, comped: 0, gift: 0 }, + }); +} + describe('Analytics overview', () => { it('renders zero KPIs when growth history is empty', async () => { fakeAnalyticsOverview(); @@ -138,6 +173,66 @@ describe('Analytics overview', () => { await expect.element(analyticsScreen.dateRangeSelect()).toHaveTextContent('Last 7 days'); await expect.poll(() => kpisApi.lastRequest?.params.get('date_from')).not.toBe(initialDateFrom); }); + + it('renders the latest and top posts with zeroed stats when nothing is recorded', async () => { + seedEmptyAnalyticsWorld(); + fakePosts([ + post({ + id: LATEST_POST_ID, + title: 'Attack of the Clones', + status: 'published', + published_at: `${daysAgo(3)}T10:00:00.000Z`, + url: 'https://example.com/attack-of-the-clones/', + }), + ]); + fakeAdminStats.post(LATEST_POST_ID); + // The server lists recently published posts in top posts even before + // they record any views; the row must render its zeros as zeros. + fakeAdminStats.topPostViews([ + { + post_id: LATEST_POST_ID, + title: 'Attack of the Clones', + published_at: `${daysAgo(3)}T10:00:00.000Z`, + }, + ]); + await renderAdminApp('/analytics', { boot: webAnalyticsBootOverrides() }); + + await expect.element(analyticsScreen.latestPost()).toHaveTextContent('Attack of the Clones'); + await expect.element(analyticsScreen.latestPostVisitors()).toHaveTextContent('0'); + await expect.element(analyticsScreen.latestPostMembers()).toHaveTextContent('0'); + + await expect.element(analyticsScreen.topPostsCard()).toHaveTextContent('Attack of the Clones'); + const visitorsStatistics = analyticsScreen.topPostsVisitorsStatistics(); + await expect.element(visitorsStatistics).toHaveTextContent('Unique visitors'); + await expect.element(visitorsStatistics).toHaveTextContent('0'); + const membersStatistics = analyticsScreen.topPostsMembersStatistics(); + await expect.element(membersStatistics).toHaveTextContent('New members'); + await expect.element(membersStatistics).toHaveTextContent('Free'); + await expect.element(membersStatistics).toHaveTextContent('0'); + }); + + it('navigates to the web traffic view from the visitors KPI', async () => { + seedEmptyAnalyticsWorld(); + seedEmptyWebTraffic(); + await renderAdminApp('/analytics', { boot: webAnalyticsBootOverrides() }); + + await analyticsScreen.uniqueVisitorsViewMoreButton().click(); + + await expect.poll(currentRoute).toMatch(/^\/analytics\/web\/?$/); + await expect.element(analyticsScreen.uniqueVisitorsTab()).toBeVisible(); + }); + + it('navigates to the growth view from the members KPI', async () => { + seedEmptyAnalyticsWorld(); + seedZeroMemberHistory(); + fakeAdminStats.topPosts(); + await renderAdminApp('/analytics', { boot: webAnalyticsBootOverrides() }); + + await analyticsScreen.membersViewMoreButton().click(); + + await expect.poll(currentRoute).toMatch(/^\/analytics\/growth\//); + await expect.element(analyticsScreen.totalMembersCard()).toBeVisible(); + }); }); describe('Analytics web traffic', () => { @@ -184,6 +279,30 @@ describe('Analytics web traffic', () => { await expect.element(analyticsScreen.sourceRow('google.com')).toHaveTextContent('170'); await expect.element(analyticsScreen.locationRow('US')).toHaveTextContent('United States'); }); + + it('renders zeroed KPIs and empty cards when there are no visits', async () => { + seedEmptyAnalyticsWorld(); + seedEmptyWebTraffic(); + await renderAdminApp('/analytics/web', { boot: webAnalyticsBootOverrides() }); + + await expect.element(analyticsScreen.uniqueVisitorsTab()).toHaveTextContent('0'); + await expect.element(analyticsScreen.totalViewsTab()).toHaveTextContent('0'); + await expect.element(analyticsScreen.topContentCard()).toHaveTextContent('No visitors'); + await expect.element(analyticsScreen.topSourcesCard()).toHaveTextContent('No visitors'); + await expect.element(analyticsScreen.locationsCard()).toHaveTextContent('No visitors'); + }); + + it('keeps the empty state across the top content tabs', async () => { + seedEmptyAnalyticsWorld(); + seedEmptyWebTraffic(); + await renderAdminApp('/analytics/web', { boot: webAnalyticsBootOverrides() }); + + await analyticsScreen.topContentTab('Posts').click(); + await expect.element(analyticsScreen.topContentCard()).toHaveTextContent('No visitors'); + + await analyticsScreen.topContentTab('Pages').click(); + await expect.element(analyticsScreen.topContentCard()).toHaveTextContent('No visitors'); + }); }); describe('Analytics growth', () => { @@ -212,6 +331,38 @@ describe('Analytics growth', () => { .toHaveTextContent('Attack of the Clones'); await expect.element(analyticsScreen.topContentCard()).toHaveTextContent('+30'); }); + + it('shows No conversions across the top content tabs when nothing converted', async () => { + seedEmptyAnalyticsWorld(); + seedZeroMemberHistory(); + fakeAdminStats.topPosts(); + fakeAdminEndpoint('GET', /^\/stats\/top-sources-growth/, { stats: [], meta: {} }); + await renderAdminApp('/analytics/growth', { boot: webAnalyticsBootOverrides() }); + + const contentCard = analyticsScreen.topContentCard(); + await expect + .element(contentCard) + .toHaveTextContent('Which posts or pages drove the most growth in the last 30 days'); + await expect.element(contentCard).toHaveTextContent('No conversions'); + + await analyticsScreen.topContentTab('Posts').click(); + await expect + .element(contentCard) + .toHaveTextContent('Which posts drove the most growth in the last 30 days'); + await expect.element(contentCard).toHaveTextContent('No conversions'); + + await analyticsScreen.topContentTab('Pages').click(); + await expect + .element(contentCard) + .toHaveTextContent('Which pages drove the most growth in the last 30 days'); + await expect.element(contentCard).toHaveTextContent('No conversions'); + + await analyticsScreen.topContentTab('Sources').click(); + await expect + .element(contentCard) + .toHaveTextContent('Which sources drove the most growth in the last 30 days'); + await expect.element(contentCard).toHaveTextContent('No conversions'); + }); }); describe('Analytics newsletters', () => { @@ -255,4 +406,28 @@ describe('Analytics newsletters', () => { .element(analyticsScreen.topNewslettersCard()) .toHaveTextContent('Weekly Digest Issue #1'); }); + + it('shows the empty state on every newsletter card when none were sent', async () => { + seedEmptyAnalyticsWorld(); + fakeNewsletters([newsletter({ name: 'Weekly Digest', status: 'active', sort_order: 0 })]); + fakeAdminStats.newsletterSubscribers(); + fakeAdminStats.newsletterBasic(); + fakeAdminStats.newsletterClicks(); + await renderAdminApp('/analytics/newsletters', { boot: webAnalyticsBootOverrides() }); + + await expect.element(analyticsScreen.newslettersCard()).toBeVisible(); + await expect + .element(analyticsScreen.topNewslettersCard()) + .toHaveTextContent('newsletters in the last 30 days'); + + await analyticsScreen.newslettersCardTab('Avg. open rate').click(); + await expect + .element(analyticsScreen.newslettersCard()) + .toHaveTextContent('No newsletters in the last 30 days'); + + await analyticsScreen.newslettersCardTab('Avg. click rate').click(); + await expect + .element(analyticsScreen.newslettersCard()) + .toHaveTextContent('No newsletters in the last 30 days'); + }); }); diff --git a/apps/admin/src/analytics/analytics.screen.ts b/apps/admin/src/analytics/analytics.screen.ts index 4105c804370..32e7734ea16 100644 --- a/apps/admin/src/analytics/analytics.screen.ts +++ b/apps/admin/src/analytics/analytics.screen.ts @@ -9,13 +9,28 @@ export const analyticsScreen = { page.getByTestId(sel.uniqueVisitors).getByTestId(sel.kpiCardHeaderValue), membersCard: () => page.getByTestId(sel.members), membersValue: () => page.getByTestId(sel.members).getByTestId(sel.kpiCardHeaderValue), + membersViewMoreButton: () => + page.getByTestId(sel.members).getByRole('button', { name: 'View more' }), mrrValue: () => page.getByTestId(sel.mrr).getByTestId(sel.kpiCardHeaderValue), + uniqueVisitorsViewMoreButton: () => + page.getByTestId(sel.uniqueVisitors).getByRole('button', { name: 'View more' }), latestPost: () => page.getByTestId(sel.latestPost), + latestPostVisitors: () => page.getByTestId(sel.latestPost).getByTestId(sel.latestPostVisitors), + latestPostMembers: () => page.getByTestId(sel.latestPost).getByTestId(sel.latestPostMembers), topPostsCard: () => page.getByTestId(sel.topPostsCard), + topPostsVisitorsStatistics: () => + page.getByTestId(sel.topPostsCard).getByTestId(sel.statisticsVisitors), + topPostsMembersStatistics: () => + page.getByTestId(sel.topPostsCard).getByTestId(sel.statisticsMembers), // Web webGraph: () => page.getByTestId(sel.webGraph), + uniqueVisitorsTab: () => + page.getByTestId(sel.webGraph).getByRole('tab', { name: 'Unique visitors' }), + totalViewsTab: () => page.getByTestId(sel.webGraph).getByRole('tab', { name: 'Total views' }), topContentCard: () => page.getByTestId(sel.topContentCard), + topContentTab: (name: string) => + page.getByTestId(sel.topContentCard).getByRole('tab', { name, exact: true }), topSourcesCard: () => page.getByTestId(sel.topSourcesCard), sourceRow: (source: string) => page.getByTestId(`${sel.sourceRowPrefix}${source}`), locationsCard: () => page.getByTestId(sel.visitorsCard), @@ -26,6 +41,8 @@ export const analyticsScreen = { // Newsletters newslettersCard: () => page.getByTestId(sel.newslettersCard), + newslettersCardTab: (name: string) => + page.getByTestId(sel.newslettersCard).getByRole('tab', { name }), totalSubscribersValue: () => page.getByTestId(sel.totalSubscribersValue), topNewslettersCard: () => page.getByTestId(sel.topNewslettersCard), diff --git a/apps/admin/src/layout/sidebar.screen.ts b/apps/admin/src/layout/sidebar.screen.ts index 3dfec8f9b89..9abf21e5922 100644 --- a/apps/admin/src/layout/sidebar.screen.ts +++ b/apps/admin/src/layout/sidebar.screen.ts @@ -26,7 +26,9 @@ export const sidebarScreen = { shellNav: () => page.getByTestId(adminSidebar), /** The shell's content area, rendered by AdminLayout alongside the sidebar. */ shellMain: () => page.getByRole('main').first(), - navLink: (name: string) => page.getByRole('navigation').getByRole('link', { name, exact: true }), + // Scoped to the shell sidebar: in-content navigation (e.g. the tag + // detail breadcrumb) repeats the same link names. + navLink: (name: string) => sidebarScreen.shellNav().getByRole('link', { name, exact: true }), postsToggle: () => page.getByRole('button', { name: postsToggle }), networkBadge: () => page.getByTestId(networkNotificationBadge), userMenuTrigger: () => page.getByRole('button', { name: userMenuTrigger }), diff --git a/apps/admin/src/posts/analytics/post-analytics.acceptance.test.tsx b/apps/admin/src/posts/analytics/post-analytics.acceptance.test.tsx index e3fbcba54ff..5853b681935 100644 --- a/apps/admin/src/posts/analytics/post-analytics.acceptance.test.tsx +++ b/apps/admin/src/posts/analytics/post-analytics.acceptance.test.tsx @@ -5,13 +5,16 @@ import { currentRoute, fakeAdminStats, fakeAdminEndpoint, + fakeMembers, fakePosts, fakeTinybirdPipe, fakeTinybirdToken, post, renderAdminApp, + settingsResponse, webAnalyticsBootOverrides, } from '@test-utils/acceptance'; +import { membersScreen } from '@/members/members.screen'; import { postAnalyticsScreen } from './post-analytics.screen'; const POST_ID = '64d623b64676110001e897d9'; @@ -94,6 +97,35 @@ function seedPostAnalyticsWorld() { }; } +/** + * The world for a freshly published post with no activity at all: no visits, + * no attributed members, no link clicks — the empty states every tab shows. + * The post never went out as an email, so no newsletter endpoints fire. + */ +function seedEmptyPostAnalyticsWorld() { + fakePosts([ + post({ + id: POST_ID, + uuid: POST_UUID, + title: 'Attack of the Clones', + slug: 'attack-of-the-clones', + status: 'published', + visibility: 'public', + published_at: `${daysAgo(1)}T10:00:00.000Z`, + url: 'https://example.com/attack-of-the-clones/', + }), + ]); + fakeAdminStats.postReferrers(POST_ID); + fakeAdminStats.postGrowth(POST_ID); + fakeAdminStats.mrr(); + fakeAdminEndpoint('GET', /^\/links\//, { links: [], meta: {} }); + fakeTinybirdToken(); + fakeTinybirdPipe('api_active_visitors', []); + fakeTinybirdPipe('api_kpis', []); + fakeTinybirdPipe('api_top_sources', []); + fakeTinybirdPipe('api_top_locations', []); +} + describe('Post analytics overview', () => { it('renders the seeded post with web and growth sections', async () => { const { postsApi } = seedPostAnalyticsWorld(); @@ -128,6 +160,57 @@ describe('Post analytics overview', () => { await expect.poll(() => kpisApi.requests.length).toBeGreaterThan(overviewKpiRequestCount); await expect.poll(() => kpisApi.lastRequest?.params.get('post_uuid')).toBe(POST_UUID); }); + + it('renders every tab and zeroed sections for a post with no activity', async () => { + seedEmptyPostAnalyticsWorld(); + await renderAdminApp(`/posts/analytics/${POST_ID}`, { boot: webAnalyticsBootOverrides() }); + + await expect.element(postAnalyticsScreen.overviewTab()).toBeVisible(); + await expect.element(postAnalyticsScreen.webTrafficTab()).toBeVisible(); + await expect.element(postAnalyticsScreen.growthTab()).toBeVisible(); + + await expect.element(postAnalyticsScreen.growthCard()).toHaveTextContent('Free members'); + await expect.element(postAnalyticsScreen.growthCard()).toHaveTextContent('0'); + }); + + it('reaches the empty web traffic view through web performance view more', async () => { + seedEmptyPostAnalyticsWorld(); + await renderAdminApp(`/posts/analytics/${POST_ID}`, { boot: webAnalyticsBootOverrides() }); + + await postAnalyticsScreen.webPerformanceViewMoreButton().click(); + + await expect.poll(currentRoute).toBe(`/posts/analytics/${POST_ID}/web`); + // No visits at all: the web view renders its whole-view empty state. + await expect.element(page.getByText('No visitors in the last 30 days').first()).toBeVisible(); + }); + + it('reaches the empty growth view through growth view more', async () => { + seedEmptyPostAnalyticsWorld(); + await renderAdminApp(`/posts/analytics/${POST_ID}`, { boot: webAnalyticsBootOverrides() }); + + await postAnalyticsScreen.growthViewMoreButton().click(); + + await expect.poll(currentRoute).toBe(`/posts/analytics/${POST_ID}/growth`); + await expect + .element(postAnalyticsScreen.topSourcesCard()) + .toHaveTextContent('No sources data available'); + }); + + it('hides the growth tab and section when member source tracking is off', async () => { + seedEmptyPostAnalyticsWorld(); + const boot = webAnalyticsBootOverrides(); + boot.browseSettings = { + response: settingsResponse({ + settings: { web_analytics_enabled: true, members_track_sources: false }, + }), + }; + await renderAdminApp(`/posts/analytics/${POST_ID}`, { boot }); + + await expect.element(postAnalyticsScreen.overviewTab()).toBeVisible(); + await expect.element(postAnalyticsScreen.webTrafficTab()).toBeVisible(); + await expect.element(postAnalyticsScreen.growthTab()).not.toBeInTheDocument(); + await expect.element(postAnalyticsScreen.growthCard()).not.toBeInTheDocument(); + }); }); describe('Post analytics web', () => { @@ -183,6 +266,40 @@ describe('Post analytics growth', () => { await expect.element(page.getByText('Top sources')).toBeVisible(); await expect.element(page.getByText('Google')).toBeVisible(); }); + + it('renders the zeroed members card and empty sources when nothing converted', async () => { + seedEmptyPostAnalyticsWorld(); + await renderAdminApp(`/posts/analytics/${POST_ID}/growth`, { + boot: webAnalyticsBootOverrides(), + }); + + await expect.element(postAnalyticsScreen.membersCard()).toHaveTextContent('Free members'); + await expect.element(postAnalyticsScreen.membersCard()).toHaveTextContent('0'); + await expect + .element(postAnalyticsScreen.topSourcesCard()) + .toHaveTextContent('No sources data available'); + }); + + it('links the members KPI to the members list filtered to this post', async () => { + seedEmptyPostAnalyticsWorld(); + const membersApi = fakeMembers([]); + // The filter bar resolves attribution ids to post/page titles. + fakeAdminEndpoint('GET', /^\/pages\//, { + pages: [], + meta: { pagination: { page: 1, limit: 25, pages: 1, total: 0, next: null, prev: null } }, + }); + await renderAdminApp(`/posts/analytics/${POST_ID}/growth`, { + boot: webAnalyticsBootOverrides(), + }); + + await expect.element(postAnalyticsScreen.membersCard()).toHaveTextContent('Free members'); + await postAnalyticsScreen.freeMembersViewMembersButton().click(); + + // The members screen re-serializes the handed-over filter clauses. + await expect.poll(currentRoute).toMatch(/^\/members\?/); + await expect(membersApi).toHaveSentFilter(`conversion:-'${POST_ID}'+signup:'${POST_ID}'`); + await expect.element(membersScreen.noResults()).toBeVisible(); + }); }); describe('Post analytics newsletter', () => { diff --git a/apps/admin/src/posts/analytics/post-analytics.screen.ts b/apps/admin/src/posts/analytics/post-analytics.screen.ts index b76451824bc..82583f607b0 100644 --- a/apps/admin/src/posts/analytics/post-analytics.screen.ts +++ b/apps/admin/src/posts/analytics/post-analytics.screen.ts @@ -13,8 +13,12 @@ export const postAnalyticsScreen = { // Overview webPerformanceCard: () => page.getByTestId(sel.webPerformance), + webPerformanceViewMoreButton: () => + page.getByTestId(sel.webPerformance).getByRole('button', { name: 'View more' }), uniqueVisitors: () => page.getByTestId(sel.uniqueVisitors), growthCard: () => page.getByTestId(sel.growth), + growthViewMoreButton: () => + page.getByTestId(sel.growth).getByRole('button', { name: 'View more' }), // Web — the post view's row testids carry the lowercased country code and // the source with non-alphanumerics dashed ("google.com" → "google-com"). @@ -26,6 +30,12 @@ export const postAnalyticsScreen = { page.getByTestId(`${sel.sourceRowPrefix}${source.toLowerCase().replace(/[^a-z0-9]/g, '-')}`), filterContainer: () => page.getByTestId(sel.statsFilterContainer), - // Growth + // Growth — every KPI in the members card carries its own "View members" + // button; the free-members KPI renders first. membersCard: () => page.getByTestId(sel.membersCard), + freeMembersViewMembersButton: () => + page + .getByTestId(sel.membersCard) + .getByRole('button', { name: /View members/ }) + .first(), }; diff --git a/apps/admin/src/settings/advanced/integrations.acceptance.test.tsx b/apps/admin/src/settings/advanced/integrations.acceptance.test.tsx index b59f0e7f0cb..717dc4f03d2 100644 --- a/apps/admin/src/settings/advanced/integrations.acceptance.test.tsx +++ b/apps/admin/src/settings/advanced/integrations.acceptance.test.tsx @@ -51,7 +51,6 @@ const customIntegrationsLimit = { function limitedConfig(upgradeUrl?: string) { const response = configResponse(); - response.config.labs = { ...response.config.labs, transistor: true }; response.config.hostSettings = upgradeUrl ? { limits: customIntegrationsLimit, billing: { upgradeUrl } } : { limits: customIntegrationsLimit }; @@ -283,12 +282,17 @@ describe('Advanced integrations', () => { await modal.getByRole('switch').click(); await modal.getByRole('button', { name: 'Save' }).click(); await expect(settingsApi).toHaveEditedSettings([{ key: 'pintura', value: true }]); + + await modal.getByRole('switch').click(); + await modal.getByRole('button', { name: 'Save' }).click(); + await expect(settingsApi).toHaveEditedSettings([{ key: 'pintura', value: false }]); + expect(settingsApi.requests).toHaveLength(2); }); it('shows the Active badge after enabling the Transistor integration', async () => { fakeSettingsScreens(); const settingsApi = fakeEditSettings(); - await renderAdminApp('/settings/integrations', { labs: { transistor: true } }); + await renderAdminApp('/settings/integrations'); const item = settingsScreen.section('integrations').getByTestId('transistor-integration'); const badge = item.getByText('Active', { exact: true }); @@ -297,6 +301,7 @@ describe('Advanced integrations', () => { await item.hover(); await item.getByRole('button', { name: 'Configure' }).click(); const modal = settingsScreen.section('transistor-modal'); + await expect.element(modal.getByRole('switch')).toBeVisible(); await modal.getByRole('switch').click(); await modal.getByRole('button', { name: 'Save' }).click(); await expect(settingsApi).toHaveEditedSettings([{ key: 'transistor', value: true }]); @@ -307,6 +312,38 @@ describe('Advanced integrations', () => { await expect.element(badge).toBeVisible(); }); + it('clears the Active badge after disabling the Transistor integration', async () => { + fakeSettingsScreens(); + const settingsApi = fakeEditSettings(); + await renderAdminApp('/settings/integrations'); + + const item = settingsScreen.section('integrations').getByTestId('transistor-integration'); + const badge = item.getByText('Active', { exact: true }); + await item.hover(); + await item.getByRole('button', { name: 'Configure' }).click(); + const modal = settingsScreen.section('transistor-modal'); + await modal.getByRole('switch').click(); + await modal.getByRole('button', { name: 'Save' }).click(); + await expect(settingsApi).toHaveEditedSettings([{ key: 'transistor', value: true }]); + await expect(badge).toHaveCount(1); + + await modal.getByRole('switch').click(); + await modal.getByRole('button', { name: 'Save' }).click(); + await expect(settingsApi).toHaveEditedSettings([{ key: 'transistor', value: false }]); + await expect(badge).toHaveCount(0); + expect(settingsApi.requests).toHaveLength(2); + }); + + it('shows the Transistor integration in the built-in list', async () => { + // The transistor labs flag was removed (#27082); the card is not gated. + fakeSettingsScreens(); + await renderAdminApp('/settings/integrations'); + + const section = settingsScreen.section('integrations'); + await expect.element(section.getByTestId('unsplash-integration')).toBeVisible(); + await expect.element(section.getByTestId('transistor-integration')).toBeVisible(); + }); + it('shows the upgrade CTA on host-limited integration cards', async () => { fakeSettingsScreens(); await renderAdminApp('/settings/integrations', { @@ -502,7 +539,6 @@ describe('Advanced integrations', () => { it('moves host-limited integrations to the bottom without disturbing relative order', async () => { fakeSettingsScreens(); await renderAdminApp('/settings/integrations', { - labs: { transistor: true }, boot: { browseConfig: { response: limitedConfig() } }, }); diff --git a/apps/admin/src/tags/detail/tag-detail.acceptance.test.tsx b/apps/admin/src/tags/detail/tag-detail.acceptance.test.tsx index 291b0d8341e..9e600ceeebb 100644 --- a/apps/admin/src/tags/detail/tag-detail.acceptance.test.tsx +++ b/apps/admin/src/tags/detail/tag-detail.acceptance.test.tsx @@ -619,6 +619,38 @@ describe('Tag detail (tagDetailsReact on)', () => { .toBeVisible(); }); + it('shows the not-found page when the tag does not exist', async () => { + fakeAdminEndpoint( + 'GET', + new RegExp('^/tags/slug/unknown/'), + { errors: [{ type: 'NotFoundError', message: 'Tag not found.' }] }, + { status: 404 }, + ); + await renderAdminApp('/tags/unknown', FLAGS); + + await expect.element(tagDetailScreen.notFound()).toBeVisible(); + }); + + it('keeps the Tags nav item active on the new tag route', async () => { + fakeTags([]); + await renderAdminApp('/tags/new', FLAGS); + + await expect.element(tagDetailScreen.title()).toHaveTextContent('New tag'); + await expect.element(sidebarScreen.navLink('Tags')).toHaveAttribute('aria-current', 'page'); + }); + + it('keeps the Tags nav item active while editing a tag from the list', async () => { + const t = tag({ name: 'News', slug: 'news' }); + fakeTags([t]); + fakeTagWorld(t); + await renderAdminApp('/tags', FLAGS); + + await tagsScreen.tagRows().getByRole('link', { name: 'News' }).click(); + + await expect.poll(currentRoute).toBe('/tags/news'); + await expect.element(sidebarScreen.navLink('Tags')).toHaveAttribute('aria-current', 'page'); + }); + it('guards leaving with unsaved edits via the breadcrumb', async () => { const t = tag({ name: 'News', slug: 'news' }); fakeTags([t]); diff --git a/apps/admin/src/tags/detail/tag-detail.screen.ts b/apps/admin/src/tags/detail/tag-detail.screen.ts index adb04d8b5d4..64e16c6160b 100644 --- a/apps/admin/src/tags/detail/tag-detail.screen.ts +++ b/apps/admin/src/tags/detail/tag-detail.screen.ts @@ -46,6 +46,8 @@ export const tagDetailScreen = { footerEditor: () => page.getByRole('textbox', { name: new RegExp(`^${sel.tagFooterEditorLabel}`) }), + notFound: () => page.getByText('Page not found', { exact: true }), + actionsButton: () => page.getByRole('button', { name: sel.tagActionsButton }), viewPostsMenuItem: () => page.getByRole('menuitem', { name: sel.viewPostsMenuItem }), deleteTagMenuItem: () => page.getByRole('menuitem', { name: sel.deleteTagMenuItem, exact: true }), diff --git a/apps/portal/src/components/pages/beta-gift-redemption-page.jsx b/apps/portal/src/components/pages/beta-gift-redemption-page.jsx index 59f36c59f36..c9c7111e1e2 100644 --- a/apps/portal/src/components/pages/beta-gift-redemption-page.jsx +++ b/apps/portal/src/components/pages/beta-gift-redemption-page.jsx @@ -60,7 +60,7 @@ const BetaGiftRedemptionPage = () => { const gift = pageData?.gift; const isLoggedIn = !!member; const [name, setName] = useState(gift?.recipient_name || member?.name || ''); - const [email, setEmail] = useState(member?.email || ''); + const [email, setEmail] = useState(member?.email || gift?.recipient_email || ''); const [errors, setErrors] = useState({}); const [showDetails, setShowDetails] = useState(false); const { cardRef, containerProps: cardTiltProps } = useCardTilt(); @@ -69,9 +69,9 @@ const BetaGiftRedemptionPage = () => { // Prefill with the recipient name the buyer entered, so the gift card // is personal before the recipient types anything. setName(gift?.recipient_name || member?.name || ''); - setEmail(member?.email || ''); + setEmail(member?.email || gift?.recipient_email || ''); setErrors({}); - }, [member?.email, member?.name, gift?.recipient_name]); + }, [member?.email, member?.name, gift?.recipient_email, gift?.recipient_name]); useEffect(() => { if (gift) { diff --git a/apps/portal/test/unit/components/pages/gift-redemption-page.test.jsx b/apps/portal/test/unit/components/pages/gift-redemption-page.test.jsx index 96091f2bb9a..fc03032e6c8 100644 --- a/apps/portal/test/unit/components/pages/gift-redemption-page.test.jsx +++ b/apps/portal/test/unit/components/pages/gift-redemption-page.test.jsx @@ -141,17 +141,17 @@ describe('BetaGiftRedemptionPage', () => { expect(queryByText(member.free.name)).not.toBeInTheDocument(); }); - test('presents the buyer details and prefills the intended recipient name', () => { + test('presents the buyer details and prefills the intended recipient details', async () => { const personalizedGift = { ...gift, buyer_name: 'Jamie', recipient_name: 'Taylor', + recipient_email: 'taylor@example.com', message: 'Enjoy this!', expires_at: '2030-01-01T00:00:00.000Z', }; - const { container, getByLabelText, getByTestId, getByText } = renderGiftRedemptionPage( - BetaGiftRedemptionPage, - { + const { container, getByLabelText, getByRole, getByTestId, getByText, mockDoActionFn } = + renderGiftRedemptionPage(BetaGiftRedemptionPage, { site: { ...testSite, url: 'https://example.com/', @@ -162,10 +162,10 @@ describe('BetaGiftRedemptionPage', () => { token: 'gift-token-123', gift: personalizedGift, }, - }, - ); + }); expect(getByLabelText(/your name/i)).toHaveValue('Taylor'); + expect(getByLabelText(/your email/i)).toHaveValue('taylor@example.com'); expect(getByLabelText(/your email/i)).toHaveFocus(); const subtitle = container.querySelector('.gh-portal-gift-checkout-subtitle'); expect(subtitle).toHaveTextContent( @@ -175,6 +175,16 @@ describe('BetaGiftRedemptionPage', () => { expect(getByTestId('gift-message')).toHaveTextContent('Enjoy this!'); expect(getByTestId('gift-message')).toHaveTextContent('Jamie'); expect(getByText(/This gift can only be redeemed once and expires on/i)).toBeInTheDocument(); + + fireEvent.click(getByRole('button', { name: 'Redeem your gift' })); + + await waitFor(() => { + expect(mockDoActionFn).toHaveBeenCalledWith('redeemGift', { + email: 'taylor@example.com', + name: 'Taylor', + giftToken: 'gift-token-123', + }); + }); }); test('presents the claim deadline in the publication locale and timezone', () => { diff --git a/compose.dev.yaml b/compose.dev.yaml index 9749999ab95..af8d62ee90a 100644 --- a/compose.dev.yaml +++ b/compose.dev.yaml @@ -144,6 +144,8 @@ services: context: ./docker/dev-gateway dockerfile: Dockerfile container_name: ghost-dev-gateway + environment: + ANALYTICS_PROXY_TARGET: ${ANALYTICS_PROXY_TARGET:-analytics:3000} ports: - '2368:80' - '80:80' diff --git a/docs/contributing/development-setup.md b/docs/contributing/development-setup.md index e5173f34c3c..2380cbfbb6f 100644 --- a/docs/contributing/development-setup.md +++ b/docs/contributing/development-setup.md @@ -105,15 +105,16 @@ runs. Run one root command at a time. Each variant includes the standard development environment and adds the listed tooling: -| Command | Use it when working on | -| -------------------- | --------------------------------------------------------------------------------------- | -| `pnpm dev` | Ghost Core, Admin, or Portal | -| `pnpm dev:public` | Comments UI, Signup Form, Search, Announcement Bar, or Admin Toolbar | -| `pnpm dev:lexical` | Koenig's Lexical editor inside Ghost Admin | -| `pnpm dev:analytics` | Tinybird-backed analytics; also exposes Tinybird on port `7181` | -| `pnpm dev:storage` | S3-compatible storage through MinIO on ports `9000` and `9001` | -| `pnpm dev:stripe` | Stripe webhooks; requires `STRIPE_SECRET_KEY` in the environment or a local `.env` file | -| `pnpm dev:full` | Public app watchers plus analytics, storage, and Stripe | +| Command | Use it when working on | +| -------------------------- | --------------------------------------------------------------------------------------------- | +| `pnpm dev` | Ghost Core, Admin, or Portal | +| `pnpm dev:public` | Comments UI, Signup Form, Search, Announcement Bar, or Admin Toolbar | +| `pnpm dev:lexical` | Koenig's Lexical editor inside Ghost Admin | +| `pnpm dev:analytics` | Tinybird-backed analytics with the latest published version of the Traffic Analytics service | +| `pnpm dev:analytics:local` | Tinybird-backed analytics with your locally running instance of the Traffic Analytics service | +| `pnpm dev:storage` | S3-compatible storage through MinIO on ports `9000` and `9001` | +| `pnpm dev:stripe` | Stripe webhooks; requires `STRIPE_SECRET_KEY` in the environment or a local `.env` file | +| `pnpm dev:full` | Public app watchers plus analytics, storage, and Stripe | Copy [`.env.example`](../../.env.example) to `.env` only when you need an optional integration. Never commit credentials or the local `.env` file. diff --git a/e2e/helpers/pages/admin/analytics/post-analytics/index.ts b/e2e/helpers/pages/admin/analytics/post-analytics/index.ts index 3265723594c..75e11cee8e7 100644 --- a/e2e/helpers/pages/admin/analytics/post-analytics/index.ts +++ b/e2e/helpers/pages/admin/analytics/post-analytics/index.ts @@ -1,4 +1,3 @@ export * from './post-analytics-page'; -export * from './post-analytics-growth-page'; export * from './post-analytics-overview-page'; export * from './post-analytics-web-traffic-page'; diff --git a/e2e/helpers/pages/admin/analytics/post-analytics/post-analytics-growth-page.ts b/e2e/helpers/pages/admin/analytics/post-analytics/post-analytics-growth-page.ts deleted file mode 100644 index a1e4cb6b425..00000000000 --- a/e2e/helpers/pages/admin/analytics/post-analytics/post-analytics-growth-page.ts +++ /dev/null @@ -1,18 +0,0 @@ -import * as postAnalyticsSel from '@tryghost/test-data/selectors/post-analytics'; -import { AdminPage } from '@/admin-pages'; -import { Locator, Page } from '@playwright/test'; - -export class PostAnalyticsGrowthPage extends AdminPage { - readonly membersCard: Locator; - readonly viewMemberButton: Locator; - readonly topSourcesCard: Locator; - - constructor(page: Page) { - super(page); - - this.membersCard = this.page.getByTestId(postAnalyticsSel.membersCard); - this.viewMemberButton = this.membersCard.getByRole('button', { name: 'View member' }); - - this.topSourcesCard = this.page.getByTestId(postAnalyticsSel.topSourcesCard); - } -} diff --git a/e2e/helpers/pages/admin/members/member-details-page.ts b/e2e/helpers/pages/admin/members/member-details-page.ts index cd506d739fa..7cf368f231d 100644 --- a/e2e/helpers/pages/admin/members/member-details-page.ts +++ b/e2e/helpers/pages/admin/members/member-details-page.ts @@ -1,6 +1,17 @@ import { AdminPage } from '@/admin-pages'; import { BasePage } from '@/helpers/pages'; import { Locator, Page } from '@playwright/test'; +import { + cancelDeleteMember, + confirmDeleteMember, + memberActions, + memberCustomFieldEditModal, + memberCustomFieldsField, + memberDetailEngagement, + memberDetailTitle, + memberSigninUrl, + memberSubscriptionToggle, +} from '@tryghost/test-data/selectors/members'; /** * Page object for the member detail screen. @@ -26,7 +37,7 @@ class SettingsSection extends BasePage { constructor(page: Page) { super(page); - this.memberActionsButton = page.getByTestId('member-actions').filter({ visible: true }); + this.memberActionsButton = page.getByTestId(memberActions).filter({ visible: true }); this.impersonateButton = menuAction(page, 'Impersonate'); this.signOutOfAllDevices = menuAction(page, 'Sign out of all devices'); @@ -34,8 +45,8 @@ class SettingsSection extends BasePage { this.enableCommentingButton = menuAction(page, 'Enable commenting'); this.deleteButton = menuAction(page, 'Delete member'); - this.confirmDeleteButton = page.getByTestId('confirm-delete-member').filter({ visible: true }); - this.cancelDeleteButton = page.getByTestId('cancel-delete-member').filter({ visible: true }); + this.confirmDeleteButton = page.getByTestId(confirmDeleteMember).filter({ visible: true }); + this.cancelDeleteButton = page.getByTestId(cancelDeleteMember).filter({ visible: true }); } } @@ -89,7 +100,7 @@ export class MemberDetailsPage extends AdminPage { this.labelsInput = page.getByText('Labels').locator('+ div'); this.labels = this.labelsInput.getByRole('listitem'); this.newsletterSubscriptionToggles = page - .getByTestId('member-subscription-toggle') + .getByTestId(memberSubscriptionToggle) .filter({ visible: true }); this.saveButton = page.getByRole('button', { name: 'Save' }); @@ -98,7 +109,7 @@ export class MemberDetailsPage extends AdminPage { .locator('[data-test-link="members-back"]') .filter({ visible: true }); this.copyLinkButton = page.getByRole('button', { name: 'Copy link' }); - this.magicLinkInput = page.getByTestId('member-signin-url').filter({ visible: true }); + this.magicLinkInput = page.getByTestId(memberSigninUrl).filter({ visible: true }); this.confirmLeaveButton = page.getByRole('button', { name: 'Leave' }); this.settingsSection = new SettingsSection(page); @@ -116,7 +127,7 @@ export class MemberDetailsPage extends AdminPage { }); this.commentingDisabledIndicator = page.getByText('Comments disabled'); - this.screenTitle = page.getByTestId('member-detail-title'); + this.screenTitle = page.getByTestId(memberDetailTitle); this.logoutConfirmModal = page.getByRole('alertdialog', { name: 'Sign out member from all devices?', }); @@ -125,7 +136,7 @@ export class MemberDetailsPage extends AdminPage { this.newsletterSubscriptionCheckboxes = this.newsletterSubscriptionToggles.and( page.getByRole('switch'), ); - this.engagementSection = page.getByTestId('member-detail-engagement').filter({ visible: true }); + this.engagementSection = page.getByTestId(memberDetailEngagement).filter({ visible: true }); this.subscriptionActionsButton = page.getByRole('button', { name: 'Subscription menu' }); this.cancelSubscriptionButton = menuAction(page, 'Cancel subscription'); @@ -133,8 +144,8 @@ export class MemberDetailsPage extends AdminPage { this.removeComplimentaryButton = menuAction(page, 'Remove complimentary subscription'); this.compTierOptions = page.getByRole('option'); - this.customFieldsCard = page.getByTestId('member-custom-fields-field'); - this.customFieldModal = page.getByTestId('member-custom-field-edit-modal'); + this.customFieldsCard = page.getByTestId(memberCustomFieldsField); + this.customFieldModal = page.getByTestId(memberCustomFieldEditModal); } // The row's accessible name is "Edit {field}" (plus ": {value}" once set), so diff --git a/e2e/helpers/pages/admin/settings/sections/access-section.ts b/e2e/helpers/pages/admin/settings/sections/access-section.ts index 5c594d783e8..6b2415a6655 100644 --- a/e2e/helpers/pages/admin/settings/sections/access-section.ts +++ b/e2e/helpers/pages/admin/settings/sections/access-section.ts @@ -1,5 +1,10 @@ import { BasePage } from '@/helpers/pages'; import { Locator, Page } from '@playwright/test'; +import { + access, + siteAccessCode, + siteVisibilitySelect, +} from '@tryghost/test-data/selectors/settings'; export class AccessSection extends BasePage { readonly section: Locator; @@ -10,10 +15,10 @@ export class AccessSection extends BasePage { constructor(page: Page) { super(page, '/ghost/#/settings'); - this.section = page.getByTestId('access'); + this.section = page.getByTestId(access); this.saveButton = this.section.getByRole('button', { name: 'Save' }); - this.visibilitySelect = this.section.getByTestId('site-visibility-select'); - this.passwordInput = this.section.getByTestId('site-access-code'); + this.visibilitySelect = this.section.getByTestId(siteVisibilitySelect); + this.passwordInput = this.section.getByTestId(siteAccessCode); } async enablePrivateMode(password: string): Promise { diff --git a/e2e/helpers/pages/admin/settings/sections/announcement-bar-section.ts b/e2e/helpers/pages/admin/settings/sections/announcement-bar-section.ts index 172299d452d..bdfdb58e8c9 100644 --- a/e2e/helpers/pages/admin/settings/sections/announcement-bar-section.ts +++ b/e2e/helpers/pages/admin/settings/sections/announcement-bar-section.ts @@ -1,5 +1,6 @@ import { BasePage } from '@/helpers/pages'; import { FrameLocator, Locator, Page } from '@playwright/test'; +import { announcementBar, announcementBarModal } from '@tryghost/test-data/selectors/settings'; export class AnnouncementBarSection extends BasePage { readonly section: Locator; @@ -15,9 +16,9 @@ export class AnnouncementBarSection extends BasePage { constructor(page: Page) { super(page, '/ghost/#/settings'); - this.section = page.getByTestId('announcement-bar'); + this.section = page.getByTestId(announcementBar); this.customizeButton = this.section.getByRole('button', { name: 'Customize' }); - this.modal = page.getByTestId('announcement-bar-modal'); + this.modal = page.getByTestId(announcementBarModal); this.freeMembersCheckbox = this.modal.getByLabel('Free members'); this.editor = this.modal.locator('.koenig-react-editor'); this.contentEditable = this.modal.locator('[contenteditable="true"]'); diff --git a/e2e/helpers/pages/admin/settings/sections/custom-fields-section.ts b/e2e/helpers/pages/admin/settings/sections/custom-fields-section.ts index 664da2754f4..2f1c276ba67 100644 --- a/e2e/helpers/pages/admin/settings/sections/custom-fields-section.ts +++ b/e2e/helpers/pages/admin/settings/sections/custom-fields-section.ts @@ -1,5 +1,10 @@ import { BasePage } from '@/helpers/pages'; import { Locator, Page } from '@playwright/test'; +import { + customFieldListItem, + customFieldModal, + customFields, +} from '@tryghost/test-data/selectors/settings'; /** * Settings -> Membership -> Custom fields. The whole section is behind the @@ -14,13 +19,13 @@ export class CustomFieldsSection extends BasePage { constructor(page: Page) { super(page, '/ghost/#/settings'); - this.section = page.getByTestId('custom-fields'); + this.section = page.getByTestId(customFields); this.addButton = this.section.getByRole('button', { name: 'Add custom field' }); - this.modal = page.getByTestId('custom-field-modal'); + this.modal = page.getByTestId(customFieldModal); } listItem(name: string): Locator { - return this.section.getByTestId('custom-field-list-item').filter({ hasText: name }); + return this.section.getByTestId(customFieldListItem).filter({ hasText: name }); } /** diff --git a/e2e/helpers/pages/admin/settings/sections/danger-zone-section.ts b/e2e/helpers/pages/admin/settings/sections/danger-zone-section.ts index 83519e91cf3..3ca684727b6 100644 --- a/e2e/helpers/pages/admin/settings/sections/danger-zone-section.ts +++ b/e2e/helpers/pages/admin/settings/sections/danger-zone-section.ts @@ -1,5 +1,6 @@ import { BasePage } from '@/helpers/pages'; import { Locator, Page } from '@playwright/test'; +import { confirmationModal, dangerZone } from '@tryghost/test-data/selectors/settings'; export class DangerZoneSection extends BasePage { readonly section: Locator; @@ -15,13 +16,13 @@ export class DangerZoneSection extends BasePage { constructor(page: Page) { super(page, '/ghost/#/settings/advanced'); - this.section = page.getByTestId('dangerzone'); + this.section = page.getByTestId(dangerZone); this.heading = page.getByRole('heading', { level: 5, name: 'Danger zone' }); this.deleteAllContentButton = this.section.getByRole('button', { name: 'Delete all content' }); this.resetAuthButton = this.section.getByRole('button', { name: 'Reset all authentication' }); - this.confirmationModal = page.getByTestId('confirmation-modal'); + this.confirmationModal = page.getByTestId(confirmationModal); this.deleteAllContentOkButton = this.confirmationModal.getByRole('button', { name: 'Delete', exact: true, diff --git a/e2e/helpers/pages/admin/settings/sections/design-section.ts b/e2e/helpers/pages/admin/settings/sections/design-section.ts index 69a6adb9b75..26fcc00dc74 100644 --- a/e2e/helpers/pages/admin/settings/sections/design-section.ts +++ b/e2e/helpers/pages/admin/settings/sections/design-section.ts @@ -1,5 +1,13 @@ import { BasePage } from '@/helpers/pages'; import { Locator, Page } from '@playwright/test'; +import { + design, + designModal, + imageDeleteButton, + imageUploadContainer, + publicationCover, + toggleUnsplashButton, +} from '@tryghost/test-data/selectors/settings'; export class DesignSection extends BasePage { readonly section: Locator; @@ -13,13 +21,13 @@ export class DesignSection extends BasePage { constructor(page: Page) { super(page, '/ghost/#/settings'); - this.section = page.getByTestId('design'); + this.section = page.getByTestId(design); this.customizeButton = this.section.getByRole('button', { name: 'Customize' }); - this.designModal = page.getByTestId('design-modal'); - this.unsplashButton = page.getByTestId('toggle-unsplash-button'); + this.designModal = page.getByTestId(designModal); + this.unsplashButton = page.getByTestId(toggleUnsplashButton); this.unsplashHeading = page.getByRole('heading', { name: 'Unsplash', level: 1 }); this.unsplashPhotos = page.locator('[data-kg-unsplash-gallery-img]'); - this.coverImage = page.getByTestId('publication-cover'); + this.coverImage = page.getByTestId(publicationCover); } async openDesignModal(): Promise { @@ -28,9 +36,9 @@ export class DesignSection extends BasePage { } async deleteCoverImage(): Promise { - const imageContainer = this.coverImage.getByTestId('image-upload-container'); + const imageContainer = this.coverImage.getByTestId(imageUploadContainer); await imageContainer.hover(); - await imageContainer.getByTestId('image-delete-button').click(); + await imageContainer.getByTestId(imageDeleteButton).click(); } async openUnsplashSelector(): Promise { diff --git a/e2e/helpers/pages/admin/settings/sections/integration-modal.ts b/e2e/helpers/pages/admin/settings/sections/integration-modal.ts index b6f6ab25013..1135cdb5c20 100644 --- a/e2e/helpers/pages/admin/settings/sections/integration-modal.ts +++ b/e2e/helpers/pages/admin/settings/sections/integration-modal.ts @@ -1,5 +1,6 @@ import { BasePage } from '@/helpers/pages'; import { Locator, Page } from '@playwright/test'; +import { integrations } from '@tryghost/test-data/selectors/settings'; export interface IntegrationConfig { name: string; @@ -53,7 +54,7 @@ export class IntegrationModal extends BasePage { this.config = typeof integration === 'string' ? INTEGRATIONS[integration] : integration; - this.integrationsSection = page.getByTestId('integrations'); + this.integrationsSection = page.getByTestId(integrations); this.integrationItem = page.getByTestId(this.config.testId); this.modal = page.getByTestId(this.config.modalTestId); this.enableToggle = this.modal.getByLabel(this.config.toggleLabel); diff --git a/e2e/helpers/pages/admin/settings/sections/integrations-section.ts b/e2e/helpers/pages/admin/settings/sections/integrations-section.ts index b25ddbb0dca..4687ec844bb 100644 --- a/e2e/helpers/pages/admin/settings/sections/integrations-section.ts +++ b/e2e/helpers/pages/admin/settings/sections/integrations-section.ts @@ -1,5 +1,6 @@ import { BasePage } from '@/helpers/pages'; import { Locator, Page } from '@playwright/test'; +import { integrations } from '@tryghost/test-data/selectors/settings'; export class IntegrationsSection extends BasePage { readonly integrationsSection: Locator; @@ -10,7 +11,7 @@ export class IntegrationsSection extends BasePage { constructor(page: Page) { super(page, 'ghost/#/settings/integrations'); - this.integrationsSection = page.getByTestId('integrations'); + this.integrationsSection = page.getByTestId(integrations); this.integrationsHeading = page.getByRole('heading', { level: 5, name: 'Integrations' }); this.integrationsDescription = page.getByText('Make Ghost work with apps and tools'); this.integrationsAddButton = page.getByRole('button', { name: 'Add custom integration' }); diff --git a/e2e/helpers/pages/admin/settings/sections/member-welcome-emails-section.ts b/e2e/helpers/pages/admin/settings/sections/member-welcome-emails-section.ts index 50a7dac320f..6140d11517f 100644 --- a/e2e/helpers/pages/admin/settings/sections/member-welcome-emails-section.ts +++ b/e2e/helpers/pages/admin/settings/sections/member-welcome-emails-section.ts @@ -1,5 +1,17 @@ import { BasePage } from '@/helpers/pages'; import { FrameLocator, Locator, Page } from '@playwright/test'; +import { + freeWelcomeEmailRow, + headerImageField, + memberEmails, + welcomeEmailCustomizeModal, + welcomeEmailEditor, + welcomeEmailModal, + welcomeEmailModeEdit, + welcomeEmailModePreview, + welcomeEmailPreviewIframe, + welcomeEmailPreviewSubject, +} from '@tryghost/test-data/selectors/settings'; export class MemberWelcomeEmailsSection extends BasePage { readonly section: Locator; @@ -47,17 +59,15 @@ export class MemberWelcomeEmailsSection extends BasePage { constructor(page: Page) { super(page, '/ghost/#/settings/memberemails'); - this.section = page.getByTestId('memberemails'); - this.freeWelcomeEmailToggle = this.section - .getByTestId('free-welcome-email-row') - .getByRole('switch'); + this.section = page.getByTestId(memberEmails); + this.freeWelcomeEmailToggle = this.section.getByTestId(freeWelcomeEmailRow).getByRole('switch'); this.freeWelcomeEmailEditButton = this.section - .getByTestId('free-welcome-email-row') + .getByTestId(freeWelcomeEmailRow) .getByRole('button', { name: 'Edit' }); // Customize button and modal this.customizeButton = this.section.getByRole('button', { name: 'Customize' }); - this.customizeModal = page.getByTestId('welcome-email-customize-modal'); + this.customizeModal = page.getByTestId(welcomeEmailCustomizeModal); this.customizeModalSaveButton = this.customizeModal.getByRole('button', { name: 'Save' }); this.customizeModalCloseButton = this.customizeModal.getByRole('button', { name: 'Close' }); this.customizeModalUnsavedChangesDialog = page.getByRole('alertdialog', { @@ -72,7 +82,7 @@ export class MemberWelcomeEmailsSection extends BasePage { .locator('..') .getByRole('switch'); this.customizeModalFooterTextarea = this.customizeModal.getByLabel('Email footer'); - this.customizeModalHeaderImageUpload = this.customizeModal.getByTestId('header-image-field'); + this.customizeModalHeaderImageUpload = this.customizeModal.getByTestId(headerImageField); this.customizeModalBadgeToggle = this.customizeModal .getByText('Promote independent publishing') .locator('../..') @@ -105,18 +115,16 @@ export class MemberWelcomeEmailsSection extends BasePage { this.customizeModalColorPickerPopover = page.locator('[data-radix-popper-content-wrapper]'); // Modal locators - this.welcomeEmailModal = page.getByTestId('welcome-email-modal'); - this.modalEditor = this.welcomeEmailModal.getByTestId('welcome-email-editor'); - this.modalEditTab = this.welcomeEmailModal.getByTestId('welcome-email-mode-edit'); - this.modalPreviewTab = this.welcomeEmailModal.getByTestId('welcome-email-mode-preview'); - this.modalPreviewSubjectInput = this.welcomeEmailModal.getByTestId( - 'welcome-email-preview-subject', - ); + this.welcomeEmailModal = page.getByTestId(welcomeEmailModal); + this.modalEditor = this.welcomeEmailModal.getByTestId(welcomeEmailEditor); + this.modalEditTab = this.welcomeEmailModal.getByTestId(welcomeEmailModeEdit); + this.modalPreviewTab = this.welcomeEmailModal.getByTestId(welcomeEmailModePreview); + this.modalPreviewSubjectInput = this.welcomeEmailModal.getByTestId(welcomeEmailPreviewSubject); this.modalSubjectInput = this.modalPreviewSubjectInput; this.modalSaveButton = this.welcomeEmailModal.getByRole('button', { name: 'Save' }); this.modalSavedButton = this.welcomeEmailModal.getByRole('button', { name: 'Saved' }); this.modalLexicalEditor = this.modalEditor.getByRole('textbox').first(); - this.modalPreviewIframe = this.welcomeEmailModal.getByTestId('welcome-email-preview-iframe'); + this.modalPreviewIframe = this.welcomeEmailModal.getByTestId(welcomeEmailPreviewIframe); this.modalPreviewFrame = page.frameLocator('iframe[title="Welcome email preview"]'); } @@ -140,9 +148,7 @@ export class MemberWelcomeEmailsSection extends BasePage { } private async waitForFreeToggle(checked: boolean): Promise { - const toggle = this.section - .getByTestId('free-welcome-email-row') - .getByRole('switch', { checked }); + const toggle = this.section.getByTestId(freeWelcomeEmailRow).getByRole('switch', { checked }); await toggle.waitFor({ state: 'visible' }); } diff --git a/e2e/helpers/pages/admin/settings/sections/portal-section.ts b/e2e/helpers/pages/admin/settings/sections/portal-section.ts index 94be9490573..be9334182bc 100644 --- a/e2e/helpers/pages/admin/settings/sections/portal-section.ts +++ b/e2e/helpers/pages/admin/settings/sections/portal-section.ts @@ -1,5 +1,6 @@ import { BasePage } from '@/helpers/pages'; import { Locator, Page } from '@playwright/test'; +import { portal, portalModal } from '@tryghost/test-data/selectors/settings'; type PaidSignupCadence = 'monthly' | 'yearly'; @@ -14,9 +15,9 @@ export class PortalSection extends BasePage { constructor(page: Page) { super(page, '/ghost/#/settings'); - this.section = page.getByTestId('portal'); + this.section = page.getByTestId(portal); this.customizeButton = this.section.getByRole('button', { name: 'Customize' }); - this.portalModal = page.getByTestId('portal-modal'); + this.portalModal = page.getByTestId(portalModal); this.linksTab = this.portalModal.getByRole('tab', { name: 'Links' }); this.linksTierSelectControl = this.portalModal.getByRole('combobox', { name: 'Tier' }); this.freeTierToggleLabel = this.portalModal diff --git a/e2e/helpers/pages/admin/settings/sections/staff-section.ts b/e2e/helpers/pages/admin/settings/sections/staff-section.ts index d4e3d00191d..5dab59d8ef6 100644 --- a/e2e/helpers/pages/admin/settings/sections/staff-section.ts +++ b/e2e/helpers/pages/admin/settings/sections/staff-section.ts @@ -1,5 +1,6 @@ import { BasePage } from '@/helpers/pages'; import { Locator, Page } from '@playwright/test'; +import { ownerUser, users } from '@tryghost/test-data/selectors/settings'; export class StaffSection extends BasePage { readonly requireTwoFaButton: Locator; @@ -8,8 +9,8 @@ export class StaffSection extends BasePage { constructor(page: Page) { super(page, '/ghost/#/settings/staff'); - this.ownerUser = this.page.getByTestId('owner-user'); - this.requireTwoFaButton = page.getByTestId('users').getByRole('switch'); + this.ownerUser = this.page.getByTestId(ownerUser); + this.requireTwoFaButton = page.getByTestId(users).getByRole('switch'); this.inviteStaffButton = page.getByRole('button', { name: 'Invite people' }); } @@ -60,7 +61,7 @@ export class StaffSection extends BasePage { } private async waitForSwitch(checked: boolean): Promise { - const switchState = this.page.getByTestId('users').getByRole('switch', { checked: checked }); + const switchState = this.page.getByTestId(users).getByRole('switch', { checked: checked }); await switchState.waitFor({ state: 'visible' }); } } diff --git a/e2e/helpers/pages/admin/settings/sections/tiers-section.ts b/e2e/helpers/pages/admin/settings/sections/tiers-section.ts index 583fbee250f..285dcb2e486 100644 --- a/e2e/helpers/pages/admin/settings/sections/tiers-section.ts +++ b/e2e/helpers/pages/admin/settings/sections/tiers-section.ts @@ -1,5 +1,12 @@ import { BasePage } from '@/helpers/pages'; import { Locator, Page } from '@playwright/test'; +import { + confirmationModal, + portal, + portalModal, + tierDetailModal, + tiers, +} from '@tryghost/test-data/selectors/settings'; export interface TierFormData { name: string; @@ -19,10 +26,10 @@ export class TiersSection extends BasePage { constructor(page: Page) { super(page, '/ghost/#/settings'); - this.section = page.getByTestId('tiers'); + this.section = page.getByTestId(tiers); this.addTierButton = this.section.getByRole('button', { name: 'Add tier' }); - this.tierDetailModal = page.getByTestId('tier-detail-modal'); - this.confirmationModal = page.getByTestId('confirmation-modal'); + this.tierDetailModal = page.getByTestId(tierDetailModal); + this.confirmationModal = page.getByTestId(confirmationModal); this.activeTab = this.section.getByRole('tab', { name: 'Active' }); this.archivedTab = this.section.getByRole('tab', { name: 'Archived' }); } @@ -89,27 +96,27 @@ export class TiersSection extends BasePage { } async enableTierInPortal(tierName: string): Promise { - const portalSection = this.page.getByTestId('portal'); + const portalSection = this.page.getByTestId(portal); await portalSection.getByRole('button', { name: 'Customize' }).click(); - const portalModal = this.page.getByTestId('portal-modal'); - await portalModal.waitFor({ state: 'visible' }); + const modal = this.page.getByTestId(portalModal); + await modal.waitFor({ state: 'visible' }); - const tierCheckbox = portalModal.getByLabel(tierName).first(); + const tierCheckbox = modal.getByLabel(tierName).first(); if (!(await tierCheckbox.isChecked())) { await tierCheckbox.check(); } - const monthlyCheckbox = portalModal.getByLabel('Monthly').first(); + const monthlyCheckbox = modal.getByLabel('Monthly').first(); if (!(await monthlyCheckbox.isChecked())) { await monthlyCheckbox.check(); } - const yearlyCheckbox = portalModal.getByLabel('Yearly').first(); + const yearlyCheckbox = modal.getByLabel('Yearly').first(); if (!(await yearlyCheckbox.isChecked())) { await yearlyCheckbox.check(); } - await portalModal.getByRole('button', { name: 'Save' }).click(); - await portalModal.getByRole('button', { name: 'Close' }).click(); + await modal.getByRole('button', { name: 'Save' }).click(); + await modal.getByRole('button', { name: 'Close' }).click(); } } diff --git a/e2e/helpers/pages/admin/settings/settings-page.ts b/e2e/helpers/pages/admin/settings/settings-page.ts index 8e8eee799ea..9d2de4a8fca 100644 --- a/e2e/helpers/pages/admin/settings/settings-page.ts +++ b/e2e/helpers/pages/admin/settings/settings-page.ts @@ -8,6 +8,7 @@ import { } from './sections'; import { Locator, Page } from '@playwright/test'; import { StaffSection } from './sections/staff-section'; +import { settingsSidebar } from '@tryghost/test-data/selectors/settings'; export class SettingsPage extends BasePage { readonly integrationsSection: IntegrationsSection; @@ -22,7 +23,7 @@ export class SettingsPage extends BasePage { constructor(page: Page) { super(page, '/ghost/#/settings'); - this.sidebar = page.getByTestId('sidebar'); + this.sidebar = page.getByTestId(settingsSidebar); this.portalSection = new PortalSection(page); this.integrationsSection = new IntegrationsSection(page); diff --git a/e2e/helpers/pages/admin/settings/staff-details-page.ts b/e2e/helpers/pages/admin/settings/staff-details-page.ts index c647c86d7a6..49ae54ae3a3 100644 --- a/e2e/helpers/pages/admin/settings/staff-details-page.ts +++ b/e2e/helpers/pages/admin/settings/staff-details-page.ts @@ -1,5 +1,12 @@ import { BasePage } from '@/helpers/pages'; import { Locator, Page } from '@playwright/test'; +import { + coverImagePreview, + coverImageUpload, + profileImagePreview, + profileImageUpload, + userDetailModal, +} from '@tryghost/test-data/selectors/settings'; type FilePayload = { name: string; @@ -20,13 +27,13 @@ export class AdminStaffDetailsPage extends BasePage { constructor(page: Page) { super(page, '/ghost/#/settings/staff'); - this.userDetailModal = page.getByTestId('user-detail-modal'); + this.userDetailModal = page.getByTestId(userDetailModal); this.emailInput = this.userDetailModal.getByRole('textbox', { name: /Email/i }); this.slugInput = this.userDetailModal.getByRole('textbox', { name: 'Slug' }); - this.profileImageInput = this.userDetailModal.getByTestId('profile-image-upload'); - this.coverImageInput = this.userDetailModal.getByTestId('cover-image-upload'); - this.profileImagePreview = this.userDetailModal.getByTestId('profile-image-preview'); - this.coverImagePreview = this.userDetailModal.getByTestId('cover-image-preview'); + this.profileImageInput = this.userDetailModal.getByTestId(profileImageUpload); + this.coverImageInput = this.userDetailModal.getByTestId(coverImageUpload); + this.profileImagePreview = this.userDetailModal.getByTestId(profileImagePreview); + this.coverImagePreview = this.userDetailModal.getByTestId(coverImagePreview); this.savedButton = this.userDetailModal.getByRole('button', { name: 'Saved' }); } diff --git a/e2e/tests/admin/analytics/growth.test.ts b/e2e/tests/admin/analytics/growth.test.ts index 2adbb14d640..ffb82e257d0 100644 --- a/e2e/tests/admin/analytics/growth.test.ts +++ b/e2e/tests/admin/analytics/growth.test.ts @@ -1,45 +1,17 @@ import { AnalyticsGrowthPage } from '@/admin-pages'; import { expect, test } from '@/helpers/playwright'; +// The growth family's server journey: a fresh site must return a zero-filled +// member count history (members-stats zero-fill contract) so the growth view +// renders instead of collapsing. The empty-state UI variations live in the +// admin acceptance tier, which fakes this contract and depends on it holding. test.describe('Ghost Admin - Growth', () => { - let growthPage: AnalyticsGrowthPage; - - test.beforeEach(async ({ page }) => { - growthPage = new AnalyticsGrowthPage(page); + test('renders the growth view with zeroed KPIs on a fresh site', async ({ page }) => { + const growthPage = new AnalyticsGrowthPage(page); await growthPage.goto(); - }); - - test('empty top content card - posts and pages', async () => { - await expect(growthPage.topContent.contentCard).toContainText( - 'Which posts or pages drove the most growth in the last 30 days', - ); - await expect(growthPage.topContent.contentCard).toContainText('No conversions'); - }); - - test('empty top content card - posts', async () => { - await growthPage.topContent.postsButton.click(); - - await expect(growthPage.topContent.contentCard).toContainText( - 'Which posts drove the most growth in the last 30 days', - ); - await expect(growthPage.topContent.contentCard).toContainText('No conversions'); - }); - - test('empty top content card - pages', async () => { - await growthPage.topContent.pagesButton.click(); - - await expect(growthPage.topContent.contentCard).toContainText( - 'Which pages drove the most growth in the last 30 days', - ); - await expect(growthPage.topContent.contentCard).toContainText('No conversions'); - }); - - test('empty top content card - sources', async () => { - await growthPage.topContent.sourcesButton.click(); - await expect(growthPage.topContent.contentCard).toContainText( - 'Which sources drove the most growth in the last 30 days', - ); + await expect(growthPage.totalMembersCard).toBeVisible(); + await expect(growthPage.totalMembersCard).toContainText('0'); await expect(growthPage.topContent.contentCard).toContainText('No conversions'); }); }); diff --git a/e2e/tests/admin/analytics/newsletters.test.ts b/e2e/tests/admin/analytics/newsletters.test.ts index e11bb05cf51..14803222299 100644 --- a/e2e/tests/admin/analytics/newsletters.test.ts +++ b/e2e/tests/admin/analytics/newsletters.test.ts @@ -16,28 +16,6 @@ test.describe('Ghost Admin - Newsletters', () => { await newslettersPage.goto(); }); - test('empty newsletters card', async () => { - await expect(newslettersPage.newslettersCard).toBeVisible(); - }); - - test('empty average open rate and click rate card', async () => { - await newslettersPage.averageOpenRateTab.click(); - await expect(newslettersPage.newslettersCard).toContainText( - 'No newsletters in the last 30 days', - ); - - await newslettersPage.averageClickRateTab.click(); - await expect(newslettersPage.newslettersCard).toContainText( - 'No newsletters in the last 30 days', - ); - }); - - test('empty top newsletters card', async () => { - await expect(newslettersPage.topNewslettersCard).toContainText( - 'newsletters in the last 30 days', - ); - }); - test('total subscribers percent change calculation', async ({ page }) => { const membersService = new MembersImportService(page.request); diff --git a/e2e/tests/admin/analytics/overview.test.ts b/e2e/tests/admin/analytics/overview.test.ts index 2e5f9fc52d9..7317280558b 100644 --- a/e2e/tests/admin/analytics/overview.test.ts +++ b/e2e/tests/admin/analytics/overview.test.ts @@ -1,4 +1,4 @@ -import { AnalyticsGrowthPage, AnalyticsOverviewPage, AnalyticsWebTrafficPage } from '@/admin-pages'; +import { AnalyticsOverviewPage } from '@/admin-pages'; import { HomePage } from '@/public-pages'; import { createPostFactory } from '@/data-factory'; import { expect, test, withIsolatedPage } from '@/helpers/playwright'; @@ -24,52 +24,4 @@ test.describe('Ghost Admin - Analytics Overview', () => { expect(await analyticsOverviewPage.uniqueVisitors.count()).toBe(1); }); - - test('latest post', async ({ page }) => { - const analyticsOverviewPage = new AnalyticsOverviewPage(page); - await analyticsOverviewPage.goto(); - - const membersCount = await analyticsOverviewPage.latestPost.membersCount(); - const visitorsCount = await analyticsOverviewPage.latestPost.visitorsCount(); - - await expect(analyticsOverviewPage.latestPost.post).toBeVisible(); - expect(visitorsCount).toContain('0'); - expect(membersCount).toContain('0'); - }); - - test('top posts', async ({ page }) => { - const analyticsOverviewPage = new AnalyticsOverviewPage(page); - await analyticsOverviewPage.goto(); - - await expect(analyticsOverviewPage.topPosts.post).toBeVisible(); - - const visitorsStatistics = await analyticsOverviewPage.topPosts.uniqueVisitorsStatistics(); - const membersStatistics = await analyticsOverviewPage.topPosts.membersStatistics(); - - expect(visitorsStatistics).toContain('Unique visitors'); - expect(visitorsStatistics).toContain('0'); - expect(membersStatistics).toContain('New members'); - expect(membersStatistics).toContain('Free'); - expect(membersStatistics).toContain('0'); - }); - - test('view more unique visitors details', async ({ page }) => { - const analyticsOverviewPage = new AnalyticsOverviewPage(page); - await analyticsOverviewPage.goto(); - - await analyticsOverviewPage.viewMoreUniqueVisitorDetails(); - - const analyticsWebTrafficPage = new AnalyticsWebTrafficPage(page); - await expect(analyticsWebTrafficPage.totalUniqueVisitorsTab).toBeVisible(); - }); - - test('view more members details', async ({ page }) => { - const analyticsOverviewPage = new AnalyticsOverviewPage(page); - await analyticsOverviewPage.goto(); - - await analyticsOverviewPage.viewMoreMembersDetails(); - - const analyticsGrowthPage = new AnalyticsGrowthPage(page); - await expect(analyticsGrowthPage.totalMembersCard).toBeVisible(); - }); }); diff --git a/e2e/tests/admin/analytics/post-analytics/growth.test.ts b/e2e/tests/admin/analytics/post-analytics/growth.test.ts deleted file mode 100644 index 354f5a2cdde..00000000000 --- a/e2e/tests/admin/analytics/post-analytics/growth.test.ts +++ /dev/null @@ -1,49 +0,0 @@ -import { - MembersPage, - PostAnalyticsGrowthPage, - PostAnalyticsOverviewPage, - PostAnalyticsPage, -} from '@/admin-pages'; -import { createPostFactory } from '@/data-factory'; -import { expect, test } from '@/helpers/playwright'; - -test.describe('Ghost Admin - Post Analytics - Growth', () => { - test.beforeEach(async ({ page }) => { - const postFactory = createPostFactory(page.request); - const post = await postFactory.create({ - title: 'Post analytics growth test', - status: 'published', - }); - - const postAnalyticsOverviewPage = new PostAnalyticsOverviewPage(page); - await postAnalyticsOverviewPage.gotoForPost(post.id); - - // TODO: check post analytics component, we shouldn't need to wait on page load to be able to click growth link - const postAnalyticsPage = new PostAnalyticsPage(page); - await postAnalyticsPage.waitForPageLoad(); - await postAnalyticsPage.growthButton.click(); - }); - - test('empty members card', async ({ page }) => { - const postAnalyticsPageGrowthPage = new PostAnalyticsGrowthPage(page); - - await expect(postAnalyticsPageGrowthPage.membersCard).toContainText('Free members'); - await expect(postAnalyticsPageGrowthPage.membersCard).toContainText('0'); - }); - - test('empty members card - view member', async ({ page }) => { - const postAnalyticsPageGrowthPage = new PostAnalyticsGrowthPage(page); - await postAnalyticsPageGrowthPage.viewMemberButton.click(); - - const membersPage = new MembersPage(page); - await expect(membersPage.body).toContainText('No matching members found.'); - }); - - test('empty top sources card', async ({ page }) => { - const postAnalyticsPageGrowthPage = new PostAnalyticsGrowthPage(page); - - await expect(postAnalyticsPageGrowthPage.topSourcesCard).toContainText( - 'No sources data available', - ); - }); -}); diff --git a/e2e/tests/admin/analytics/post-analytics/overview.test.ts b/e2e/tests/admin/analytics/post-analytics/overview.test.ts index 9d87d0929d9..d9a14af081a 100644 --- a/e2e/tests/admin/analytics/post-analytics/overview.test.ts +++ b/e2e/tests/admin/analytics/post-analytics/overview.test.ts @@ -1,76 +1,38 @@ -import { - PostAnalyticsGrowthPage, - PostAnalyticsOverviewPage, - PostAnalyticsPage, - PostAnalyticsWebTrafficPage, -} from '@/admin-pages'; +import { PostAnalyticsOverviewPage, PostAnalyticsPage } from '@/admin-pages'; import { SettingsService } from '@/helpers/services/settings/settings-service'; import { createPostFactory } from '@/data-factory'; import { expect, test } from '@/helpers/playwright'; +// The family's server journey: the members_track_sources setting round-trips +// through the real settings API and the analytics chrome reacts after reload. +// The pure-rendering variants of these screens live in the admin acceptance +// tier against faked data. test.describe('Ghost Admin - Post Analytics - Overview', () => { - test.beforeEach(async ({ page }) => { + test('hides growth after member source tracking is disabled and persisted', async ({ page }) => { const postFactory = createPostFactory(page.request); const post = await postFactory.create({ - title: 'Post analytics overview test', + title: 'Post analytics settings journey', status: 'published', }); const postAnalyticsOverviewPage = new PostAnalyticsOverviewPage(page); await postAnalyticsOverviewPage.gotoForPost(post.id); - }); - - test('empty page with all tabs', async ({ page }) => { - const postAnalyticsPage = new PostAnalyticsPage(page); - - await expect(postAnalyticsPage.overviewButton).toBeVisible(); - await expect(postAnalyticsPage.webTrafficButton).toBeVisible(); - await expect(postAnalyticsPage.growthButton).toBeVisible(); - }); - - test('empty page - overview - web performance - view more', async ({ page }) => { - const postAnalyticsPage = new PostAnalyticsPage(page); - await postAnalyticsPage.webPerformanceSection.viewMoreButton.click(); - - const postAnalyticsWebTrafficPage = new PostAnalyticsWebTrafficPage(page); - await expect(postAnalyticsWebTrafficPage.body).toContainText('No visitors in the last 30 days'); - }); - test('empty page - overview - growth', async ({ page }) => { - const postAnalyticsPage = new PostAnalyticsPage(page); - - await expect(postAnalyticsPage.growthSection.card).toContainText('Free members'); - await expect(postAnalyticsPage.growthSection.card).toContainText('0'); - }); - - test('empty page - overview - growth - view more', async ({ page }) => { - const postAnalyticsPage = new PostAnalyticsPage(page); - await postAnalyticsPage.growthSection.viewMoreButton.click(); - - const postAnalyticsGrowthPage = new PostAnalyticsGrowthPage(page); - await expect(postAnalyticsGrowthPage.topSourcesCard).toContainText('No sources data available'); - }); - - test('growth tab and section - hidden when member sources tracking disabled', async ({ - page, - }) => { const settingsService = new SettingsService(page.request); const postAnalyticsPage = new PostAnalyticsPage(page); await expect(postAnalyticsPage.growthButton).toBeVisible(); - await expect(postAnalyticsPage.growthSection.card).toBeVisible(); try { await settingsService.setMembersTrackSources(false); await page.reload(); - await expect(postAnalyticsPage.growthButton).toBeHidden(); - await expect(postAnalyticsPage.growthSection.card).toBeHidden(); await expect(postAnalyticsPage.overviewButton).toBeVisible(); - await expect(postAnalyticsPage.webTrafficButton).toBeVisible(); + await expect(postAnalyticsPage.growthButton).toBeHidden(); } finally { await settingsService.setMembersTrackSources(true); await page.reload(); + await expect(postAnalyticsPage.growthButton).toBeVisible(); } }); }); diff --git a/e2e/tests/admin/analytics/web-traffic.test.ts b/e2e/tests/admin/analytics/web-traffic.test.ts deleted file mode 100644 index 0d7eb22a182..00000000000 --- a/e2e/tests/admin/analytics/web-traffic.test.ts +++ /dev/null @@ -1,40 +0,0 @@ -import { AnalyticsWebTrafficPage } from '@/admin-pages'; -import { expect, test } from '@/helpers/playwright'; - -test.describe('Ghost Admin - Analytics Web Traffic', () => { - let analyticsWebTrafficPage: AnalyticsWebTrafficPage; - - test.beforeEach(async ({ page }) => { - analyticsWebTrafficPage = new AnalyticsWebTrafficPage(page); - await analyticsWebTrafficPage.goto(); - }); - - test('empty web traffic general graph', async () => { - await expect(analyticsWebTrafficPage.totalUniqueVisitorsTab).toContainText('0'); - await expect(analyticsWebTrafficPage.totalViewsTab).toContainText('0'); - }); - - test('empty top content card', async () => { - await expect(analyticsWebTrafficPage.topContentCard).toContainText('No visitors'); - }); - - test('empty top content card - posts', async () => { - await analyticsWebTrafficPage.postsButton.click(); - - await expect(analyticsWebTrafficPage.topContentCard).toContainText('No visitors'); - }); - - test('empty top content card - pages', async () => { - await analyticsWebTrafficPage.pagesButton.click(); - - await expect(analyticsWebTrafficPage.topContentCard).toContainText('No visitors'); - }); - - test('empty top sources card', async () => { - await expect(analyticsWebTrafficPage.topSourcesCard).toContainText('No visitors'); - }); - - test('empty locations card', async () => { - await expect(analyticsWebTrafficPage.locationsCard).toContainText('No visitors'); - }); -}); diff --git a/e2e/tests/admin/settings/integrations.test.ts b/e2e/tests/admin/settings/integrations.test.ts index de36f592645..5791daf536b 100644 --- a/e2e/tests/admin/settings/integrations.test.ts +++ b/e2e/tests/admin/settings/integrations.test.ts @@ -1,199 +1,21 @@ import { IntegrationModal } from '@/admin-pages'; import { expect, test } from '@/helpers/playwright'; -interface Setting { - key: string; - value: string | boolean | null; -} - -interface SettingsResponse { - settings: Setting[]; -} - test.describe('Ghost Admin - Integrations', () => { - test.describe('Transistor Integration', () => { - test.describe('with transistor labs flag enabled', () => { - test.use({ labs: { transistor: true } }); - - test('transistor integration is visible when labs flag is enabled', async ({ page }) => { - const transistor = new IntegrationModal(page, 'transistor'); - - await transistor.goto(); - - await expect(transistor.integrationItem).toBeVisible(); - }); - - test('can open transistor modal', async ({ page }) => { - const transistor = new IntegrationModal(page, 'transistor'); - - await transistor.goto(); - await transistor.openModal(); - - await expect(transistor.modal).toBeVisible(); - await expect(transistor.enableToggle).toBeVisible(); - }); - - test('can enable transistor integration', async ({ page }) => { - const transistor = new IntegrationModal(page, 'transistor'); - - await transistor.goto(); - await transistor.openModal(); - await transistor.enable(); - await transistor.save(); - - const response = await page.request.get('/ghost/api/admin/settings/'); - expect(response.ok()).toBe(true); - - const data = (await response.json()) as SettingsResponse; - const transistorSetting = data.settings.find((s) => s.key === 'transistor'); - expect(transistorSetting).toBeDefined(); - expect(transistorSetting?.value).toBe(true); - }); - - test('can disable transistor integration', async ({ page }) => { - const transistor = new IntegrationModal(page, 'transistor'); - - await transistor.goto(); - await transistor.openModal(); - await transistor.enable(); - await transistor.save(); - - await transistor.closeModal(); - await transistor.openModal(); - await transistor.disable(); - await transistor.save(); - - const response = await page.request.get('/ghost/api/admin/settings/'); - expect(response.ok()).toBe(true); - - const data = (await response.json()) as SettingsResponse; - const transistorSetting = data.settings.find((s) => s.key === 'transistor'); - expect(transistorSetting).toBeDefined(); - expect(transistorSetting?.value).toBe(false); - }); - - test('transistor enabled state persists after page reload', async ({ page }) => { - const transistor = new IntegrationModal(page, 'transistor'); - - await transistor.goto(); - await transistor.openModal(); - await transistor.enable(); - await transistor.save(); - await transistor.closeModal(); - - await page.reload(); - await transistor.integrationsSection.waitFor({ state: 'visible' }); - - await transistor.openModal(); - - await expect(transistor.enableToggle).toHaveAttribute('aria-checked', 'true'); - }); - - test('transistor shows active badge when enabled', async ({ page }) => { - const transistor = new IntegrationModal(page, 'transistor'); - - await transistor.goto(); - await transistor.openModal(); - await transistor.enable(); - await transistor.save(); - await transistor.closeModal(); - - await expect(transistor.integrationItem.getByText('Active')).toBeVisible(); - }); - }); - - test.describe('with transistor labs flag disabled', () => { - test.use({ labs: { transistor: false } }); - - test('transistor integration is hidden when labs flag is disabled', async ({ page }) => { - const transistor = new IntegrationModal(page, 'transistor'); - - await transistor.goto(); - - await expect(transistor.integrationItem).toBeHidden(); - }); - }); - }); - - test.describe('Unsplash Integration', () => { - test('can enable unsplash integration', async ({ page }) => { - const unsplash = new IntegrationModal(page, 'unsplash'); - - await unsplash.goto(); - await unsplash.openModal(); - await unsplash.enable(); - await unsplash.save(); - - const response = await page.request.get('/ghost/api/admin/settings/'); - expect(response.ok()).toBe(true); - - const data = (await response.json()) as SettingsResponse; - const unsplashSetting = data.settings.find((s) => s.key === 'unsplash'); - expect(unsplashSetting).toBeDefined(); - expect(unsplashSetting?.value).toBe(true); - }); - - test('can disable unsplash integration', async ({ page }) => { - const unsplash = new IntegrationModal(page, 'unsplash'); - - await unsplash.goto(); - await unsplash.openModal(); - await unsplash.enable(); - await unsplash.save(); - - await unsplash.closeModal(); - await unsplash.openModal(); - await unsplash.disable(); - await unsplash.save(); - - const response = await page.request.get('/ghost/api/admin/settings/'); - expect(response.ok()).toBe(true); - - const data = (await response.json()) as SettingsResponse; - const unsplashSetting = data.settings.find((s) => s.key === 'unsplash'); - expect(unsplashSetting).toBeDefined(); - expect(unsplashSetting?.value).toBe(false); - }); - }); - - test.describe('Pintura Integration', () => { - test('can enable pintura integration', async ({ page }) => { - const pintura = new IntegrationModal(page, 'pintura'); - - await pintura.goto(); - await pintura.openModal(); - await pintura.enable(); - await pintura.save(); - - const response = await page.request.get('/ghost/api/admin/settings/'); - expect(response.ok()).toBe(true); - - const data = (await response.json()) as SettingsResponse; - const pinturaSetting = data.settings.find((s) => s.key === 'pintura'); - expect(pinturaSetting).toBeDefined(); - expect(pinturaSetting?.value).toBe(true); - }); - - test('can disable pintura integration', async ({ page }) => { - const pintura = new IntegrationModal(page, 'pintura'); + test('transistor enabled state persists after page reload', async ({ page }) => { + const transistor = new IntegrationModal(page, 'transistor'); - await pintura.goto(); - await pintura.openModal(); - await pintura.enable(); - await pintura.save(); + await transistor.goto(); + await transistor.openModal(); + await transistor.enable(); + await transistor.save(); + await transistor.closeModal(); - await pintura.closeModal(); - await pintura.openModal(); - await pintura.disable(); - await pintura.save(); + await page.reload(); + await transistor.integrationsSection.waitFor({ state: 'visible' }); - const response = await page.request.get('/ghost/api/admin/settings/'); - expect(response.ok()).toBe(true); + await transistor.openModal(); - const data = (await response.json()) as SettingsResponse; - const pinturaSetting = data.settings.find((s) => s.key === 'pintura'); - expect(pinturaSetting).toBeDefined(); - expect(pinturaSetting?.value).toBe(false); - }); + await expect(transistor.enableToggle).toHaveAttribute('aria-checked', 'true'); }); }); diff --git a/e2e/tests/admin/tags/editor.test.ts b/e2e/tests/admin/tags/editor.test.ts index 41560728156..3e7661befbf 100644 --- a/e2e/tests/admin/tags/editor.test.ts +++ b/e2e/tests/admin/tags/editor.test.ts @@ -1,4 +1,4 @@ -import { NewTagsPage, SidebarPage, TagEditorPage, TagsPage } from '@/admin-pages'; +import { NewTagsPage, TagEditorPage, TagsPage } from '@/admin-pages'; import { expect, test } from '@/helpers/playwright'; test.describe('Ghost Admin - Tags Editor', () => { @@ -97,39 +97,4 @@ test.describe('Ghost Admin - Tags Editor', () => { await expect(page).toHaveURL(tagsPage.pageUrl); await expect(tagsPage.getTagLinkByName('News')).toBeHidden(); }); - - test('can load tag via slug in url', async ({ page }) => { - const tagEditor = new TagEditorPage(page); - await tagEditor.gotoTagBySlug('news'); - - await expect(page).toHaveURL('/ghost/#/tags/news'); - await expect(tagEditor.nameInput).toHaveValue('News'); - await expect(tagEditor.slugInput).toHaveValue('news'); - }); - - test('redirects to 404 when tag does not exist', async ({ page }) => { - const tagEditor = new TagEditorPage(page); - await tagEditor.gotoTagBySlug('unknown'); - - await expect(page.getByText('Page not found')).toBeVisible(); - }); - - test('maintains active state in nav menu when creating a new tag', async ({ page }) => { - const newTagsPage = new NewTagsPage(page); - const sidebar = new SidebarPage(page); - await newTagsPage.goto(); - - await expect(page).toHaveURL(newTagsPage.pageUrl); - await expect(sidebar.getNavLink('Tags')).toHaveAttribute('aria-current', 'page'); - }); - - test('maintains active state in nav menu when editing a tag', async ({ page }) => { - const tagsPage = new TagsPage(page); - const sidebar = new SidebarPage(page); - - await tagsPage.goto(); - await tagsPage.getTagLinkByName('News').click(); - - await expect(sidebar.getNavLink('Tags')).toHaveAttribute('aria-current', 'page'); - }); }); diff --git a/ghost/core/core/server/api/endpoints/index.js b/ghost/core/core/server/api/endpoints/index.js index 94db512857e..430bedcc601 100644 --- a/ghost/core/core/server/api/endpoints/index.js +++ b/ghost/core/core/server/api/endpoints/index.js @@ -116,6 +116,10 @@ module.exports = { return apiFramework.pipeline(require('./member-custom-fields'), localUtils); }, + get tiersCheckoutConfig() { + return apiFramework.pipeline(require('./tiers-checkout-config'), localUtils); + }, + get memberCommenting() { return apiFramework.pipeline(require('./member-commenting'), localUtils); }, diff --git a/ghost/core/core/server/api/endpoints/tiers-checkout-config.ts b/ghost/core/core/server/api/endpoints/tiers-checkout-config.ts new file mode 100644 index 00000000000..98d960a2a4c --- /dev/null +++ b/ghost/core/core/server/api/endpoints/tiers-checkout-config.ts @@ -0,0 +1,76 @@ +import { actingContext } from '../../services/members-custom-fields'; +import { emptyCheckoutConfig } from '../../services/tier-checkout-config'; +import type { TierCheckoutConfig } from '../../services/tier-checkout-config'; + +const tiersService = require('../../services/tiers'); + +interface Frame { + data: { tiers_checkout_config?: unknown[] }; + options: { id: string; context: unknown; [key: string]: unknown }; +} + +export type TierCheckoutResult = TierCheckoutConfig[]; + +/** + * Reads one tier's checkout settings for the API to return. + * + * The service answers with null when nobody has ever set up checkout for the tier, because + * there is genuinely nothing stored for it. The API still answers with a resource: the tier + * itself exists, so a client asking what it collects should be told "nothing" in the same + * shape as any other answer, rather than getting a 404 or an empty body to puzzle over. + */ +async function forTier(id: string): Promise { + return (await tiersService.checkout.read(id)) ?? emptyCheckoutConfig(id); +} + +/** + * A tier's checkout configuration, as a sub-resource of the tier rather than an attribute + * of it. + * + * The tier resource is generally available and this concept is not, so putting it on the + * tier payload would add a key to every tier response on every site whether or not the + * feature is on. A route of its own can carry the flag, and be removed with it. + * + * Every operation here is one call. A tier's configuration is one shape the service hands + * out and takes back, and that it spans tables, that destinations are site-wide, and that a + * binding exists at all are facts about the inside of that domain. + */ +const controller = { + docName: 'tiers_checkout_config', + + browse: { + headers: { cacheInvalidate: false }, + permissions: { docName: 'products', method: 'browse' }, + query(): Promise { + return tiersService.checkout.browse(); + }, + }, + + read: { + headers: { cacheInvalidate: false }, + options: ['id'], + validation: { options: { id: { required: true } } }, + permissions: { docName: 'products', method: 'read' }, + async query(frame: Frame): Promise { + return [await forTier(frame.options.id)]; + }, + }, + + edit: { + headers: { cacheInvalidate: true }, + options: ['id'], + validation: { options: { id: { required: true } } }, + permissions: { docName: 'products', method: 'edit' }, + async query(frame: Frame): Promise { + await tiersService.checkout.edit( + actingContext(frame.options.context), + frame.options.id, + frame.data.tiers_checkout_config?.[0] ?? {}, + ); + return [await forTier(frame.options.id)]; + }, + }, +}; + +// module.exports (not export): the API framework loads controllers via require(). +module.exports = controller; diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/index.js b/ghost/core/core/server/api/endpoints/utils/serializers/output/index.js index 9645b314584..ce83186c76d 100644 --- a/ghost/core/core/server/api/endpoints/utils/serializers/output/index.js +++ b/ghost/core/core/server/api/endpoints/utils/serializers/output/index.js @@ -84,6 +84,10 @@ module.exports = { return require('./tiers'); }, + get tiers_checkout_config() { + return require('./tiers-checkout-config'); + }, + get images() { return require('./images'); }, diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/tiers-checkout-config.ts b/ghost/core/core/server/api/endpoints/utils/serializers/output/tiers-checkout-config.ts new file mode 100644 index 00000000000..c252654e5a6 --- /dev/null +++ b/ghost/core/core/server/api/endpoints/utils/serializers/output/tiers-checkout-config.ts @@ -0,0 +1,17 @@ +import { toCheckoutConfigResponse } from '../../../../../services/tier-checkout-config'; +import type { TierCheckoutConfig } from '../../../../../services/tier-checkout-config'; + +interface Frame { + response?: unknown; +} + +const serialize = (configs: TierCheckoutConfig[], _apiConfig: unknown, frame: Frame): void => { + frame.response = toCheckoutConfigResponse.parse(configs); +}; + +// module.exports (not export): the API framework loads serializers via require(). +module.exports = { + browse: serialize, + read: serialize, + edit: serialize, +}; diff --git a/ghost/core/core/server/data/exporter/table-lists.js b/ghost/core/core/server/data/exporter/table-lists.js index 5f9141c3c84..a570b54ba2f 100644 --- a/ghost/core/core/server/data/exporter/table-lists.js +++ b/ghost/core/core/server/data/exporter/table-lists.js @@ -44,6 +44,9 @@ const BACKUP_TABLES = [ 'members_newsletters', 'members_custom_fields', 'members_custom_field_values', + 'members_custom_field_bindings', + 'products_checkout_fields', + 'products_checkout_config', 'mentions', 'comments', 'comment_likes', diff --git a/ghost/core/core/server/data/migrations/versions/6.61/2026-08-19-14-38-53-add-members-custom-field-bindings-table.js b/ghost/core/core/server/data/migrations/versions/6.61/2026-08-19-14-38-53-add-members-custom-field-bindings-table.js new file mode 100644 index 00000000000..013d0894ad7 --- /dev/null +++ b/ghost/core/core/server/data/migrations/versions/6.61/2026-08-19-14-38-53-add-members-custom-field-bindings-table.js @@ -0,0 +1,26 @@ +const { addTable } = require('../../utils'); + +module.exports = addTable('members_custom_field_bindings', { + id: { type: 'string', maxlength: 24, nullable: false, primary: true }, + product_id: { + type: 'string', + maxlength: 24, + nullable: false, + references: 'products.id', + cascadeDelete: true, + }, + port: { type: 'string', maxlength: 191, nullable: false }, + custom_field_key: { + type: 'string', + maxlength: 191, + nullable: false, + references: 'members_custom_fields.key', + cascadeDelete: true, + }, + created_at: { type: 'dateTime', nullable: false }, + updated_at: { type: 'dateTime', nullable: true }, + '@@UNIQUE_CONSTRAINTS@@': [ + { columns: ['product_id', 'port'], indexName: 'members_custom_field_bindings_unique' }, + ], + '@@INDEXES@@': [['custom_field_key']], +}); diff --git a/ghost/core/core/server/data/migrations/versions/6.61/2026-08-19-14-48-06-add-tier-checkout-field-tables.js b/ghost/core/core/server/data/migrations/versions/6.61/2026-08-19-14-48-06-add-tier-checkout-field-tables.js new file mode 100644 index 00000000000..f3c6ad787c2 --- /dev/null +++ b/ghost/core/core/server/data/migrations/versions/6.61/2026-08-19-14-48-06-add-tier-checkout-field-tables.js @@ -0,0 +1,35 @@ +const { combineNonTransactionalMigrations, addTable } = require('../../utils'); + +module.exports = combineNonTransactionalMigrations( + addTable('products_checkout_fields', { + id: { type: 'string', maxlength: 24, nullable: false, primary: true }, + binding_id: { + type: 'string', + maxlength: 24, + nullable: false, + unique: true, + references: 'members_custom_field_bindings.id', + cascadeDelete: true, + }, + sort_order: { type: 'integer', nullable: false, unsigned: true, defaultTo: 0 }, + label: { type: 'string', maxlength: 191, nullable: true }, + optional: { type: 'boolean', nullable: false, defaultTo: true }, + created_at: { type: 'dateTime', nullable: false }, + updated_at: { type: 'dateTime', nullable: true }, + }), + addTable('products_checkout_config', { + id: { type: 'string', maxlength: 24, nullable: false, primary: true }, + product_id: { + type: 'string', + maxlength: 24, + nullable: false, + unique: true, + references: 'products.id', + cascadeDelete: true, + }, + shipping_allowed_countries: { type: 'string', maxlength: 2000, nullable: true }, + tax_number_collect: { type: 'boolean', nullable: false, defaultTo: false }, + created_at: { type: 'dateTime', nullable: false }, + updated_at: { type: 'dateTime', nullable: true }, + }), +); diff --git a/ghost/core/core/server/data/schema/schema.js b/ghost/core/core/server/data/schema/schema.js index ade08d0f8e9..c9e4234ef83 100644 --- a/ghost/core/core/server/data/schema/schema.js +++ b/ghost/core/core/server/data/schema/schema.js @@ -1064,6 +1064,79 @@ module.exports = { created_at: { type: 'dateTime', nullable: false }, updated_at: { type: 'dateTime', nullable: true }, }, + // Where a source sends what it collected. The source is who collects, the port is that + // source's own name for the thing, and the destination is the publisher's field, which + // they can repoint without the source knowing. The row is the collecting: there is one + // and the source writes through it, or there is none and it does not. + // + // A second kind of source becomes a `source_type` beside a widened id. What it must not + // become is a second table holding destinations. + members_custom_field_bindings: { + id: { type: 'string', maxlength: 24, nullable: false, primary: true }, + product_id: { + type: 'string', + maxlength: 24, + nullable: false, + references: 'products.id', + cascadeDelete: true, + }, + port: { type: 'string', maxlength: 191, nullable: false }, + // Indexed rather than unique: several sources landing in one field is expected. + custom_field_key: { + type: 'string', + maxlength: 191, + nullable: false, + references: 'members_custom_fields.key', + cascadeDelete: true, + }, + created_at: { type: 'dateTime', nullable: false }, + updated_at: { type: 'dateTime', nullable: true }, + '@@UNIQUE_CONSTRAINTS@@': [ + { columns: ['product_id', 'port'], indexName: 'members_custom_field_bindings_unique' }, + ], + '@@INDEXES@@': [['custom_field_key']], + }, + // How a tier's checkout question is asked. Where the answer lands is the binding it + // hangs off. + products_checkout_fields: { + id: { type: 'string', maxlength: 24, nullable: false, primary: true }, + binding_id: { + type: 'string', + maxlength: 24, + nullable: false, + unique: true, + references: 'members_custom_field_bindings.id', + cascadeDelete: true, + }, + sort_order: { type: 'integer', nullable: false, unsigned: true, defaultTo: 0 }, + // Processors cap a label far shorter than a field name may be. Null asks under the + // field's own name. + label: { type: 'string', maxlength: 191, nullable: true }, + optional: { type: 'boolean', nullable: false, defaultTo: true }, + created_at: { type: 'dateTime', nullable: false }, + updated_at: { type: 'dateTime', nullable: true }, + }, + // The options a tier's collection needs, and the one thing it collects without keeping. + // Whether it collects anything it *does* keep is the binding above. + products_checkout_config: { + id: { type: 'string', maxlength: 24, nullable: false, primary: true }, + product_id: { + type: 'string', + maxlength: 24, + nullable: false, + unique: true, + references: 'products.id', + cascadeDelete: true, + }, + // ISO 3166-1 alpha-2, comma-joined. A processor will not render an address form + // without them, and a wrong code fails the session create. + shipping_allowed_countries: { type: 'string', maxlength: 2000, nullable: true }, + // Stripe keeps a tax number against the customer it invoices, so there is no + // destination to bind and nothing to record but whether to ask. + tax_number_collect: { type: 'boolean', nullable: false, defaultTo: false }, + created_at: { type: 'dateTime', nullable: false }, + updated_at: { type: 'dateTime', nullable: true }, + }, members_custom_field_values: { id: { type: 'string', maxlength: 24, nullable: false, primary: true }, // The field's stable key, not its id: a value is addressed by key everywhere it diff --git a/ghost/core/core/server/services/email-analytics/email-analytics-service.ts b/ghost/core/core/server/services/email-analytics/email-analytics-service.ts index ad0560940e4..fcbb7870dc3 100644 --- a/ghost/core/core/server/services/email-analytics/email-analytics-service.ts +++ b/ghost/core/core/server/services/email-analytics/email-analytics-service.ts @@ -24,6 +24,11 @@ type FetchDataScheduled = FetchData & { schedule?: { begin: Date; end: Date } }; type EmailAnalyticsEvent = 'delivered' | 'opened' | 'failed' | 'unsubscribed' | 'complained'; +type FetchEventsResult = { + /** Earliest processed timestamp for a sending domain that stopped at its event limit. */ + safeCursor?: Date; +}; + /** * Names of the jobs this service runs. Each pipeline needs its own set so their * cursors don't overwrite each other in the jobs table. @@ -68,7 +73,7 @@ type FetchEvents = (options: { end: Date; maxEvents: number; events?: EmailAnalyticsEvent[]; -}) => Promise; +}) => Promise; const TRUST_THRESHOLD_MS = 30 * 60 * 1000; // 30 minutes const FETCH_LATEST_END_MARGIN_MS = 1 * 60 * 1000; // Do not fetch events newer than 1 minute (yet). Reduces the chance of having missed events in fetchLatest. @@ -539,14 +544,25 @@ export class EmailAnalyticsService { }; try { - await this.#fetchEvents({ + const fetchResult = await this.#fetchEvents({ batchHandler: processBatch, begin, end, maxEvents, events: eventTypes, }); + + if ( + fetchResult?.safeCursor && + (!fetchData.lastEventTimestamp || fetchResult.safeCursor < fetchData.lastEventTimestamp) + ) { + fetchData.lastEventTimestamp = fetchResult.safeCursor; + } } catch (err) { + // A fetch can process events from one domain before another domain fails. Keep the + // in-memory cursor at the start of this run so the next attempt retries every domain. + fetchData.lastEventTimestamp = begin; + if (!(err instanceof Error) || err.message !== 'Fetching canceled') { logging.error('[EmailAnalytics] Error while fetching'); logging.error(err); diff --git a/ghost/core/core/server/services/email-analytics/fetch-mailgun-events.ts b/ghost/core/core/server/services/email-analytics/fetch-mailgun-events.ts index 9af38e6e660..cda02b5c678 100644 --- a/ghost/core/core/server/services/email-analytics/fetch-mailgun-events.ts +++ b/ghost/core/core/server/services/email-analytics/fetch-mailgun-events.ts @@ -9,7 +9,7 @@ type FetchMailgunEventsOptions = { settings: { get: (key: string) => unknown }; tags: string[]; batchHandler: Function; - /** Not a strict maximum. We stop fetching after we reached the maximum AND received at least one event after begin (not equal) to prevent deadlocks. */ + /** Per-domain soft maximum. We stop fetching a domain after we reached the maximum AND received at least one event after begin (not equal) to prevent deadlocks. */ maxEvents?: number; begin?: Date; end?: Date; diff --git a/ghost/core/core/server/services/gifts/CONTEXT.md b/ghost/core/core/server/services/gifts/CONTEXT.md index 02f8c0b50fd..25fe19fef0f 100644 --- a/ghost/core/core/server/services/gifts/CONTEXT.md +++ b/ghost/core/core/server/services/gifts/CONTEXT.md @@ -37,7 +37,7 @@ The buyer's chosen way of handing over a gift subscription: the publication emai _Avoid_: Delivery mode **Redemption link**: -A single-use, time-limited link through which a purchased gift subscription can be viewed and claimed. Its bearer may see the gift's buyer name, intended recipient name, and personal message, but not email-routing or delivery details. +A single-use, time-limited link through which a purchased gift subscription can be viewed and claimed. Its bearer may see the gift's buyer name, intended recipient name, recipient email, and personal message; the intended recipient does not reserve redemption. _Avoid_: Gift Link **Gift redemption**: 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 331e2ea2ae2..a248a9fae5d 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 @@ -24,6 +24,10 @@ export interface GiftDeliveryRepository { giftId: string, options?: RepositoryTransactionOptions, ): Promise; + getByGiftToken( + giftToken: string, + options?: RepositoryTransactionOptions, + ): Promise; getByProviderMessageId(providerMessageId: string): Promise; findRecoverableForPurchasedGifts( now: Date, @@ -124,6 +128,20 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { return model ? decodeGiftDeliveryRow(model.toJSON()) : null; } + async getByGiftToken( + giftToken: string, + options: RepositoryTransactionOptions = {}, + ): Promise { + const db = options.transacting ?? this.knex; + const row = await db('gift_deliveries') + .select('gift_deliveries.*') + .join('gifts', 'gifts.id', 'gift_deliveries.gift_id') + .where('gifts.token', giftToken) + .first(); + + return row ? decodeGiftDeliveryRow(row) : null; + } + async getByProviderMessageId(providerMessageId: string): Promise { const model = await this.model.findOne( { email_provider_message_id: providerMessageId }, diff --git a/ghost/core/core/server/services/gifts/gift-delivery-service.ts b/ghost/core/core/server/services/gifts/gift-delivery-service.ts index 5bbc7bd6e33..1d5640eefe1 100644 --- a/ghost/core/core/server/services/gifts/gift-delivery-service.ts +++ b/ghost/core/core/server/services/gifts/gift-delivery-service.ts @@ -124,6 +124,15 @@ export class GiftDeliveryService { return delivery.id; } + async getRecipientEmailForGift( + giftToken: string, + options: RepositoryTransactionOptions = {}, + ): Promise { + const delivery = await this.deps.giftDeliveryRepository.getByGiftToken(giftToken, options); + + return delivery?.recipientEmail ?? null; + } + async dispatchForGift({ giftId, }: { diff --git a/ghost/core/core/server/services/gifts/gift-service.ts b/ghost/core/core/server/services/gifts/gift-service.ts index 05e17912245..d9607258282 100644 --- a/ghost/core/core/server/services/gifts/gift-service.ts +++ b/ghost/core/core/server/services/gifts/gift-service.ts @@ -9,6 +9,7 @@ import type { GiftEventBrowseOptions, GiftEventPage, GiftRepository, + RepositoryTransactionOptions, } from './gift-bookshelf-repository'; import type { GiftDeliveryDispatchResult, GiftDeliveryService } from './gift-delivery-service'; import type { SignedFlushScheduler } from '../../adapters/scheduling/signed-flush-scheduler'; @@ -175,7 +176,11 @@ interface GiftServiceDeps { giftRepository: GiftRepository; giftDeliveryService: Pick< GiftDeliveryService, - 'createForCheckout' | 'dispatchForGift' | 'cancelPendingForGift' | 'recoverPending' + | 'createForCheckout' + | 'getRecipientEmailForGift' + | 'dispatchForGift' + | 'cancelPendingForGift' + | 'recoverPending' >; memberRepository: MemberRepository; tiersService: TiersService; @@ -286,6 +291,7 @@ export interface GiftRedemption { amount: number; buyer_name: string | null; recipient_name: string | null; + recipient_email: string | null; message: string | null; expires_at: Date; consumes_at: Date | null; @@ -893,7 +899,7 @@ export class GiftService { transacting, newMember: input.newMember, }); - const redemption = await this.serializeRedemption(redeemed); + const redemption = await this.serializeRedemption(redeemed, { transacting }); return { redeemed, member, redemption }; }; @@ -1567,7 +1573,14 @@ export class GiftService { return true; } - private async serializeRedemption(gift: Gift): Promise { + private async serializeRedemption( + gift: Gift, + options: RepositoryTransactionOptions = {}, + ): Promise { + const recipientEmail = await this.deps.giftDeliveryService.getRecipientEmailForGift( + gift.token, + options, + ); const tier = await this.deps.tiersService.api.read(gift.tierId); if (!tier) { @@ -1586,6 +1599,7 @@ export class GiftService { amount: gift.amount, buyer_name: gift.buyerName, recipient_name: gift.recipientName, + recipient_email: recipientEmail, message: gift.personalMessage, expires_at: gift.expiresAt!, consumes_at: gift.consumesAt, diff --git a/ghost/core/core/server/services/lib/mailgun-client.js b/ghost/core/core/server/services/lib/mailgun-client.js index c533edb899a..618f9a5a50e 100644 --- a/ghost/core/core/server/services/lib/mailgun-client.js +++ b/ghost/core/core/server/services/lib/mailgun-client.js @@ -165,8 +165,8 @@ module.exports = class MailgunClient { * @param {Object} mailgunOptions * @param {Function} batchHandler * @param {Object} options - * @param {number} [options.maxEvents] Not a strict maximum. We stop fetching after we reached the maximum AND received at least one event after begin (not equal) to prevent deadlocks. - * @returns {Promise} + * @param {number} [options.maxEvents] Per-domain soft maximum. We stop fetching a domain after we reached the maximum AND received at least one event after begin (not equal) to prevent deadlocks. + * @returns {Promise<{safeCursor?: Date} | void>} */ async fetchEvents(mailgunOptions, batchHandler, { maxEvents = Infinity } = {}) { const mailgunInstance = this.getInstance(); @@ -184,12 +184,33 @@ module.exports = class MailgunClient { // Determine which domains to fetch from const domains = this.#getDomainsToFetch(mailgunConfig); + const cappedDomainCursors = []; + // Fetch events from each domain for (const domain of domains) { - await this.#fetchEventsFromDomain(domain, mailgunInstance, mailgunOptions, batchHandler, { - maxEvents, - }); + const result = await this.#fetchEventsFromDomain( + domain, + mailgunInstance, + mailgunOptions, + batchHandler, + { maxEvents }, + ); + + if ( + result.capped && + result.lastEventTimestamp && + Number.isFinite(result.lastEventTimestamp.getTime()) + ) { + cappedDomainCursors.push(result.lastEventTimestamp); + } } + + const safeCursor = cappedDomainCursors.reduce( + (earliest, cursor) => (!earliest || cursor < earliest ? cursor : earliest), + undefined, + ); + + return { safeCursor }; } /** @@ -220,7 +241,7 @@ module.exports = class MailgunClient { * @param {Function} batchHandler * @param {Object} options * @param {number} options.maxEvents - * @returns {Promise} + * @returns {Promise<{capped: boolean, eventCount: number, lastEventTimestamp?: Date}>} */ async #fetchEventsFromDomain( domain, @@ -235,6 +256,9 @@ module.exports = class MailgunClient { let batchCount = 0; let totalBatchTime = 0; + let eventCount = 0; + let lastEventTimestamp; + let capped = false; try { let page = await this.getEventsFromMailgun(mailgunInstance, domain, mailgunOptions); @@ -248,7 +272,6 @@ module.exports = class MailgunClient { `[MailgunClient fetchEventsFromDomain ${domain}]: finished fetching first page with ${events.length} events`, ); - let eventCount = 0; const beginTimestamp = mailgunOptions.begin ? Math.ceil(mailgunOptions.begin * 1000) : undefined; // ceil here if we have rounding errors @@ -263,6 +286,13 @@ module.exports = class MailgunClient { totalBatchTime += batchDuration; eventCount += events.length; + const batchLastEventTimestamp = events[events.length - 1].timestamp; + if ( + batchLastEventTimestamp && + (!lastEventTimestamp || batchLastEventTimestamp > lastEventTimestamp) + ) { + lastEventTimestamp = batchLastEventTimestamp; + } if ( eventCount >= maxEvents && @@ -270,6 +300,7 @@ module.exports = class MailgunClient { !events[events.length - 1].timestamp || events[events.length - 1].timestamp.getTime() > beginTimestamp) ) { + capped = true; break; } @@ -294,10 +325,15 @@ module.exports = class MailgunClient { const overallEndTime = Date.now(); const totalDuration = overallEndTime - overallStartTime; const averageBatchTime = batchCount > 0 ? totalBatchTime / batchCount : 0; + const cursor = Number.isFinite(lastEventTimestamp?.getTime()) + ? lastEventTimestamp.toISOString() + : 'none'; logging.info( - `[MailgunClient fetchEventsFromDomain ${domain}]: Processed ${batchCount} batches in ${(totalDuration / 1000).toFixed(2)}s. Average batch time: ${(averageBatchTime / 1000).toFixed(2)}s`, + `[MailgunClient fetchEventsFromDomain ${domain}]: Processed ${eventCount} events in ${batchCount} batches in ${(totalDuration / 1000).toFixed(2)}s. Average batch time: ${(averageBatchTime / 1000).toFixed(2)}s. Status: ${capped ? 'capped' : 'exhausted'}. Cursor: ${cursor}`, ); + + return { capped, eventCount, lastEventTimestamp }; } catch (error) { logging.error(`[MailgunClient fetchEventsFromDomain ${domain}]: Error fetching events`); logging.error(error); diff --git a/ghost/core/core/server/services/members-custom-fields/bindings-service.ts b/ghost/core/core/server/services/members-custom-fields/bindings-service.ts new file mode 100644 index 00000000000..afe5b66d899 --- /dev/null +++ b/ghost/core/core/server/services/members-custom-fields/bindings-service.ts @@ -0,0 +1,151 @@ +import ObjectID from 'bson-objectid'; +import logging from '@tryghost/logging'; +import type { Knex } from 'knex'; +import type { FieldType } from '@tryghost/custom-field-types'; +import { DbBoundField, FIELD_STATUS } from './schema'; +import type { CustomFieldValuesService, PlannedWrite } from './values-service'; + +const FIELDS_TABLE = 'members_custom_fields'; +const BINDINGS_TABLE = 'members_custom_field_bindings'; + +export interface BoundField { + bindingId: string; + key: string; + type: FieldType; +} + +/** + * Where a source sends what it collected: a `port` is the name that source uses for a + * thing, and the binding resolves it to one of the publisher's fields. + */ +export class CustomFieldBindingsService { + private knex: Knex; + private values: CustomFieldValuesService; + + constructor({ knex, values }: { knex: Knex; values: CustomFieldValuesService }) { + this.knex = knex; + this.values = values; + } + + async bind( + db: Knex, + productId: string, + port: string, + customFieldKey: string, + now: Date, + ): Promise { + const existing = await db(BINDINGS_TABLE).where({ product_id: productId, port }).first(); + if (existing?.custom_field_key === customFieldKey) { + await db(BINDINGS_TABLE).where('id', existing.id).update({ updated_at: now }); + return existing.id; + } + if (existing) { + await db(BINDINGS_TABLE).where('id', existing.id).del(); + } + + const bindingId = new ObjectID().toHexString(); + await db(BINDINGS_TABLE).insert({ + id: bindingId, + product_id: productId, + port, + custom_field_key: customFieldKey, + created_at: now, + updated_at: now, + }); + return bindingId; + } + + /** Stops the writing. Whatever hangs off the binding cascades with it. */ + async remove(db: Knex, productId: string, port: string): Promise { + await db(BINDINGS_TABLE).where({ product_id: productId, port }).del(); + } + + /** + * Values arrive in the order they are to be applied: where two land in one field, the + * last of them is what the field holds. + */ + async writeCollected( + memberId: string, + productId: string, + collected: Array<{ port: string; value: unknown }>, + ): Promise { + for (const { port, value } of collected) { + const destination = await this.resolve(productId, port); + if (!destination) { + continue; + } + await this.writeOne(memberId, destination, value); + } + } + + private async writeOne(memberId: string, into: BoundField, value: unknown): Promise { + let planned: PlannedWrite[]; + try { + planned = await this.values.planWrite({ [into.key]: value }); + } catch (err) { + logging.warn( + { + event: { name: 'members.custom_fields.collected_value_rejected' }, + err, + memberId, + customFieldKey: into.key, + }, + 'A collected value could not be saved', + ); + return; + } + + try { + await this.values.applyWrite(memberId, planned, { + writtenBy: { type: 'binding', id: into.bindingId }, + }); + } catch (err) { + logging.error( + { + event: { name: 'members.custom_fields.collected_value_write_failed' }, + err, + memberId, + customFieldKey: into.key, + bindingId: into.bindingId, + }, + 'Failed to store a collected custom field value', + ); + } + } + + private async resolve(productId: string, port: string): Promise { + const row = await this.knex(BINDINGS_TABLE) + .join(FIELDS_TABLE, `${BINDINGS_TABLE}.custom_field_key`, `${FIELDS_TABLE}.key`) + .where(`${BINDINGS_TABLE}.product_id`, productId) + .where(`${BINDINGS_TABLE}.port`, port) + // An archived destination is still where this goes, and still not somewhere a value + // can land, so the write drops rather than waiting. + .where(`${FIELDS_TABLE}.status`, FIELD_STATUS.active) + .select(`${BINDINGS_TABLE}.id as binding_id`, `${FIELDS_TABLE}.key`, `${FIELDS_TABLE}.type`) + .first(); + + if (!row) { + return null; + } + + // Decoded rather than trusted: a join is a read boundary, and `type` is what decides + // how the collected value is read. Unreadable counts as unresolved rather than + // throwing, so one bad row skips its value the way an unbound port does instead of + // failing everything else the same checkout collected. + const bound = DbBoundField.safeParse(row); + if (!bound.success) { + logging.warn( + { + event: { name: 'members.custom_fields.binding_unreadable' }, + err: bound.error, + productId, + port, + }, + 'A binding could not be read', + ); + return null; + } + + return { bindingId: bound.data.binding_id, key: bound.data.key, type: bound.data.type }; + } +} diff --git a/ghost/core/core/server/services/members-custom-fields/definitions-service.ts b/ghost/core/core/server/services/members-custom-fields/definitions-service.ts index fc926b7dea4..aba034b8c0f 100644 --- a/ghost/core/core/server/services/members-custom-fields/definitions-service.ts +++ b/ghost/core/core/server/services/members-custom-fields/definitions-service.ts @@ -3,11 +3,11 @@ import errors from '@tryghost/errors'; import type { Knex } from 'knex'; import { z } from 'zod'; import { CustomField } from './models'; -import { FieldTypeSchema } from '@tryghost/custom-field-types'; +import { FieldTypeSchema, type FieldType } from '@tryghost/custom-field-types'; import { customFieldCodec } from './codec'; import { FIELD_STATUS, FieldStatusSchema } from './schema'; -import { activeFields, inFieldOrder } from './queries'; -import { mintableKey } from './key'; +import { activeFields, fieldByKey, inFieldOrder, type DefinitionQuery } from './queries'; +import { KEY_CHARACTERS, mintableKey } from './key'; import { type RecordCustomFieldAction, type RequestContext } from './actions'; // The same NQL -> knex bridge Bookshelf's filter plugin uses, applied directly to @@ -38,11 +38,13 @@ const MAX_KEY_BASE_LENGTH = MAX_KEY_LENGTH - (String(MAX_KEY_ITERATIONS).length // object from JSON.parse. A key naming a member of Object.prototype reads back as // inherited rather than absent wherever one of those objects is indexed. // -// Derived rather than listed, because the set is a consequence of how keys are -// minted: minting lowercases and trims leading underscores, so a prototype name -// survives only if it has neither. `constructor` is the only one. -const RESERVED_KEYS = Object.getOwnPropertyNames(Object.prototype).filter( - (name) => mintableKey(name) === name, +// Derived rather than listed, from the format a key may take rather than from how one +// is minted: a caller can state a key instead of deriving it from a name, so what has +// to be reserved is every prototype name a well-formed key could spell. The ones that +// carry a capital cannot be spelled at all and need no reserving; `constructor` and +// `__proto__` can. +const RESERVED_KEYS = Object.getOwnPropertyNames(Object.prototype).filter((name) => + KEY_CHARACTERS.test(name), ); const FieldName = z @@ -136,17 +138,17 @@ export class CustomFieldDefinitionsService { * Typed off `activeFields` so the builder keeps the table's row type: every caller * hands over a query against the definitions table, whatever it has narrowed. */ - private async list(query: ReturnType): Promise { + private async list(query: DefinitionQuery): Promise { const rows = await inFieldOrder(query).select('*'); return rows.map((row) => z.decode(customFieldCodec, row)); } async read(key: string): Promise { - const row = await this.knex(TABLE).where('key', key).first(); - if (!row) { + const [field] = await this.list(fieldByKey(this.knex, key)); + if (!field) { throw new errors.NotFoundError({ message: 'Custom field not found.' }); } - return z.decode(customFieldCodec, row); + return field; } /** @@ -201,13 +203,11 @@ export class CustomFieldDefinitionsService { for (const [index, field] of fields.entries()) { await this.assertNameAvailable(trx, field.name); const key = await this.mintKey(trx, bases[index]); - await trx(TABLE).insert({ - id: new ObjectID().toHexString(), + await this.insertField(trx, { key, name: field.name, type: field.type, - sort_order: firstSortOrder + index, - created_at: new Date(), + sortOrder: firstSortOrder + index, }); keys.push(key); } @@ -228,7 +228,86 @@ export class CustomFieldDefinitionsService { // Logged after the commit: the action log is a separate Bookshelf write // outside this transaction, so recording inside it would leave orphaned // "added" entries for fields a rollback never created. - for (const field of created) { + await this.recordCreated(context, created); + return created; + } + + /** + * Given an executor it joins that transaction; given none it opens its own. Unlike + * `add`, the key is stated rather than minted from the name, and nothing about it is + * worked around: a caller states a key because something else already names that key, + * so a variant of it would be a field nothing points at. + * + * The field is returned rather than logged: the history writes on its own connection, + * which a single-connection pool would deadlock against an open transaction. + */ + async addOne( + wanted: { key: string; name: string; type: FieldType }, + { executor = this.knex }: { executor?: Knex } = {}, + ): Promise { + // Before any database access, the way `add` mints before opening its transaction: + // an unusable key is a payload problem worth reporting on its own terms. + assertKeyUsable(wanted.key); + + const write = async (db: Knex) => { + await this.assertWithinLimit(db, 1); + await this.assertKeyAvailable(db, wanted.key); + await this.assertNameAvailable(db, wanted.name); + + await this.insertField(db, { + key: wanted.key, + name: wanted.name, + type: wanted.type, + sortOrder: await this.nextSortOrder(db), + }); + const [created] = await this.readMany(db, [wanted.key]); + return created; + }; + + // knex's marker for a transactor: join it rather than nesting a savepoint under it. + return executor.isTransaction ? write(executor) : executor.transaction(write); + } + + /** + * Only for a stated key. Minting picks a free one instead, so this reads as a clean 422 + * where the unique index would read as a 500. + */ + private async assertKeyAvailable(db: Knex, key: string): Promise { + const taken = await db(TABLE).where('key', key).first(); + if (taken) { + throw new errors.ValidationError({ + message: 'A custom field with this key already exists.', + property: 'key', + }); + } + } + + private async insertField( + db: Knex, + field: { key: string; name: string; type: FieldType; sortOrder: number }, + ): Promise { + await db(TABLE).insert({ + id: new ObjectID().toHexString(), + key: field.key, + name: field.name, + type: field.type, + sort_order: field.sortOrder, + created_at: new Date(), + }); + } + + /** `read` for a caller that is deciding rather than serving: absent is an answer. */ + async findByKey( + key: string, + { executor = this.knex }: { executor?: Knex } = {}, + ): Promise { + const row = await executor(TABLE).where('key', key).first(); + return row ? z.decode(customFieldCodec, row) : null; + } + + /** The same entry `add` writes. Only the caller knows its transaction committed. */ + async recordCreated(context: RequestContext, fields: CustomField[]): Promise { + for (const field of fields) { await this.recordAction({ context, verb: 'create', @@ -236,7 +315,6 @@ export class CustomFieldDefinitionsService { details: { primary_name: field.name, key: field.key }, }); } - return created; } /** @@ -518,6 +596,37 @@ export class CustomFieldDefinitionsService { } } +/** + * The shape a stated key has to have. Minting derives one that is usable by + * construction, so this is the check that path never needed: a caller stating its own + * key has said nothing about the format, and guarding here covers every route in + * rather than whichever one arrived first. + * + * The length bound is the column's own, not `mintKey`'s: that one holds back room for a + * `_` collision suffix, and a stated key is written exactly as given and never + * suffixed, so the whole column is available to it. + */ +function assertKeyUsable(key: string): void { + if (!KEY_CHARACTERS.test(key)) { + throw new errors.ValidationError({ + message: 'A custom field key can only contain lowercase letters, numbers and underscores.', + property: 'key', + }); + } + if (key.length > MAX_KEY_LENGTH) { + throw new errors.ValidationError({ + message: `A custom field key can be at most ${MAX_KEY_LENGTH} characters.`, + property: 'key', + }); + } + if (RESERVED_KEYS.includes(key)) { + throw new errors.ValidationError({ + message: `${key} cannot be used as a custom field key.`, + property: 'key', + }); + } +} + // The field a zod issue points at. Create validates an array, so an issue's path // is prefixed with the item's index (`[0, 'name']`); `property` names the field // that is wrong, so the numeric prefix is dropped. Which item it was is reported diff --git a/ghost/core/core/server/services/members-custom-fields/index.ts b/ghost/core/core/server/services/members-custom-fields/index.ts index 546cd4d3d25..ff9f5d84fd9 100644 --- a/ghost/core/core/server/services/members-custom-fields/index.ts +++ b/ghost/core/core/server/services/members-custom-fields/index.ts @@ -1,28 +1,31 @@ import { CustomFieldDefinitionsService } from './definitions-service'; import { CustomFieldValuesService } from './values-service'; +import { CustomFieldBindingsService } from './bindings-service'; import { recordCustomFieldAction, type RecordCustomFieldAction } from './actions'; import { resolveMaxDefinitions } from './config'; export type { CustomField } from './models'; export type { RequestContext } from './actions'; export { actingContext } from './actions'; +export type { BoundField } from './bindings-service'; export type { WrittenBy } from './schema'; -// Two services from one module, split along an aggregate boundary rather than a -// technical layer: `definitions` owns the field definitions, which belong to the -// site's settings, and `values` owns the per-member values, which belong to the -// member. The values service reads the definitions table directly for the -// reference data it needs — a value referencing its definition, not a boundary -// crossing. +// Three services from one module, split along aggregate boundaries rather than +// technical layers: `definitions` owns the field definitions, which belong to the +// site's settings, `values` owns the per-member values, which belong to the +// member, and `bindings` owns which of a source's ports writes into which field. +// The values service reads the definitions table directly for the reference data +// it needs — a value referencing its definition, not a boundary crossing. // // Constructed by init() at boot, not at import: knex is only available once the DB has connected. export let definitions: CustomFieldDefinitionsService | undefined; export let values: CustomFieldValuesService | undefined; +export let bindings: CustomFieldBindingsService | undefined; export function init(): void { - // The two are constructed together below, so checking both keeps the "both or - // neither" invariant explicit rather than trusting one to stand in for the pair. - if (definitions && values) { + // The three are constructed together below, so checking all of them keeps the "all or + // none" invariant explicit rather than trusting one to stand in for the rest. + if (definitions && values && bindings) { return; } @@ -51,4 +54,9 @@ export function init(): void { getMaxDefinitions: () => resolveMaxDefinitions(config.get('members:customFields:maxDefinitions')), }); + + // Built after the values, which is what a binding routes into. It has no handle on the + // definitions: making a field is not part of binding to one, so a caller that needs both + // asks for both. + bindings = new CustomFieldBindingsService({ knex, values }); } diff --git a/ghost/core/core/server/services/members-custom-fields/queries.ts b/ghost/core/core/server/services/members-custom-fields/queries.ts index 40a8a51b7b4..1dc8ddd60ee 100644 --- a/ghost/core/core/server/services/members-custom-fields/queries.ts +++ b/ghost/core/core/server/services/members-custom-fields/queries.ts @@ -12,6 +12,20 @@ export function activeFields(db: Knex) { return db(FIELDS_TABLE).where('status', FIELD_STATUS.active); } +/** + * A narrowed query against the definitions table, whatever it has narrowed by. + * + * Named rather than inferred from `activeFields`, so a caller that narrows some other way — + * one key, a publisher's filter — states the same type instead of asserting its way back to + * it. + */ +export type DefinitionQuery = ReturnType; + +/** One field by key, as the same kind of query the rest of this reads through. */ +export function fieldByKey(db: Knex, key: string): DefinitionQuery { + return db(FIELDS_TABLE).where(`${FIELDS_TABLE}.key`, key); +} + /** * The publisher's order, applied to every read of the list. Here for the same reason the * status filter is: a read that forgets it comes back in whatever order the engine chose. 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 1e401b3308c..7d24f509231 100644 --- a/ghost/core/core/server/services/members-custom-fields/schema.ts +++ b/ghost/core/core/server/services/members-custom-fields/schema.ts @@ -75,6 +75,24 @@ export const DbCustomFieldLeaf = z.object({ value_text: z.string(), }); +export const DbCustomFieldBinding = z.object({ + id: z.string(), + product_id: z.string(), + port: z.string(), + custom_field_key: z.string(), + created_at: DbDate, + updated_at: DbDate.nullable(), +}); + +type CustomFieldBindingRow = z.infer; + +/** A binding joined to the field it points at, which is how a collected value is routed. */ +export const DbBoundField = z.object({ + binding_id: z.string(), + key: z.string(), + type: FieldTypeSchema, +}); + declare module 'knex/types/tables' { interface Tables { members_custom_fields: Knex.CompositeTableType< @@ -89,5 +107,12 @@ declare module 'knex/types/tables' { Omit, 'updated_at'>, Partial >; + members_custom_field_bindings: Knex.CompositeTableType< + CustomFieldBindingRow, + // `updated_at` is set on insert as well as update: a binding is a setting, and + // "when was this last stated" is the same question whichever way it got there. + z.input, + Partial + >; } } diff --git a/ghost/core/core/server/services/stripe/services/checkout/field-ports.ts b/ghost/core/core/server/services/stripe/services/checkout/field-ports.ts new file mode 100644 index 00000000000..7d88d6f135d --- /dev/null +++ b/ghost/core/core/server/services/stripe/services/checkout/field-ports.ts @@ -0,0 +1,38 @@ +/** + * What Stripe Checkout will render, measured against the live API at Ghost's pinned + * version rather than read from the reference — which disagreed with the API in three of + * five probes, missing the field cap and the key format entirely. A wrong bound here fails + * the session create, which fails the checkout. + */ + +/** The names Stripe returns values under, and the only ports a binding for it may use. */ +export const STRIPE_PORTS = ['shipping_name', 'shipping_address', 'phone'] as const; +export type StripePort = (typeof STRIPE_PORTS)[number]; + +export function isStripePort(key: string): key is StripePort { + return (STRIPE_PORTS as readonly string[]).includes(key); +} + +export const STRIPE_PORT = { + shippingName: 'shipping_name', + shippingAddress: 'shipping_address', + phone: 'phone', +} as const satisfies Record; + +/** Stripe rejects a fourth. */ +export const MAX_CHECKOUT_CUSTOM_FIELDS = 3; + +/** Stripe caps a custom label at 50, where a field name may be 191 — hence a question label. */ +export const MAX_CHECKOUT_LABEL_LENGTH = 50; + +/** + * What Stripe Checkout can ask for. No `long_text`: its text input caps shorter than that + * type allows. No `address`: Stripe has no custom-field equivalent, so an address is + * collected through its own parameter instead. + */ +export const CHECKOUT_ELIGIBLE_FIELD_TYPES = ['short_text'] as const; +export type CheckoutEligibleFieldType = (typeof CHECKOUT_ELIGIBLE_FIELD_TYPES)[number]; + +export function isCheckoutEligible(type: string): type is CheckoutEligibleFieldType { + return (CHECKOUT_ELIGIBLE_FIELD_TYPES as readonly string[]).includes(type); +} diff --git a/ghost/core/core/server/services/tier-checkout-config/codec.ts b/ghost/core/core/server/services/tier-checkout-config/codec.ts new file mode 100644 index 00000000000..2113c4b4cec --- /dev/null +++ b/ghost/core/core/server/services/tier-checkout-config/codec.ts @@ -0,0 +1,112 @@ +import { z } from 'zod'; +import { FieldTypeSchema } from '@tryghost/custom-field-types'; +import { DbBoolean } from '../../lib/db-types/boolean'; +import { DbCheckoutOptions, DbCheckoutQuestion } from './schema'; +import { CheckoutOptions } from './models'; + +const SEPARATOR = ','; + +export const optionsCodec = z.codec(DbCheckoutOptions, CheckoutOptions, { + decode: (columns) => ({ + shippingAllowedCountries: splitList(columns.shipping_allowed_countries), + taxNumber: columns.tax_number_collect, + }), + encode: (options) => ({ + shipping_allowed_countries: options.shippingAllowedCountries.length + ? joinList(options.shippingAllowedCountries) + : null, + tax_number_collect: options.taxNumber, + }), +}); + +function splitList(stored: string | null): string[] { + return stored ? stored.split(SEPARATOR).filter(Boolean) : []; +} + +function joinList(values: string[]): string { + return [...new Set(values)].join(SEPARATOR); +} + +export const CollectionRow = z.object({ + product_id: z.string(), + config_id: z.string().nullable(), + + shipping_allowed_countries: z.string().nullable(), + tax_number_collect: DbBoolean.nullable(), + + shipping_name_key: z.string().nullable(), + shipping_name_collectable: z.string().nullable(), + shipping_address_key: z.string().nullable(), + shipping_address_collectable: z.string().nullable(), + phone_key: z.string().nullable(), + phone_collectable: z.string().nullable(), +}); +export type CollectionRow = z.input; + +export const collectionRowCodec = CollectionRow.transform((row) => { + const collection = collectedInto(row); + + return { + tierId: row.product_id, + configured: row.config_id !== null, + collection, + collecting: { + // Stripe collects the recipient's name and their address under one parameter, so it + // asks for both or neither. Archiving one of the two destinations is the publisher + // saying they no longer want that half: the step is still worth asking for while the + // other half can land, and whatever arrives for the archived one is dropped. Only + // archiving both leaves nothing worth asking for. + shipping: + row.shipping_name_collectable === null && row.shipping_address_collectable === null + ? null + : collection.shipping, + taxNumber: collection.taxNumber, + phone: row.phone_collectable === null ? null : collection.phone, + }, + }; +}); +export type CollectionParts = z.infer; + +export const QuestionRow = z.object({ + product_id: z.string(), + port: z.string(), + label: DbCheckoutQuestion.shape.label, + optional: DbCheckoutQuestion.shape.optional, + question_name: z.string().nullable(), + question_type: FieldTypeSchema.nullable(), +}); +export type QuestionRow = z.input; + +export const questionRowCodec = QuestionRow.transform((row) => ({ + tierId: row.product_id, + question: { + key: row.port, + label: row.label, + optional: row.optional, + }, + askable: + row.question_type === null + ? null + : { prompt: row.label ?? row.question_name ?? row.port, type: row.question_type }, +})); +export type QuestionParts = z.infer; + +function collectedInto(row: z.output) { + const { shippingAllowedCountries, taxNumber } = z.decode(optionsCodec, { + shipping_allowed_countries: row.shipping_allowed_countries, + tax_number_collect: row.tax_number_collect ?? false, + }); + + return { + shipping: + row.shipping_name_key !== null && row.shipping_address_key !== null + ? { + allowedCountries: shippingAllowedCountries, + nameCustomFieldKey: row.shipping_name_key, + addressCustomFieldKey: row.shipping_address_key, + } + : null, + taxNumber, + phone: row.phone_key === null ? null : { customFieldKey: row.phone_key }, + }; +} diff --git a/ghost/core/core/server/services/tier-checkout-config/destinations.ts b/ghost/core/core/server/services/tier-checkout-config/destinations.ts new file mode 100644 index 00000000000..877cb094c5e --- /dev/null +++ b/ghost/core/core/server/services/tier-checkout-config/destinations.ts @@ -0,0 +1,17 @@ +import type { FieldType } from '@tryghost/custom-field-types'; +import type { StripePort } from '../stripe/services/checkout/field-ports'; + +/** + * What a port supplies, and what to call a field made to hold it. The type is a rule about + * any destination: a port that returns an address can only be collected into a field that + * keeps one. The name is used only when the request names a key the site does not keep + * yet, which is the one moment a publisher has not chosen a label themselves. + * + * A key is not a port. `phone` is what Stripe calls what it returns; where it lands is + * whatever the request said, and `Shipping Phone` is only how that field is listed. + */ +export const PORT_FIELD = { + shipping_name: { name: 'Shipping Name', type: 'short_text' }, + shipping_address: { name: 'Shipping Address', type: 'address' }, + phone: { name: 'Shipping Phone', type: 'short_text' }, +} as const satisfies Record; diff --git a/ghost/core/core/server/services/tier-checkout-config/index.ts b/ghost/core/core/server/services/tier-checkout-config/index.ts new file mode 100644 index 00000000000..38135ca2653 --- /dev/null +++ b/ghost/core/core/server/services/tier-checkout-config/index.ts @@ -0,0 +1,24 @@ +/** + * What a tier's checkout asks and collects. + * + * A domain of its own rather than part of the Tier aggregate: a tier is loaded into memory + * once at boot, and these rows are read live because deleting a custom field cascades a + * question away without that repository ever seeing it. + * + * Constructed by the tiers service wrapper at boot, which already holds the collaborators + * this needs, rather than by an init() here — the custom field services are built before + * it, so both are ready by the time it runs. + */ +export { TierCheckoutConfigService } from './service'; + +export { + CheckoutQuestion, + emptyCheckoutConfig, + PhoneCollection, + ResolvedCheckout, + ResolvedQuestion, + ShippingCollection, + TierCheckoutConfig, +} from './models'; + +export { toCheckoutConfigResponse } from './serializers'; diff --git a/ghost/core/core/server/services/tier-checkout-config/models.ts b/ghost/core/core/server/services/tier-checkout-config/models.ts new file mode 100644 index 00000000000..22248675904 --- /dev/null +++ b/ghost/core/core/server/services/tier-checkout-config/models.ts @@ -0,0 +1,71 @@ +import { z } from 'zod'; +import { FieldTypeSchema } from '@tryghost/custom-field-types'; + +export const CheckoutQuestion = z.object({ + key: z.string(), + label: z.string().nullable(), + optional: z.boolean(), +}); +export type CheckoutQuestion = z.infer; + +/** + * One toggle and two destinations: a processor returns the recipient and the address + * under one parameter, but a publisher keeps a name and an address in different fields. + */ +export const ShippingCollection = z.object({ + /** ISO 3166-1 alpha-2. A processor will not render an address form without them. */ + allowedCountries: z.array(z.string()), + nameCustomFieldKey: z.string(), + addressCustomFieldKey: z.string(), +}); +export type ShippingCollection = z.infer; + +export const PhoneCollection = z.object({ + customFieldKey: z.string(), +}); +export type PhoneCollection = z.infer; + +export const TierCheckoutConfig = z.object({ + tierId: z.string(), + customFields: z.array(CheckoutQuestion), + shipping: ShippingCollection.nullable(), + /** Stripe keeps a tax number against the customer it invoices; Ghost never stores one. */ + taxNumber: z.boolean(), + phone: PhoneCollection.nullable(), +}); +export type TierCheckoutConfig = z.infer; + +export const emptyCollection = (): Pick< + TierCheckoutConfig, + 'shipping' | 'taxNumber' | 'phone' +> => ({ + shipping: null, + taxNumber: false, + phone: null, +}); + +export const emptyCheckoutConfig = (tierId: string): TierCheckoutConfig => ({ + tierId, + customFields: [], + ...emptyCollection(), +}); + +export const CheckoutOptions = z.object({ + shippingAllowedCountries: z.array(z.string()), + taxNumber: z.boolean(), +}); +export type CheckoutOptions = z.infer; + +export const ResolvedQuestion = CheckoutQuestion.extend({ + prompt: z.string(), + type: FieldTypeSchema, +}); +export type ResolvedQuestion = z.infer; + +export const ResolvedCheckout = z.object({ + customFields: z.array(ResolvedQuestion), + shipping: ShippingCollection.nullable(), + taxNumber: z.boolean(), + phone: PhoneCollection.nullable(), +}); +export type ResolvedCheckout = z.infer; diff --git a/ghost/core/core/server/services/tier-checkout-config/queries.ts b/ghost/core/core/server/services/tier-checkout-config/queries.ts new file mode 100644 index 00000000000..521a5f82855 --- /dev/null +++ b/ghost/core/core/server/services/tier-checkout-config/queries.ts @@ -0,0 +1,98 @@ +import type { Knex } from 'knex'; +import { FIELD_STATUS } from '../members-custom-fields/schema'; +import { STRIPE_PORT } from '../stripe/services/checkout/field-ports'; +import { DbCheckoutOptions } from './schema'; +import type { CollectionRow, QuestionRow } from './codec'; + +export const QUESTIONS_TABLE = 'products_checkout_fields'; +export const CONFIG_TABLE = 'products_checkout_config'; +export const BINDINGS_TABLE = 'members_custom_field_bindings'; +export const FIELDS_TABLE = 'members_custom_fields'; + +const ACTIVE = FIELD_STATUS.active; + +const optionColumns = Object.keys(DbCheckoutOptions.shape).map( + (column) => `${CONFIG_TABLE}.${column}`, +); + +function collectionQuery(db: Knex) { + const bindTo = (alias: string, port: string) => + function (this: Knex.JoinClause) { + this.on(`${alias}.product_id`, 'products.id').andOn(db.raw(`${alias}.port = ?`, [port])); + }; + + const landsIn = (alias: string, binding: string) => + function (this: Knex.JoinClause) { + this.on(`${alias}.key`, `${binding}.custom_field_key`).andOn( + db.raw(`${alias}.status = ?`, [ACTIVE]), + ); + }; + + const query = db('products') + .leftJoin(CONFIG_TABLE, `${CONFIG_TABLE}.product_id`, 'products.id') + .leftJoin({ name_binding: BINDINGS_TABLE }, bindTo('name_binding', STRIPE_PORT.shippingName)) + .leftJoin({ name_field: FIELDS_TABLE }, landsIn('name_field', 'name_binding')) + .leftJoin( + { shipping_binding: BINDINGS_TABLE }, + bindTo('shipping_binding', STRIPE_PORT.shippingAddress), + ) + .leftJoin({ shipping_field: FIELDS_TABLE }, landsIn('shipping_field', 'shipping_binding')) + .leftJoin({ phone_binding: BINDINGS_TABLE }, bindTo('phone_binding', STRIPE_PORT.phone)) + .leftJoin({ phone_field: FIELDS_TABLE }, landsIn('phone_field', 'phone_binding')) + .orderBy('products.id', 'asc') + .select([ + 'products.id as product_id', + `${CONFIG_TABLE}.id as config_id`, + ...optionColumns, + 'name_binding.custom_field_key as shipping_name_key', + 'name_field.key as shipping_name_collectable', + 'shipping_binding.custom_field_key as shipping_address_key', + 'shipping_field.key as shipping_address_collectable', + 'phone_binding.custom_field_key as phone_key', + 'phone_field.key as phone_collectable', + ]); + + return query; +} + +/** One row, whether or not that tier has ever been configured. */ +export function collectionRowsForTier(db: Knex, productId: string) { + return collectionQuery(db).where('products.id', productId); +} + +/** + * This query starts from products, so without a filter it would return every tier on the + * site, configured or not. A tier only gets a row in products_checkout_config when someone + * saves checkout settings for it, so requiring that row narrows the list to the tiers + * somebody has actually set up. A tier that was set up and then had everything switched off + * keeps its row and stays in the list, reporting that it collects nothing. + */ +export function configuredCollectionRows(db: Knex) { + return collectionQuery(db).whereNotNull(`${CONFIG_TABLE}.id`); +} + +export function questionRows(db: Knex, productId?: string) { + const query = db(QUESTIONS_TABLE) + .join(BINDINGS_TABLE, `${BINDINGS_TABLE}.id`, `${QUESTIONS_TABLE}.binding_id`) + .leftJoin({ question_field: FIELDS_TABLE }, function () { + this.on('question_field.key', `${BINDINGS_TABLE}.custom_field_key`).andOn( + db.raw('question_field.status = ?', [ACTIVE]), + ); + }) + .orderBy(`${BINDINGS_TABLE}.product_id`, 'asc') + .orderBy(`${QUESTIONS_TABLE}.sort_order`, 'asc') + .orderBy(`${QUESTIONS_TABLE}.id`, 'asc') + .select([ + `${BINDINGS_TABLE}.product_id`, + `${BINDINGS_TABLE}.port`, + `${QUESTIONS_TABLE}.label`, + `${QUESTIONS_TABLE}.optional`, + 'question_field.name as question_name', + 'question_field.type as question_type', + ]); + + if (productId) { + return query.where(`${BINDINGS_TABLE}.product_id`, productId); + } + return query; +} diff --git a/ghost/core/core/server/services/tier-checkout-config/schema.ts b/ghost/core/core/server/services/tier-checkout-config/schema.ts new file mode 100644 index 00000000000..0ef3d44c1dd --- /dev/null +++ b/ghost/core/core/server/services/tier-checkout-config/schema.ts @@ -0,0 +1,53 @@ +import { z } from 'zod'; +import type { Knex } from 'knex'; +import { DbBoolean } from '../../lib/db-types/boolean'; +import { DbDate } from '../../lib/db-types/date'; + +export const DbCheckoutQuestion = z.object({ + id: z.string(), + binding_id: z.string(), + sort_order: z.number(), + label: z.string().nullable(), + optional: DbBoolean, + created_at: DbDate, + updated_at: DbDate.nullable(), +}); + +/** + * Options only. Whether a tier collects something it keeps is the binding; a tax number is + * the exception, since Stripe keeps it and Ghost never does, so there is nothing to bind. + */ +export const DbCheckoutConfig = z.object({ + id: z.string(), + product_id: z.string(), + shipping_allowed_countries: z.string().nullable(), + tax_number_collect: DbBoolean, + created_at: DbDate, + updated_at: DbDate.nullable(), +}); + +type CheckoutConfigRow = z.infer; + +export const DbCheckoutOptions = DbCheckoutConfig.omit({ + id: true, + product_id: true, + created_at: true, + updated_at: true, +}); +export type DbCheckoutOptions = z.infer; + +declare module 'knex/types/tables' { + interface Tables { + products_checkout_fields: Knex.CompositeTableType< + z.infer, + Omit, 'updated_at'>, + Partial> + >; + products_checkout_config: Knex.CompositeTableType< + CheckoutConfigRow, + Omit, keyof DbCheckoutOptions> & + Partial>, + Partial> + >; + } +} diff --git a/ghost/core/core/server/services/tier-checkout-config/serializers.ts b/ghost/core/core/server/services/tier-checkout-config/serializers.ts new file mode 100644 index 00000000000..48106509e72 --- /dev/null +++ b/ghost/core/core/server/services/tier-checkout-config/serializers.ts @@ -0,0 +1,135 @@ +import { z } from 'zod'; +import { MAX_CHECKOUT_CUSTOM_FIELDS } from '../stripe/services/checkout/field-ports'; +import { TierCheckoutConfig } from './models'; + +/** Each code costs three characters once comma-joined, against a 2000-character column. */ +const MAX_ALLOWED_COUNTRIES = 600; + +const QuestionInput = z.object({ + key: z.string().min(1, { error: 'Every checkout question needs a custom field key.' }), + label: z.string().trim().min(1).nullish(), + optional: z.boolean().optional(), +}); +export type QuestionInput = z.infer; + +// Not checked against a list of countries: membership of that list is contested, and Ghost +// is not its arbiter. +const CountryCode = z + .string() + .trim() + .regex(/^[A-Za-z]{2}$/, { error: 'Enter a 2-letter country code, like US.' }) + .toUpperCase(); + +/** + * Where a collected value lands is the request's to state. Ghost keeps no convention about + * it, so a block that collects names its destination and one that does not carries nothing + * to name. + */ +const DESTINATION_REQUIRED = 'Say which custom field this is collected into.'; +const CustomFieldKey = z + .string({ error: DESTINATION_REQUIRED }) + .min(1, { error: DESTINATION_REQUIRED }); +const Destination = z.strictObject( + { custom_field_key: CustomFieldKey }, + { error: DESTINATION_REQUIRED }, +); + +export const CheckoutConfigInput = z.strictObject({ + custom_fields: z + .array(QuestionInput) + .max(MAX_CHECKOUT_CUSTOM_FIELDS, { + error: `A checkout can ask at most ${MAX_CHECKOUT_CUSTOM_FIELDS} questions.`, + }) + .refine( + (questions) => new Set(questions.map((question) => question.key)).size === questions.length, + { error: 'This checkout already asks for that field.' }, + ) + .optional(), + + shipping: z + .discriminatedUnion('collect', [ + z.strictObject({ collect: z.literal(false) }), + z.strictObject({ + collect: z.literal(true), + allowed_countries: z + .array(CountryCode, { error: 'Choose at least one country you deliver to.' }) + .min(1, { error: 'Choose at least one country you deliver to.' }) + .max(MAX_ALLOWED_COUNTRIES), + name: Destination, + address: Destination, + }), + ]) + .optional(), + + tax_number: z.strictObject({ collect: z.boolean() }).optional(), + + phone: z + .discriminatedUnion('collect', [ + z.strictObject({ collect: z.literal(false) }), + z.strictObject({ collect: z.literal(true), custom_field_key: CustomFieldKey }), + ]) + .optional(), +}); +export type CheckoutConfigInput = z.infer; + +const QuestionResource = z.object({ + key: z.string(), + label: z.string().nullable(), + optional: z.boolean(), +}); + +const CollectionResource = z.object({ + collect: z.literal(true), + custom_field_key: z.string(), +}); + +const ShippingResource = z.object({ + collect: z.literal(true), + allowed_countries: z.array(z.string()), + name: z.object({ custom_field_key: z.string() }), + address: z.object({ custom_field_key: z.string() }), +}); + +const CheckoutConfigResource = z.object({ + tier_id: z.string(), + custom_fields: z.array(QuestionResource), + shipping: ShippingResource.optional(), + tax_number: z.object({ collect: z.literal(true) }).optional(), + phone: CollectionResource.optional(), +}); + +const CheckoutConfigResponse = z.object({ + tiers_checkout_config: z.array(CheckoutConfigResource), +}); + +/** One resource per tier, so a browse and a read differ only in how many come back. */ +export const toCheckoutConfigResponse = z + .array(TierCheckoutConfig) + .transform((configs): z.input => ({ + tiers_checkout_config: configs.map((config) => ({ + tier_id: config.tierId, + custom_fields: config.customFields, + // A block appears only when the tier collects that thing, so a client reads + // presence rather than a flag it would have to check. + ...(config.shipping + ? { + shipping: { + collect: true as const, + allowed_countries: config.shipping.allowedCountries, + name: { custom_field_key: config.shipping.nameCustomFieldKey }, + address: { custom_field_key: config.shipping.addressCustomFieldKey }, + }, + } + : {}), + ...(config.taxNumber ? { tax_number: { collect: true as const } } : {}), + ...(config.phone + ? { + phone: { + collect: true as const, + custom_field_key: config.phone.customFieldKey, + }, + } + : {}), + })), + })) + .pipe(CheckoutConfigResponse); diff --git a/ghost/core/core/server/services/tier-checkout-config/service.ts b/ghost/core/core/server/services/tier-checkout-config/service.ts new file mode 100644 index 00000000000..45eaea32754 --- /dev/null +++ b/ghost/core/core/server/services/tier-checkout-config/service.ts @@ -0,0 +1,420 @@ +import ObjectID from 'bson-objectid'; +import errors from '@tryghost/errors'; +import { z } from 'zod'; +import type { Knex } from 'knex'; +import type { FieldType } from '@tryghost/custom-field-types'; +import { DbCustomField, FIELD_STATUS } from '../members-custom-fields/schema'; +import type { CustomField, RequestContext } from '../members-custom-fields'; +import { + MAX_CHECKOUT_LABEL_LENGTH, + STRIPE_PORT, + isCheckoutEligible, + isStripePort, + type StripePort, +} from '../stripe/services/checkout/field-ports'; +import { + collectionRowCodec, + optionsCodec, + questionRowCodec, + type CollectionParts, + type CollectionRow, + type QuestionParts, +} from './codec'; +import { + BINDINGS_TABLE, + CONFIG_TABLE, + FIELDS_TABLE, + QUESTIONS_TABLE, + collectionRowsForTier, + configuredCollectionRows, + questionRows, +} from './queries'; +import { + emptyCollection, + type ResolvedCheckout, + type ResolvedQuestion, + type TierCheckoutConfig, +} from './models'; +import { PORT_FIELD } from './destinations'; +import { CheckoutConfigInput } from './serializers'; + +type FieldRow = Pick, 'key' | 'name' | 'type' | 'status'>; + +type NewField = { key: string; name: string; type: FieldType }; + +/** + * A request states its settings in named sections: one for shipping, one for phone. Each + * section stands for one or more of the values Stripe hands back when a checkout is + * completed — shipping covers both the recipient's name and their address, phone covers + * only the phone number. + * + * Naming those values here means that a request which includes a section settles every + * value in it: each one is either given a destination or has its old one removed. Without + * this list, a value the request never mentioned could keep a destination from an earlier + * save that the publisher believes they have turned off. + */ +const BLOCK_PORTS = { + shipping: [STRIPE_PORT.shippingName, STRIPE_PORT.shippingAddress], + phone: [STRIPE_PORT.phone], +} as const satisfies Record; + +interface CollectionPlan { + clear: StripePort[]; + create: NewField[]; + bind: Array<{ port: StripePort; key: string }>; +} + +export interface PortBinder { + bind( + db: Knex, + productId: string, + port: string, + customFieldKey: string, + now: Date, + ): Promise; + remove(db: Knex, productId: string, port: string): Promise; +} + +export interface FieldMaker { + findByKey(key: string, options?: { executor?: Knex }): Promise; + addOne(wanted: NewField, options?: { executor?: Knex }): Promise; + recordCreated(context: RequestContext, fields: CustomField[]): Promise; +} + +export class TierCheckoutConfigService { + private knex: Knex; + private bindings: PortBinder; + private fields: FieldMaker; + + constructor({ + knex, + bindings, + fields, + }: { + knex: Knex; + bindings: PortBinder; + fields: FieldMaker; + }) { + this.knex = knex; + this.bindings = bindings; + this.fields = fields; + } + + async browse(): Promise { + const rows = decodeCollection(await configuredCollectionRows(this.knex)); + if (rows.length === 0) { + return []; + } + + const asked = await this.questions(); + return rows.map((row) => assemble(row, asked.get(row.tierId) ?? [])); + } + + async read(productId: string): Promise { + const [row] = decodeCollection(await collectionRowsForTier(this.knex, productId)); + if (!row) { + throw new errors.NotFoundError({ message: 'Tier not found.' }); + } + if (!row.configured) { + return null; + } + + const asked = await this.questions(productId); + return assemble(row, asked.get(productId) ?? []); + } + + async resolve(productId: string): Promise { + const [row] = decodeCollection(await collectionRowsForTier(this.knex, productId)); + if (!row?.configured) { + return { customFields: [], ...emptyCollection() }; + } + + const asked = await this.questions(productId); + const customFields: ResolvedQuestion[] = (asked.get(productId) ?? []).flatMap((entry) => + entry.askable ? [{ ...entry.question, ...entry.askable }] : [], + ); + return { customFields, ...row.collecting }; + } + + /** + * Saves the checkout settings a request states, and leaves the rest alone. + * + * A request only has to include the sections it wants to change. Say nothing about + * shipping and the shipping settings stay exactly as they were, so a client that only + * knows how to edit the questions cannot wipe out the shipping settings by omitting + * them. + */ + async edit(context: RequestContext, productId: string, input: unknown): Promise { + const stated = parseInput(input); + const now = new Date(); + + if (stated.custom_fields) { + await assertQuestionsAskable(this.knex, stated.custom_fields); + } + const plan = await this.planCollection(stated); + + const created = await this.knex.transaction(async (trx) => { + await assertTierExists(trx, productId); + await writeOptions(trx, productId, stated, now); + + for (const port of plan.clear) { + await this.bindings.remove(trx, productId, port); + } + + const made: CustomField[] = []; + for (const wanted of plan.create) { + made.push(await this.fields.addOne(wanted, { executor: trx })); + } + for (const { port, key } of plan.bind) { + await this.bindings.bind(trx, productId, port, key, now); + } + + if (stated.custom_fields) { + await this.writeQuestions(trx, productId, stated.custom_fields, now); + } + return made; + }); + + await this.fields.recordCreated(context, created); + } + + private async questions(productId?: string): Promise> { + const rows = await questionRows(this.knex, productId); + const byTier = new Map(); + for (const row of rows) { + const parts = z.decode(questionRowCodec, row); + const forTier = byTier.get(parts.tierId) ?? []; + forTier.push(parts); + byTier.set(parts.tierId, forTier); + } + return byTier; + } + + private async planCollection(stated: CheckoutConfigInput): Promise { + const wanted: Array<{ port: StripePort; key: string }> = []; + + if (stated.shipping?.collect) { + wanted.push( + { port: STRIPE_PORT.shippingName, key: stated.shipping.name.custom_field_key }, + { port: STRIPE_PORT.shippingAddress, key: stated.shipping.address.custom_field_key }, + ); + } + if (stated.phone?.collect) { + wanted.push({ port: STRIPE_PORT.phone, key: stated.phone.custom_field_key }); + } + + const bound = new Set(wanted.map(({ port }) => port)); + const clear: StripePort[] = []; + for (const block of ['shipping', 'phone'] as const) { + if (stated[block]) { + clear.push(...BLOCK_PORTS[block].filter((port) => !bound.has(port))); + } + } + + const create = new Map(); + for (const { port, key } of wanted) { + const wants = PORT_FIELD[port]; + const existing = await this.fields.findByKey(key); + if (existing) { + assertCollectableInto(port, existing, wants.type); + continue; + } + + const alreadyPlanned = create.get(key); + if (alreadyPlanned && alreadyPlanned.type !== wants.type) { + throw new errors.ValidationError({ + message: `This can only be collected into a ${wants.type} field.`, + property: `checkout.${port}.custom_field_key`, + }); + } + if (!alreadyPlanned) { + create.set(key, { key, name: wants.name, type: wants.type }); + } + } + + return { clear, create: [...create.values()], bind: wanted }; + } + + /** + * Saves the questions this tier asks its buyers during checkout. + * + * A binding records that one value coming back from Stripe belongs in one custom field, + * and it identifies the value by the name Stripe uses for it. For a question that name + * is the custom field's own key: Ghost sends the key to Stripe as the question's + * identifier, and Stripe returns the buyer's answer labelled with that same key. That is + * why the key is passed twice below — once as the name Stripe will answer under, and + * once as the field the answer is stored in. + */ + private async writeQuestions( + trx: Knex.Transaction, + productId: string, + questions: NonNullable, + now: Date, + ): Promise { + const asked = new Set(questions.map((question) => question.key)); + const alreadyAsked: Array<{ port: string }> = await trx(QUESTIONS_TABLE) + .join(BINDINGS_TABLE, `${BINDINGS_TABLE}.id`, `${QUESTIONS_TABLE}.binding_id`) + .where(`${BINDINGS_TABLE}.product_id`, productId) + .select(`${BINDINGS_TABLE}.port`); + for (const { port } of alreadyAsked) { + if (!asked.has(port)) { + await this.bindings.remove(trx, productId, port); + } + } + + for (const [index, question] of questions.entries()) { + const bindingId = await this.bindings.bind(trx, productId, question.key, question.key, now); + await trx(QUESTIONS_TABLE).where('binding_id', bindingId).del(); + await trx(QUESTIONS_TABLE).insert({ + id: new ObjectID().toHexString(), + binding_id: bindingId, + sort_order: index, + label: question.label ?? null, + optional: question.optional ?? true, + created_at: now, + }); + } + } +} + +function parseInput(input: unknown): CheckoutConfigInput { + const parsed = CheckoutConfigInput.safeParse(input); + if (parsed.success) { + return parsed.data; + } + const issue = parsed.error.issues[0]; + throw new errors.ValidationError({ + message: issue.message, + property: issue.path.join('.') || 'checkout', + }); +} + +function decodeCollection(rows: CollectionRow[]): CollectionParts[] { + return rows.map((row) => z.decode(collectionRowCodec, row)); +} + +function assemble(row: CollectionParts, asked: QuestionParts[]): TierCheckoutConfig { + return { + tierId: row.tierId, + customFields: asked.map((entry) => entry.question), + ...row.collection, + }; +} + +async function assertTierExists(db: Knex, productId: string): Promise { + const tier = await db('products').where('id', productId).first(); + if (!tier) { + throw new errors.NotFoundError({ message: 'Tier not found.' }); + } +} + +/** + * Refuses a custom field that cannot hold what Stripe will send back. + * + * A request names the custom field each collected value should be saved into. That field + * has to be active, because an archived field accepts no new values, and it has to store + * the right kind of data: Stripe returns a structured address for the shipping address, + * and plain text for a name or a phone number. + * + * Both refusals are deliberate rather than defensive. Ghost could save the value into some + * other field, or accept the request and quietly collect nothing, but a publisher who named + * a field meant that field. Being told now is better than finding out weeks later that + * nothing was ever recorded. + */ +function assertCollectableInto(port: StripePort, field: CustomField, valueType: FieldType): void { + if (field.status !== FIELD_STATUS.active) { + throw new errors.ValidationError({ + message: 'An archived custom field cannot receive collected data. Restore it first.', + property: `checkout.${port}.custom_field_key`, + }); + } + if (field.type !== valueType) { + throw new errors.ValidationError({ + message: `This can only be collected into a ${valueType} field.`, + property: `checkout.${port}.custom_field_key`, + }); + } +} + +async function writeOptions( + trx: Knex.Transaction, + productId: string, + stated: CheckoutConfigInput, + now: Date, +): Promise { + const all = z.encode(optionsCodec, { + shippingAllowedCountries: stated.shipping?.collect ? stated.shipping.allowed_countries : [], + taxNumber: stated.tax_number?.collect ?? false, + }); + + // Only the columns this request spoke about are written, so two requests changing + // different parts of the same tier cannot undo each other, and a request that mentions + // neither still leaves the row behind as the record that this tier has been set up. + const columns = { + ...(stated.shipping ? { shipping_allowed_countries: all.shipping_allowed_countries } : {}), + ...(stated.tax_number ? { tax_number_collect: all.tax_number_collect } : {}), + }; + + await trx(CONFIG_TABLE) + .insert({ + id: new ObjectID().toHexString(), + product_id: productId, + created_at: now, + updated_at: now, + ...columns, + }) + .onConflict('product_id') + .merge({ ...columns, updated_at: now }); +} + +async function assertQuestionsAskable( + db: Knex, + questions: NonNullable, +): Promise { + const collides = questions.find((question) => isStripePort(question.key)); + if (collides) { + throw new errors.ValidationError({ + message: `A field keyed ${collides.key} cannot be asked at checkout, because that is what this checkout calls something it collects for itself.`, + property: 'checkout.custom_fields', + }); + } + + const keys = questions.map((question) => question.key); + if (keys.length === 0) { + return; + } + + const rows: FieldRow[] = await db(FIELDS_TABLE) + .whereIn(`${FIELDS_TABLE}.key`, keys) + .where(`${FIELDS_TABLE}.status`, FIELD_STATUS.active) + .select( + `${FIELDS_TABLE}.key`, + `${FIELDS_TABLE}.name`, + `${FIELDS_TABLE}.type`, + `${FIELDS_TABLE}.status`, + ); + const byKey = new Map(rows.map((row) => [row.key, row])); + + for (const question of questions) { + const field = byKey.get(question.key); + if (!field) { + throw new errors.ValidationError({ + message: `Unknown custom field: ${question.key}`, + property: 'checkout.custom_fields', + }); + } + if (!isCheckoutEligible(field.type)) { + throw new errors.ValidationError({ + message: `A ${field.type} field cannot be asked for at checkout.`, + property: 'checkout.custom_fields', + }); + } + const prompt = question.label ?? field.name; + if (prompt.length > MAX_CHECKOUT_LABEL_LENGTH) { + throw new errors.ValidationError({ + message: `A checkout question can be at most ${MAX_CHECKOUT_LABEL_LENGTH} characters. Give this one a shorter label.`, + property: 'checkout.custom_fields', + }); + } + } +} diff --git a/ghost/core/core/server/services/tiers/service.js b/ghost/core/core/server/services/tiers/service.js index b03a7ca6b8a..e119d341090 100644 --- a/ghost/core/core/server/services/tiers/service.js +++ b/ghost/core/core/server/services/tiers/service.js @@ -30,6 +30,27 @@ class TiersServiceWrapper { repository, slugService, }); + + // What a tier's checkout asks, kept beside the tier rather than inside it: these + // rows are read live on every request, because deleting a custom field cascades a + // question away without the repository above ever seeing it, and a cached copy + // would go on naming a field the site no longer has. + // + // Boot builds the custom fields services before this one, so both collaborators + // are ready by the time this runs. + const { TierCheckoutConfigService } = require('../tier-checkout-config'); + const { bindings, definitions } = require('../members-custom-fields'); + this.checkout = new TierCheckoutConfigService({ + knex: models.Base.knex, + // A binding is where a collected value lands, and the same rows are what a completed + // checkout is routed through later. Handed over rather than reached for, so the + // checkout domain states what it needs of them and nothing more. + bindings, + // Turning a collection on has to leave it somewhere to land, and making a field is + // the definitions' to do. A separate collaborator because it is a separate act, and + // because deciding whether one is needed belongs to the checkout rather than to them. + fields: definitions, + }); } } diff --git a/ghost/core/core/server/web/api/endpoints/admin/routes.js b/ghost/core/core/server/web/api/endpoints/admin/routes.js index 6c6023c1d87..263584c260a 100644 --- a/ghost/core/core/server/web/api/endpoints/admin/routes.js +++ b/ghost/core/core/server/web/api/endpoints/admin/routes.js @@ -146,6 +146,25 @@ module.exports = function apiRoutes() { // Tiers router.get('/tiers', mw.authAdminApi, http(api.tiers.browse)); router.post('/tiers', mw.authAdminApi, http(api.tiers.add)); + // What a tier's checkout asks for, read when a Stripe checkout session is built. + // Registered before /tiers/:id so the literal path isn't captured by :id. + // + // A sub-resource rather than a key on the tier, because the tier payload is a public + // projection: `tiers-public` shares this docName's serializer, so anything on a tier is + // rendered by themes. This is admin-only configuration that no client renders, since + // the questions are drawn by Stripe's own checkout page rather than by Portal. + // + // Named for the configuration rather than the checkout, so it cannot be mistaken for + // the session that `create-stripe-checkout-session` creates from it. + router.get('/tiers/checkout_config', mw.authAdminApi, http(api.tiersCheckoutConfig.browse)); + router.get('/tiers/:id/checkout_config', mw.authAdminApi, http(api.tiersCheckoutConfig.read)); + router.put( + '/tiers/:id/checkout_config', + mw.authAdminApi, + labs.enabledMiddleware('membersCustomFields'), + http(api.tiersCheckoutConfig.edit), + ); + router.get('/tiers/:id', mw.authAdminApi, http(api.tiers.read)); router.put('/tiers/:id', mw.authAdminApi, http(api.tiers.edit)); @@ -176,14 +195,11 @@ module.exports = function apiRoutes() { router.get('/members/stripe_connect', mw.authAdminApi, http(api.membersStripeConnect.auth)); - // Member custom field definitions — gated by the members_custom_fields flag. + // Member custom field definitions. Reading them is open: a site that has never + // turned the flag on has none, so the answer is an empty list rather than a 404, and + // Admin can ask without first knowing whether it may. Changing them is gated. // Registered before /members/:id so the literal path isn't captured by :id. - router.get( - '/members/custom_fields', - mw.authAdminApi, - labs.enabledMiddleware('membersCustomFields'), - http(api.membersCustomFields.browse), - ); + router.get('/members/custom_fields', mw.authAdminApi, http(api.membersCustomFields.browse)); router.post( '/members/custom_fields', mw.authAdminApi, @@ -197,12 +213,7 @@ module.exports = function apiRoutes() { labs.enabledMiddleware('membersCustomFields'), http(api.membersCustomFields.reorder), ); - router.get( - '/members/custom_fields/:key', - mw.authAdminApi, - labs.enabledMiddleware('membersCustomFields'), - http(api.membersCustomFields.read), - ); + router.get('/members/custom_fields/:key', mw.authAdminApi, http(api.membersCustomFields.read)); router.put( '/members/custom_fields/:key', mw.authAdminApi, diff --git a/ghost/core/test/e2e-api/admin/member-custom-fields.test.ts b/ghost/core/test/e2e-api/admin/member-custom-fields.test.ts index b3e71216d5c..a03ce7d6707 100644 --- a/ghost/core/test/e2e-api/admin/member-custom-fields.test.ts +++ b/ghost/core/test/e2e-api/admin/member-custom-fields.test.ts @@ -435,6 +435,20 @@ describe('Member Custom Fields Admin API', function () { }); }); + // Admin sends `?include=` on resources that have relations to load. This one has none, + // so the parameter has nothing to act on — which is a request that returns a definition, + // not a request that got something wrong. + describe('Asking for relations', function () { + it('ignores an include, rather than failing on it', async function () { + const field = await createField({ name: 'T-shirt size' }); + + const { body } = await agent.get('members/custom_fields/?include=bindings').expectStatus(200); + + assert.equal(body.members_custom_fields[0].key, field.key); + assert.equal('bindings' in body.members_custom_fields[0], false); + }); + }); + describe('Creating several definitions at once', function () { it('creates every definition in the request, in order', async function () { const { body } = await agent @@ -2254,14 +2268,30 @@ describe('Member Custom Fields Admin API', function () { }); }); + // The flag governs whether a publisher can set custom fields up, not whether the rest of + // Ghost may ask what they are. Admin reads this list to draw screens it renders either + // way, and a site that has never enabled the flag has no fields, so the honest answer is + // an empty list. Every route that changes something stays behind the flag. describe('Flag disabled', function () { beforeEach(function () { mockManager.restore(); mockManager.mockLabsDisabled('membersCustomFields'); }); - it('404s the definitions endpoint', async function () { - await agent.get('members/custom_fields/').expectStatus(404); + it('serves the definitions endpoint, with nothing on it', async function () { + const { body } = await agent.get('members/custom_fields/').expectStatus(200); + assert.deepEqual(body.members_custom_fields, []); + }); + + it('404s a read of a field that does not exist, rather than hiding the route', async function () { + await agent.get('members/custom_fields/company/').expectStatus(404); + }); + + it('404s the create endpoint', async function () { + await agent + .post('members/custom_fields/') + .body({ members_custom_fields: [{ name: 'Company', type: 'short_text' }] }) + .expectStatus(404); }); it('404s the reorder endpoint', async function () { @@ -2270,5 +2300,16 @@ describe('Member Custom Fields Admin API', function () { .body({ members_custom_fields: [{ key: 'company' }] }) .expectStatus(404); }); + + it('404s the edit endpoint', async function () { + await agent + .put('members/custom_fields/company/') + .body({ members_custom_fields: [{ name: 'Employer' }] }) + .expectStatus(404); + }); + + it('404s the delete endpoint', async function () { + await agent.delete('members/custom_fields/company/').expectStatus(404); + }); }); }); diff --git a/ghost/core/test/e2e-api/admin/tiers-checkout-config.test.ts b/ghost/core/test/e2e-api/admin/tiers-checkout-config.test.ts new file mode 100644 index 00000000000..a4f3e99f54b --- /dev/null +++ b/ghost/core/test/e2e-api/admin/tiers-checkout-config.test.ts @@ -0,0 +1,1063 @@ +import assert from 'node:assert/strict'; + +const { agentProvider, fixtureManager, mockManager } = require('../../utils/e2e-framework'); +const models = require('../../../core/server/models'); + +describe('Tier Checkout Admin API', function () { + let agent: { + get: (_url: string) => any; + put: (_url: string) => any; + post: (_url: string) => any; + delete: (_url: string) => any; + loginAsOwner: () => Promise; + }; + let tierId: string; + + async function createField(field: { name: string; type?: string }) { + const { body } = await agent + .post('members/custom_fields/') + .body({ members_custom_fields: [{ type: 'short_text', ...field }] }) + .expectStatus(201); + return body.members_custom_fields[0]; + } + + async function setStatus(key: string, status: 'active' | 'archived') { + await agent + .put(`members/custom_fields/${key}/`) + .body({ members_custom_fields: [{ status }] }) + .expectStatus(200); + } + + async function setCheckout(config: Record, status = 200) { + const { body } = await agent + .put(`tiers/${tierId}/checkout_config/`) + .body({ tiers_checkout_config: [config] }) + .expectStatus(status); + return body; + } + + async function readCheckout() { + const { body } = await agent.get(`tiers/${tierId}/checkout_config/`).expectStatus(200); + return body.tiers_checkout_config[0]; + } + + const shipping = (over: Record = {}) => ({ + shipping: { + collect: true, + allowed_countries: ['GB'], + name: { custom_field_key: 'shipping_name' }, + address: { custom_field_key: 'delivery_address' }, + ...over, + }, + }); + + beforeAll(async function () { + agent = await agentProvider.getAdminAPIAgent(); + await fixtureManager.init('users'); + await agent.loginAsOwner(); + + const { body } = await agent.get('tiers/?limit=1&filter=type:paid').expectStatus(200); + tierId = body.tiers[0].id; + }); + + beforeEach(function () { + mockManager.mockLabsEnabled('membersCustomFields'); + }); + + afterEach(async function () { + mockManager.restore(); + await models.Base.knex('products_checkout_fields').del(); + await models.Base.knex('products_checkout_config').del(); + await models.Base.knex('members_custom_field_bindings').del(); + await models.Base.knex('members_custom_fields').del(); + // The history outlives the fields it describes, which is the point of it — but across + // tests in one file it would leave each one reading the ones before. + await models.Base.knex('actions').where('resource_type', 'member_custom_field').del(); + await models.Base.knex('products').where('id', 'ffffffffffffffffffffffff').del(); + }); + + describe('Questions the checkout asks', function () { + it('starts with nothing configured', async function () { + assert.deepEqual(await readCheckout(), { tier_id: tierId, custom_fields: [] }); + }); + + it('keeps the questions in the order they were given', async function () { + const size = await createField({ name: 'T-shirt size' }); + const diet = await createField({ name: 'Dietary requirements' }); + + await setCheckout({ custom_fields: [{ key: diet.key }, { key: size.key }] }); + + const { custom_fields: questions } = await readCheckout(); + assert.deepEqual( + questions.map((question: { key: string }) => question.key), + ['dietary_requirements', 't_shirt_size'], + ); + }); + + // A question is optional unless a publisher chooses otherwise: a required question + // at the payment step costs conversion. + it("asks optionally, and under the field's own name, unless told otherwise", async function () { + const size = await createField({ name: 'T-shirt size' }); + + await setCheckout({ custom_fields: [{ key: size.key }] }); + assert.deepEqual(await readCheckout(), { + tier_id: tierId, + custom_fields: [{ key: 't_shirt_size', label: null, optional: true }], + }); + + await setCheckout({ + custom_fields: [{ key: size.key, label: 'Which size?', optional: false }], + }); + assert.deepEqual((await readCheckout()).custom_fields, [ + { key: 't_shirt_size', label: 'Which size?', optional: false }, + ]); + }); + + it('states the whole list, so a question left out is no longer asked', async function () { + const size = await createField({ name: 'T-shirt size' }); + const diet = await createField({ name: 'Dietary requirements' }); + await setCheckout({ custom_fields: [{ key: size.key }, { key: diet.key }] }); + + await setCheckout({ custom_fields: [{ key: diet.key }] }); + assert.deepEqual( + (await readCheckout()).custom_fields.map((question: { key: string }) => question.key), + ['dietary_requirements'], + ); + }); + + it('refuses more questions than the processor will render', async function () { + const keys = []; + for (const name of ['One', 'Two', 'Three', 'Four']) { + keys.push((await createField({ name })).key); + } + + const body = await setCheckout({ custom_fields: keys.map((key) => ({ key })) }, 422); + assert.match(body.errors[0].context, /at most 3 questions/); + }); + + // A publisher can name a field something no processor will render as a label, so + // the label exists to be shorter than the name. + it('refuses a question the processor could not label', async function () { + const long = await createField({ name: `Tell us ${'x'.repeat(60)}` }); + + const body = await setCheckout({ custom_fields: [{ key: long.key }] }, 422); + assert.match(body.errors[0].context, /at most 50 characters/); + + await setCheckout({ custom_fields: [{ key: long.key, label: 'Tell us more' }] }); + assert.equal((await readCheckout()).custom_fields[0].label, 'Tell us more'); + }); + + it('refuses a field type the processor cannot ask for', async function () { + // Named so its key is not one of the things this checkout collects for itself, + // which is refused earlier and for a different reason. + const address = await createField({ name: 'Postal address', type: 'address' }); + + const body = await setCheckout({ custom_fields: [{ key: address.key }] }, 422); + assert.match(body.errors[0].context, /cannot be asked for at checkout/); + }); + + it('refuses the same field twice', async function () { + const size = await createField({ name: 'T-shirt size' }); + + const body = await setCheckout( + { custom_fields: [{ key: size.key }, { key: size.key }] }, + 422, + ); + assert.match(body.errors[0].context, /already asks/); + }); + + it('refuses a field the site does not have', async function () { + const body = await setCheckout({ custom_fields: [{ key: 'no_such_field' }] }, 422); + assert.match(body.errors[0].context, /Unknown custom field/); + }); + + // A question's port is the field's own key, so a field keyed like something this + // checkout already collects would want a port that is taken. Refused in its own + // words, rather than left to the unique index to report unreadably. + it('refuses a field keyed like something the checkout collects itself', async function () { + const phone = await createField({ name: 'Phone' }); + assert.equal(phone.key, 'phone', 'the name mints the key the phone port uses'); + + const body = await setCheckout({ custom_fields: [{ key: phone.key }] }, 422); + assert.match(body.errors[0].context, /cannot be asked at checkout/); + }); + }); + + describe('What the checkout collects for itself', function () { + // Collecting and choosing where it lands are one statement, because a publisher + // makes them as one choice: a checkbox and the field beside it. + it('collects a port and binds its destination in one write', async function () { + await createField({ name: 'Delivery address', type: 'address' }); + + await setCheckout(shipping({ allowed_countries: ['GB', 'ie'] })); + assert.deepEqual((await readCheckout()).shipping, { + collect: true, + // Uppercased on the way in, so one country is one value. + allowed_countries: ['GB', 'IE'], + // Nothing kept the recipient's name yet, so a field was made under that key. + name: { custom_field_key: 'shipping_name' }, + address: { custom_field_key: 'delivery_address' }, + }); + }); + + // Turning collection on is a publisher saying they want the data. Making them build a + // field for it first turns one checkbox into an errand, so naming a key the site does + // not keep yet makes it. + it('makes a field for a key the site does not keep yet', async function () { + await setCheckout({ + shipping: { + collect: true, + allowed_countries: ['GB'], + name: { custom_field_key: 'shipping_name' }, + address: { custom_field_key: 'shipping_address' }, + }, + }); + + assert.deepEqual((await readCheckout()).shipping, { + collect: true, + allowed_countries: ['GB'], + name: { custom_field_key: 'shipping_name' }, + address: { custom_field_key: 'shipping_address' }, + }); + + const { body } = await agent.get('members/custom_fields/').expectStatus(200); + assert.deepEqual( + body.members_custom_fields.map((field: { name: string; type: string }) => [ + field.name, + field.type, + ]), + [ + ['Shipping Name', 'short_text'], + ['Shipping Address', 'address'], + ], + 'and it is listed under the name the port supplies', + ); + }); + + // Turning collection off and on again is the ordinary case. A second "Shipping Address" + // beside the first would split one thing across two columns of every export. + it('binds the field it made rather than making another', async function () { + const collect = { + shipping: { + collect: true, + allowed_countries: ['GB'], + name: { custom_field_key: 'shipping_name' }, + address: { custom_field_key: 'shipping_address' }, + }, + }; + await setCheckout(collect); + await setCheckout({ shipping: { collect: false } }); + await setCheckout(collect); + + const { body } = await agent.get('members/custom_fields/').expectStatus(200); + assert.deepEqual( + body.members_custom_fields.map((field: { key: string }) => field.key), + ['shipping_name', 'shipping_address'], + ); + assert.equal( + (await readCheckout()).shipping.address.custom_field_key, + 'shipping_address', + 'and bound to it again', + ); + }); + + // Names are unique across the whole list, so the label a made field would be listed + // under can be taken by something else. Numbering past it would leave a publisher with + // two things called nearly the same, neither of them obviously the one collected into. + it('refuses to make a field whose label is already taken', async function () { + await createField({ name: 'Shipping Address', type: 'short_text' }); + + await setCheckout(shipping(), 422); + + const { body } = await agent.get('members/custom_fields/').expectStatus(200); + assert.deepEqual( + body.members_custom_fields.map((field: { key: string }) => field.key), + ['shipping_address'], + 'and nothing was made under a numbered label instead', + ); + }); + + // The key is the field's identity and the name is not, so renaming what Ghost made + // does not cost a publisher a second copy of it: the request names the same key and + // finds the same field. + it('binds the same field after a publisher renames it', async function () { + const collect = { + shipping: { + collect: true, + allowed_countries: ['GB'], + name: { custom_field_key: 'shipping_name' }, + address: { custom_field_key: 'shipping_address' }, + }, + }; + await setCheckout(collect); + await agent + .put('members/custom_fields/shipping_address/') + .body({ members_custom_fields: [{ name: 'Delivery address' }] }) + .expectStatus(200); + + await setCheckout({ shipping: { collect: false } }); + await setCheckout(collect); + + const { body } = await agent.get('members/custom_fields/').expectStatus(200); + assert.deepEqual( + body.members_custom_fields + .filter((field: { type: string }) => field.type === 'address') + .map((field: { key: string; name: string }) => [field.key, field.name]), + [['shipping_address', 'Delivery address']], + 'the renamed field is still the one, and there is only one', + ); + assert.equal((await readCheckout()).shipping.address.custom_field_key, 'shipping_address'); + }); + + // A field of the right type under the key that was named is that field, whoever made it. + it('collects into a field the publisher already keeps under that key', async function () { + const theirs = await createField({ name: 'Shipping Address', type: 'address' }); + assert.equal(theirs.key, 'shipping_address'); + + await setCheckout(shipping({ address: { custom_field_key: theirs.key } })); + + const { body } = await agent.get('members/custom_fields/').expectStatus(200); + assert.equal( + body.members_custom_fields.filter((field: { type: string }) => field.type === 'address') + .length, + 1, + 'nothing was made beside it', + ); + assert.equal((await readCheckout()).shipping.address.custom_field_key, theirs.key); + }); + + // Binding to it would succeed and collect nothing until someone restored it, which the + // publisher who asked for the collection has no way to see. Restoring the field is one + // step; working out why a checkout stopped asking is not. + it('refuses a destination the publisher has archived', async function () { + await createField({ name: 'Delivery address', type: 'address' }); + await setStatus('delivery_address', 'archived'); + + const body = await setCheckout(shipping(), 422); + assert.match(body.errors[0].context, /archived/); + + assert.deepEqual( + await models.Base.knex('members_custom_field_bindings').select(), + [], + 'and nothing was bound', + ); + }); + + // A field appearing in a publisher's list without them creating it is exactly the case + // the history is for, and it is theirs from that point on: the same "added" entry a + // field they typed gets, attributed to whoever turned the collection on. + it('records the fields it made in the history', async function () { + await setCheckout({ + shipping: { + collect: true, + allowed_countries: ['GB'], + name: { custom_field_key: 'shipping_name' }, + address: { custom_field_key: 'shipping_address' }, + }, + }); + + const { body } = await agent + .get('actions/?filter=resource_type:member_custom_field&limit=all') + .expectStatus(200); + // The name rides in the action's context; resource_id holds the row id. Sorted + // because both were written in the same statement, so their order is not meaningful. + const provisioned = body.actions + .map((action: { event: string; actor_type: string; context: unknown }) => ({ + event: action.event, + actorType: action.actor_type, + ...(typeof action.context === 'string' ? JSON.parse(action.context) : action.context), + })) + .sort((a: { key: string }, b: { key: string }) => a.key.localeCompare(b.key)); + + assert.deepEqual( + provisioned.map((action: { event: string; primary_name: string; actorType: string }) => [ + action.event, + action.primary_name, + action.actorType, + ]), + [ + ['added', 'Shipping Address', 'user'], + ['added', 'Shipping Name', 'user'], + ], + ); + }); + + // The phone number is the port whose name is not the publisher's word for it: Stripe + // calls what it returns `phone`, and a field made for it is listed as Shipping Phone. + // So the two are asserted together, because a declaration that muddled them would list + // a publisher's field under the processor's own vocabulary. + it('makes a field for the phone number under its own name', async function () { + await setCheckout({ phone: { collect: true, custom_field_key: 'shipping_phone' } }); + + const { body } = await agent.get('members/custom_fields/').expectStatus(200); + assert.deepEqual( + body.members_custom_fields.map((field: { key: string; name: string; type: string }) => [ + field.key, + field.name, + field.type, + ]), + [['shipping_phone', 'Shipping Phone', 'short_text']], + ); + assert.equal((await readCheckout()).phone.custom_field_key, 'shipping_phone'); + }); + + // A publisher stated one thing, so it either happened or it did not. Every custom field + // a request names is now checked before anything at all is written, so a request that + // will be refused never gets as far as creating one. This test holds that line: were + // the checking ever to move back inside the writing, a refused request could leave a + // field behind that nobody asked for, sitting in the publisher's list, made by a change + // they were told did not happen. + it('leaves nothing behind when a later part of the same request is refused', async function () { + // An address cannot be collected into short_text, so naming this one is refused — + // but only after the recipient's name has already been given a field of its own. + const theirs = await createField({ name: 'Delivery address', type: 'short_text' }); + + await setCheckout(shipping({ address: { custom_field_key: theirs.key } }), 422); + + const { body } = await agent.get('members/custom_fields/').expectStatus(200); + assert.deepEqual( + body.members_custom_fields.map((field: { key: string }) => field.key), + [theirs.key], + 'the field made for the recipient went back with the request', + ); + assert.deepEqual( + await models.Base.knex('members_custom_field_bindings').select(), + [], + 'and so did the binding made before it', + ); + }); + + it('makes nothing when both destinations already exist', async function () { + await createField({ name: 'Recipient', type: 'short_text' }); + await createField({ name: 'Delivery address', type: 'address' }); + + await setCheckout(shipping({ name: { custom_field_key: 'recipient' } })); + + const { body } = await agent.get('members/custom_fields/').expectStatus(200); + assert.deepEqual( + body.members_custom_fields.map((field: { key: string }) => field.key), + ['recipient', 'delivery_address'], + ); + }); + + // Turning it on is one decision; where a parcel goes is not something Ghost can guess. + it('still refuses to collect an address without a country to deliver to', async function () { + const body = await setCheckout({ shipping: { collect: true } }, 422); + assert.match(body.errors[0].context, /at least one country/); + }); + + // Ghost keeps no convention about where a collected value belongs, so a request that + // collects without saying where is missing half the decision rather than deferring it. + it('refuses to collect without saying where the value lands', async function () { + await createField({ name: 'Delivery address', type: 'address' }); + + const body = await setCheckout(shipping({ name: undefined }), 422); + assert.match(body.errors[0].context, /which custom field this is collected into/); + + await setCheckout(shipping({ address: { custom_field_key: '' } }), 422); + await setCheckout({ phone: { collect: true } }, 422); + + assert.deepEqual(await models.Base.knex('members_custom_field_bindings').select(), []); + }); + + // The destination is site-wide, so it is one answer however many tiers collect it. + it('reports the destination against the thing collected', async function () { + await createField({ name: 'Delivery address', type: 'address' }); + await setCheckout(shipping()); + + assert.equal((await readCheckout()).shipping.address.custom_field_key, 'delivery_address'); + }); + + // An address form cannot be rendered without a country list. Stripe's reference + // calls it optional, but an empty collection object form-encodes to nothing, so a + // request built that way never asks Stripe to collect anything at all. + it('refuses to collect an address without countries to collect it in', async function () { + await createField({ name: 'Delivery address', type: 'address' }); + + const body = await setCheckout(shipping({ allowed_countries: undefined }), 422); + assert.match(body.errors[0].context, /at least one country/); + }); + + it('refuses a country code that is not one', async function () { + await createField({ name: 'Delivery address', type: 'address' }); + + const body = await setCheckout(shipping({ allowed_countries: ['IRL'] }), 422); + assert.match(body.errors[0].context, /2-letter country code/); + }); + + it('refuses a destination whose type is not what the port supplies', async function () { + await createField({ name: 'Delivery notes', type: 'short_text' }); + + const body = await setCheckout( + shipping({ address: { custom_field_key: 'delivery_notes' } }), + 422, + ); + assert.match(body.errors[0].context, /address field/); + }); + + // A key is stated rather than derived from a name now, so nothing has put it into + // the shape Ghost's own keys take, and a request that gets it wrong is refused + // rather than written into the column as given. + it('makes one field when two ports name the same key that does not exist yet', async function () { + await setCheckout({ + shipping: { + collect: true, + allowed_countries: ['GB'], + name: { custom_field_key: 'contact' }, + address: { custom_field_key: 'delivery' }, + }, + phone: { collect: true, custom_field_key: 'contact' }, + }); + + const { body } = await agent + .get('members/custom_fields/?filter=status:[active,archived]') + .expectStatus(200); + const named = body.members_custom_fields.filter( + (field: { key: string }) => field.key === 'contact', + ); + assert.equal(named.length, 1, 'the key both ports named was made once'); + + const { body: read } = await agent.get(`tiers/${tierId}/checkout_config/`).expectStatus(200); + const [config] = read.tiers_checkout_config; + assert.equal(config.shipping.name.custom_field_key, 'contact'); + assert.equal(config.phone.custom_field_key, 'contact'); + }); + + it('refuses a key of a shape nothing could be keyed', async function () { + const body = await setCheckout( + { phone: { collect: true, custom_field_key: 'Shipping Phone' } }, + 422, + ); + assert.match(body.errors[0].context, /lowercase letters, numbers and underscores/); + + assert.deepEqual(await models.Base.knex('members_custom_fields').select(), []); + }); + + // A key is a property name on the plain object a member's values travel as, so one + // naming something every object already has reads back as inherited rather than + // absent. Both of these are well-formed keys, which is exactly why the format check + // is not enough on its own. + it('refuses a key that names something every object already has', async function () { + for (const key of ['constructor', '__proto__']) { + const body = await setCheckout({ phone: { collect: true, custom_field_key: key } }, 422); + assert.match(body.errors[0].context, /cannot be used as a custom field key/); + } + + assert.deepEqual(await models.Base.knex('members_custom_fields').select(), []); + }); + + // Minting holds back room for a numbering suffix; a stated key is written as given, + // so what it has to fit in is the whole column and nothing less. + it('refuses a key longer than the column that holds it', async function () { + const body = await setCheckout( + { phone: { collect: true, custom_field_key: 'a'.repeat(192) } }, + 422, + ); + assert.match(body.errors[0].context, /at most 191 characters/); + + assert.deepEqual(await models.Base.knex('members_custom_fields').select(), []); + }); + + // Which kinds of thing exist is the request schema's to state, so naming one that + // does not is a malformed body rather than a lookup that came back empty. + it('refuses a kind of thing nothing can collect', async function () { + await createField({ name: 'Delivery address', type: 'address' }); + await setCheckout( + { inside_leg: { collect: true, custom_field_key: 'delivery_address' } }, + 422, + ); + }); + }); + + describe('Naming one list and not the other', function () { + // A client that knows about the questions must not erase the collection by staying + // silent about it. + it('leaves a list the request does not name alone', async function () { + const size = await createField({ name: 'T-shirt size' }); + await createField({ name: 'Delivery address', type: 'address' }); + + await setCheckout({ custom_fields: [{ key: size.key }] }); + await setCheckout(shipping()); + + const config = await readCheckout(); + assert.deepEqual( + config.custom_fields.map((question: { key: string }) => question.key), + ['t_shirt_size'], + ); + assert.deepEqual(config.shipping, { + collect: true, + allowed_countries: ['GB'], + name: { custom_field_key: 'shipping_name' }, + address: { custom_field_key: 'delivery_address' }, + }); + }); + + // Every collectable thing shares one row, so leaving one alone is a property of + // the write rather than of the storage. + it('leaves a collectable thing the request does not name alone', async function () { + await createField({ name: 'Delivery address', type: 'address' }); + + await setCheckout(shipping()); + await setCheckout({ tax_number: { collect: true } }); + + const config = await readCheckout(); + assert.equal(config.shipping.collect, true); + assert.deepEqual(config.shipping.allowed_countries, ['GB']); + assert.equal(config.tax_number.collect, true); + }); + + // A tax number is collected and not kept, so it is stated as an option beside the + // countries rather than as a binding, and nothing is made to put it in. + it('collects a tax number without keeping it anywhere', async function () { + await setCheckout({ tax_number: { collect: true } }); + + assert.deepEqual((await readCheckout()).tax_number, { collect: true }); + + const { body } = await agent.get('members/custom_fields/').expectStatus(200); + assert.deepEqual(body.members_custom_fields, [], 'no field was made for it'); + + assert.deepEqual( + await models.Base.knex('members_custom_field_bindings').select(), + [], + 'and nothing was bound', + ); + }); + + it('names nowhere to keep a tax number', async function () { + await setCheckout({ tax_number: { collect: true, custom_field_key: 'anything' } }, 422); + }); + + // Turning collection back on is a fresh statement of where a publisher delivers, not a + // resumption of the last one: the countries have to be given again, so a tier cannot + // quietly start delivering somewhere the publisher has since stopped. + it('stops collecting, and asks where to deliver again before it will resume', async function () { + await createField({ name: 'Delivery address', type: 'address' }); + + await setCheckout(shipping()); + await setCheckout({ shipping: { collect: false } }); + assert.equal((await readCheckout()).shipping, undefined); + + const body = await setCheckout(shipping({ allowed_countries: undefined }), 422); + assert.match(body.errors[0].context, /at least one country/); + }); + }); + + describe('When a field stops being usable', function () { + // Refused at write, tolerated at read. Archiving is reversible, so the question + // waits for the field to come back rather than being torn out. + it('keeps a question whose field was archived', async function () { + const size = await createField({ name: 'T-shirt size' }); + await setCheckout({ custom_fields: [{ key: size.key }] }); + + await setStatus(size.key, 'archived'); + assert.deepEqual( + (await readCheckout()).custom_fields.map((question: { key: string }) => question.key), + ['t_shirt_size'], + ); + + await setStatus(size.key, 'active'); + assert.deepEqual( + (await readCheckout()).custom_fields.map((question: { key: string }) => question.key), + ['t_shirt_size'], + ); + }); + + // Deleting is irreversible and already gated behind archiving, so the question goes + // with the field rather than pointing at nothing. + it('drops a question whose field was permanently deleted', async function () { + const size = await createField({ name: 'T-shirt size' }); + await setCheckout({ custom_fields: [{ key: size.key }] }); + + await setStatus(size.key, 'archived'); + await agent.delete(`members/custom_fields/${size.key}/`).expectStatus(204); + + assert.deepEqual((await readCheckout()).custom_fields, []); + }); + }); + + describe('Browsing every tier', function () { + it('returns the tiers that ask for something, so a list needs one request', async function () { + const size = await createField({ name: 'T-shirt size' }); + await setCheckout({ custom_fields: [{ key: size.key }] }); + + const { body } = await agent.get('tiers/checkout_config/').expectStatus(200); + assert.deepEqual(body.tiers_checkout_config, [ + { + tier_id: tierId, + custom_fields: [{ key: 't_shirt_size', label: null, optional: true }], + }, + ]); + }); + + it('still lists a tier that turned everything back off, collecting nothing', async function () { + await createField({ name: 'Delivery address', type: 'address' }); + await setCheckout(shipping()); + await setCheckout({ shipping: { collect: false } }); + + const { body } = await agent.get('tiers/checkout_config/').expectStatus(200); + assert.deepEqual(body.tiers_checkout_config, [{ tier_id: tierId, custom_fields: [] }]); + }); + }); + + // The litmus test for the data model: if someone administers this with plain SQL, what + // states can they reach, and does anything break when they do. Each of these reaches a + // state the API refuses to create, and asserts the system stays sane in it. + describe('When the database is edited by hand', function () { + async function collectShipping() { + await createField({ name: 'Delivery address', type: 'address' }); + await setCheckout(shipping()); + } + + // The options row is the other half that could disagree. It holds no opinion about + // whether anything is collected, so one left behind changes nothing. + it('collects nothing when only the options row is left', async function () { + await collectShipping(); + await models.Base.knex('members_custom_field_bindings').del(); + + const [options] = await models.Base.knex('products_checkout_config').select(); + assert.ok(options, 'the options row outlived the collection'); + assert.deepEqual(await readCheckout(), { tier_id: tierId, custom_fields: [] }); + }); + + it('takes the binding with the field when a definition is deleted', async function () { + await collectShipping(); + // Deleting takes archiving first, which is the API's own rule; the cascade below + // is the database's, and it is what a hand-written DELETE would hit too. + await setStatus('delivery_address', 'archived'); + await agent.delete('members/custom_fields/delivery_address/').expectStatus(204); + + // Cascade, so no binding can name a field that is gone. The recipient's name is + // kept elsewhere, so its binding survives — but a recipient with nowhere to send + // the parcel is not a delivery, so the tier reports collecting nothing. + assert.deepEqual( + (await models.Base.knex('members_custom_field_bindings').select()).map( + (row: { port: string }) => row.port, + ), + ['shipping_name'], + ); + assert.equal((await readCheckout()).shipping, undefined); + }); + + it('keeps the binding when a definition is archived, and resumes on restore', async function () { + await collectShipping(); + await setStatus('delivery_address', 'archived'); + + // Archiving is reversible, so the binding waits rather than being torn out, and + // the tier goes on reporting where the value goes. + const [binding] = await models.Base.knex('members_custom_field_bindings') + .where('port', 'shipping_address') + .select(); + assert.equal(binding.custom_field_key, 'delivery_address'); + assert.equal((await readCheckout()).shipping.address.custom_field_key, 'delivery_address'); + + await setStatus('delivery_address', 'active'); + assert.equal((await readCheckout()).shipping.address.custom_field_key, 'delivery_address'); + }); + }); + + // Asking for a field and collecting into it are two writers aimed at one destination: + // the checkout shows the question and the widget together, and whichever the webhook + // writes second is what the field holds. Allowed rather than refused, because a + // destination is not exclusive — another tier can already be writing into the same + // field — so refusing it here would only forbid the one arrangement Ghost can see. + describe('One field, asked for and collected into', function () { + it('accepts both, in one statement or in two', async function () { + const field = await createField({ name: 'Contact' }); + + await setCheckout({ + custom_fields: [{ key: field.key }], + phone: { collect: true, custom_field_key: field.key }, + }); + + const config = await readCheckout(); + assert.deepEqual( + config.custom_fields.map((question: { key: string }) => question.key), + [field.key], + ); + assert.equal(config.phone.custom_field_key, field.key); + }); + }); + + // A destination is stated, not remembered. The request says where every value it + // collects lands, so the same statement made twice reaches the same field and two tiers + // making different statements keep them apart. + describe('A destination the publisher chose', function () { + it('is still theirs after turning collection off and on', async function () { + await createField({ name: 'Delivery address', type: 'address' }); + await setCheckout(shipping()); + + await setCheckout({ shipping: { collect: false } }); + await setCheckout(shipping()); + + assert.equal((await readCheckout()).shipping.address.custom_field_key, 'delivery_address'); + }); + + // Another tier binding the same port first says nothing about this one: each states + // its own destination, and neither is worked out from what the other settled on. + it('is not decided by another tier that bound the port first', async function () { + const [existing] = await models.Base.knex('products').where('id', tierId); + const secondId = 'ffffffffffffffffffffffff'; + await models.Base.knex('products').insert({ + ...existing, + id: secondId, + name: 'Digital', + slug: 'digital-second', + }); + + await createField({ name: 'Delivery address', type: 'address' }); + await createField({ name: 'Digital address', type: 'address' }); + + // The other tier goes first, so its binding is the older one. + await agent + .put(`tiers/${secondId}/checkout_config/`) + .body({ + tiers_checkout_config: [shipping({ address: { custom_field_key: 'digital_address' } })], + }) + .expectStatus(200); + + await setCheckout(shipping()); + + assert.equal( + (await readCheckout()).shipping.address.custom_field_key, + 'delivery_address', + 'this tier kept its own destination', + ); + const { body } = await agent.get(`tiers/${secondId}/checkout_config/`).expectStatus(200); + assert.equal( + body.tiers_checkout_config[0].shipping.address.custom_field_key, + 'digital_address', + 'and the other kept its own', + ); + }); + }); + + // A binding is the collecting: there is one and the tier collects, or there is none and + // it does not. What the database does to a row nobody is looking at is the whole of the + // rest of it. + describe('When collection stops', function () { + const bindingsFor = (port: string) => + models.Base.knex('members_custom_field_bindings').where('port', port).select(); + + it('forgets the binding rather than keeping it switched off', async function () { + await createField({ name: 'Delivery address', type: 'address' }); + await setCheckout(shipping()); + + await setCheckout({ shipping: { collect: false } }); + + assert.deepEqual(await models.Base.knex('members_custom_field_bindings').select(), []); + assert.deepEqual(await readCheckout(), { tier_id: tierId, custom_fields: [] }); + }); + + // Nothing is left pointing at it, so the same request that made it once makes it again. + it('makes the field again once it has been deleted', async function () { + await setCheckout({ + shipping: { + collect: true, + allowed_countries: ['GB'], + name: { custom_field_key: 'shipping_name' }, + address: { custom_field_key: 'shipping_address' }, + }, + }); + await setCheckout({ shipping: { collect: false } }); + await setStatus('shipping_address', 'archived'); + await agent.delete('members/custom_fields/shipping_address/').expectStatus(204); + + await setCheckout({ + shipping: { + collect: true, + allowed_countries: ['GB'], + name: { custom_field_key: 'shipping_name' }, + address: { custom_field_key: 'shipping_address' }, + }, + }); + + assert.equal((await readCheckout()).shipping.address.custom_field_key, 'shipping_address'); + }); + + // Deleting the tier takes its bindings, so nothing it configured outlives it. + it('takes a binding with its tier', async function () { + const [existing] = await models.Base.knex('products').where('id', tierId); + const secondId = 'ffffffffffffffffffffffff'; + await models.Base.knex('products').insert({ + ...existing, + id: secondId, + name: 'Digital', + slug: 'digital-second', + }); + + await agent + .put(`tiers/${secondId}/checkout_config/`) + .body({ + tiers_checkout_config: [ + { + shipping: { + collect: true, + allowed_countries: ['GB'], + name: { custom_field_key: 'shipping_name' }, + address: { custom_field_key: 'shipping_address' }, + }, + }, + ], + }) + .expectStatus(200); + + await models.Base.knex('products').where('id', secondId).del(); + + assert.deepEqual(await bindingsFor('shipping_address'), []); + }); + }); + + // Archiving a bound field stops the tier that bound it, silently and by design. A second + // tier asking to collect into the same field is a publisher walking into that state + // rather than out of it, so it is refused where the first tier's collection is left + // alone: one of them chose this before the archiving, and the other is choosing it after. + describe('A destination archived before another tier wants it', function () { + it('is refused, rather than bound to collect nothing', async function () { + const [existing] = await models.Base.knex('products').where('id', tierId); + const secondId = 'ffffffffffffffffffffffff'; + await models.Base.knex('products').insert({ + ...existing, + id: secondId, + name: 'Digital', + slug: 'digital-second', + }); + + await createField({ name: 'Delivery address', type: 'address' }); + await setCheckout(shipping()); + await setStatus('delivery_address', 'archived'); + + await agent + .put(`tiers/${secondId}/checkout_config/`) + .body({ tiers_checkout_config: [shipping()] }) + .expectStatus(422); + + assert.deepEqual( + await models.Base.knex('members_custom_field_bindings') + .where('product_id', secondId) + .select(), + [], + ); + + // The tier that bound it first is untouched: it still reports where the address + // goes, and restoring the field is what starts it collecting again. + assert.equal((await readCheckout()).shipping.address.custom_field_key, 'delivery_address'); + + await setStatus('delivery_address', 'active'); + await agent + .put(`tiers/${secondId}/checkout_config/`) + .body({ tiers_checkout_config: [shipping()] }) + .expectStatus(200); + + const { body: fields } = await agent + .get('members/custom_fields/?filter=status:[active,archived]') + .expectStatus(200); + assert.deepEqual( + fields.members_custom_fields + .filter((field: { type: string }) => field.type === 'address') + .map((field: { key: string }) => field.key), + ['delivery_address'], + 'and no second address field was made along the way', + ); + }); + }); + + // A destination belongs to a tier, not to the site: the binding is keyed by tier and + // port, so each tier names its own and one turning collection off says nothing about + // any other. What these pin is that the two stay independent in both directions. + describe('Two tiers, one destination', function () { + it('leaves another tier collecting when this one stops', async function () { + await createField({ name: 'Delivery address', type: 'address' }); + + // Copied from the tier the suite already has, because what is being tested is + // what happens between two of them rather than how one is made. + const [existing] = await models.Base.knex('products').where('id', tierId); + const secondId = 'ffffffffffffffffffffffff'; + await models.Base.knex('products').insert({ + ...existing, + id: secondId, + name: 'Digital', + slug: 'digital-second', + }); + + await setCheckout(shipping()); + + // The digital tier does not ship anything, so a publisher turns it off there. + await agent + .put(`tiers/${secondId}/checkout_config/`) + .body({ tiers_checkout_config: [{ shipping: { collect: false } }] }) + .expectStatus(200); + + // The print tier never changed, so it must still be collecting into its field. + const config = await readCheckout(); + assert.equal(config.shipping.collect, true); + assert.equal(config.shipping.address.custom_field_key, 'delivery_address'); + }); + + // The other half of the same rule. The field outlives every binding into it, because + // it holds everything collected so far, and stating it again reaches the same one. + it('keeps the field once no tier collects into it', async function () { + await createField({ name: 'Delivery address', type: 'address' }); + await setCheckout(shipping()); + + await setCheckout({ shipping: { collect: false } }); + + // Off, so the tier reports nothing and the checkout asks for nothing. + assert.deepEqual(await models.Base.knex('members_custom_field_bindings').select(), []); + assert.deepEqual(await readCheckout(), { tier_id: tierId, custom_fields: [] }); + + const { body } = await agent.get('members/custom_fields/').expectStatus(200); + assert.deepEqual( + body.members_custom_fields.map((field: { key: string }) => field.key), + ['delivery_address', 'shipping_name'], + 'the fields it collected into are still there', + ); + + // And on again, it is the same field rather than a second one beside it. + await setCheckout(shipping()); + assert.equal((await readCheckout()).shipping.address.custom_field_key, 'delivery_address'); + }); + }); + + // A tier that asks for nothing and a tier that does not exist read the same off the + // join, and answering an empty configuration for the second would tell a client the + // tier is there. + describe('A tier that does not exist', function () { + it('404s rather than reading as unconfigured', async function () { + await agent.get('tiers/6a8dbb6a2668becb3f92f000/checkout_config/').expectStatus(404); + }); + + // Every table a write touches references the tier, so the tier is checked before any + // of them is written. Otherwise the write reaches a foreign key and answers with a + // database error, where reading the same tier answers 404. + it('404s a write, rather than failing against a foreign key', async function () { + await agent + .put('tiers/6a8dbb6a2668becb3f92f000/checkout_config/') + .body({ tiers_checkout_config: [{ tax_number: { collect: true } }] }) + .expectStatus(404); + }); + }); + + describe('Flag', function () { + // Reading is open, because a tier that collects nothing reads the same either way and + // the checkout has to build its session whatever the flag says. Configuring is not. + it('still reads with the flag off', async function () { + mockManager.mockLabsDisabled('membersCustomFields'); + const { body } = await agent.get(`tiers/${tierId}/checkout_config/`).expectStatus(200); + assert.deepEqual(body.tiers_checkout_config[0].custom_fields, []); + }); + + it('cannot be configured with the flag off', async function () { + mockManager.mockLabsDisabled('membersCustomFields'); + await agent + .put(`tiers/${tierId}/checkout_config/`) + .body({ tiers_checkout_config: [{ custom_fields: [] }] }) + .expectStatus(404); + }); + + // The tier resource is generally available, so this concept must not appear on it. + it('adds nothing to the tier itself', async function () { + const { body } = await agent.get(`tiers/${tierId}/`).expectStatus(200); + assert.equal(body.tiers[0].checkout, undefined); + }); + }); +}); diff --git a/ghost/core/test/e2e-api/members/gift-subscriptions.test.js b/ghost/core/test/e2e-api/members/gift-subscriptions.test.js index cfe13fe471e..35ed0b70e23 100644 --- a/ghost/core/test/e2e-api/members/gift-subscriptions.test.js +++ b/ghost/core/test/e2e-api/members/gift-subscriptions.test.js @@ -856,6 +856,12 @@ describe('Gift Subscriptions', function () { recipient_name: 'Taylor', personal_message: 'Enjoy!', }); + await models.GiftDelivery.add({ + gift_id: gift.id, + recipient_email: 'taylor@example.com', + status: 'failed', + outcome: 'permanent_failed', + }); const { body } = await membersAgent .get(`/api/gifts/${gift.get('token')}/redeem/`) @@ -869,6 +875,7 @@ describe('Gift Subscriptions', function () { assert.equal(body.gifts[0].amount, 5000); assert.equal(body.gifts[0].buyer_name, 'Jamie'); assert.equal(body.gifts[0].recipient_name, 'Taylor'); + assert.equal(body.gifts[0].recipient_email, 'taylor@example.com'); assert.equal(body.gifts[0].message, 'Enjoy!'); assert.equal(body.gifts[0].expires_at, new Date(gift.get('expires_at')).toISOString()); assert.deepEqual(body.gifts[0].tier, { @@ -881,7 +888,6 @@ describe('Gift Subscriptions', function () { .map((item) => item.name), }); assert.equal(body.gifts[0].buyer_email, undefined); - assert.equal(body.gifts[0].recipient_email, undefined); assert.equal(body.gifts[0].delivery_status, undefined); assert.equal(body.gifts[0].redeemed_at, undefined); assert.equal(body.gifts[0].status, undefined); @@ -897,6 +903,7 @@ describe('Gift Subscriptions', function () { const { body } = await agent.get(`/api/gifts/${gift.get('token')}/redeem/`).expectStatus(200); assert.equal(body.gifts[0].token, gift.get('token')); + assert.equal(body.gifts[0].recipient_email, null); }); it('returns 404 when the gift token does not exist', async function () { @@ -975,6 +982,11 @@ describe('Gift Subscriptions', function () { const agent = membersAgent.duplicate(); const email = `gift-post-free-${giftSequence + 1}@example.com`; const gift = await createGift(); + await models.GiftDelivery.add({ + gift_id: gift.id, + recipient_email: 'recipient@example.com', + status: 'pending', + }); await agent.loginAs(email); @@ -990,6 +1002,7 @@ describe('Gift Subscriptions', function () { const member = await models.Member.findOne({ email }, { require: true }); assert.equal(body.gifts[0].token, gift.get('token')); + assert.equal(body.gifts[0].recipient_email, 'recipient@example.com'); assert.equal(body.gifts[0].status, undefined); assert.ok(body.gifts[0].consumes_at); assert.equal(member.get('status'), 'gift'); diff --git a/ghost/core/test/integration/exporter/exporter.test.js b/ghost/core/test/integration/exporter/exporter.test.js index bb7bf345138..03295b5715e 100644 --- a/ghost/core/test/integration/exporter/exporter.test.js +++ b/ghost/core/test/integration/exporter/exporter.test.js @@ -60,6 +60,7 @@ describe('Exporter', function () { 'members_click_events', 'members_created_events', 'members_current_subscription', + 'members_custom_field_bindings', 'members_custom_field_values', 'members_custom_fields', 'members_email_change_events', @@ -97,6 +98,8 @@ describe('Exporter', function () { 'posts_tags', 'products', 'products_benefits', + 'products_checkout_config', + 'products_checkout_fields', 'recommendation_click_events', 'recommendation_subscribe_events', 'recommendations', diff --git a/ghost/core/test/integration/services/gifts/gift-delivery-bookshelf-repository.test.ts b/ghost/core/test/integration/services/gifts/gift-delivery-bookshelf-repository.test.ts index c68445dea3d..be4741dd030 100644 --- a/ghost/core/test/integration/services/gifts/gift-delivery-bookshelf-repository.test.ts +++ b/ghost/core/test/integration/services/gifts/gift-delivery-bookshelf-repository.test.ts @@ -117,6 +117,16 @@ describe('GiftDeliveryBookshelfRepository (integration)', function () { return { gift, delivery }; } + it('finds an email delivery by gift token', async function () { + const { gift, delivery } = await createPendingEmailGift({ deliveryStatus: 'failed' }); + + assert.equal( + (await deliveryRepository.getByGiftToken(gift.get('token')))?.recipientEmail, + delivery.get('recipient_email'), + ); + assert.equal(await deliveryRepository.getByGiftToken('missing-token'), null); + }); + it('allows exactly one concurrent caller to start a pending delivery', async function () { const startedAt = new Date(); startedAt.setMilliseconds(0); diff --git a/ghost/core/test/unit/server/data/schema/integrity.test.js b/ghost/core/test/unit/server/data/schema/integrity.test.js index 9acbc4aa6dc..0cf5224feaf 100644 --- a/ghost/core/test/unit/server/data/schema/integrity.test.js +++ b/ghost/core/test/unit/server/data/schema/integrity.test.js @@ -37,7 +37,7 @@ const parseYaml = require('../../../../../core/server/services/route-settings/ya */ describe('DB version integrity', function () { // Only these variables should need updating - const currentSchemaHash = 'dff334a3ab0ee69919d330d3c46752a2'; + const currentSchemaHash = '0d83dfdf9142d606e0660871c5adcf1a'; const currentFixturesHash = '1727a789194847da33d68dd95301b416'; const currentSettingsHash = '6ea42a00cca61a1ba87f66eb6e25a78a'; const currentRoutesHash = 'd8c25fa01bf6d22a2bcb05ba0de70dc1'; diff --git a/ghost/core/test/unit/server/services/email-analytics/email-analytics-service.test.ts b/ghost/core/test/unit/server/services/email-analytics/email-analytics-service.test.ts index b3fe7d8bc1a..cddb0d92e38 100644 --- a/ghost/core/test/unit/server/services/email-analytics/email-analytics-service.test.ts +++ b/ghost/core/test/unit/server/services/email-analytics/email-analytics-service.test.ts @@ -740,6 +740,35 @@ describe('EmailAnalyticsService', function () { ); }); + it('retries from the original cursor after a fetch error', async function () { + const initialCursor = new Date(Date.now() - 10 * 60 * 1000); + const fetchBegins: Date[] = []; + const eventProcessor = createStubEventProcessor(); + eventProcessor.processBatch.callsFake(async (_events, _result, fetchData) => { + fetchData.lastEventTimestamp = new Date(initialCursor.getTime() + 1000); + }); + const fetchEvents = sinon.stub().callsFake(async ({ batchHandler, begin }) => { + fetchBegins.push(begin); + await batchHandler([{ timestamp: new Date(initialCursor.getTime() + 1000) }]); + throw new Error('fallback fetch failed'); + }); + const service = createService({ + queries: { + getLastEventTimestamp: sinon.stub().resolves(initialCursor), + setJobTimestamp: sinon.stub().resolves(), + setJobStatus: sinon.stub().resolves(), + }, + fetchEvents, + createEventProcessor: () => eventProcessor, + }); + + await assert.rejects(service.fetchLatestNonOpenedEvents(), /fallback fetch failed/); + await assert.rejects(service.fetchLatestNonOpenedEvents(), /fallback fetch failed/); + + assert.deepEqual(fetchBegins, [initialCursor, initialCursor]); + assert.deepEqual(service.getStatus().latest.lastEventTimestamp, initialCursor); + }); + it('persists and advances the last processed event timestamp', async function () { const lastEventTimestamp = new Date(Date.now() - 10_000); const setJobTimestamp = sinon.stub().resolves(); @@ -772,6 +801,72 @@ describe('EmailAnalyticsService', function () { new Date(lastEventTimestamp.getTime() + 1000), ); }); + + it('resumes from a capped sending domain until all of its events are processed', async function () { + const newerDomainTimestamp = new Date(Date.now() - 70_000); + const fallbackTimestamps = [ + new Date(Date.now() - 120_000), + new Date(Date.now() - 119_000), + new Date(Date.now() - 118_000), + new Date(Date.now() - 117_000), + ]; + const setJobTimestamp = sinon.stub().resolves(); + const processedEventIds = new Set(); + const eventProcessor = createStubEventProcessor(); + eventProcessor.processBatch.callsFake(async (events, _result, fetchData) => { + for (const event of events) { + processedEventIds.add(event.id); + if (!fetchData.lastEventTimestamp || event.timestamp > fetchData.lastEventTimestamp) { + fetchData.lastEventTimestamp = event.timestamp; + } + } + }); + const service = createService({ + queries: { + getLastEventTimestamp: sinon.stub().resolves(), + setJobTimestamp, + setJobStatus: sinon.stub().resolves(), + }, + fetchEvents: async ({ batchHandler, begin }) => { + const fetchIndex = fetchBegins.push(begin) - 1; + const firstFallbackIndex = fetchIndex; + const safeCursor = fallbackTimestamps[firstFallbackIndex + 1]; + + await batchHandler([{ id: 'primary-10', timestamp: newerDomainTimestamp }]); + await batchHandler([ + { + id: `fallback-${firstFallbackIndex + 1}`, + timestamp: fallbackTimestamps[firstFallbackIndex], + }, + { + id: `fallback-${firstFallbackIndex + 2}`, + timestamp: safeCursor, + }, + ]); + return { safeCursor }; + }, + createEventProcessor: () => eventProcessor, + }); + const fetchBegins: Date[] = []; + + await service.fetchLatestNonOpenedEvents({ maxEvents: 2 }); + await service.fetchLatestNonOpenedEvents({ maxEvents: 2 }); + await service.fetchLatestNonOpenedEvents({ maxEvents: 2 }); + + assert.deepEqual( + setJobTimestamp + .getCalls() + .filter((call) => call.args[1] === 'finished') + .map((call) => call.args[2]), + fallbackTimestamps.slice(1), + ); + assert.deepEqual(fetchBegins.slice(1), fallbackTimestamps.slice(1, 3)); + assert.deepEqual( + [...processedEventIds], + ['primary-10', 'fallback-1', 'fallback-2', 'fallback-3', 'fallback-4'], + ); + assert.deepEqual(service.getStatus().latest.lastEventTimestamp, fallbackTimestamps[3]); + }); }); describe('restoreScheduled', function () { diff --git a/ghost/core/test/unit/server/services/email-analytics/fetch-mailgun-events.test.ts b/ghost/core/test/unit/server/services/email-analytics/fetch-mailgun-events.test.ts index 416e106e5ef..ac25bca45d0 100644 --- a/ghost/core/test/unit/server/services/email-analytics/fetch-mailgun-events.test.ts +++ b/ghost/core/test/unit/server/services/email-analytics/fetch-mailgun-events.test.ts @@ -1,3 +1,4 @@ +import assert from 'node:assert/strict'; import sinon from 'sinon'; // @ts-expect-error This module lacks type definitions. @@ -58,6 +59,20 @@ describe('fetchMailgunEvents', function () { }); }); + it('returns the domain-safe cursor from the Mailgun client', async function () { + const safeCursor = new Date('Thu Feb 25 2021 13:00:00 GMT+0000'); + sinon.stub(MailgunClient.prototype, 'fetchEvents').resolves({ safeCursor }); + + const result = await fetchMailgunEvents({ + config, + settings, + tags: DEFAULT_TAGS, + batchHandler: sinon.spy(), + }); + + assert.deepEqual(result, { safeCursor }); + }); + it('uses supplied end timestamp and max events', async function () { const { batchHandler, mailgunFetchEventsStub } = await fetchEvents({ begin: LATEST_TIMESTAMP, diff --git a/ghost/core/test/unit/server/services/gifts/gift-delivery-service.test.ts b/ghost/core/test/unit/server/services/gifts/gift-delivery-service.test.ts index 929f9227ef8..13e26e57432 100644 --- a/ghost/core/test/unit/server/services/gifts/gift-delivery-service.test.ts +++ b/ghost/core/test/unit/server/services/gifts/gift-delivery-service.test.ts @@ -50,6 +50,7 @@ describe('GiftDeliveryService', function () { giftDeliveryRepository = { getById: sinon.stub().resolves(null), getByGiftId: sinon.stub().resolves(null), + getByGiftToken: sinon.stub().resolves(null), getByProviderMessageId: sinon.stub().resolves(null), findRecoverableForPurchasedGifts: sinon.stub().resolves([]), findScheduledTimesForPurchasedGifts: sinon.stub().resolves([]), @@ -111,6 +112,31 @@ describe('GiftDeliveryService', function () { sinon.restore(); }); + it('returns the recipient email for a gift regardless of delivery state', async function () { + giftDeliveryRepository.getByGiftToken.resolves( + buildGiftDelivery({ + recipientEmail: 'recipient@example.com', + status: 'failed', + outcome: 'permanent_failed', + }), + ); + const service = createService(); + + assert.equal( + await service.getRecipientEmailForGift('gift-token', { transacting }), + 'recipient@example.com', + ); + sinon.assert.calledOnceWithExactly(giftDeliveryRepository.getByGiftToken, 'gift-token', { + transacting, + }); + }); + + it('returns null when a gift has no email delivery', async function () { + const service = createService(); + + assert.equal(await service.getRecipientEmailForGift('gift-token'), null); + }); + it('dispatches an immediate send for a pending delivery after purchase', async function () { giftDeliveryRepository.getByGiftId.resolves( buildGiftDelivery({ id: 'delivery_1', recipientEmail: 'recipient@example.com' }), diff --git a/ghost/core/test/unit/server/services/gifts/gift-service-interface.test.ts b/ghost/core/test/unit/server/services/gifts/gift-service-interface.test.ts index 248daeec8c4..a54ed0b59fa 100644 --- a/ghost/core/test/unit/server/services/gifts/gift-service-interface.test.ts +++ b/ghost/core/test/unit/server/services/gifts/gift-service-interface.test.ts @@ -65,6 +65,7 @@ describe('GiftService interface', function () { }; const giftDeliveryService = { createForCheckout: sinon.stub().resolves(), + getRecipientEmailForGift: sinon.stub().resolves(null), dispatchForGift: sinon.stub().resolves(null), cancelPendingForGift: sinon.stub().resolves(false), }; @@ -652,6 +653,7 @@ describe('GiftService interface', function () { amount: 5000, buyer_name: 'Jamie', recipient_name: 'Taylor', + recipient_email: null, message: 'Enjoy!', expires_at: new Date('2030-01-01T00:00:00.000Z'), consumes_at: null, diff --git a/ghost/core/test/unit/server/services/gifts/gift-service.test.ts b/ghost/core/test/unit/server/services/gifts/gift-service.test.ts index 8a2cbe3e804..f48472391ba 100644 --- a/ghost/core/test/unit/server/services/gifts/gift-service.test.ts +++ b/ghost/core/test/unit/server/services/gifts/gift-service.test.ts @@ -87,6 +87,7 @@ describe('GiftService', function () { let giftRepository: GiftRepositoryStub; let giftDeliveryService: { createForCheckout: sinon.SinonStub; + getRecipientEmailForGift: sinon.SinonStub; dispatchForGift: sinon.SinonStub; cancelPendingForGift: sinon.SinonStub; recoverPending: sinon.SinonStub; @@ -177,6 +178,7 @@ describe('GiftService', function () { }; giftDeliveryService = { createForCheckout: sinon.stub().resolves(undefined), + getRecipientEmailForGift: sinon.stub().resolves(null), dispatchForGift: sinon.stub().resolves(null), cancelPendingForGift: sinon.stub().resolves(false), recoverPending: sinon.stub().resolves({ sentCount: 0, skippedCount: 0, failedCount: 0 }), @@ -838,11 +840,18 @@ describe('GiftService', function () { const service = createService(); giftRepository.getByToken.resolves(gift); + giftDeliveryService.getRecipientEmailForGift.resolves('recipient@example.com'); const result = await service.getRedeemable({ token: 'gift-token', memberStatus: 'free' }); sinon.assert.calledOnceWithExactly(giftRepository.getByToken, 'gift-token'); + sinon.assert.calledOnceWithExactly( + giftDeliveryService.getRecipientEmailForGift, + 'gift-token', + {}, + ); assert.equal(result.token, gift.token); + assert.equal(result.recipient_email, 'recipient@example.com'); assert.equal('buyerEmail' in result, false); }); @@ -1668,6 +1677,7 @@ describe('GiftService', function () { memberGet.withArgs('email').returns('member@example.com'); giftRepository.getByToken.resolves(gift); + giftDeliveryService.getRecipientEmailForGift.resolves('recipient@example.com'); memberRepository.get.resolves({ id: 'member_1', get: memberGet, @@ -1707,6 +1717,11 @@ describe('GiftService', function () { sinon.assert.calledOnceWithExactly(giftDeliveryService.cancelPendingForGift, redeemed.token, { transacting, }); + sinon.assert.calledOnceWithExactly( + giftDeliveryService.getRecipientEmailForGift, + redeemed.token, + { transacting }, + ); sinon.assert.calledTwice(tiersService.api.read); sinon.assert.alwaysCalledWithExactly(tiersService.api.read, 'tier_1'); sinon.assert.calledOnceWithExactly(staffServiceEmails.notifyGiftSubscriptionStarted, { @@ -1722,6 +1737,7 @@ describe('GiftService', function () { assert.equal(redeemed.redeemerMemberId, 'member_1'); assert.notEqual(redeemed.consumesAt, null); assert.equal(redemption.token, 'gift-token'); + assert.equal(redemption.recipient_email, 'recipient@example.com'); assert.deepEqual(redemption.consumes_at, redeemed.consumesAt); }); diff --git a/ghost/core/test/unit/server/services/lib/mailgun-client.test.js b/ghost/core/test/unit/server/services/lib/mailgun-client.test.js index dc4d2d0d6e9..4a435caac2a 100644 --- a/ghost/core/test/unit/server/services/lib/mailgun-client.test.js +++ b/ghost/core/test/unit/server/services/lib/mailgun-client.test.js @@ -30,6 +30,31 @@ const createBatchCounter = (customHandler) => { return batchCounter; }; +const createEventsPage = ({ domain, nextPage, timestamps }) => ({ + items: timestamps.map((timestamp, index) => ({ + event: 'delivered', + recipient: `recipient${index}@example.com`, + 'user-variables': { + 'email-id': '5fbe5d9607bdfa3765dc3819', + }, + message: { + headers: { + 'message-id': `message-${index}@${domain}`, + }, + }, + timestamp, + })), + paging: { + next: `https://api.mailgun.net/v3/${domain}/events/${nextPage}`, + }, +}); + +const mockEventsPage = ({ domain, mailgunOptions, nextPage, page, timestamps }) => + nock('https://api.mailgun.net') + .get(`/v3/${domain}/events${page ? `/${page}` : ''}`) + .query(mailgunOptions) + .reply(200, createEventsPage({ domain, nextPage, timestamps })); + describe('MailgunClient', function () { let config, settings; @@ -1119,6 +1144,165 @@ describe('MailgunClient', function () { nock.cleanAll(); }); + it('returns the capped fallback cursor when the primary domain is exhausted further ahead', async function () { + setupDomainWarmingConfig(true); + + const begin = 1606399300; + const mailgunOptions = { ...MAILGUN_OPTIONS, begin }; + + const primaryPageMock = mockEventsPage({ + domain: 'primary.com', + mailgunOptions, + nextPage: 'primary-next', + timestamps: [begin + 10], + }); + const primaryEmptyPageMock = mockEventsPage({ + domain: 'primary.com', + mailgunOptions, + nextPage: 'primary-empty', + page: 'primary-next', + timestamps: [], + }); + const fallbackPageMock = mockEventsPage({ + domain: 'fallback.com', + mailgunOptions, + nextPage: 'fallback-unconsumed', + timestamps: [begin + 1, begin + 2], + }); + const fallbackNextPageMock = mockEventsPage({ + domain: 'fallback.com', + mailgunOptions, + nextPage: 'fallback-empty', + page: 'fallback-unconsumed', + timestamps: [begin + 3], + }); + + const processedTimestamps = []; + + const result = await new MailgunClient({ config, settings }).fetchEvents( + mailgunOptions, + (events) => { + processedTimestamps.push(...events.map((event) => event.timestamp)); + }, + { maxEvents: 2 }, + ); + + assert.equal(primaryPageMock.isDone(), true); + assert.equal(primaryEmptyPageMock.isDone(), true); + assert.equal(fallbackPageMock.isDone(), true); + assert.equal(fallbackNextPageMock.isDone(), false); + assert.deepEqual(processedTimestamps, [ + new Date((begin + 10) * 1000), + new Date((begin + 1) * 1000), + new Date((begin + 2) * 1000), + ]); + assert.deepEqual(result, { + safeCursor: new Date((begin + 2) * 1000), + }); + }); + + it('returns the capped primary cursor when the fallback domain is exhausted further ahead', async function () { + setupDomainWarmingConfig(true); + + const begin = 1606399300; + const mailgunOptions = { ...MAILGUN_OPTIONS, begin }; + + const primaryPageMock = mockEventsPage({ + domain: 'primary.com', + mailgunOptions, + nextPage: 'primary-unconsumed', + timestamps: [begin + 1, begin + 2], + }); + const primaryNextPageMock = mockEventsPage({ + domain: 'primary.com', + mailgunOptions, + nextPage: 'primary-empty', + page: 'primary-unconsumed', + timestamps: [begin + 3], + }); + const fallbackPageMock = mockEventsPage({ + domain: 'fallback.com', + mailgunOptions, + nextPage: 'fallback-next', + timestamps: [begin + 10], + }); + const fallbackEmptyPageMock = mockEventsPage({ + domain: 'fallback.com', + mailgunOptions, + nextPage: 'fallback-empty', + page: 'fallback-next', + timestamps: [], + }); + + const processedTimestamps = []; + const result = await new MailgunClient({ config, settings }).fetchEvents( + mailgunOptions, + (events) => { + processedTimestamps.push(...events.map((event) => event.timestamp)); + }, + { maxEvents: 2 }, + ); + + assert.equal(primaryPageMock.isDone(), true); + assert.equal(primaryNextPageMock.isDone(), false); + assert.equal(fallbackPageMock.isDone(), true); + assert.equal(fallbackEmptyPageMock.isDone(), true); + assert.deepEqual(processedTimestamps, [ + new Date((begin + 1) * 1000), + new Date((begin + 2) * 1000), + new Date((begin + 10) * 1000), + ]); + assert.deepEqual(result, { + safeCursor: new Date((begin + 2) * 1000), + }); + }); + + it('returns the earliest capped cursor when both domains reach their limits', async function () { + setupDomainWarmingConfig(true); + + const begin = 1606399300; + const mailgunOptions = { ...MAILGUN_OPTIONS, begin }; + + mockEventsPage({ + domain: 'primary.com', + mailgunOptions, + nextPage: 'primary-unconsumed', + timestamps: [begin + 5, begin + 6], + }); + const primaryNextPageMock = mockEventsPage({ + domain: 'primary.com', + mailgunOptions, + nextPage: 'primary-empty', + page: 'primary-unconsumed', + timestamps: [begin + 7], + }); + mockEventsPage({ + domain: 'fallback.com', + mailgunOptions, + nextPage: 'fallback-unconsumed', + timestamps: [begin + 1, begin + 2], + }); + const fallbackNextPageMock = mockEventsPage({ + domain: 'fallback.com', + mailgunOptions, + nextPage: 'fallback-empty', + page: 'fallback-unconsumed', + timestamps: [begin + 3], + }); + + const result = await new MailgunClient({ config, settings }).fetchEvents( + mailgunOptions, + () => {}, + { maxEvents: 2 }, + ); + + assert.equal(primaryNextPageMock.isDone(), false); + assert.equal(fallbackNextPageMock.isDone(), false); + assert.deepEqual(result, { + safeCursor: new Date((begin + 2) * 1000), + }); + }); + it('fetches from both primary and fallback domains when enabled', async function () { setupDomainWarmingConfig(true); @@ -1141,12 +1325,13 @@ describe('MailgunClient', function () { const counter = createBatchCounter(); const mailgunClient = new MailgunClient({ config, settings }); - await mailgunClient.fetchEvents(MAILGUN_OPTIONS, counter.batchHandler); + const result = await mailgunClient.fetchEvents(MAILGUN_OPTIONS, counter.batchHandler); assert.equal(primaryMock.isDone(), true); assert.equal(fallbackMock.isDone(), true); assert.equal(counter.batches, 2); assert.equal(counter.events, 6); + assert.deepEqual(result, { safeCursor: undefined }); }); it('only fetches from primary when disabled', async function () { diff --git a/package.json b/package.json index 2993c7c281f..4c5dcf5f9a3 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,7 @@ "dev:public": "pnpm nx run ghost-monorepo:docker:dev:public", "dev:full": "DEV_COMPOSE_FILES='-f compose.dev.analytics.yaml -f compose.dev.storage.yaml' ./docker/stripe/with-stripe.sh pnpm nx run ghost-monorepo:docker:dev:public", "dev:analytics": "DEV_COMPOSE_FILES='-f compose.dev.analytics.yaml' pnpm nx run ghost-monorepo:docker:dev", + "dev:analytics:local": "ANALYTICS_PROXY_TARGET=traffic-analytics-local:3000 DEV_COMPOSE_FILES='-f compose.dev.analytics.yaml' pnpm nx run ghost-monorepo:docker:dev", "dev:storage": "DEV_COMPOSE_FILES='-f compose.dev.storage.yaml' pnpm nx run ghost-monorepo:docker:dev", "dev:stripe": "./docker/stripe/with-stripe.sh pnpm nx run ghost-monorepo:docker:dev", "dev:all": "DEV_COMPOSE_FILES='-f compose.dev.analytics.yaml -f compose.dev.storage.yaml' ./docker/stripe/with-stripe.sh pnpm nx run ghost-monorepo:docker:dev", diff --git a/packages/testing/test-data/src/selectors/members.ts b/packages/testing/test-data/src/selectors/members.ts index 45b93083121..a68fd38f48e 100644 --- a/packages/testing/test-data/src/selectors/members.ts +++ b/packages/testing/test-data/src/selectors/members.ts @@ -8,6 +8,13 @@ export const membersListItem = 'members-list-item'; export const membersSearchInput = 'members-search-input'; export const membersActions = 'members-actions'; export const memberDetail = 'member-detail'; +export const memberDetailTitle = 'member-detail-title'; +export const memberDetailEngagement = 'member-detail-engagement'; +export const memberActions = 'member-actions'; +export const memberSubscriptionToggle = 'member-subscription-toggle'; +export const memberSigninUrl = 'member-signin-url'; +export const confirmDeleteMember = 'confirm-delete-member'; +export const cancelDeleteMember = 'cancel-delete-member'; export const memberCustomFieldsField = 'member-custom-fields-field'; export const memberCustomFieldEditModal = 'member-custom-field-edit-modal'; export const importCreateCustomField = 'import-create-custom-field'; diff --git a/packages/testing/test-data/src/selectors/settings.ts b/packages/testing/test-data/src/selectors/settings.ts index 02edd53920e..47153bb967f 100644 --- a/packages/testing/test-data/src/selectors/settings.ts +++ b/packages/testing/test-data/src/selectors/settings.ts @@ -22,6 +22,9 @@ export const copyShareableLink = 'copy-shareable-link'; export const titleAndDescription = 'title-and-description'; export const design = 'design'; export const users = 'users'; +export const ownerUser = 'owner-user'; +export const integrations = 'integrations'; +export const dangerZone = 'dangerzone'; export const portal = 'portal'; export const explore = 'explore'; export const network = 'network'; @@ -49,6 +52,7 @@ export const addNewsletterModal = 'add-newsletter-modal'; export const newsletterModal = 'newsletter-modal'; export const toastInfo = 'toast-info'; export const access = 'access'; +export const siteVisibilitySelect = 'site-visibility-select'; export const siteAccessCode = 'site-access-code'; export const regenerateAccessCode = 'regenerate-access-code'; export const subscriptionAccessSelect = 'subscription-access-select'; @@ -56,6 +60,7 @@ export const defaultPostAccessSelect = 'default-post-access-select'; export const commentingSelect = 'commenting-select'; export const tiersSelect = 'tiers-select'; export const customFields = 'custom-fields'; +export const customFieldListItem = 'custom-field-list-item'; export const customFieldModal = 'custom-field-modal'; export const stripeModal = 'stripe-modal'; export const tiers = 'tiers'; @@ -87,9 +92,12 @@ export const welcomeEmailPreviewSubject = 'welcome-email-preview-subject'; export const welcomeEmailPreviewIframe = 'welcome-email-preview-iframe'; export const welcomeEmailPreviewError = 'welcome-email-preview-error'; export const welcomeEmailPreviewLoading = 'welcome-email-preview-loading'; +export const welcomeEmailEditor = 'welcome-email-editor'; export const freeWelcomeEmailPreview = 'free-welcome-email-preview'; export const freeWelcomeEmailTitle = 'free-welcome-email-title'; +export const freeWelcomeEmailRow = 'free-welcome-email-row'; export const paidWelcomeEmailRow = 'paid-welcome-email-row'; +export const headerImageField = 'header-image-field'; export const testEmailDropdown = 'test-email-dropdown'; export const automationsTransactionalRow = 'automations-transactional-row'; export const embedIframe = 'embed-iframe'; @@ -101,6 +109,15 @@ export const themeListItem = 'theme-list-item'; export const themeCodeEditorModal = 'theme-code-editor-modal'; export const themeEditorConfirmModal = 'theme-editor-confirm-modal'; export const themeEditorInputModal = 'theme-editor-input-modal'; +export const publicationCover = 'publication-cover'; +export const profileImageUpload = 'profile-image-upload'; +export const profileImagePreview = 'profile-image-preview'; +export const coverImageUpload = 'cover-image-upload'; +export const coverImagePreview = 'cover-image-preview'; + +// testids rendered by the Shade image-upload pattern (apps/shade) +export const imageUploadContainer = 'image-upload-container'; +export const imageDeleteButton = 'image-delete-button'; // accessible names export const settingsSearchLabel = 'Search settings';