diff --git a/apps/admin-x-framework/package.json b/apps/admin-x-framework/package.json index 80a580c645a..74208b4d87f 100644 --- a/apps/admin-x-framework/package.json +++ b/apps/admin-x-framework/package.json @@ -75,7 +75,6 @@ "dependencies": { "@sentry/react": "catalog:", "@tanstack/react-query": "catalog:", - "@tinybirdco/charts": "0.3.0", "@tryghost/custom-field-types": "workspace:*", "@tryghost/limit-service": "catalog:", "@tryghost/nql-string": "workspace:*", diff --git a/apps/admin-x-framework/src/api/tiers-checkout-config.ts b/apps/admin-x-framework/src/api/tiers-checkout-config.ts new file mode 100644 index 00000000000..ab3370a3147 --- /dev/null +++ b/apps/admin-x-framework/src/api/tiers-checkout-config.ts @@ -0,0 +1,82 @@ +import { Meta, createMutation, createQuery } from '../utils/api/hooks'; + +/** + * A tier's checkout configuration, as the API serves and takes it. + * + * Mirrors the server's wire shape (tier-checkout-config/serializers.ts): each collectable + * thing is its own named block, present in a response only when the tier collects it, so + * a client reads presence rather than a flag. A write states only the blocks it wants to + * change; an unnamed block is left alone. + */ + +export type TierCheckoutQuestion = { + key: string; + label: string | null; + optional: boolean; +}; + +export type TierCheckoutCollection = { + collect: true; + custom_field_key: string; +}; + +export type TierCheckoutConfig = { + tier_id: string; + custom_fields: TierCheckoutQuestion[]; + /** + * One toggle for the shipping step, two destinations: the processor collects the + * recipient name and the address together, and each lands in its own field. + */ + shipping?: { + collect: true; + /** Absent means everywhere the processor ships; a list is a restriction. */ + allowed_countries?: string[]; + name: { custom_field_key: string }; + address: { custom_field_key: string }; + }; + /** + * The tax number itself stays on Stripe, against the member's invoices; Ghost only + * records that the checkout asks for one, so there is no destination to name. + */ + tax_number?: { collect: true }; + phone?: TierCheckoutCollection; +}; + +/** The blocks a write may state. `collect: false` turns a collection off. */ +export type TierCheckoutConfigInput = { + custom_fields?: Array<{ key: string; label?: string | null; optional?: boolean }>; + shipping?: + | { collect: false } + | { + collect: true; + /** Omit to deliver everywhere. An empty list is refused, not read as everywhere. */ + allowed_countries?: string[]; + name: { custom_field_key: string }; + address: { custom_field_key: string }; + }; + tax_number?: { collect: boolean }; + phone?: { collect: false } | { collect: true; custom_field_key: string }; +}; + +export interface TiersCheckoutConfigResponseType { + meta?: Meta; + tiers_checkout_config: TierCheckoutConfig[]; +} + +const dataType = 'TiersCheckoutConfigResponseType'; + +// Every tier's configuration in one read, so a tier list needs one request. +export const useBrowseTiersCheckoutConfig = createQuery({ + dataType, + path: '/tiers/checkout_config/', +}); + +export const useEditTierCheckoutConfig = createMutation< + TiersCheckoutConfigResponseType, + { tierId: string; config: TierCheckoutConfigInput } +>({ + method: 'PUT', + path: ({ tierId }) => `/tiers/${tierId}/checkout_config/`, + body: ({ config }) => ({ tiers_checkout_config: [config] }), + invalidateQueries: { dataType }, +}); diff --git a/apps/admin-x-framework/src/hooks/use-active-visitors.ts b/apps/admin-x-framework/src/hooks/use-active-visitors.ts index 84f3278fa8b..80dcf3edaa1 100644 --- a/apps/admin-x-framework/src/hooks/use-active-visitors.ts +++ b/apps/admin-x-framework/src/hooks/use-active-visitors.ts @@ -8,38 +8,26 @@ interface UseActiveVisitorsOptions { enabled?: boolean; } +export const ACTIVE_VISITORS_REFETCH_INTERVAL = 60 * 1000; + export const useActiveVisitors = (options: UseActiveVisitorsOptions = {}) => { const { postUuid, statsConfig, enabled = true } = options; - const [refreshKey, setRefreshKey] = useState(0); const [lastKnownCount, setLastKnownCount] = useState(null); - // Set up 60-second interval only if enabled - useEffect(() => { - if (!enabled) { - return; - } - - const interval = setInterval(() => { - setRefreshKey((prev) => prev + 1); - }, 60000); // 60 seconds - - return () => clearInterval(interval); - }, [enabled]); - const params = { site_uuid: statsConfig?.id || '', - // Add postUuid if provided ...(postUuid && { post_uuid: postUuid }), - // Add refreshKey to force refetch - _refresh: refreshKey.toString(), }; - // Use useTinybirdQuery for consistent token handling const { data, loading, error } = useTinybirdQuery({ statsConfig, endpoint: 'api_active_visitors', params, enabled, + refetchInterval: ACTIVE_VISITORS_REFETCH_INTERVAL, + // Keep counting while the tab is hidden (matches the old interval tick, + // which the browser throttled to roughly this cadence anyway). + refetchIntervalInBackground: true, }); const currentCount = data?.[0]?.active_visitors; diff --git a/apps/admin-x-framework/src/hooks/use-tinybird-query.ts b/apps/admin-x-framework/src/hooks/use-tinybird-query.ts index ff382461298..8496bcacc3e 100644 --- a/apps/admin-x-framework/src/hooks/use-tinybird-query.ts +++ b/apps/admin-x-framework/src/hooks/use-tinybird-query.ts @@ -1,19 +1,134 @@ -import { useQuery } from '@tinybirdco/charts'; +import { useQuery } from '@tanstack/react-query'; import { useTinybirdToken } from './use-tinybird-token'; import { StatsConfig } from '../providers/framework-provider'; import { getStatEndpointUrl } from '../utils/stats-config'; import { useWebAnalyticsEnabled } from '../api/settings'; +export type TinybirdRow = Record; + +export interface TinybirdMeta { + name: string; + type: string; +} + +interface TinybirdPipeResponse { + data?: TinybirdRow[] | null; + meta?: TinybirdMeta[] | null; +} + export interface UseTinybirdQueryOptions { statsConfig?: StatsConfig | null; endpoint: string; params: Record; enabled?: boolean; + /** Poll interval in ms (e.g. active visitors); no polling by default. */ + refetchInterval?: number; + refetchIntervalInBackground?: boolean; +} + +export interface UseTinybirdQueryResult { + data: TinybirdRow[] | null; + meta: TinybirdMeta[] | null; + loading: boolean; + error: Error | null; +} + +// Analytics reads fresh: pipe responses go stale after a minute instead of +// the app-wide five. +export const TINYBIRD_STALE_TIME = 60 * 1000; + +/** Full pipe request URL with params applied; undefined disables the query. */ +export const buildTinybirdRequestUrl = ( + endpointUrl: string | undefined, + params: Record, +): string | undefined => { + if (!endpointUrl) { + return undefined; + } + let url: URL; + try { + url = new URL(endpointUrl); + } catch { + return undefined; + } + for (const [key, value] of Object.entries(params)) { + url.searchParams.set(key, value); + } + return url.toString(); +}; + +interface FetchTinybirdPipeOptions { + url: string; + token: string; + refreshToken: () => Promise; + signal?: AbortSignal; } -// Wrapper around Tinybird's useQuery hook that handles the token loading state -export const useTinybirdQuery = (options: UseTinybirdQueryOptions) => { - const { statsConfig, endpoint, params, enabled = true } = options; +// Direct browser→Tinybird request: bearer token only, no Ghost cookies. +export const fetchTinybirdPipe = async ({ + url, + token, + refreshToken, + signal, +}: FetchTinybirdPipeOptions): Promise => { + const request = (bearer: string) => + fetch(url, { + credentials: 'omit', + headers: { Authorization: `Bearer ${bearer}` }, + signal, + }); + + let response = await request(token); + + // A fetch can race a server-side token rotation: refresh the token once + // and retry before surfacing the error. + if (response.status === 401 || response.status === 403) { + const freshToken = await refreshToken(); + if (freshToken && freshToken !== token) { + response = await request(freshToken); + } + } + + if (!response.ok) { + const body = await response.text(); + throw new Error(`Tinybird request failed (${response.status}): ${body || response.statusText}`); + } + + const body: unknown = await response.json(); + return parseTinybirdPipeResponse(body); +}; + +/** + * Structural guard for the external response envelope. Rows are pipe-shaped + * (each pipe returns its own columns), so cells are deliberately not + * validated per field — only the envelope: `data`/`meta` must be arrays of + * objects when present. + */ +export const parseTinybirdPipeResponse = (body: unknown): TinybirdPipeResponse => { + if (body === null || typeof body !== 'object' || Array.isArray(body)) { + throw new Error('Tinybird returned an unexpected response shape'); + } + const { data, meta } = body as { data?: unknown; meta?: unknown }; + const isObjectArray = (value: unknown): boolean => + Array.isArray(value) && value.every((row) => row !== null && typeof row === 'object'); + if (data !== undefined && data !== null && !isObjectArray(data)) { + throw new Error('Tinybird returned an unexpected response shape'); + } + if (meta !== undefined && meta !== null && !isObjectArray(meta)) { + throw new Error('Tinybird returned an unexpected response shape'); + } + return body as TinybirdPipeResponse; +}; + +export const useTinybirdQuery = (options: UseTinybirdQueryOptions): UseTinybirdQueryResult => { + const { + statsConfig, + endpoint, + params, + enabled = true, + refetchInterval, + refetchIntervalInBackground, + } = options; // Web analytics kill-switch, read from settings so no call site threads it. // When off, shouldQuery is false and the hook returns empty state. const webAnalyticsEnabled = useWebAnalyticsEnabled(); @@ -22,22 +137,38 @@ export const useTinybirdQuery = (options: UseTinybirdQueryOptions) => { const tokenQuery = useTinybirdToken({ enabled: shouldQuery }); const endpointUrl = shouldQuery && statsConfig ? getStatEndpointUrl(statsConfig, endpoint) : undefined; + const requestUrl = buildTinybirdRequestUrl(endpointUrl, params); + const token = tokenQuery.token; + const refreshToken = tokenQuery.refetch; - // Set the endpoint to undefined if: - // - Token is not loaded (prevents 403 errors) - // - Query is disabled via enabled flag - // - No statsConfig provided - // - No endpoint specified - const { data, meta, loading, error } = useQuery({ - endpoint: !tokenQuery.isLoading && tokenQuery.token && shouldQuery ? endpointUrl : undefined, - token: shouldQuery ? tokenQuery.token : undefined, - params: params, + const query = useQuery({ + // The token stays out of the key — it is auth material, not data identity; + // keying on it would refetch every pipe when the scheduled token refresh + // lands. The queryFn reads the current token instead. + queryKey: ['tinybird', requestUrl], + // Fetch only once the token has loaded (prevents guaranteed 403s). + enabled: Boolean(shouldQuery && requestUrl && token), + staleTime: TINYBIRD_STALE_TIME, + retry: false, + // Analytics wants fresh data on return to the tab — the app-wide + // focus-refetch opt-out is for CRUD data, and the old charts-lib SWR + // layer revalidated on focus too. + refetchOnWindowFocus: true, + refetchInterval, + refetchIntervalInBackground, + queryFn: ({ signal }) => + fetchTinybirdPipe({ + url: requestUrl as string, + token: token as string, + refreshToken, + signal, + }), }); return { - data: shouldQuery ? data : null, - meta: shouldQuery ? meta : null, - loading: shouldQuery ? tokenQuery.isLoading || loading : false, - error: shouldQuery ? (error ?? tokenQuery.error) : null, + data: shouldQuery ? (query.data?.data ?? null) : null, + meta: shouldQuery ? (query.data?.meta ?? null) : null, + loading: shouldQuery ? tokenQuery.isLoading || query.isLoading : false, + error: shouldQuery ? (query.error ?? tokenQuery.error) : null, }; }; diff --git a/apps/admin-x-framework/src/hooks/use-tinybird-token.ts b/apps/admin-x-framework/src/hooks/use-tinybird-token.ts index f25b8d4b60d..01cbb8714be 100644 --- a/apps/admin-x-framework/src/hooks/use-tinybird-token.ts +++ b/apps/admin-x-framework/src/hooks/use-tinybird-token.ts @@ -1,3 +1,4 @@ +import { useCallback } from 'react'; import { useTinybirdTokenQuery } from '../api/tinybird'; import { useWebAnalyticsEnabled } from '../api/settings'; @@ -5,7 +6,8 @@ export interface UseTinybirdTokenResult { token: string | undefined; isLoading: boolean; error: Error | null; - refetch: () => void; + /** Refetches the token query and resolves the fresh token, if any. */ + refetch: () => Promise; } export interface UseTinybirdTokenOptions { @@ -22,6 +24,13 @@ export const useTinybirdToken = (options: UseTinybirdTokenOptions = {}): UseTiny const effectiveEnabled = enabled && webAnalyticsEnabled; const tinybirdQuery = useTinybirdTokenQuery({ enabled: effectiveEnabled }); + const refetchQuery = tinybirdQuery.refetch; + const refetch = useCallback(async () => { + const result = await refetchQuery(); + const freshToken = result.data?.tinybird?.token; + return typeof freshToken === 'string' && freshToken ? freshToken : undefined; + }, [refetchQuery]); + // A disabled React Query can keep cached data/errors, so return an idle // result — else direct consumers (the providers) leak a stale token. if (!effectiveEnabled) { @@ -29,7 +38,7 @@ export const useTinybirdToken = (options: UseTinybirdTokenOptions = {}): UseTiny token: undefined, isLoading: false, error: null, - refetch: tinybirdQuery.refetch, + refetch, }; } @@ -52,6 +61,6 @@ export const useTinybirdToken = (options: UseTinybirdTokenOptions = {}): UseTiny token: apiToken && typeof apiToken === 'string' ? apiToken : undefined, isLoading: tinybirdQuery.isLoading, error, - refetch: tinybirdQuery.refetch, + refetch, }; }; diff --git a/apps/admin-x-framework/test/unit/api/tiers-checkout-config.test.ts b/apps/admin-x-framework/test/unit/api/tiers-checkout-config.test.ts new file mode 100644 index 00000000000..f13f70da0cb --- /dev/null +++ b/apps/admin-x-framework/test/unit/api/tiers-checkout-config.test.ts @@ -0,0 +1,130 @@ +import { describe, expect, it } from 'vitest'; + +import type { + TierCheckoutConfig, + TierCheckoutConfigInput, +} from '../../../src/api/tiers-checkout-config'; + +// Compile-time cases: the build failing is the assertion. Each `@ts-expect-error` fails +// the build if the case it names stops being an error, so a drift between these types and +// the server's serializers (tier-checkout-config/serializers.ts) surfaces as a build +// break instead of a runtime 422. Directives sit immediately above the value they judge. + +const everythingOff: TierCheckoutConfigInput = { + shipping: { collect: false }, + tax_number: { collect: false }, + phone: { collect: false }, +}; + +const everythingOn: TierCheckoutConfigInput = { + shipping: { + collect: true, + allowed_countries: ['US'], + name: { custom_field_key: 'recipient_name' }, + address: { custom_field_key: 'shipping_address' }, + }, + tax_number: { collect: true }, + phone: { collect: true, custom_field_key: 'phone' }, +}; + +// The tax number stays on Stripe, against the member's invoices: a destination is not +// part of its contract, and the server's strict schema refuses one. +const taxWithDestination: TierCheckoutConfigInput = { + // @ts-expect-error tax_number takes no custom_field_key + tax_number: { collect: true, custom_field_key: 'vat' }, +}; + +// Shipping that collects must state everything it needs; the checkout asks for the name +// and address together and both land in required destinations. +const shippingWithoutName: TierCheckoutConfigInput = { + // @ts-expect-error a collecting shipping block must name where the recipient name goes + shipping: { + collect: true, + allowed_countries: ['US'], + address: { custom_field_key: 'shipping_address' }, + }, +}; + +const shippingWithoutAddress: TierCheckoutConfigInput = { + // @ts-expect-error a collecting shipping block must name where the address goes + shipping: { + collect: true, + allowed_countries: ['US'], + name: { custom_field_key: 'recipient_name' }, + }, +}; + +// Countries are a restriction, so omitting them is how a tier says it delivers everywhere. +// No directive here on purpose: this has to keep compiling, or Admin loses the only way to +// express that without enumerating a set that moves. +const shippingEverywhere: TierCheckoutConfigInput = { + shipping: { + collect: true, + name: { custom_field_key: 'recipient_name' }, + address: { custom_field_key: 'shipping_address' }, + }, +}; + +const phoneWithoutDestination: TierCheckoutConfigInput = { + // @ts-expect-error a collecting phone block must name its destination + phone: { collect: true }, +}; + +// A response block is present only when the tier collects that thing, and a present +// block carries its destinations as definite strings, never null. +const response: TierCheckoutConfig = { + tier_id: 'abc', + custom_fields: [{ key: 'company', label: null, optional: true }], + shipping: { + collect: true, + allowed_countries: ['US'], + name: { custom_field_key: 'recipient_name' }, + address: { custom_field_key: 'shipping_address' }, + }, + tax_number: { collect: true }, + phone: { collect: true, custom_field_key: 'phone' }, +}; + +// And the same absence comes back, so a client reads everywhere the way it wrote it. +const responseEverywhere: TierCheckoutConfig = { + tier_id: 'abc', + custom_fields: [], + shipping: { + collect: true, + name: { custom_field_key: 'recipient_name' }, + address: { custom_field_key: 'shipping_address' }, + }, +}; + +const responseWithNullDestination: TierCheckoutConfig = { + tier_id: 'abc', + custom_fields: [], + shipping: { + collect: true, + allowed_countries: ['US'], + // @ts-expect-error a served shipping block always names both destinations + name: { custom_field_key: null }, + address: { custom_field_key: 'shipping_address' }, + }, +}; + +describe('tiers-checkout-config wire contract', () => { + it('compiles the shapes the server accepts and serves', () => { + // The `@ts-expect-error` cases above are the real assertions; this keeps the + // compile-time values referenced and the file a test. + for (const value of [ + everythingOff, + everythingOn, + taxWithDestination, + shippingWithoutName, + shippingWithoutAddress, + shippingEverywhere, + phoneWithoutDestination, + response, + responseEverywhere, + responseWithNullDestination, + ]) { + expect(value).toBeTruthy(); + } + }); +}); diff --git a/apps/admin-x-framework/test/unit/hooks/use-active-visitors.test.ts b/apps/admin-x-framework/test/unit/hooks/use-active-visitors.test.ts index 3c2fcf2ae21..08bb31aac7b 100644 --- a/apps/admin-x-framework/test/unit/hooks/use-active-visitors.test.ts +++ b/apps/admin-x-framework/test/unit/hooks/use-active-visitors.test.ts @@ -1,90 +1,38 @@ -import { renderHook, act } from '@testing-library/react'; -import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { renderHook } from '@testing-library/react'; import { useActiveVisitors } from '../../../src/hooks/use-active-visitors'; -import React from 'react'; -// Mock the @tinybirdco/charts module -vi.mock('@tinybirdco/charts', () => ({ - useQuery: vi.fn(), +vi.mock('../../../src/hooks/use-tinybird-query', () => ({ + useTinybirdQuery: vi.fn(), })); -// Mock the stats-config utils -vi.mock('../../../src/utils/stats-config', () => ({ - getStatEndpointUrl: vi.fn(), -})); - -// Mock the useTinybirdToken hook -vi.mock('../../../src/hooks/use-tinybird-token', () => ({ - useTinybirdToken: vi.fn(), -})); +import { useTinybirdQuery } from '../../../src/hooks/use-tinybird-query'; -// Web analytics kill-switch: on for these tests, so useTinybirdQuery queries. -vi.mock('../../../src/api/settings', () => ({ - useWebAnalyticsEnabled: vi.fn(), -})); - -import { useQuery } from '@tinybirdco/charts'; -import { getStatEndpointUrl } from '../../../src/utils/stats-config'; -import { useTinybirdToken } from '../../../src/hooks/use-tinybird-token'; -import { useWebAnalyticsEnabled } from '../../../src/api/settings'; +const mockUseTinybirdQuery = vi.mocked(useTinybirdQuery); -const mockUseQuery = vi.mocked(useQuery); -const mockGetStatEndpointUrl = vi.mocked(getStatEndpointUrl); -const mockUseTinybirdToken = vi.mocked(useTinybirdToken); -const mockUseWebAnalyticsEnabled = vi.mocked(useWebAnalyticsEnabled); const statsConfig = { id: 'test-site-id', endpoint: 'https://api.test.com', - token: 'test-token', }; -describe('useActiveVisitors', () => { - let queryClient: QueryClient; - let wrapper: React.FC<{ children: React.ReactNode }>; +const queryState = (overrides: Partial> = {}) => ({ + data: null, + meta: null, + loading: false, + error: null, + ...overrides, +}); +describe('useActiveVisitors', () => { beforeEach(() => { - vi.useFakeTimers(); - queryClient = new QueryClient({ - defaultOptions: { - queries: { retry: false }, - mutations: { retry: false }, - }, - }); - wrapper = ({ children }) => - React.createElement(QueryClientProvider, { client: queryClient }, children); - - mockUseQuery.mockReturnValue({ - data: null, - loading: false, - error: null, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/api_active_visitors', - token: 'mock-token', - refresh: vi.fn(), - }); - mockGetStatEndpointUrl.mockImplementation( - (_config: any, endpoint: any) => `https://api.example.com/${endpoint}`, - ); - mockUseTinybirdToken.mockReturnValue({ - token: 'mock-token', - isLoading: false, - error: null, - refetch: vi.fn(), - }); - mockUseWebAnalyticsEnabled.mockReturnValue(true); + mockUseTinybirdQuery.mockReturnValue(queryState()); }); afterEach(() => { - vi.useRealTimers(); vi.clearAllMocks(); - vi.restoreAllMocks(); }); it('returns initial state when enabled is true', () => { - const { result } = renderHook(() => useActiveVisitors({ statsConfig, enabled: true }), { - wrapper, - }); + const { result } = renderHook(() => useActiveVisitors({ statsConfig, enabled: true })); expect(result.current).toEqual({ activeVisitors: 0, @@ -94,578 +42,159 @@ describe('useActiveVisitors', () => { }); it('returns zero state when enabled is false', () => { - const { result } = renderHook(() => useActiveVisitors({ enabled: false }), { wrapper }); + const { result } = renderHook(() => useActiveVisitors({ enabled: false })); expect(result.current).toEqual({ activeVisitors: 0, isLoading: false, error: null, }); + expect(mockUseTinybirdQuery).toHaveBeenCalledWith(expect.objectContaining({ enabled: false })); + }); + + it('polls via refetchInterval instead of a cache-busting param', () => { + renderHook(() => useActiveVisitors({ statsConfig, enabled: true })); + + expect(mockUseTinybirdQuery).toHaveBeenCalledWith( + expect.objectContaining({ + endpoint: 'api_active_visitors', + statsConfig, + refetchInterval: 60 * 1000, + }), + ); + const params = mockUseTinybirdQuery.mock.calls[0][0].params; + expect(params).not.toHaveProperty('_refresh'); }); it('shows loading state only on initial load with no last known count', () => { - mockUseQuery.mockReturnValue({ - data: null, - loading: true, - error: null, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/api_active_visitors', - token: 'mock-token', - refresh: vi.fn(), - }); + mockUseTinybirdQuery.mockReturnValue(queryState({ loading: true })); - const { result } = renderHook(() => useActiveVisitors({ statsConfig, enabled: true }), { - wrapper, - }); + const { result } = renderHook(() => useActiveVisitors({ statsConfig, enabled: true })); - // Should show loading on initial load when no lastKnownCount exists expect(result.current.isLoading).toBe(true); expect(result.current.activeVisitors).toBe(0); }); - it('does not show loading when lastKnownCount exists', () => { - // First render with data to establish lastKnownCount - mockUseQuery.mockReturnValue({ - data: [{ active_visitors: 25 }], - loading: false, - error: null, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/api_active_visitors', - token: 'mock-token', - refresh: vi.fn(), - }); + it('does not show loading when a last known count exists', () => { + mockUseTinybirdQuery.mockReturnValue(queryState({ data: [{ active_visitors: 25 }] })); - const { result, rerender } = renderHook( - () => useActiveVisitors({ statsConfig, enabled: true }), - { wrapper }, + const { result, rerender } = renderHook(() => + useActiveVisitors({ statsConfig, enabled: true }), ); expect(result.current.activeVisitors).toBe(25); - // Second render with loading but data should not show loading - mockUseQuery.mockReturnValue({ - data: null, - loading: true, - error: null, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/api_active_visitors', - token: 'mock-token', - refresh: vi.fn(), - }); - + mockUseTinybirdQuery.mockReturnValue(queryState({ data: null, loading: true })); rerender(); expect(result.current.isLoading).toBe(false); - expect(result.current.activeVisitors).toBe(25); // Retains last known count + expect(result.current.activeVisitors).toBe(25); }); it('returns active visitor count from data', () => { - mockUseQuery.mockReturnValue({ - data: [{ active_visitors: 42 }], - loading: false, - error: null, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/api_active_visitors', - token: 'mock-token', - refresh: vi.fn(), - }); + mockUseTinybirdQuery.mockReturnValue(queryState({ data: [{ active_visitors: 42 }] })); - const { result } = renderHook(() => useActiveVisitors({ statsConfig, enabled: true }), { - wrapper, - }); + const { result } = renderHook(() => useActiveVisitors({ statsConfig, enabled: true })); expect(result.current.activeVisitors).toBe(42); expect(result.current.isLoading).toBe(false); }); it('handles error state', () => { - const mockError = 'Network error'; - mockUseQuery.mockReturnValue({ - data: null, - loading: false, - error: mockError, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/api_active_visitors', - token: 'mock-token', - refresh: vi.fn(), - }); + const mockError = new Error('Network error'); + mockUseTinybirdQuery.mockReturnValue(queryState({ error: mockError })); - const { result } = renderHook(() => useActiveVisitors({ statsConfig, enabled: true }), { - wrapper, - }); + const { result } = renderHook(() => useActiveVisitors({ statsConfig, enabled: true })); expect(result.current.error).toBe(mockError); }); - it('calls getStatEndpointUrl with correct parameters and uses tinybirdToken', () => { - renderHook(() => useActiveVisitors({ statsConfig, enabled: true }), { wrapper }); - - expect(mockGetStatEndpointUrl).toHaveBeenCalledWith(statsConfig, 'api_active_visitors'); - expect(mockUseTinybirdToken).toHaveBeenCalled(); - }); - - it('calls useTinybirdQuery with undefined endpoint and token when no statsConfig', () => { - renderHook(() => useActiveVisitors({ enabled: true }), { wrapper }); - - expect(mockUseQuery).toHaveBeenCalledWith( - expect.objectContaining({ - endpoint: undefined, - token: undefined, - params: expect.objectContaining({ - site_uuid: '', - }), - }), - ); - expect(mockUseTinybirdToken).toHaveBeenCalledWith({ enabled: false }); - }); - - it('sets up 60-second interval when enabled', () => { - renderHook(() => useActiveVisitors({ enabled: true }), { wrapper }); - - // Initially refreshKey should be 0 - expect(mockUseQuery).toHaveBeenCalledWith( - expect.objectContaining({ - params: expect.objectContaining({ - _refresh: '0', - }), - }), - ); - - // Fast-forward 60 seconds - act(() => { - vi.advanceTimersByTime(60000); - }); - - // Should increment refreshKey - expect(mockUseQuery).toHaveBeenCalledWith( - expect.objectContaining({ - params: expect.objectContaining({ - _refresh: '1', - }), - }), - ); - }); - - it('does not set up interval when disabled', () => { - renderHook(() => useActiveVisitors({ enabled: false }), { wrapper }); - - // Fast-forward 60 seconds - act(() => { - vi.advanceTimersByTime(60000); - }); - - // Should still be at refreshKey 0 - expect(mockUseQuery).toHaveBeenCalledWith( - expect.objectContaining({ - params: expect.objectContaining({ - _refresh: '0', - }), - }), - ); - }); - it('includes postUuid in params when provided', () => { const postUuid = 'test-post-uuid'; - renderHook(() => useActiveVisitors({ postUuid, enabled: true }), { wrapper }); + renderHook(() => useActiveVisitors({ postUuid, enabled: true })); - expect(mockUseQuery).toHaveBeenCalledWith( + expect(mockUseTinybirdQuery).toHaveBeenCalledWith( expect.objectContaining({ - params: expect.objectContaining({ - post_uuid: postUuid, - }), + params: expect.objectContaining({ post_uuid: postUuid }), }), ); }); it('does not include postUuid in params when not provided', () => { - renderHook(() => useActiveVisitors({ enabled: true }), { wrapper }); + renderHook(() => useActiveVisitors({ enabled: true })); - expect(mockUseQuery).toHaveBeenCalledWith( + expect(mockUseTinybirdQuery).toHaveBeenCalledWith( expect.objectContaining({ - params: expect.not.objectContaining({ - post_uuid: expect.anything(), - }), + params: expect.not.objectContaining({ post_uuid: expect.anything() }), }), ); }); - it('uses statsConfig for site_uuid', () => { - renderHook(() => useActiveVisitors({ statsConfig, enabled: true }), { wrapper }); + it('uses statsConfig for site_uuid, falling back to an empty string', () => { + const { rerender } = renderHook( + ({ config }: { config?: typeof statsConfig }) => + useActiveVisitors({ statsConfig: config, enabled: true }), + { initialProps: { config: statsConfig as typeof statsConfig | undefined } }, + ); - expect(mockUseQuery).toHaveBeenCalledWith( + expect(mockUseTinybirdQuery).toHaveBeenLastCalledWith( expect.objectContaining({ - params: expect.objectContaining({ - site_uuid: 'test-site-id', - }), + params: expect.objectContaining({ site_uuid: 'test-site-id' }), }), ); - }); - it('uses empty string for site_uuid when no statsConfig', () => { - renderHook(() => useActiveVisitors({ enabled: true }), { wrapper }); + rerender({ config: undefined }); - expect(mockUseQuery).toHaveBeenCalledWith( + expect(mockUseTinybirdQuery).toHaveBeenLastCalledWith( expect.objectContaining({ - params: expect.objectContaining({ - site_uuid: '', - }), + params: expect.objectContaining({ site_uuid: '' }), }), ); }); - it('retains last known count after refresh', () => { - // Initial data - mockUseQuery.mockReturnValue({ - data: [{ active_visitors: 25 }], - loading: false, - error: null, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/api_active_visitors', - token: 'mock-token', - refresh: vi.fn(), - }); + it('retains the last known count when data becomes null', () => { + mockUseTinybirdQuery.mockReturnValue(queryState({ data: [{ active_visitors: 15 }] })); - const { result, rerender } = renderHook( - () => useActiveVisitors({ statsConfig, enabled: true }), - { wrapper }, + const { result, rerender } = renderHook(() => + useActiveVisitors({ statsConfig, enabled: true }), ); - expect(result.current.activeVisitors).toBe(25); - - // Simulate refresh with loading state but no new data - mockUseQuery.mockReturnValue({ - data: null, - loading: true, - error: null, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/api_active_visitors', - token: 'mock-token', - refresh: vi.fn(), - }); + expect(result.current.activeVisitors).toBe(15); + mockUseTinybirdQuery.mockReturnValue(queryState({ data: null })); rerender(); - // Should retain last known count and not show loading - expect(result.current.activeVisitors).toBe(25); - expect(result.current.isLoading).toBe(false); + expect(result.current.activeVisitors).toBe(15); }); it('handles zero active visitors correctly', () => { - mockUseQuery.mockReturnValue({ - data: [{ active_visitors: 0 }], - loading: false, - error: null, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/api_active_visitors', - token: 'mock-token', - refresh: vi.fn(), - }); + mockUseTinybirdQuery.mockReturnValue(queryState({ data: [{ active_visitors: 0 }] })); - const { result } = renderHook(() => useActiveVisitors({ enabled: true }), { wrapper }); + const { result } = renderHook(() => useActiveVisitors({ enabled: true })); expect(result.current.activeVisitors).toBe(0); }); it('handles invalid data format gracefully', () => { - mockUseQuery.mockReturnValue({ - data: [{ some_other_field: 42 }], - loading: false, - error: null, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/api_active_visitors', - token: 'mock-token', - refresh: vi.fn(), - }); + mockUseTinybirdQuery.mockReturnValue(queryState({ data: [{ some_other_field: 42 }] })); - const { result } = renderHook(() => useActiveVisitors({ enabled: true }), { wrapper }); + const { result } = renderHook(() => useActiveVisitors({ enabled: true })); expect(result.current.activeVisitors).toBe(0); }); - it('cleans up interval on unmount', () => { - // Spy on clearInterval before creating the hook - const clearIntervalSpy = vi.spyOn(global, 'clearInterval'); - - const { unmount } = renderHook(() => useActiveVisitors({ enabled: true }), { wrapper }); - - unmount(); - - expect(clearIntervalSpy).toHaveBeenCalled(); - }); - - it('updates lastKnownCount when new valid data is received', () => { - // Start with no data - mockUseQuery.mockReturnValue({ - data: null, - loading: false, - error: null, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/api_active_visitors', - token: 'mock-token', - refresh: vi.fn(), - }); + it('does not update the count when disabled', () => { + mockUseTinybirdQuery.mockReturnValue(queryState({ data: [{ active_visitors: 20 }] })); const { result, rerender } = renderHook( - () => useActiveVisitors({ statsConfig, enabled: true }), - { wrapper }, - ); - expect(result.current.activeVisitors).toBe(0); - - // Provide valid data - mockUseQuery.mockReturnValue({ - data: [{ active_visitors: 15 }], - loading: false, - error: null, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/api_active_visitors', - token: 'mock-token', - refresh: vi.fn(), - }); - - rerender(); - - expect(result.current.activeVisitors).toBe(15); - - // Now when data becomes null again, should retain the count - mockUseQuery.mockReturnValue({ - data: null, - loading: false, - error: null, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/api_active_visitors', - token: 'mock-token', - refresh: vi.fn(), - }); - - rerender(); - - expect(result.current.activeVisitors).toBe(15); - }); - - it('handles statsConfig changes correctly', () => { - const initialStatsConfig = { - id: 'initial-site-id', - endpoint: 'https://initial.api.com', - token: 'initial-token', - }; - - const { rerender } = renderHook( - ({ statsConfig: currentStatsConfig }) => - useActiveVisitors({ statsConfig: currentStatsConfig, enabled: true }), - { initialProps: { statsConfig: initialStatsConfig }, wrapper }, - ); - - expect(mockGetStatEndpointUrl).toHaveBeenCalledWith(initialStatsConfig, 'api_active_visitors'); - expect(mockUseTinybirdToken).toHaveBeenCalled(); - - // Change statsConfig - const newStatsConfig = { - id: 'new-site-id', - endpoint: 'https://new.api.com', - token: 'new-token', - }; - - rerender({ statsConfig: newStatsConfig }); - - expect(mockGetStatEndpointUrl).toHaveBeenCalledWith(newStatsConfig, 'api_active_visitors'); - expect(mockUseTinybirdToken).toHaveBeenCalled(); - }); - - it('does not update lastKnownCount when disabled', () => { - // Start enabled with data - mockUseQuery.mockReturnValue({ - data: [{ active_visitors: 20 }], - loading: false, - error: null, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/api_active_visitors', - token: 'mock-token', - refresh: vi.fn(), - }); - - const { result, rerender } = renderHook( - ({ enabled }) => useActiveVisitors({ statsConfig, enabled }), - { initialProps: { enabled: true }, wrapper }, + ({ enabled }: { enabled: boolean }) => useActiveVisitors({ statsConfig, enabled }), + { initialProps: { enabled: true } }, ); expect(result.current.activeVisitors).toBe(20); - // Disable and provide new data - mockUseQuery.mockReturnValue({ - data: [{ active_visitors: 30 }], - loading: false, - error: null, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/api_active_visitors', - token: 'mock-token', - refresh: vi.fn(), - }); - + mockUseTinybirdQuery.mockReturnValue(queryState({ data: [{ active_visitors: 30 }] })); rerender({ enabled: false }); - // Should return 0 when disabled, regardless of new data expect(result.current.activeVisitors).toBe(0); expect(result.current.error).toBeNull(); }); - - it('resets interval when enabled state changes', () => { - const clearIntervalSpy = vi.spyOn(global, 'clearInterval'); - const setIntervalSpy = vi.spyOn(global, 'setInterval'); - - // Clear any previous calls - setIntervalSpy.mockClear(); - clearIntervalSpy.mockClear(); - - const { rerender } = renderHook(({ enabled }) => useActiveVisitors({ enabled }), { - initialProps: { enabled: true }, - wrapper, - }); - - expect(setIntervalSpy).toHaveBeenCalledTimes(1); - const firstIntervalId = setIntervalSpy.mock.results[0]?.value; - - // Disable - rerender({ enabled: false }); - - expect(clearIntervalSpy).toHaveBeenCalledWith(firstIntervalId); - - // Re-enable - rerender({ enabled: true }); - - // Should create a new interval - expect(setIntervalSpy).toHaveBeenCalledTimes(2); - }); - - it('should call useQuery with undefined endpoint when token is loading (preventing HTTP requests)', () => { - // Mock useTinybirdToken to return undefined token (still loading) - mockUseTinybirdToken.mockReturnValue({ - token: undefined, - isLoading: true, - error: null, - refetch: vi.fn(), - }); - - // Clear any previous calls - mockUseQuery.mockClear(); - - // Render the hook - renderHook(() => useActiveVisitors({ statsConfig, enabled: true }), { wrapper }); - - // EXPECTED: useQuery should be called with undefined endpoint when token is loading - // This prevents HTTP requests by disabling the SWR query entirely - expect(mockUseQuery).toHaveBeenCalledWith( - expect.objectContaining({ - endpoint: undefined, - token: undefined, - }), - ); - }); - - it('calls useQuery with token when token is available', () => { - // Mock useTinybirdToken to return a valid token - mockUseTinybirdToken.mockReturnValue({ - token: 'valid-token', - isLoading: false, - error: null, - refetch: vi.fn(), - }); - - // Clear any previous calls - mockUseQuery.mockClear(); - - // Render the hook - renderHook(() => useActiveVisitors({ statsConfig, enabled: true }), { wrapper }); - - // Should call useQuery with the valid token - expect(mockUseQuery).toHaveBeenCalledWith( - expect.objectContaining({ - token: 'valid-token', - }), - ); - }); - - it('transitions from undefined to valid token correctly', () => { - // Start with undefined token - mockUseTinybirdToken.mockReturnValue({ - token: undefined, - isLoading: true, - error: null, - refetch: vi.fn(), - }); - - // Render the hook - const { rerender } = renderHook(() => useActiveVisitors({ statsConfig, enabled: true }), { - wrapper, - }); - - // Verify first call with undefined token (due to tokenLoading: true) - expect(mockUseQuery).toHaveBeenLastCalledWith( - expect.objectContaining({ - token: undefined, - }), - ); - - // Clear previous calls - mockUseQuery.mockClear(); - - // Now provide a valid token - mockUseTinybirdToken.mockReturnValue({ - token: 'valid-token', - isLoading: false, - error: null, - refetch: vi.fn(), - }); - - // Rerender - rerender(); - - // Should now call useQuery with the valid token - expect(mockUseQuery).toHaveBeenLastCalledWith( - expect.objectContaining({ - token: 'valid-token', - }), - ); - }); - - it('shows loading state when token is loading', () => { - // Mock token as loading - mockUseTinybirdToken.mockReturnValue({ - token: undefined, - isLoading: true, - error: null, - refetch: vi.fn(), - }); - - // Mock useQuery to return no loading (since token loading should be considered) - mockUseQuery.mockReturnValue({ - data: null, - loading: false, - error: null, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/api_active_visitors', - token: undefined, - refresh: vi.fn(), - }); - - const { result } = renderHook(() => useActiveVisitors({ statsConfig, enabled: true }), { - wrapper, - }); - - // Should show loading because token is loading and no lastKnownCount - expect(result.current.isLoading).toBe(true); - expect(result.current.activeVisitors).toBe(0); - }); }); diff --git a/apps/admin-x-framework/test/unit/hooks/use-tinybird-query.test.ts b/apps/admin-x-framework/test/unit/hooks/use-tinybird-query.test.ts index 9c00585fa1f..5b88ec38541 100644 --- a/apps/admin-x-framework/test/unit/hooks/use-tinybird-query.test.ts +++ b/apps/admin-x-framework/test/unit/hooks/use-tinybird-query.test.ts @@ -1,16 +1,7 @@ -import { renderHook } from '@testing-library/react'; -import { useTinybirdQuery } from '../../../src/hooks/use-tinybird-query'; -import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { act, renderHook, waitFor } from '@testing-library/react'; +import { focusManager, QueryClient, QueryClientProvider } from '@tanstack/react-query'; import React from 'react'; -vi.mock('@tinybirdco/charts', () => ({ - useQuery: vi.fn(), -})); - -vi.mock('../../../src/utils/stats-config', () => ({ - getStatEndpointUrl: vi.fn(), -})); - vi.mock('../../../src/hooks/use-tinybird-token', () => ({ useTinybirdToken: vi.fn(), })); @@ -19,372 +10,321 @@ vi.mock('../../../src/api/settings', () => ({ useWebAnalyticsEnabled: vi.fn(), })); +import { + fetchTinybirdPipe, + parseTinybirdPipeResponse, + useTinybirdQuery, +} from '../../../src/hooks/use-tinybird-query'; import { useTinybirdToken } from '../../../src/hooks/use-tinybird-token'; import { useWebAnalyticsEnabled } from '../../../src/api/settings'; -import { getStatEndpointUrl } from '../../../src/utils/stats-config'; -import { useQuery } from '@tinybirdco/charts'; -const mockUseQuery = vi.mocked(useQuery); const mockUseTinybirdToken = vi.mocked(useTinybirdToken); -const mockGetStatEndpointUrl = vi.mocked(getStatEndpointUrl); const mockUseWebAnalyticsEnabled = vi.mocked(useWebAnalyticsEnabled); +const statsConfig = { id: 'site-1', endpoint: 'https://tinybird.example.com' }; +const PIPE_URL = 'https://tinybird.example.com/v0/pipes/api_test.json'; + +const rows = [{ visits: 42 }]; +const meta = [{ name: 'visits', type: 'UInt64' }]; + +const okResponse = (body: unknown = { data: rows, meta }) => ({ + ok: true, + status: 200, + statusText: 'OK', + json: async () => body, + text: async () => JSON.stringify(body), +}); + +const errorResponse = (status: number, body = '') => ({ + ok: false, + status, + statusText: 'Error', + json: async () => ({}), + text: async () => body, +}); + +const tokenState = (overrides: Partial> = {}) => ({ + token: 'token-a' as string | undefined, + isLoading: false, + error: null, + refetch: vi.fn().mockResolvedValue('token-a'), + ...overrides, +}); + describe('useTinybirdQuery', () => { let queryClient: QueryClient; let wrapper: React.FC<{ children: React.ReactNode }>; + let fetchMock: ReturnType; + + const renderQuery = (options: Partial[0]> = {}) => + renderHook( + () => + useTinybirdQuery({ + statsConfig, + endpoint: 'api_test', + params: { site_uuid: 'site-1' }, + ...options, + }), + { wrapper }, + ); beforeEach(() => { queryClient = new QueryClient(); wrapper = ({ children }) => React.createElement(QueryClientProvider, { client: queryClient }, children); - mockUseTinybirdToken.mockReturnValue({ - token: undefined, - isLoading: true, - error: null, - refetch: vi.fn(), - }); - mockUseQuery.mockReturnValue({ - data: null, - loading: false, - error: null, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/test', - token: undefined, - refresh: vi.fn(), - }); - mockGetStatEndpointUrl.mockImplementation( - (_config: any, endpoint: any) => `https://api.example.com/${endpoint}`, - ); + fetchMock = vi.fn().mockResolvedValue(okResponse()); + vi.stubGlobal('fetch', fetchMock); + mockUseTinybirdToken.mockReturnValue(tokenState()); mockUseWebAnalyticsEnabled.mockReturnValue(true); }); afterEach(() => { + queryClient.clear(); + vi.unstubAllGlobals(); vi.clearAllMocks(); }); - it('should return data, meta, loading, and error', () => { - const { result } = renderHook( - () => - useTinybirdQuery({ - statsConfig: { id: '123' }, - endpoint: 'test', - params: {}, - }), - { wrapper }, - ); + it('fetches the pipe with params and a bearer token and returns data and meta', async () => { + const { result } = renderQuery(); - expect(result.current.data).toBeDefined(); - expect(result.current.loading).toBeDefined(); - expect(result.current.error).toBeDefined(); - }); - - it('should set the endpoint to undefined if the token is not loaded', () => { - // This prevents an initial 403 error by waiting for the token to load before making the request - mockUseTinybirdToken.mockReturnValue({ - token: undefined, - isLoading: true, - error: null, - refetch: vi.fn(), + await waitFor(() => { + expect(result.current.data).toEqual(rows); }); + expect(result.current.meta).toEqual(meta); + expect(result.current.loading).toBe(false); + expect(result.current.error).toBe(null); - renderHook( - () => - useTinybirdQuery({ - statsConfig: { id: '123' }, - endpoint: 'test', - params: {}, - }), - { wrapper }, - ); + expect(fetchMock).toHaveBeenCalledTimes(1); + const [url, init] = fetchMock.mock.calls[0]; + expect(url).toBe(`${PIPE_URL}?site_uuid=site-1`); + expect(init.headers).toEqual({ Authorization: 'Bearer token-a' }); + expect(init.credentials).toBe('omit'); + }); + + it('appends the version suffix from statsConfig to the pipe name', async () => { + const { result } = renderQuery({ statsConfig: { ...statsConfig, version: 'v2' } }); - expect(mockUseQuery).toHaveBeenCalledWith( - expect.objectContaining({ - endpoint: undefined, - }), + await waitFor(() => { + expect(result.current.data).toEqual(rows); + }); + expect(fetchMock.mock.calls[0][0]).toBe( + 'https://tinybird.example.com/v0/pipes/api_test_v2.json?site_uuid=site-1', ); }); - it('should not fetch a token or query Tinybird when disabled', () => { - const mockError = new Error('Token error'); - mockUseTinybirdToken.mockReturnValue({ - token: 'mock-token', - isLoading: true, - error: mockError, - refetch: vi.fn(), - }); - mockUseQuery.mockReturnValue({ - data: [{ visits: 1 }], - loading: true, - error: 'Query error', - meta: [{ name: 'visits', type: 'UInt64' }], - statistics: null, - endpoint: 'https://api.example.com/test', - token: 'mock-token', - refresh: vi.fn(), + it('reports loading and does not fetch until the token has loaded', async () => { + mockUseTinybirdToken.mockReturnValue(tokenState({ token: undefined, isLoading: true })); + const { result, rerender } = renderQuery(); + + expect(result.current.loading).toBe(true); + expect(fetchMock).not.toHaveBeenCalled(); + + mockUseTinybirdToken.mockReturnValue(tokenState()); + rerender(); + + await waitFor(() => { + expect(result.current.data).toEqual(rows); }); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); - const { result } = renderHook( - () => - useTinybirdQuery({ - statsConfig: { id: '123' }, - endpoint: 'test', - params: {}, - enabled: false, - }), - { wrapper }, - ); + it('does not fetch a token or query Tinybird when disabled', () => { + const { result } = renderQuery({ enabled: false }); expect(mockUseTinybirdToken).toHaveBeenCalledWith({ enabled: false }); - expect(mockGetStatEndpointUrl).not.toHaveBeenCalled(); - expect(mockUseQuery).toHaveBeenCalledWith( - expect.objectContaining({ - endpoint: undefined, - token: undefined, - }), - ); - expect(result.current.loading).toBe(false); - expect(result.current.error).toBe(null); - expect(result.current.data).toBe(null); - expect(result.current.meta).toBe(null); + expect(fetchMock).not.toHaveBeenCalled(); + expect(result.current).toEqual({ data: null, meta: null, loading: false, error: null }); }); - it('should not fetch a token or query Tinybird without statsConfig', () => { - renderHook( - () => - useTinybirdQuery({ - endpoint: 'test', - params: {}, - }), - { wrapper }, - ); + it('does not fetch a token or query Tinybird without statsConfig', () => { + const { result } = renderQuery({ statsConfig: undefined }); expect(mockUseTinybirdToken).toHaveBeenCalledWith({ enabled: false }); - expect(mockGetStatEndpointUrl).not.toHaveBeenCalled(); - expect(mockUseQuery).toHaveBeenCalledWith( - expect.objectContaining({ - endpoint: undefined, - token: undefined, - }), - ); + expect(fetchMock).not.toHaveBeenCalled(); + expect(result.current).toEqual({ data: null, meta: null, loading: false, error: null }); }); - it('should call useQuery with the correct token', () => { - mockUseTinybirdToken.mockReturnValue({ - token: 'mock-token', - isLoading: false, - error: null, - refetch: vi.fn(), - }); + it('noops when the web analytics kill-switch is off', () => { + mockUseWebAnalyticsEnabled.mockReturnValue(false); - renderHook( - () => - useTinybirdQuery({ - statsConfig: { id: '123' }, - endpoint: 'test', - params: {}, - }), - { wrapper }, - ); + const { result } = renderQuery(); - expect(mockUseQuery).toHaveBeenCalledWith( - expect.objectContaining({ - token: 'mock-token', - }), - ); + expect(mockUseTinybirdToken).toHaveBeenCalledWith({ enabled: false }); + expect(fetchMock).not.toHaveBeenCalled(); + expect(result.current).toEqual({ data: null, meta: null, loading: false, error: null }); }); - it('should call useQuery with the correct endpoint once the token is loaded', () => { - mockUseTinybirdToken.mockReturnValue({ - token: 'mock-token', - isLoading: false, - error: null, - refetch: vi.fn(), + it('keeps the cached result when the token rotates', async () => { + const { result, rerender } = renderQuery(); + + await waitFor(() => { + expect(result.current.data).toEqual(rows); }); + expect(fetchMock).toHaveBeenCalledTimes(1); - renderHook( - () => - useTinybirdQuery({ - statsConfig: { id: '123' }, - endpoint: 'test', - params: {}, - }), - { wrapper }, - ); + // Rotate the token: same query key, so no refetch and no data reset. + mockUseTinybirdToken.mockReturnValue(tokenState({ token: 'token-b' })); + rerender(); - expect(mockUseQuery).toHaveBeenCalledWith( - expect.objectContaining({ - endpoint: 'https://api.example.com/test', - }), - ); + expect(result.current.data).toEqual(rows); + expect(fetchMock).toHaveBeenCalledTimes(1); }); - it('should return loading state that includes token loading', () => { - mockUseTinybirdToken.mockReturnValue({ - token: 'mock-token', - isLoading: true, - error: null, - refetch: vi.fn(), + it('uses the current token when refetching after a rotation', async () => { + const { result, rerender } = renderQuery(); + + await waitFor(() => { + expect(result.current.data).toEqual(rows); }); - const { result } = renderHook( - () => - useTinybirdQuery({ - statsConfig: { id: '123' }, - endpoint: 'test', - params: {}, - }), - { wrapper }, - ); + mockUseTinybirdToken.mockReturnValue(tokenState({ token: 'token-b' })); + rerender(); - expect(result.current.loading).toBe(true); - }); + await act(async () => { + await queryClient.invalidateQueries({ queryKey: ['tinybird'] }); + }); - it('should pass the correct params to useQuery', () => { - mockUseTinybirdToken.mockReturnValue({ - token: 'mock-token', - isLoading: false, - error: null, - refetch: vi.fn(), + await waitFor(() => { + expect(fetchMock).toHaveBeenCalledTimes(2); }); + expect(fetchMock.mock.calls[1][1].headers).toEqual({ Authorization: 'Bearer token-b' }); + }); - renderHook( - () => - useTinybirdQuery({ - statsConfig: { id: '123' }, - endpoint: 'test', - params: { test: 'test' }, - }), - { wrapper }, - ); + it('polls at the configured refetchInterval', async () => { + renderQuery({ refetchInterval: 30 }); - expect(mockUseQuery).toHaveBeenCalledWith( - expect.objectContaining({ - params: { test: 'test' }, - }), - ); + await waitFor(() => { + expect(fetchMock.mock.calls.length).toBeGreaterThanOrEqual(2); + }); }); - it('handles errors from useQuery', () => { - const mockError = 'Network error'; - mockUseQuery.mockReturnValue({ - data: null, - loading: false, - error: mockError, - meta: null, - statistics: null, - endpoint: 'https://api.example.com/test', - token: undefined, - refresh: vi.fn(), + it('surfaces request failures as errors without retrying', async () => { + fetchMock.mockResolvedValue(errorResponse(500, 'pipe exploded')); + + const { result } = renderQuery(); + + await waitFor(() => { + expect(result.current.error).toBeInstanceOf(Error); }); + expect(result.current.error?.message).toBe('Tinybird request failed (500): pipe exploded'); + expect(result.current.data).toBe(null); + expect(result.current.loading).toBe(false); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); - const { result } = renderHook( - () => - useTinybirdQuery({ - statsConfig: { id: '123' }, - endpoint: 'test', - params: {}, - }), - { wrapper }, - ); + it('surfaces the token query error', () => { + const tokenError = new Error('Token error'); + mockUseTinybirdToken.mockReturnValue(tokenState({ token: undefined, error: tokenError })); - expect(result.current.error).toBe(mockError); + const { result } = renderQuery(); + + expect(result.current.error).toBe(tokenError); + expect(result.current.loading).toBe(false); + expect(fetchMock).not.toHaveBeenCalled(); }); - it('should return the error from useQuery if the token query has an error', () => { - const mockError = new Error('Token error'); - mockUseTinybirdToken.mockReturnValue({ - token: undefined, - isLoading: false, - error: mockError, - refetch: vi.fn(), + it('refetches stale data when the window regains focus', async () => { + const { result } = renderQuery(); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(fetchMock).toHaveBeenCalledTimes(1); + + // Age the cached entry past the stale window, then simulate a tab return. + const entry = queryClient.getQueryCache().getAll()[0]; + entry.state.dataUpdatedAt = Date.now() - 61_000; + act(() => { + focusManager.setFocused(false); + focusManager.setFocused(true); }); - const { result } = renderHook( - () => - useTinybirdQuery({ - statsConfig: { id: '123' }, - endpoint: 'test', - params: {}, - }), - { wrapper }, - ); - - expect(result.current.error).toBe(mockError); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2)); + focusManager.setFocused(undefined); }); - describe('web analytics gate', () => { - it('noops without a per-call flag when web analytics is disabled', () => { - mockUseWebAnalyticsEnabled.mockReturnValue(false); - mockUseTinybirdToken.mockReturnValue({ - token: 'mock-token', - isLoading: false, - error: null, - refetch: vi.fn(), - }); - mockUseQuery.mockReturnValue({ - data: [{ visits: 1 }], - loading: true, - error: 'Query error', - meta: [{ name: 'visits', type: 'UInt64' }], - statistics: null, - endpoint: 'https://api.example.com/test', - token: 'mock-token', - refresh: vi.fn(), - }); - - // enabled defaults to true and statsConfig/endpoint are provided — - // only the kill-switch should suppress the query. - const { result } = renderHook( - () => - useTinybirdQuery({ - statsConfig: { id: '123' }, - endpoint: 'test', - params: {}, - }), - { wrapper }, - ); - - expect(mockGetStatEndpointUrl).not.toHaveBeenCalled(); - expect(mockUseTinybirdToken).toHaveBeenCalledWith({ enabled: false }); - expect(mockUseQuery).toHaveBeenCalledWith( - expect.objectContaining({ - endpoint: undefined, - token: undefined, - }), - ); - expect(result.current.data).toBe(null); - expect(result.current.meta).toBe(null); - expect(result.current.loading).toBe(false); - expect(result.current.error).toBe(null); + it('passes the background polling flag through to the query', () => { + const { result } = renderQuery({ + refetchInterval: 60_000, + refetchIntervalInBackground: true, }); + void result; + const entry = queryClient.getQueryCache().getAll()[0]; + expect(entry.observers[0]?.options.refetchIntervalInBackground).toBe(true); + expect(entry.observers[0]?.options.refetchInterval).toBe(60_000); + }); +}); - it('queries normally when web analytics is enabled', () => { - mockUseWebAnalyticsEnabled.mockReturnValue(true); - mockUseTinybirdToken.mockReturnValue({ - token: 'mock-token', - isLoading: false, - error: null, - refetch: vi.fn(), - }); - - renderHook( - () => - useTinybirdQuery({ - statsConfig: { id: '123' }, - endpoint: 'test', - params: {}, - }), - { wrapper }, - ); - - expect(mockUseTinybirdToken).toHaveBeenCalledWith({ enabled: true }); - expect(mockUseQuery).toHaveBeenCalledWith( - expect.objectContaining({ - endpoint: 'https://api.example.com/test', - token: 'mock-token', - }), - ); +describe('fetchTinybirdPipe', () => { + let fetchMock: ReturnType; + + beforeEach(() => { + fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + vi.clearAllMocks(); + }); + + it('refreshes the token and retries once on a 403', async () => { + fetchMock.mockResolvedValueOnce(errorResponse(403)).mockResolvedValueOnce(okResponse()); + const refreshToken = vi.fn().mockResolvedValue('token-b'); + + const result = await fetchTinybirdPipe({ url: PIPE_URL, token: 'token-a', refreshToken }); + + expect(result).toEqual({ data: rows, meta }); + expect(refreshToken).toHaveBeenCalledTimes(1); + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(fetchMock.mock.calls[1][1].headers).toEqual({ Authorization: 'Bearer token-b' }); + }); + + it('does not retry when the refreshed token is unchanged', async () => { + fetchMock.mockResolvedValue(errorResponse(403, 'forbidden')); + const refreshToken = vi.fn().mockResolvedValue('token-a'); + + await expect( + fetchTinybirdPipe({ url: PIPE_URL, token: 'token-a', refreshToken }), + ).rejects.toThrow('Tinybird request failed (403): forbidden'); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('does not retry when no fresh token is available', async () => { + fetchMock.mockResolvedValue(errorResponse(401)); + const refreshToken = vi.fn().mockResolvedValue(undefined); + + await expect( + fetchTinybirdPipe({ url: PIPE_URL, token: 'token-a', refreshToken }), + ).rejects.toThrow('Tinybird request failed (401): Error'); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('does not refresh the token for non-auth failures', async () => { + fetchMock.mockResolvedValue(errorResponse(500, 'boom')); + const refreshToken = vi.fn(); + + await expect( + fetchTinybirdPipe({ url: PIPE_URL, token: 'token-a', refreshToken }), + ).rejects.toThrow('Tinybird request failed (500): boom'); + expect(refreshToken).not.toHaveBeenCalled(); + }); +}); + +describe('parseTinybirdPipeResponse', () => { + it('accepts the envelope with loose pipe-shaped rows', () => { + const body = { + data: [{ pathname: '/', visits: 3, referrer: null }], + meta: [{ name: 'visits', type: 'UInt64' }], + }; + expect(parseTinybirdPipeResponse(body)).toBe(body); + expect(parseTinybirdPipeResponse({})).toEqual({}); + expect(parseTinybirdPipeResponse({ data: null, meta: null })).toEqual({ + data: null, + meta: null, }); }); + + it('rejects non-envelope bodies', () => { + for (const bad of [null, [], 'error', { data: 'nope' }, { data: [1, 2] }, { meta: 'x' }]) { + expect(() => parseTinybirdPipeResponse(bad)).toThrow('unexpected response shape'); + } + }); }); diff --git a/apps/admin-x-framework/test/unit/hooks/use-tinybird-token.test.tsx b/apps/admin-x-framework/test/unit/hooks/use-tinybird-token.test.tsx index ab4d5d53398..8abeff14e8e 100644 --- a/apps/admin-x-framework/test/unit/hooks/use-tinybird-token.test.tsx +++ b/apps/admin-x-framework/test/unit/hooks/use-tinybird-token.test.tsx @@ -138,8 +138,26 @@ describe('useTinybirdToken', () => { expect(result.current.error).toBe(apiError); }); - it('exposes refetch function', () => { - const mockRefetch = vi.fn(); + it('exposes a refetch that resolves the fresh token', async () => { + const mockRefetch = vi.fn().mockResolvedValue({ + data: { tinybird: { token: 'fresh-token' } }, + }); + + mockUseTinybirdTokenQuery.mockReturnValue({ + data: { tinybird: { token: 'test-token' } }, + isLoading: false, + error: null, + refetch: mockRefetch, + } as any); + + const { result } = renderHook(() => useTinybirdToken(), { wrapper }); + + await expect(result.current.refetch()).resolves.toBe('fresh-token'); + expect(mockRefetch).toHaveBeenCalledTimes(1); + }); + + it('resolves undefined from refetch when no fresh token comes back', async () => { + const mockRefetch = vi.fn().mockResolvedValue({ data: null }); mockUseTinybirdTokenQuery.mockReturnValue({ data: { tinybird: { token: 'test-token' } }, @@ -150,7 +168,7 @@ describe('useTinybirdToken', () => { const { result } = renderHook(() => useTinybirdToken(), { wrapper }); - expect(result.current.refetch).toBe(mockRefetch); + await expect(result.current.refetch()).resolves.toBeUndefined(); }); it('refreshes token when stale time expires', () => { diff --git a/apps/admin/package.json b/apps/admin/package.json index eaa776a3b35..98bb64a23ce 100644 --- a/apps/admin/package.json +++ b/apps/admin/package.json @@ -31,6 +31,7 @@ "@tanstack/react-virtual": "catalog:", "@tryghost/activitypub": "workspace:*", "@tryghost/admin-x-framework": "workspace:*", + "@tryghost/checkout": "workspace:*", "@tryghost/color-utils": "catalog:", "@tryghost/custom-fonts": "catalog:", "@tryghost/i18n": "workspace:*", diff --git a/apps/admin/src/ember-bridge/ember-bridge.test.tsx b/apps/admin/src/ember-bridge/ember-bridge.test.tsx index 9344ab1e7e1..6a10f58e39f 100644 --- a/apps/admin/src/ember-bridge/ember-bridge.test.tsx +++ b/apps/admin/src/ember-bridge/ember-bridge.test.tsx @@ -416,6 +416,7 @@ describe('useEmberRouting', () => { baseTest('returns bridge routing methods when bridge is available', () => { const mock = createMockStateBridge(); + mock.stateBridge.isRouteActive = vi.fn(() => true); window.EmberBridge = { state: mock.stateBridge }; const { result } = renderHook(() => useEmberRouting()); @@ -423,9 +424,11 @@ describe('useEmberRouting', () => { expect(result.current).toHaveProperty('getRouteUrl'); expect(result.current).toHaveProperty('isRouteActive'); - // Should be using bridge methods, not defaults + // Should be using bridge methods, not defaults. The active-state method is + // wrapped so the app can ignore stale Ember state on React-owned routes. expect(result.current.getRouteUrl).toBe(mock.stateBridge.getRouteUrl); - expect(result.current.isRouteActive).toBe(mock.stateBridge.isRouteActive); + expect(result.current.isRouteActive('posts')).toBe(true); + expect(mock.stateBridge.isRouteActive).toHaveBeenCalledWith('posts'); }); baseTest('switches to bridge methods when bridge becomes available', async () => { @@ -439,6 +442,7 @@ describe('useEmberRouting', () => { // Bridge becomes available const mock = createMockStateBridge(); + mock.stateBridge.isRouteActive = vi.fn(() => true); window.EmberBridge = { state: mock.stateBridge }; // Wait for the subscription interval to fire @@ -448,7 +452,8 @@ describe('useEmberRouting', () => { // Now should be using bridge methods expect(result.current.getRouteUrl).toBe(mock.stateBridge.getRouteUrl); - expect(result.current.isRouteActive).toBe(mock.stateBridge.isRouteActive); + expect(result.current.isRouteActive('posts')).toBe(true); + expect(mock.stateBridge.isRouteActive).toHaveBeenCalledWith('posts'); }); baseTest('re-renders when route changes', async () => { diff --git a/apps/admin/src/ember-bridge/ember-bridge.tsx b/apps/admin/src/ember-bridge/ember-bridge.tsx index 57f44ef8769..d94d8a7d02b 100644 --- a/apps/admin/src/ember-bridge/ember-bridge.tsx +++ b/apps/admin/src/ember-bridge/ember-bridge.tsx @@ -1,6 +1,7 @@ -import { useCallback, useEffect, useState, useSyncExternalStore } from 'react'; +import { useCallback, useContext, useEffect, useState, useSyncExternalStore } from 'react'; import { useQueryClient } from '@tanstack/react-query'; import { useBrowseConfig } from '@tryghost/admin-x-framework/api/config'; +import { EmberContext } from './ember-context'; export interface EmberBridge { state: StateBridge; @@ -360,6 +361,7 @@ const defaultRouting: EmberRouting = { * ``` */ export function useEmberRouting(): EmberRouting { + const emberContext = useContext(EmberContext); const [bridge, setBridge] = useState(() => window.EmberBridge?.state ?? null); const [, forceUpdate] = useState(0); @@ -385,7 +387,12 @@ export function useEmberRouting(): EmberRouting { return { getRouteUrl: bridge.getRouteUrl, - isRouteActive: bridge.isRouteActive, + // React-owned navigations use pushState, which Ember does not observe. + // Only trust Ember's route state while the current route is actually + // rendering an Ember fallback. Outside EmberProvider (mainly unit tests + // and standalone consumers), preserve the bridge's original behaviour. + isRouteActive: (...args) => + (emberContext?.isFallbackPresent ?? true) && bridge.isRouteActive(...args), }; } diff --git a/apps/admin/src/ember-bridge/index.ts b/apps/admin/src/ember-bridge/index.ts index 81d7466bd40..9a411fd7dde 100644 --- a/apps/admin/src/ember-bridge/index.ts +++ b/apps/admin/src/ember-bridge/index.ts @@ -22,4 +22,5 @@ export type { EmberDataChangeEvent, EmberRouting, OpenGiftLinkModalEvent, + StateBridge, } from './ember-bridge'; diff --git a/apps/admin/src/index.css b/apps/admin/src/index.css index 48efc4518fa..7b7bb3f7432 100644 --- a/apps/admin/src/index.css +++ b/apps/admin/src/index.css @@ -5,12 +5,52 @@ @import '@tryghost/shade/styles.css'; +/* Site custom-font families for the settings font pickers. The font-* utilities + must be generated in this Tailwind lane; the font files themselves load with + settings/custom-fonts.css in the settings chunk. */ +@theme { + --font-cardo: Cardo; + --font-manrope: Manrope; + --font-merriweather: Merriweather; + --font-nunito: Nunito; + --font-tenor-sans: Tenor Sans; + --font-old-standard-tt: Old Standard TT; + --font-prata: Prata; + --font-roboto: Roboto; + --font-rufina: Rufina; + --font-inter: Inter; + --font-space-grotesk: Space Grotesk; + --font-chakra-petch: Chakra Petch; + --font-noto-sans: Noto Sans; + --font-poppins: Poppins; + --font-fira-sans: Fira Sans; + --font-noto-serif: Noto Serif; + --font-lora: Lora; + --font-ibm-plex-serif: IBM Plex Serif; + --font-space-mono: Space Mono; + --font-fira-mono: Fira Mono; + --font-jetbrains-mono: JetBrains Mono; +} + /* Koenig mounts floating toolbars to document.body. Radix modal dialogs disable body pointer events, so body-level Koenig portals need to opt back in. */ [data-kg-portal] { pointer-events: auto; } +/* Prose classes are for formatting arbitrary HTML that comes from the API */ +.gh-prose-links a { + color: #30cf43; +} + +.dark .shade .gh-loading-orb-container { + background-color: #000000; +} + +.dark .shade .gh-loading-orb { + filter: invert(100%); +} + /* Legacy utility compatibility (Spirit/Tachyons-style percentages). */ .w-100 { width: 100%; diff --git a/apps/admin/src/layout/app-sidebar/nav-main.tsx b/apps/admin/src/layout/app-sidebar/nav-main.tsx index 83ed898f883..0dc58c1f269 100644 --- a/apps/admin/src/layout/app-sidebar/nav-main.tsx +++ b/apps/admin/src/layout/app-sidebar/nav-main.tsx @@ -6,7 +6,7 @@ import { SidebarMenu, SidebarMenuBadge, } from '@tryghost/shade/components'; -import { LucideIcon } from '@tryghost/shade/utils'; +import { formatNumber, LucideIcon } from '@tryghost/shade/utils'; import { useBrowseSite } from '@tryghost/admin-x-framework/api/site'; import { useCurrentUser } from '@tryghost/admin-x-framework/api/current-user'; import { useBrowseSettings } from '@tryghost/admin-x-framework/api/settings'; @@ -34,6 +34,11 @@ function NavMain({ ...props }: React.ComponentProps) { ); const isNetworkRouteActive = useIsActiveLink({ path: 'network', activeOnSubpath: true }); const isActivitypubRouteActive = useIsActiveLink({ path: 'activitypub', activeOnSubpath: true }); + const isAnalyticsRouteActive = useIsActiveLink({ path: 'analytics', activeOnSubpath: true }); + const isPostAnalyticsRouteActive = useIsActiveLink({ + path: 'posts/analytics', + activeOnSubpath: true, + }); const showNetworkBadge = networkNotificationCount > 0 && !isNetworkRouteActive && !isActivitypubRouteActive; @@ -46,7 +51,10 @@ function NavMain({ ...props }: React.ComponentProps) { - + Analytics @@ -62,7 +70,7 @@ function NavMain({ ...props }: React.ComponentProps) { {showNetworkBadge && ( - {networkNotificationCount} + {formatNumber(networkNotificationCount)} )} diff --git a/apps/admin/src/layout/sidebar.acceptance.test.tsx b/apps/admin/src/layout/sidebar.acceptance.test.tsx index 88ad0c9082b..bdcf8a38be9 100644 --- a/apps/admin/src/layout/sidebar.acceptance.test.tsx +++ b/apps/admin/src/layout/sidebar.acceptance.test.tsx @@ -1,4 +1,5 @@ -import { describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it } from 'vitest'; +import type { StateBridge } from '@/ember-bridge'; import { activeThemeResponse, @@ -30,6 +31,29 @@ function fakeUnreadNotifications(count: number): void { fakeEndpoint('GET', UNREAD_COUNT_URL, { count }); } +function installStaleEmberRoute(activeRoute: 'members-activity' | 'pages' | 'posts'): void { + window.EmberBridge = { + state: { + onUpdate: () => {}, + onInvalidate: () => {}, + onDelete: () => {}, + isFeatureEnabled: () => false, + on: () => {}, + off: () => {}, + sidebarVisible: true, + getRouteUrl: (routeName) => routeName, + isRouteActive: (routeNames) => { + const routes = Array.isArray(routeNames) ? routeNames : routeNames.split(' '); + return routes.includes(activeRoute); + }, + } satisfies StateBridge, + }; +} + +afterEach(() => { + delete window.EmberBridge; +}); + describe('Sidebar navigation', () => { it('renders the navigation for the current user', async () => { await renderAdminApp('/site'); @@ -89,6 +113,25 @@ describe('Sidebar navigation', () => { await expect.poll(currentRoute).toBe('/pages'); }); + it.each([ + { label: 'Posts', route: '/posts', emberRoute: 'posts' }, + { label: 'Pages', route: '/pages', emberRoute: 'pages' }, + { label: 'Members', route: '/members-activity', emberRoute: 'members-activity' }, + ] as const)( + 'clears the $label active state after leaving its Ember route', + async ({ label, route, emberRoute }) => { + fakeTags([]); + installStaleEmberRoute(emberRoute); + await renderAdminApp(route); + + await expect.element(sidebarScreen.navLink(label)).toHaveAttribute('aria-current', 'page'); + + await sidebarScreen.navLink('Tags').click(); + await expect.poll(currentRoute).toBe('/tags'); + await expect.element(sidebarScreen.navLink(label)).not.toHaveAttribute('aria-current'); + }, + ); + it('shows the default post views and collapses them with the toggle', async () => { await renderAdminApp('/posts'); diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members-gate.tsx b/apps/admin/src/members/components/bulk-action-modals/import-members-gate.tsx index 21a4d7a28a9..45665a55ffb 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members-gate.tsx +++ b/apps/admin/src/members/components/bulk-action-modals/import-members-gate.tsx @@ -1,5 +1,5 @@ import { ImportMembersModal as BaselineImportMembersModal } from '@/members/components/bulk-action-modals/import-members-modal'; -import { ImportMembersModal as CustomFieldsImportMembersModal } from '@/members/components/bulk-action-modals/import-members/custom-fields/import-members-modal'; +import { ImportMembersModal as RedesignedImportMembersModal } from '@/members/components/bulk-action-modals/import-members/custom-fields/import-members-modal'; import { useFeatureFlag } from '@tryghost/admin-x-framework/hooks'; import type { ImportResponse } from '@/members/components/bulk-action-modals/import-members/state'; @@ -11,8 +11,8 @@ interface ImportMembersGateProps { } /** - * Serves the members CSV import from the custom fields experience when the `membersCustomFields` - * Labs flag is on, and from the import as it shipped otherwise. + * Serves the redesigned members CSV import when the `membersImportRedesign` Labs flag is on, + * and the import as it shipped otherwise. * * Two whole implementations rather than one with the flag threaded through it. The mapping step * diverges in almost every part — what a row offers, what a row means, what the request carries — @@ -22,12 +22,15 @@ interface ImportMembersGateProps { * * The cost is real and worth stating: a fix to the import has to be applied to both, or knowingly * to one, until the flag goes and the baseline is deleted. + * + * Custom fields are not this flag's decision. The redesign ships whether or not custom fields + * exist, and asks `membersCustomFields` itself for whether to offer them. */ export function ImportMembersGate(props: ImportMembersGateProps) { - const customFieldsEnabled = useFeatureFlag('membersCustomFields'); + const importRedesignEnabled = useFeatureFlag('membersImportRedesign'); - if (customFieldsEnabled) { - return ; + if (importRedesignEnabled) { + return ; } return ; } diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members-modal.tsx b/apps/admin/src/members/components/bulk-action-modals/import-members-modal.tsx index 2e73345d090..0fd6b09aab7 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members-modal.tsx +++ b/apps/admin/src/members/components/bulk-action-modals/import-members-modal.tsx @@ -27,10 +27,6 @@ import { isImportMembersCompleteResponse, useImportMembers, } from '@tryghost/admin-x-framework/api/members'; -import { - memberCustomFieldCsvColumns, - useBrowseMemberCustomFields, -} from '@tryghost/admin-x-framework/api/member-custom-fields'; import { parseCSV } from './import-members/csv'; import { useCallback, useEffect, useMemo, useReducer, useRef } from 'react'; import { useFeatureFlag } from '@tryghost/admin-x-framework/hooks'; @@ -54,28 +50,12 @@ export function ImportMembersModal({ const { mutateAsync: importMembers } = useImportMembers(); const importMemberTier = useFeatureFlag('importMemberTier'); - // Defined custom fields become mapping targets. Fetched only when the feature is on; - // browse returns active fields only, which are the ones the importer writes to. - const customFieldsEnabled = useFeatureFlag('membersCustomFields'); - const { data: customFieldsData } = useBrowseMemberCustomFields({ enabled: customFieldsEnabled }); - const customFieldColumns = useMemo( - () => memberCustomFieldCsvColumns(customFieldsData?.members_custom_fields ?? []), - [customFieldsData], - ); - // The file-reader effect waits for this before its first parse: with the feature on, - // the custom field definitions must be loaded or auto-detection would miss - // custom_fields.* columns on a fast upload. It flips false -> true once and stays true - // (a refetch keeps data defined), so readiness never re-triggers the read. - const customFieldsReady = !customFieldsEnabled || customFieldsData !== undefined; // Detection options are read inside the effect through this ref rather than as deps, so - // a later refetch of the options can't re-run the read and overwrite a mapping the user - // has begun editing. - const detectOptionsRef = useRef({ importMemberTier, customFieldColumns }); - detectOptionsRef.current = { importMemberTier, customFieldColumns }; - const fieldMappings = useMemo( - () => getFieldMappings({ importMemberTier, customFieldColumns }), - [importMemberTier, customFieldColumns], - ); + // a later change to them can't re-run the read and overwrite a mapping the user has + // begun editing. + const detectOptionsRef = useRef({ importMemberTier }); + detectOptionsRef.current = { importMemberTier }; + const fieldMappings = useMemo(() => getFieldMappings({ importMemberTier }), [importMemberTier]); const labelPicker = useLabelPicker({ selectedSlugs: state.selectedLabelSlugs, @@ -119,7 +99,7 @@ export function ImportMembersModal({ ); useEffect(() => { - if (!state.file || !customFieldsReady) { + if (!state.file) { return; } @@ -186,7 +166,7 @@ export function ImportMembersModal({ reader.abort(); } }; - }, [state.file, customFieldsReady]); + }, [state.file]); const validateFile = useCallback((file: File): boolean => { const match = /(?:\.([^.]+))?$/.exec(file.name); diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/field-picker.tsx b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/field-picker.tsx index a95dba8f9f7..5ba3d1e4343 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/field-picker.tsx +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/field-picker.tsx @@ -4,6 +4,7 @@ import { Button, Command, CommandCheck, + CommandEmpty, CommandGroup, CommandInput, CommandItem, @@ -15,9 +16,8 @@ import { inputSurfaceClasses, } from '@tryghost/shade/components'; import { - FIELD_SOURCES, - FIELD_SOURCE_ORDER, type FieldTarget, + type FieldTargetGroup, } from '@/members/components/bulk-action-modals/import-members/custom-fields/field-targets'; import { LucideIcon, cn } from '@tryghost/shade/utils'; import { useRef } from 'react'; @@ -66,7 +66,13 @@ interface FieldPickerProps { value: string | null; disabled?: boolean; invalid?: boolean; - targets: FieldTarget[]; + // The list as it is shown: sections in order, each already headed and populated. Everything + // about where a target sits and how it is named is settled by fieldTargets, so nothing here + // re-derives it. + targetGroups: FieldTargetGroup[]; + // Whether the list ends with an offer to make a custom field. Off, a search that matches + // nothing says so instead, which the offer had been standing in for. + canCreateField: boolean; // Open and search are the caller's, not this component's: creating a composite has to // reopen this row's picker filtered to the field it just made, so which picker is open and // what it is filtered by have to be sayable from outside. @@ -85,7 +91,8 @@ export function FieldPicker({ value, disabled, invalid, - targets, + targetGroups, + canCreateField, open, search, onOpenChange, @@ -101,9 +108,11 @@ export function FieldPicker({ // its autoFocus pulled straight back inside. Both are dealt with at teardown, below. const openingCreateForm = useRef(false); - const selected = targets.find((target) => target.value === value); - const badge = selected?.contested ? FIELD_SOURCES[selected.source].badge : null; - const ariaKind = selected ? FIELD_SOURCES[selected.source].ariaKind : null; + const selected = targetGroups + .flatMap((group) => group.targets) + .find((target) => target.value === value); + const badge = selected?.badge ?? null; + const ariaKind = selected?.ariaKind ?? null; const choose = (target: string) => { onOpenChange(false); @@ -266,44 +275,48 @@ export function FieldPicker({ and Enter would take whichever the DOM had first. Identity is the target, which is namespaced and so already distinct; the label moves to keywords for scoreByLabel. */} - {FIELD_SOURCE_ORDER.map((source) => ( - - {targets - .filter((target) => target.source === source) - .map((target) => ( - choose(target.value)} - > - - {target.label} - {value === target.value && } - - ))} + {targetGroups.map((group) => ( + + {group.targets.map((target) => ( + choose(target.value)} + > + + {target.label} + {value === target.value && } + + ))} ))} - {/* Its own group because cmdk hands forceMount down to every item in a - group, so among the fields it would have pinned all of them. Search - finding nothing is the strongest signal the field does not exist yet, - which is why nothing else stands in for an empty state. */} - - {/* A publisher can name a field "New field", so the colour is what + {canCreateField ? ( + /* Its own group because cmdk hands forceMount down to every item in a group, so + among the fields it would have pinned all of them. Search finding nothing + is the strongest signal the field does not exist yet, which is why + nothing else stands in for an empty state while this is offered. */ + + {/* A publisher can name a field "New field", so the colour is what sets this apart from one. */} - { - openingCreateForm.current = true; - onOpenChange(false); - }} - > - - Add custom field - - + { + openingCreateForm.current = true; + onOpenChange(false); + }} + > + + Add custom field + + + ) : ( + // With the offer gone nothing is force-mounted, so a search matching nothing + // would otherwise leave the list blank with no account of why. + No fields found. + )} diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/field-targets.test.ts b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/field-targets.test.ts index d6a2f8efb11..22b307c8fcf 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/field-targets.test.ts +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/field-targets.test.ts @@ -1,4 +1,4 @@ -import { FIELD_SOURCES, FIELD_SOURCE_ORDER, fieldTargets } from './field-targets'; +import { type FieldTargetGroup, fieldTargets } from './field-targets'; import { type MemberCustomFieldCsvColumn } from '@tryghost/admin-x-framework/api/member-custom-fields'; const membershipFields = [ @@ -17,25 +17,31 @@ const customColumn = ( ...overrides, }); +/** Every target across every section, for the assertions that are about the list as a whole. */ +const allTargets = (groups: FieldTargetGroup[]) => groups.flatMap((group) => group.targets); + describe('field targets', () => { describe('fieldTargets', () => { - it('gives a membership field the same two halves every target carries', () => { - const [target] = fieldTargets({ + it('gives a membership field the same halves every target carries', () => { + const [group] = fieldTargets({ membershipFields: [{ label: 'Name', value: 'name' }], customFieldColumns: [], }); - expect(target).toEqual({ - value: 'name', - source: 'membership', - fieldName: 'Name', - label: 'Name', - contested: false, - }); + expect(group.targets).toEqual([ + { + value: 'name', + source: 'membership', + fieldName: 'Name', + label: 'Name', + badge: null, + ariaKind: null, + }, + ]); }); it("carries a custom field's type, name and part through", () => { - const targets = fieldTargets({ + const groups = fieldTargets({ membershipFields: [], customFieldColumns: [ customColumn({ @@ -48,7 +54,7 @@ describe('field targets', () => { ], }); - expect(targets).toEqual([ + expect(allTargets(groups)).toEqual([ { value: 'custom_fields.shipping_address.city', source: 'custom', @@ -56,54 +62,80 @@ describe('field targets', () => { partLabel: 'City', label: 'Shipping Address (City)', type: 'address', - contested: false, + badge: null, + ariaKind: 'Custom field', }, ]); }); it('leaves a one-column field with no part at all', () => { - const [target] = fieldTargets({ membershipFields: [], customFieldColumns: [customColumn()] }); + const [target] = allTargets( + fieldTargets({ membershipFields: [], customFieldColumns: [customColumn()] }), + ); expect(target).not.toHaveProperty('partLabel'); }); + }); + + describe('sections', () => { + it('heads each section with the kind under it, in the order they are declared in', () => { + const groups = fieldTargets({ membershipFields, customFieldColumns: [customColumn()] }); + + expect(groups.map((group) => [group.source, group.heading])).toEqual([ + ['membership', 'Membership fields'], + ['custom', 'Custom fields'], + ]); + }); - it('offers the sources in the order they are declared in', () => { - const targets = fieldTargets({ membershipFields, customFieldColumns: [customColumn()] }); + // A section with nothing under it would still be announced, so a site with custom fields + // off must not be offered the heading at all. + it('drops a section nothing falls into', () => { + const groups = fieldTargets({ membershipFields, customFieldColumns: [] }); - expect([...new Set(targets.map((target) => target.source))]).toEqual([...FIELD_SOURCE_ORDER]); + expect(groups.map((group) => group.source)).toEqual(['membership']); + }); + + it('offers no section at all when there is nothing to offer', () => { + expect(fieldTargets({ membershipFields: [], customFieldColumns: [] })).toEqual([]); }); }); - describe('contested', () => { - const contestedLabels = (targets: ReturnType) => - targets.filter((target) => target.contested).map((target) => target.label); + // The badge tells two same-named targets apart. It is a fact about the list rather than the + // field, so it is settled here and the picker only shows what it is given. + describe('badge', () => { + const badged = (groups: FieldTargetGroup[]) => + allTargets(groups) + .filter((target) => target.badge !== null) + .map((target) => [target.label, target.badge]); - it('finds a custom field a membership field has the name of', () => { - const targets = fieldTargets({ + it('marks a custom field a membership field has the name of', () => { + const groups = fieldTargets({ membershipFields, customFieldColumns: [customColumn({ fieldName: 'Name', label: 'Name' })], }); - expect(contestedLabels(targets)).toEqual(['Name', 'Name']); + // Only the custom one is marked: membership is the kind a reader assumes, so naming it + // would mark both halves of the pair and tell them apart no better than marking neither. + expect(badged(groups)).toEqual([['Name', 'Custom']]); }); - it('leaves a name no other source offers alone', () => { - const targets = fieldTargets({ membershipFields, customFieldColumns: [customColumn()] }); + it('leaves a name no other source offers unmarked', () => { + const groups = fieldTargets({ membershipFields, customFieldColumns: [customColumn()] }); - expect(contestedLabels(targets)).toEqual([]); + expect(badged(groups)).toEqual([]); }); it('counts a name differing only in case or space as the same name', () => { - const targets = fieldTargets({ + const groups = fieldTargets({ membershipFields, customFieldColumns: [customColumn({ fieldName: ' name ', label: ' name ' })], }); - expect(contestedLabels(targets)).toEqual(['Name', ' name ']); + expect(badged(groups)).toEqual([[' name ', 'Custom']]); }); - it('does not contest a composite part that shares only its field name', () => { - const targets = fieldTargets({ + it('does not mark a composite part that shares only its field name', () => { + const groups = fieldTargets({ membershipFields, customFieldColumns: [ customColumn({ @@ -116,41 +148,52 @@ describe('field targets', () => { ], }); - expect(contestedLabels(targets)).toEqual([]); + expect(badged(groups)).toEqual([]); }); - it('contests only against the targets in the list it is given', () => { + it('marks only against the targets in the list it is given', () => { const custom = [ customColumn({ value: 'custom_fields.tier', fieldName: 'Tier', label: 'Tier' }), ]; + expect(badged(fieldTargets({ membershipFields, customFieldColumns: custom }))).toEqual([]); expect( - contestedLabels(fieldTargets({ membershipFields, customFieldColumns: custom })), - ).toEqual([]); - expect( - contestedLabels( + badged( fieldTargets({ membershipFields: [...membershipFields, { label: 'Tier', value: 'import_tier' }], customFieldColumns: custom, }), ), - ).toEqual(['Tier', 'Tier']); + ).toEqual([['Tier', 'Custom']]); }); }); - describe('FIELD_SOURCES', () => { - it('leaves exactly one source unmarked', () => { - const unmarked = FIELD_SOURCE_ORDER.filter((source) => FIELD_SOURCES[source].badge === null); + // Read out with the selection, so a custom field is not announced as a membership one. + describe('accessible kind', () => { + it('names a custom field in full, and says nothing for a membership field', () => { + const groups = fieldTargets({ + membershipFields: [{ label: 'Name', value: 'name' }], + customFieldColumns: [customColumn()], + }); - expect(unmarked).toEqual(['membership']); + expect(allTargets(groups).map((target) => [target.label, target.ariaKind])).toEqual([ + ['Name', null], + ['Nickname', 'Custom field'], + ]); }); - it('names a marked source in full for the accessible name', () => { - expect(FIELD_SOURCES.custom).toEqual({ - heading: 'Custom fields', - badge: 'Custom', - ariaKind: 'Custom field', + // Unlike the badge, which only appears where something else shares the label. + it('names the kind whether or not the label is shared', () => { + const groups = fieldTargets({ + membershipFields, + customFieldColumns: [customColumn({ fieldName: 'Name', label: 'Name' })], }); + + expect( + allTargets(groups) + .filter((target) => target.source === 'custom') + .map((target) => target.ariaKind), + ).toEqual(['Custom field']); }); }); }); diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/field-targets.ts b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/field-targets.ts index 5eebd5bdc99..329d5721e35 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/field-targets.ts +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/field-targets.ts @@ -19,12 +19,25 @@ type FieldOffer = | (FieldFacts & { source: 'custom'; type: MemberCustomField['type'] }); /** - * One thing a CSV column can be imported as. + * One thing a CSV column can be imported as, as the picker shows it. * - * `contested` belongs to the list rather than to the field: whether "Tier" needs telling apart - * depends on whether tiers are on offer at all. + * The badge is settled here rather than left to the picker, because it is not a fact about the + * field: whether "Tier" needs telling apart depends on whether tiers are on offer at all, which + * only the whole list knows. */ -export type FieldTarget = FieldOffer & { contested: boolean }; +export type FieldTarget = FieldOffer & { + /** The kind, shown beside the label; null where no other source offers this label. */ + badge: string | null; + /** The kind named in full, for the accessible name; null to say nothing. */ + ariaKind: string | null; +}; + +/** One section of the picker's list: what heads it, and what is under it. */ +export interface FieldTargetGroup { + source: FieldSource; + heading: string; + targets: FieldTarget[]; +} interface FieldSourcePresentation { heading: string; @@ -34,21 +47,28 @@ interface FieldSourcePresentation { ariaKind: string | null; } -export const FIELD_SOURCES: Record = { +const FIELD_SOURCES: Record = { membership: { heading: 'Membership fields', badge: null, ariaKind: null }, custom: { heading: 'Custom fields', badge: 'Custom', ariaKind: 'Custom field' }, }; -export const FIELD_SOURCE_ORDER = Object.keys(FIELD_SOURCES) as FieldSource[]; +const FIELD_SOURCE_ORDER = Object.keys(FIELD_SOURCES) as FieldSource[]; -/** Everything a column can be imported as, in the order the sections offer it. */ +/** + * Everything a column can be imported as, sectioned as the picker offers it: in the order the + * sources are declared in, and without a section nothing falls into — a site with custom fields + * off is not told there is a Custom fields section and then shown nothing under it. + * + * The picker is handed this and nothing else, so where a target sits, what heads its section and + * how it is named are all answered before it renders. + */ export function fieldTargets({ membershipFields, customFieldColumns, }: { membershipFields: { label: string; value: string }[]; customFieldColumns: MemberCustomFieldCsvColumn[]; -}): FieldTarget[] { +}): FieldTargetGroup[] { const offers: FieldOffer[] = [ ...membershipFields.map((field): FieldOffer => ({ value: field.value, @@ -67,7 +87,17 @@ export function fieldTargets({ ]; const contested = labelsSharedAcrossSources(offers); - return offers.map((offer) => ({ ...offer, contested: contested.has(readsAs(offer)) })); + const targets = offers.map((offer): FieldTarget => ({ + ...offer, + badge: contested.has(readsAs(offer)) ? FIELD_SOURCES[offer.source].badge : null, + ariaKind: FIELD_SOURCES[offer.source].ariaKind, + })); + + return FIELD_SOURCE_ORDER.map((source) => ({ + source, + heading: FIELD_SOURCES[source].heading, + targets: targets.filter((target) => target.source === source), + })).filter((group) => group.targets.length > 0); } function readsAs(offer: { label: string }): string { diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/import-members-modal.tsx b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/import-members-modal.tsx index d85b952d77a..8ea1d1676af 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/import-members-modal.tsx +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/import-members-modal.tsx @@ -77,26 +77,39 @@ export function ImportMembersModal({ const { mutateAsync: importMembers } = useImportMembers(); const importMemberTier = useFeatureFlag('importMemberTier'); + // Whether custom fields exist at all is their own flag's answer, not this dialog's: the + // redesigned import ships on `membersImportRedesign` and has to be a plain column-to-member-field + // mapper while custom fields are still an experiment. Off, they are absent from every part of + // this file — not fetched, not offered as a target, not creatable. + const customFieldsEnabled = useFeatureFlag('membersCustomFields'); // Defined custom fields become mapping targets. Browse returns active fields only, which - // are the ones the importer writes to. No flag check anywhere in this file: the gate does - // not render it unless membersCustomFields is on. - const { data: customFieldsData, isError: customFieldsFailed } = useBrowseMemberCustomFields(); + // are the ones the importer writes to. + const { data: customFieldsData, isError: customFieldsFailed } = useBrowseMemberCustomFields({ + enabled: customFieldsEnabled, + }); // A field created from the mapping step is in here the moment it is created: the create // mutation puts it into the cached list, so there is no window where a row points at a // column the picker cannot name yet. + // + // The flag is asked again rather than left to the disabled query above: disabling stops the + // fetch, not the read, so a cache another screen had warmed would still be served here. const customFieldColumns = useMemo( - () => memberCustomFieldCsvColumns(customFieldsData?.members_custom_fields ?? []), - [customFieldsData], + () => + customFieldsEnabled + ? memberCustomFieldCsvColumns(customFieldsData?.members_custom_fields ?? []) + : [], + [customFieldsEnabled, customFieldsData], ); // The file-reader effect waits for this before its first parse: the custom field // definitions must be loaded or auto-detection would miss custom_fields.* columns on a // fast upload. It flips false -> true once and stays true (a refetch keeps data defined), // so readiness never re-triggers the read. - // Ready, or never going to be. A failure has no representation in `data`, so waiting on it - // alone leaves the file unparsed and the step on a spinner with nothing said — for a query - // whose only job is to add targets to a list. Failing it costs the custom fields; blocking - // on it costs the import. - const customFieldsReady = customFieldsData !== undefined || customFieldsFailed; + // Ready, or never going to be. Neither a failed query nor a disabled one has any + // representation in `data`, so waiting on `data` alone leaves the file unparsed and the step + // on a spinner with nothing said — for a query whose only job is to add targets to a list. + // Failing it costs the custom fields; blocking on it costs the import. + const customFieldsReady = + !customFieldsEnabled || customFieldsData !== undefined || customFieldsFailed; // Detection options are read inside the effect through this ref rather than as deps, so // a later refetch of the options can't re-run the read and overwrite a mapping the user // has begun editing. @@ -108,7 +121,7 @@ export function ImportMembersModal({ }, [importMemberTier, customFieldColumns]); // Auto-detection takes customFieldColumns separately, through detectOptionsRef above: it // matches on column names rather than on what is offered. - const targets = useMemo( + const targetGroups = useMemo( () => fieldTargets({ membershipFields: getFieldMappings({ importMemberTier }), @@ -474,6 +487,7 @@ export function ImportMembersModal({ {(state.status === 'MAPPING' || state.status === 'UPLOADING') && state.fileData !== null && ( { hasEditsRef.current = true; }} diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping-step.tsx b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping-step.tsx index 027bd94c371..68239b4b93c 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping-step.tsx +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping-step.tsx @@ -32,7 +32,7 @@ import { columnsOf, } from '@/members/components/bulk-action-modals/import-members/custom-fields/mapping'; import { Fragment, useCallback, useLayoutEffect, useMemo, useRef, useState } from 'react'; -import { type FieldTarget } from '@/members/components/bulk-action-modals/import-members/custom-fields/field-targets'; +import { type FieldTargetGroup } from '@/members/components/bulk-action-modals/import-members/custom-fields/field-targets'; import { type MemberCustomField } from '@tryghost/admin-x-framework/api/member-custom-fields'; import { type UseLabelPickerResult } from '@/members/hooks/use-label-picker'; @@ -56,7 +56,10 @@ interface MappingStepProps { mappingError: string | null; showMappingErrors: boolean; dataPreviewIndex: number; - targets: FieldTarget[]; + targetGroups: FieldTargetGroup[]; + // Whether custom fields exist for this site at all. Off, no row offers to make one and the + // create form is unreachable, so the table is a plain mapping of columns onto member fields. + canCreateCustomFields: boolean; labelPicker: UseLabelPickerResult; onUpdateMapping: (from: string, to: string | null) => void; onFieldCreated: (columnKey: string, column: string | null) => void; @@ -106,7 +109,8 @@ export function MappingStep({ mappingError, showMappingErrors, dataPreviewIndex, - targets, + targetGroups, + canCreateCustomFields, labelPicker, onUpdateMapping, onFieldCreated, @@ -525,13 +529,14 @@ export function MappingStep({ second mechanism. The mapping is not lost either way — it comes back with the row when it is selected again. */} { if (node) { fieldTriggers.current.set(row.key, node); diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping.ts b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping.ts index 98f5dd11b57..c75015546cf 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping.ts +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping.ts @@ -1,7 +1,3 @@ -import { - FIELD_MAPPINGS, - IMPORT_TIER_FIELD_MAPPING, -} from '@/members/components/bulk-action-modals/import-members/mapping'; import { isCustomFieldColumn } from '@tryghost/admin-x-framework/api/member-custom-fields'; /** @@ -20,17 +16,10 @@ export { columnsOf, detectFieldTypes, formatImportError, + getFieldMappings, sampleData, } from '@/members/components/bulk-action-modals/import-members/mapping'; -// The native targets only. Custom fields are offered from their own list, chosen by kind, so -// they are not folded in here — auto-detection still sees both, through the shared detection. -export function getFieldMappings({ - importMemberTier = false, -}: { importMemberTier?: boolean } = {}) { - return [...FIELD_MAPPINGS, ...(importMemberTier ? [IMPORT_TIER_FIELD_MAPPING] : [])]; -} - /** * The field name to suggest for a column no defined field matches. * diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/mapping.test.ts b/apps/admin/src/members/components/bulk-action-modals/import-members/mapping.test.ts index 2fce21c5253..5908dd4c3e3 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/mapping.test.ts +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/mapping.test.ts @@ -148,20 +148,6 @@ describe('mapping helpers', () => { }, ]; - it('offers the custom field columns as mapping targets', () => { - const mappings = getFieldMappings({ customFieldColumns }); - - expect(mappings).toContainEqual( - expect.objectContaining({ label: 'Nickname', value: 'custom_fields.nickname' }), - ); - expect(mappings).toContainEqual( - expect.objectContaining({ - label: 'Shipping Address (Line 1)', - value: 'custom_fields.shipping_address.line1', - }), - ); - }); - it('auto-detects a custom field column by its namespaced header', () => { const mapping = detectFieldTypes( [{ email: 'user@example.com', 'custom_fields.nickname': 'Bex' }], diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/mapping.ts b/apps/admin/src/members/components/bulk-action-modals/import-members/mapping.ts index f1d1fdaa1cb..224451c6d63 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/mapping.ts +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/mapping.ts @@ -4,10 +4,16 @@ import { type MemberCustomFieldCsvColumn, } from '@tryghost/admin-x-framework/api/member-custom-fields'; -type FieldMappingOptions = { +/** + * What auto-detection is allowed to map a column onto. + * + * `customFieldColumns` is the redesigned import's alone — it is the only one that offers + * custom fields, so it is the only one that passes any. Detection is shared with the import + * as it shipped, which passes none. + */ +type DetectionOptions = { importMemberTier?: boolean; - // Custom field CSV columns offered as mapping targets (see memberCustomFieldCsvColumns); - // empty when the feature is off. + // Custom field CSV columns offered as mapping targets (see memberCustomFieldCsvColumns). customFieldColumns?: MemberCustomFieldCsvColumn[]; }; @@ -40,7 +46,7 @@ const SUPPORTED_TYPES = [ function getSupportedTypes({ importMemberTier = false, customFieldColumns = [], -}: FieldMappingOptions = {}): string[] { +}: DetectionOptions = {}): string[] { return [ ...SUPPORTED_TYPES, ...(importMemberTier ? [IMPORT_TIER_FIELD_MAPPING.value] : []), @@ -48,15 +54,15 @@ function getSupportedTypes({ ]; } +/** + * Everything a column can be imported as. Native targets only, on both sides of the flag: the + * import as it shipped has no others, and the redesigned one offers custom fields from a list + * of its own rather than folded in here. + */ export function getFieldMappings({ importMemberTier = false, - customFieldColumns = [], -}: FieldMappingOptions = {}) { - return [ - ...FIELD_MAPPINGS, - ...(importMemberTier ? [IMPORT_TIER_FIELD_MAPPING] : []), - ...customFieldColumns, - ]; +}: { importMemberTier?: boolean } = {}) { + return [...FIELD_MAPPINGS, ...(importMemberTier ? [IMPORT_TIER_FIELD_MAPPING] : [])]; } const AUTO_DETECTED_TYPES = ['email']; @@ -186,7 +192,7 @@ export function sampleData( */ export function detectFieldTypes( data: Record[], - options: FieldMappingOptions = {}, + options: DetectionOptions = {}, ): Record { const sampledData = sampleData(data); const mapping: Record = {}; diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/modal.test.tsx b/apps/admin/src/members/components/bulk-action-modals/import-members/modal.test.tsx index f865f601709..a2210cf4eb0 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/modal.test.tsx +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/modal.test.tsx @@ -39,19 +39,6 @@ vi.mock('@tryghost/admin-x-framework/api/members', async () => { }; }); -// The modal offers defined custom fields as mapping targets. Stub the browse hook -// (which needs a QueryClient this test does not mount) while keeping the real -// column-deriving helper, so a mapping built from fields still exercises production logic. -vi.mock('@tryghost/admin-x-framework/api/member-custom-fields', async () => { - const actual = await vi.importActual< - typeof import('@tryghost/admin-x-framework/api/member-custom-fields') - >('@tryghost/admin-x-framework/api/member-custom-fields'); - return { - ...actual, - useBrowseMemberCustomFields: () => ({ data: undefined }), - }; -}); - vi.mock('@/members/hooks/use-label-picker', () => ({ useLabelPicker: () => ({ labels: [], diff --git a/apps/admin/src/members/components/members-list-item.tsx b/apps/admin/src/members/components/members-list-item.tsx index 3d49712503e..cbacff1a651 100644 --- a/apps/admin/src/members/components/members-list-item.tsx +++ b/apps/admin/src/members/components/members-list-item.tsx @@ -260,7 +260,7 @@ const MembersListItem = forwardRef< > @@ -281,7 +281,7 @@ const MembersListItem = forwardRef< style={{ ...PINNED_EDGE_FADE_POSITION_STYLE, background: - 'linear-gradient(to right, var(--members-sticky-hover-bg) 0px, color-mix(in hsl, var(--members-sticky-hover-bg) 78%, transparent) 6px, color-mix(in hsl, var(--members-sticky-hover-bg) 28%, transparent) 16px, transparent 24px)', + 'linear-gradient(to right, var(--table-row-hover) 0px, color-mix(in hsl, var(--table-row-hover) 78%, transparent) 6px, color-mix(in hsl, var(--table-row-hover) 28%, transparent) 16px, transparent 24px)', }} /> diff --git a/apps/admin/src/members/import-members-custom-fields.acceptance.test.tsx b/apps/admin/src/members/import-members-custom-fields.acceptance.test.tsx index 4e1b92c6ce1..558502146db 100644 --- a/apps/admin/src/members/import-members-custom-fields.acceptance.test.tsx +++ b/apps/admin/src/members/import-members-custom-fields.acceptance.test.tsx @@ -11,7 +11,10 @@ import { import { importMembersScreen } from './import-members.screen'; import { membersScreen } from './members.screen'; -const FLAGS = { labs: { membersCustomFields: true } }; +// Both flags: the redesigned dialog is what this file exercises, and custom fields are what it +// exercises it for. They are separate switches — the redesign ships without custom fields. +const FLAGS = { labs: { membersImportRedesign: true, membersCustomFields: true } }; +const WITHOUT_CUSTOM_FIELDS = { labs: { membersImportRedesign: true } }; // A `nickname` column no defined field matches, alongside the columns auto-detection claims. // `name` is present deliberately: it takes the /name/i heuristic, which would otherwise map @@ -37,10 +40,10 @@ const EXPORTED_CSV = 'email,custom_fields.nickname\nada@example.com,Countess\n'; * key from the name the way the service does, and a browse reflecting what has been * created, so the picker behaves as it would against a real site. */ -function fakeCustomFieldsWorld() { - const fields: Array> = []; +function fakeCustomFieldsWorld(definedFields: Array> = []) { + const fields: Array> = [...definedFields]; fakeMembers([member({ name: 'Ada Lovelace' })]); - fakeMemberCustomFields(() => fields); + const browseApi = fakeMemberCustomFields(() => fields); const uploadApi = fakeAdminEndpoint('POST', '/members/upload/', { meta: { stats: { imported: 1, invalid: [] }, import_label: { name: 'Import', slug: 'import' } }, }); @@ -58,9 +61,19 @@ function fakeCustomFieldsWorld() { fields.push(field); return { members_custom_fields: [field] }; }); - return { createApi, uploadApi }; + return { browseApi, createApi, uploadApi }; } +/** A field the site has already defined, for proving what an import does and does not offer. */ +const NICKNAME_FIELD = { + key: 'nickname', + name: 'Nickname', + type: 'text', + status: 'active', + created_at: '2026-08-05T00:00:00.000Z', + updated_at: null, +}; + /** * The mapping as it went over the wire: the upload is multipart, carrying one * `mapping[]` field per column the request names. @@ -747,4 +760,43 @@ describe('Import members custom fields', () => { await expect.element(importMembersScreen.leaveConfirmationText()).toBeVisible(); }); + + // The redesigned dialog ships on its own flag, ahead of custom fields. Off, it has to be a + // plain mapping of columns onto the member fields Ghost already has, with nothing about + // custom fields anywhere in it — including on a site that has some defined. + describe('with custom fields off', () => { + it('offers no custom field, and no way to make one', async () => { + const { browseApi } = fakeCustomFieldsWorld([NICKNAME_FIELD]); + await renderAdminApp('/members', WITHOUT_CUSTOM_FIELDS); + await openMappingStep(); + + await importToggle('nickname').click(); + await fieldSelect('nickname').click(); + + // Exact, or "Email" also matches "Subscribed to emails". + await expect.element(importMembersScreen.option('Email', { exact: true })).toBeVisible(); + await expect.element(importMembersScreen.option('Nickname')).not.toBeInTheDocument(); + await expect.element(importMembersScreen.addCustomFieldOption()).not.toBeInTheDocument(); + + // Not merely unrendered: the definitions are never asked for. That query also gates the + // first parse of the file, so leaving it enabled and unanswerable would hold the mapping + // step on a spinner — which reaching the table above already rules out. + expect(browseApi.requests).toHaveLength(0); + }); + + it('says no field matches a search rather than offering to make one', async () => { + fakeCustomFieldsWorld(); + await renderAdminApp('/members', WITHOUT_CUSTOM_FIELDS); + await openMappingStep(); + + await importToggle('nickname').click(); + await fieldSelect('nickname').click(); + await userEvent.fill(importMembersScreen.searchFieldsInput(), 'zzzz'); + + // The offer to add a field was the list's only force-mounted item, so without it a + // fruitless search would leave the list blank with nothing said. + await expect.element(importMembersScreen.addCustomFieldOption()).not.toBeInTheDocument(); + await expect.element(importMembersScreen.messageText('No fields found.')).toBeVisible(); + }); + }); }); diff --git a/apps/admin/src/members/import-members-gate.acceptance.test.tsx b/apps/admin/src/members/import-members-gate.acceptance.test.tsx index eedfa00969e..a85d9a10809 100644 --- a/apps/admin/src/members/import-members-gate.acceptance.test.tsx +++ b/apps/admin/src/members/import-members-gate.acceptance.test.tsx @@ -16,7 +16,7 @@ const CSV = 'email,name\nada@example.com,Ada Lovelace\n'; * The one thing the split can break. * * Both implementations have their own tests: the import as it shipped is covered by - * import-members/modal.test.tsx, and the custom fields experience by + * import-members/modal.test.tsx, and the redesigned one by * import-members-custom-fields.acceptance.test.tsx. Neither can regress from the other's * changes, because they share no file. What is left is whether the gate hands over to the * right one, which is what this asserts — by a marker only that implementation renders. @@ -36,11 +36,11 @@ async function openMappingStep(labs: Record) { } describe('Import members gate', () => { - it('serves the custom fields import when the flag is on', async () => { - await openMappingStep({ membersCustomFields: true }); + it('serves the redesigned import when the flag is on', async () => { + await openMappingStep({ membersImportRedesign: true }); - // A checkbox per column exists only in the custom fields experience: it is what decides - // whether a column is imported there, a job the select does in the import as it shipped. + // A checkbox per column exists only in the redesigned dialog: it is what decides whether + // a column is imported there, a job the select does in the import as it shipped. await expect.element(importMembersScreen.importToggle('name')).toBeVisible(); }); @@ -55,4 +55,13 @@ describe('Import members gate', () => { await page.getByRole('combobox').first().click(); await expect.element(importMembersScreen.option('Not imported')).toBeVisible(); }); + + // Custom fields are no longer what chooses between the two: they are an experiment the + // redesign is meant to ship ahead of, so on their own they must move nothing. + it('serves the import as it shipped when only custom fields are on', async () => { + await openMappingStep({ membersCustomFields: true }); + + await expect.element(importMembersScreen.importToggle('name')).not.toBeInTheDocument(); + await expect.element(page.getByRole('combobox').first()).toBeVisible(); + }); }); 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 5853b681935..974d5fd7243 100644 --- a/apps/admin/src/posts/analytics/post-analytics.acceptance.test.tsx +++ b/apps/admin/src/posts/analytics/post-analytics.acceptance.test.tsx @@ -15,6 +15,7 @@ import { webAnalyticsBootOverrides, } from '@test-utils/acceptance'; import { membersScreen } from '@/members/members.screen'; +import { sidebarScreen } from '@/layout/sidebar.screen'; import { postAnalyticsScreen } from './post-analytics.screen'; const POST_ID = '64d623b64676110001e897d9'; @@ -133,6 +134,10 @@ describe('Post analytics overview', () => { await expect.element(postAnalyticsScreen.postTitle('Attack of the Clones')).toBeVisible(); await expect(postsApi).toHaveSentFilter(`id:${POST_ID}`); + await expect + .element(sidebarScreen.navLink('Analytics')) + .toHaveAttribute('aria-current', 'page'); + await expect.element(sidebarScreen.navLink('Posts')).not.toHaveAttribute('aria-current'); // Web performance: visitors summed from the Tinybird rows. await expect.element(postAnalyticsScreen.webPerformanceCard()).toBeVisible(); diff --git a/apps/admin/src/settings/advanced/labs/private-features.tsx b/apps/admin/src/settings/advanced/labs/private-features.tsx index 38233dd8ee7..1f6327fa2f3 100644 --- a/apps/admin/src/settings/advanced/labs/private-features.tsx +++ b/apps/admin/src/settings/advanced/labs/private-features.tsx @@ -83,6 +83,12 @@ const features: Feature[] = [ description: 'Let admins create and manage custom field definitions for members', flag: 'membersCustomFields', }, + { + title: 'Members import redesign', + description: + 'Serves the redesigned members CSV import dialog, which shows every column in the file and lets each one be mapped to a member field', + flag: 'membersImportRedesign', + }, { title: 'Paywall improvements', description: 'Enables paywall usability, discoverability and email customization improvements', diff --git a/apps/admin/src/settings/custom-fonts.css b/apps/admin/src/settings/custom-fonts.css new file mode 100644 index 00000000000..02cccfe5cd6 --- /dev/null +++ b/apps/admin/src/settings/custom-fonts.css @@ -0,0 +1,24 @@ +/* Site custom-font files for the settings font pickers. Imported from the + consuming screens so the fetches ship with the settings chunk, not the + admin entry CSS. The matching font-* utilities come from src/index.css. */ +@import url(https://fonts.bunny.net/css?family=cardo:400,700); +@import url(https://fonts.bunny.net/css?family=manrope:300,500,700); +@import url(https://fonts.bunny.net/css?family=merriweather:300,700); +@import url(https://fonts.bunny.net/css?family=nunito:400,600,700); +@import url(https://fonts.bunny.net/css?family=old-standard-tt:400,700); +@import url(https://fonts.bunny.net/css?family=prata:400); +@import url(https://fonts.bunny.net/css?family=roboto:400,500,700); +@import url(https://fonts.bunny.net/css?family=rufina:400,500,700); +@import url(https://fonts.bunny.net/css?family=tenor-sans:400); +@import url(https://fonts.bunny.net/css?family=space-grotesk:700); +@import url(https://fonts.bunny.net/css?family=chakra-petch:400); +@import url(https://fonts.bunny.net/css?family=noto-sans:400,700); +@import url(https://fonts.bunny.net/css?family=poppins:400,700); +@import url(https://fonts.bunny.net/css?family=fira-sans:400,700); +@import url(https://fonts.bunny.net/css?family=inter:400,700); +@import url(https://fonts.bunny.net/css?family=noto-serif:400,700); +@import url(https://fonts.bunny.net/css?family=lora:400,700); +@import url(https://fonts.bunny.net/css?family=ibm-plex-serif:400,700); +@import url(https://fonts.bunny.net/css?family=space-mono:400,700); +@import url(https://fonts.bunny.net/css?family=fira-mono:400,700); +@import url(https://fonts.bunny.net/css?family=jetbrains-mono:400,700); diff --git a/apps/admin/src/settings/membership/portal.tsx b/apps/admin/src/settings/membership/portal.tsx index 59a0dca93ca..ac1754ee482 100644 --- a/apps/admin/src/settings/membership/portal.tsx +++ b/apps/admin/src/settings/membership/portal.tsx @@ -1,3 +1,4 @@ +import '@/settings/custom-fonts.css'; import FakeLogo from '@/settings/assets/images/portal-splash-default-logo.png'; import React from 'react'; import TopLevelGroup from '@/settings/components/top-level-group'; diff --git a/apps/admin/src/settings/membership/tiers-checkout.acceptance.test.tsx b/apps/admin/src/settings/membership/tiers-checkout.acceptance.test.tsx new file mode 100644 index 00000000000..c83f34fdca2 --- /dev/null +++ b/apps/admin/src/settings/membership/tiers-checkout.acceptance.test.tsx @@ -0,0 +1,401 @@ +import { describe, expect, it } from 'vitest'; +import { page, userEvent } from 'vitest/browser'; + +import { + configResponse, + fakeAdminEndpoint, + fakeMemberCustomFields, + fakeSettingsScreens, + fakeTiers, + renderAdminApp, + settingsResponse, + tier, +} from '@test-utils/acceptance'; +import { settingsScreen } from '@/settings/settings.screen'; + +const freeTier = tier({ id: '645453f4d254799990dd0e21', name: 'Free', slug: 'free', type: 'free' }); +const supporterTier = tier({ + id: '645453f4d254799990dd0e22', + name: 'Basic Supporter', + slug: 'basic-supporter', +}); + +const addressField = { + key: 'shipping_address', + name: 'Shipping Address', + type: 'address', + status: 'active', + created_at: '2026-07-13T00:00:00.000Z', + updated_at: null as string | null, +}; + +const nameField = { + ...addressField, + key: 'recipient_name', + name: 'Recipient Name', + type: 'short_text', +}; + +function stripeSettings(overrides: Parameters[0] = {}) { + return settingsResponse({ + ...overrides, + settings: { + stripe_connect_display_name: 'Dummy', + stripe_connect_livemode: false, + stripe_connect_account_id: 'acct_123', + stripe_connect_publishable_key: 'pk_test_123', + stripe_connect_secret_key: 'sk_test_123', + ...overrides.settings, + }, + }); +} + +// The flag lives in settings and config in lockstep, and Stripe rides along in settings. +const flagOnBoot = { + browseConfig: { response: configResponse({ labs: { membersCustomFields: true } }) }, + browseSettings: { response: stripeSettings({ labs: { membersCustomFields: true } }) }, +}; + +const supporterConfig = { + tier_id: supporterTier.id, + custom_fields: [], + shipping: { + collect: true as const, + allowed_countries: ['FI', 'SE'], + name: { custom_field_key: nameField.key }, + address: { custom_field_key: addressField.key }, + }, + tax_number: { collect: true as const }, +}; + +/** The world most specs share: both tiers, both fields, and a declared configuration. */ +function checkoutWorld(configs: object[] = []) { + fakeSettingsScreens(); + fakeTiers([freeTier, supporterTier]); + fakeMemberCustomFields([addressField, nameField]); + fakeAdminEndpoint('GET', '/tiers/checkout_config/', { tiers_checkout_config: configs }); + return fakeAdminEndpoint('PUT', `/tiers/${supporterTier.id}/checkout_config/`, ({ body }) => ({ + tiers_checkout_config: [{ tier_id: supporterTier.id, custom_fields: [], ...(body as object) }], + })); +} + +async function openSupporterModal() { + await settingsScreen.tiers().getByText(supporterTier.name, { exact: true }).click(); + const modal = settingsScreen.tierDetailModal(); + // Exact: "Save recipient name as" would otherwise match too. + await expect.element(modal.getByLabelText('Name', { exact: true })).toBeVisible(); + return modal; +} + +describe('Tier checkout collection', () => { + it('keeps the tier modal untouched and the endpoint unqueried while the flag is off', async () => { + fakeSettingsScreens(); + fakeTiers([freeTier, supporterTier]); + const configApi = fakeAdminEndpoint('GET', '/tiers/checkout_config/', { + tiers_checkout_config: [], + }); + await renderAdminApp('/settings', { boot: { browseSettings: { response: stripeSettings() } } }); + + const modal = await openSupporterModal(); + await expect(modal.getByText('Checkout', { exact: true })).toHaveCount(0); + expect(configApi.requests).toHaveLength(0); + }); + + // The deploy-compatibility rule in apps/admin/README.md: an Admin deployed ahead of a + // Core without this endpoint must keep existing tier editing intact. + it('renders the tier modal without the section against a Core without the endpoint', async () => { + fakeSettingsScreens(); + fakeTiers([freeTier, supporterTier]); + // The flag also shows the Custom fields settings group, which browses the fields. + fakeMemberCustomFields([]); + fakeAdminEndpoint( + 'GET', + '/tiers/checkout_config/', + { errors: [{ type: 'NotFoundError', message: 'Resource not found error.' }] }, + { status: 404 }, + ); + await renderAdminApp('/settings', { boot: flagOnBoot }); + + const modal = await openSupporterModal(); + await expect(modal.getByText('Checkout', { exact: true })).toHaveCount(0); + await expect(modal.getByText(/could not be loaded/)).toHaveCount(0); + }); + + it("holds the section's place with an explanation when the configuration read fails", async () => { + fakeSettingsScreens(); + fakeTiers([freeTier, supporterTier]); + fakeMemberCustomFields([]); + fakeAdminEndpoint( + 'GET', + '/tiers/checkout_config/', + { errors: [{ type: 'InternalServerError', message: 'Something went wrong.' }] }, + { status: 500 }, + ); + await renderAdminApp('/settings', { boot: flagOnBoot }); + + const modal = await openSupporterModal(); + await expect.element(modal.getByText(/could not be loaded/)).toBeVisible(); + await expect(modal.getByLabelText('Collect shipping address')).toHaveCount(0); + }); + + it('shows no checkout section on the free tier', async () => { + checkoutWorld(); + await renderAdminApp('/settings', { boot: flagOnBoot }); + + await settingsScreen.tiers().getByText(freeTier.name, { exact: true }).click(); + const modal = settingsScreen.tierDetailModal(); + await expect.element(modal.getByLabelText('Name')).toBeVisible(); + await expect(modal.getByText('Checkout', { exact: true })).toHaveCount(0); + }); + + it('reflects the saved configuration and writes nothing when untouched', async () => { + const putApi = checkoutWorld([supporterConfig]); + await renderAdminApp('/settings', { boot: flagOnBoot }); + + const modal = await openSupporterModal(); + await expect.element(modal.getByLabelText('Collect shipping address')).toBeChecked(); + await expect.element(modal.getByLabelText('Ships to')).toHaveTextContent('Specific countries'); + await expect + .element(modal.getByLabelText('Select specific countries')) + .toHaveTextContent('Finland, Sweden'); + await expect + .element(modal.getByLabelText('Save address as')) + .toHaveTextContent(addressField.name); + await expect + .element(modal.getByLabelText('Save recipient name as')) + .toHaveTextContent(nameField.name); + await expect.element(modal.getByLabelText('Collect business tax ID')).toBeChecked(); + await expect.element(modal.getByLabelText('Collect phone number')).not.toBeChecked(); + + await modal.getByRole('button', { name: 'Save' }).click(); + await expect.element(modal.getByRole('button', { name: 'Saved' })).toBeVisible(); + expect(putApi.requests).toHaveLength(0); + + // A clean save leaves nothing unsaved: closing asks no questions. + await modal.getByRole('button', { name: 'Close' }).click(); + await expect(settingsScreen.tierDetailModal()).toHaveCount(0); + }); + + // The other half of the sentinel: a tier that delivers everywhere carries no list, and + // has to read back as "All countries" rather than as a restriction to nothing. + it('reads a configuration with no countries as delivering everywhere', async () => { + const everywhere = { ...supporterConfig.shipping }; + delete (everywhere as { allowed_countries?: string[] }).allowed_countries; + checkoutWorld([{ ...supporterConfig, shipping: everywhere }]); + await renderAdminApp('/settings', { boot: flagOnBoot }); + + const modal = await openSupporterModal(); + await expect.element(modal.getByLabelText('Collect shipping address')).toBeChecked(); + await expect.element(modal.getByLabelText('Ships to')).toHaveTextContent('All countries'); + await expect(modal.getByLabelText('Select specific countries')).toHaveCount(0); + }); + + it('validates destinations inline before anything is written', async () => { + const putApi = checkoutWorld(); + await renderAdminApp('/settings', { boot: flagOnBoot }); + + const modal = await openSupporterModal(); + await modal.getByLabelText('Collect shipping address').click(); + await modal.getByRole('button', { name: 'Save' }).click(); + + await expect(modal.getByText('Choose where this should be kept')).toHaveCount(2); + expect(putApi.requests).toHaveLength(0); + }); + + it('saves the chosen collections, stating every block explicitly', async () => { + const putApi = checkoutWorld(); + await renderAdminApp('/settings', { boot: flagOnBoot }); + + const modal = await openSupporterModal(); + await modal.getByLabelText('Collect shipping address').click(); + await modal.getByLabelText('Save address as').click(); + await page.getByRole('option', { name: addressField.name }).click(); + await modal.getByLabelText('Save recipient name as').click(); + await page.getByRole('option', { name: nameField.name }).click(); + await modal.getByRole('button', { name: 'Save' }).click(); + await expect.element(modal.getByRole('button', { name: 'Saved' })).toBeVisible(); + + const sent = ( + putApi.lastRequest?.body as { + tiers_checkout_config: [{ shipping: { collect: boolean; allowed_countries?: string[] } }]; + } + ).tiers_checkout_config[0]; + expect(sent).toMatchObject({ + shipping: { + collect: true, + name: { custom_field_key: nameField.key }, + address: { custom_field_key: addressField.key }, + }, + tax_number: { collect: false }, + phone: { collect: false }, + }); + // "All countries" is written as no list at all. Sending every country instead would + // save today's set as a restriction, and quietly exclude whatever is added next. + expect(sent.shipping).not.toHaveProperty('allowed_countries'); + }); + + // A destination can stop being usable between the picker offering it and the save + // reaching the server. Only the server sees that, so its refusal has to land on the + // picker it names, carrying the server's own words rather than a guess restated here. + it('shows a refused destination against the picker that named it', async () => { + const refusal = 'An archived custom field cannot receive collected data. Restore it first.'; + fakeSettingsScreens(); + fakeTiers([freeTier, supporterTier]); + fakeMemberCustomFields([addressField, nameField]); + fakeAdminEndpoint('GET', '/tiers/checkout_config/', { tiers_checkout_config: [] }); + fakeAdminEndpoint( + 'PUT', + `/tiers/${supporterTier.id}/checkout_config/`, + { + errors: [ + { + message: 'Validation error', + context: refusal, + type: 'ValidationError', + property: 'checkout.shipping_address.custom_field_key', + }, + ], + }, + { status: 422 }, + ); + await renderAdminApp('/settings', { boot: flagOnBoot }); + + const modal = await openSupporterModal(); + await modal.getByLabelText('Collect shipping address').click(); + await modal.getByLabelText('Save address as').click(); + await page.getByRole('option', { name: addressField.name }).click(); + await modal.getByLabelText('Save recipient name as').click(); + await page.getByRole('option', { name: nameField.name }).click(); + await modal.getByRole('button', { name: 'Save' }).click(); + + await expect.element(modal.getByText(refusal)).toBeVisible(); + // On the picker the server blamed, and only that one. + await expect + .element(modal.getByLabelText('Save address as')) + .toHaveAttribute('aria-invalid', 'true'); + await expect + .element(modal.getByLabelText('Save recipient name as')) + .not.toHaveAttribute('aria-invalid'); + }); + + it('creates a destination field from inside the picker', async () => { + let fields = [addressField]; + const created = { ...nameField, key: 'gift_recipient', name: 'Gift Recipient' }; + fakeSettingsScreens(); + fakeTiers([freeTier, supporterTier]); + fakeMemberCustomFields(() => fields); + fakeAdminEndpoint('GET', '/tiers/checkout_config/', { tiers_checkout_config: [] }); + const createApi = fakeAdminEndpoint('POST', '/members/custom_fields/', () => { + fields = [...fields, created]; + return { members_custom_fields: [created] }; + }); + await renderAdminApp('/settings', { boot: flagOnBoot }); + + const modal = await openSupporterModal(); + await modal.getByLabelText('Collect shipping address').click(); + await modal.getByLabelText('Save recipient name as').click(); + await page.getByRole('option', { name: 'Add custom field' }).click(); + await page.getByLabelText(/New custom field/).fill(created.name); + // Enter submits the inline form; a Save-button locator would be ambiguous with the + // modal's own footer Save. + await userEvent.keyboard('{Enter}'); + + await expect + .element(modal.getByLabelText('Save recipient name as')) + .toHaveTextContent(created.name); + expect(createApi.lastRequest?.body).toEqual({ + members_custom_fields: [{ name: created.name, type: 'short_text' }], + }); + }); + + it('collects checkout settings while creating a tier, in one Save', async () => { + const createdTier = tier({ + id: '645453f4d254799990dd0e99', + name: 'Print Edition', + slug: 'print-edition', + monthly_price: 800, + yearly_price: 8000, + }); + let saved = false; + fakeSettingsScreens(); + fakeTiers(() => (saved ? [freeTier, supporterTier, createdTier] : [freeTier, supporterTier])); + fakeMemberCustomFields([addressField, nameField]); + fakeAdminEndpoint('GET', '/tiers/checkout_config/', { tiers_checkout_config: [] }); + const createApi = fakeAdminEndpoint('POST', '/tiers/', () => { + saved = true; + return { tiers: [createdTier] }; + }); + const putApi = fakeAdminEndpoint( + 'PUT', + `/tiers/${createdTier.id}/checkout_config/`, + ({ body }) => ({ + tiers_checkout_config: [ + { tier_id: createdTier.id, custom_fields: [], ...(body as object) }, + ], + }), + ); + await renderAdminApp('/settings', { boot: flagOnBoot }); + + await settingsScreen.tiers().getByRole('button', { name: 'Add tier' }).click(); + const modal = settingsScreen.tierDetailModal(); + await modal.getByLabelText('Name', { exact: true }).fill(createdTier.name); + await modal.getByLabelText('Monthly price').fill('8'); + await modal.getByLabelText('Yearly price').fill('80'); + + // The checkout card is present during creation, under the same conditions as prices. + await modal.getByLabelText('Collect shipping address').click(); + await modal.getByLabelText('Save address as').click(); + await page.getByRole('option', { name: addressField.name }).click(); + await modal.getByLabelText('Save recipient name as').click(); + await page.getByRole('option', { name: nameField.name }).click(); + + await modal.getByRole('button', { name: 'Save' }).click(); + await expect.element(modal.getByRole('button', { name: 'Saved' })).toBeVisible(); + + expect(createApi.lastRequest?.body).toMatchObject({ tiers: [{ name: createdTier.name }] }); + const sent = ( + putApi.lastRequest?.body as { tiers_checkout_config: [{ shipping: { collect: boolean } }] } + ).tiers_checkout_config[0]; + expect(sent).toMatchObject({ + shipping: { + collect: true, + name: { custom_field_key: nameField.key }, + address: { custom_field_key: addressField.key }, + }, + tax_number: { collect: false }, + phone: { collect: false }, + }); + + // The one Save covered both writes: closing asks no questions. + await modal.getByRole('button', { name: 'Close' }).click(); + await expect(settingsScreen.tierDetailModal()).toHaveCount(0); + }); + + it('closes without confirmation after saving checkout edits', async () => { + const putApi = checkoutWorld(); + await renderAdminApp('/settings', { boot: flagOnBoot }); + + const modal = await openSupporterModal(); + await modal.getByLabelText('Collect business tax ID').click(); + await modal.getByRole('button', { name: 'Save' }).click(); + await expect.element(modal.getByRole('button', { name: 'Saved' })).toBeVisible(); + expect(putApi.requests).toHaveLength(1); + + await modal.getByRole('button', { name: 'Close' }).click(); + await expect(settingsScreen.tierDetailModal()).toHaveCount(0); + }); + + it('asks before discarding unsaved checkout edits', async () => { + checkoutWorld(); + await renderAdminApp('/settings', { boot: flagOnBoot }); + + const modal = await openSupporterModal(); + await modal.getByLabelText('Collect phone number').click(); + await userEvent.keyboard('{Escape}'); + + const confirmation = page.getByText('Are you sure you want to leave this page?'); + await expect.element(confirmation).toBeVisible(); + await page.getByRole('button', { name: 'Leave' }).click(); + await expect(settingsScreen.tierDetailModal()).toHaveCount(0); + }); +}); diff --git a/apps/admin/src/settings/membership/tiers.tsx b/apps/admin/src/settings/membership/tiers.tsx index e72d13c6c30..7f2c6d8068c 100644 --- a/apps/admin/src/settings/membership/tiers.tsx +++ b/apps/admin/src/settings/membership/tiers.tsx @@ -46,6 +46,7 @@ import { useConfirmation } from '@/settings/providers/confirmation-context'; import { useGlobalData } from '@/settings/providers/global-data-context'; import { useFeatureFlag, useHandleError, useLimiter } from '@tryghost/admin-x-framework/hooks'; import { useSettingsNavigation } from '@/settings/hooks/use-settings-navigation'; +import { useTierCheckoutCollection } from './tiers/use-tier-checkout-collection'; import { useUpgradeRoute } from '@/settings/hooks/use-upgrade-route'; import { withErrorBoundary } from '@/settings/components/with-error-boundary'; @@ -69,6 +70,9 @@ const StripeConnectedButton: React.FC<{ className?: string; onClick: () => void }; const Tiers: React.FC<{ keywords: string[] }> = ({ keywords }) => { + // Warms the tier-independent checkout-config read (cached, one request) so the tier + // modal's deferred first paint has nothing left to wait for by the time one opens. + useTierCheckoutCollection(undefined); const [selectedTab, setSelectedTab] = useState('active-tiers'); const [currencyOpen, setCurrencyOpen] = useState(false); const [machinePaymentsAmountError, setMachinePaymentsAmountError] = useState< diff --git a/apps/admin/src/settings/membership/tiers/tier-checkout-collection.tsx b/apps/admin/src/settings/membership/tiers/tier-checkout-collection.tsx new file mode 100644 index 00000000000..3169ae9e394 --- /dev/null +++ b/apps/admin/src/settings/membership/tiers/tier-checkout-collection.tsx @@ -0,0 +1,613 @@ +import countries from 'i18n-iso-countries'; +import enLocale from 'i18n-iso-countries/langs/en.json'; +import { CustomFieldPicker } from '@/shared/member-custom-fields/custom-field-picker'; +import { + Combobox, + ComboboxContent, + ComboboxTrigger, + ComboboxValue, + Field, + FieldDescription, + FieldError, + FieldGroup, + FieldLabel, + FieldLegend, + FieldSet, + MultiSelectCombobox, + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, + Separator, + Switch, +} from '@tryghost/shade/components'; +import { + PORT_FIELD, + STRIPE_ALLOWED_COUNTRIES, + STRIPE_PORT, + isStripePort, + type StripePort, +} from '@tryghost/checkout'; +import { JSONError, getErrorMessage } from '@tryghost/admin-x-framework/errors'; +import { type ErrorMessages, useHandleError } from '@tryghost/admin-x-framework/hooks'; +import { Text } from '@tryghost/shade/primitives'; +import { + type MemberCustomField, + useBrowseMemberCustomFields, +} from '@tryghost/admin-x-framework/api/member-custom-fields'; +import { + type TierCheckoutConfig, + useEditTierCheckoutConfig, +} from '@tryghost/admin-x-framework/api/tiers-checkout-config'; +import { + type ReactNode, + forwardRef, + useEffect, + useImperativeHandle, + useMemo, + useState, +} from 'react'; + +countries.registerLocale(enLocale); + +// Names for codes ISO 3166-1 does not carry as standalone entries but Stripe ships to. +const EXTRA_COUNTRY_NAMES: Record = { + AC: 'Ascension Island', + TA: 'Tristan da Cunha', + ZZ: 'Unknown region', +}; + +const countryName = (code: string) => + EXTRA_COUNTRY_NAMES[code] ?? countries.getName(code, 'en', { select: 'alias' }) ?? code; + +const countryOptions = STRIPE_ALLOWED_COUNTRIES.map((code) => ({ + value: code, + label: countryName(code), +})).sort((a, b) => a.label.localeCompare(b.label)); + +const SHIPS_TO_OPTIONS = [ + { value: 'all', label: 'All countries', hint: 'Everywhere Stripe supports shipping' }, + { value: 'specific', label: 'Specific countries', hint: 'Choose the countries you deliver to' }, +] as const; +type ShipsToMode = (typeof SHIPS_TO_OPTIONS)[number]['value']; + +/** Which picker a refusal belongs to, by the collection the server names in it. */ +const DESTINATION_ERROR: Record = { + [STRIPE_PORT.shippingAddress]: 'shippingField', + [STRIPE_PORT.shippingName]: 'shippingName', + [STRIPE_PORT.phone]: 'phoneField', +}; + +/** + * The picker a refused save blamed, if it blamed one. + * + * A destination can stop being usable between the picker listing it and the save reaching + * the server — archived, or retyped to something that cannot hold what is collected. Only + * the server sees that, and it says which collection was refused, so the refusal can be + * shown against the picker that named it instead of as a toast that leaves every picker + * looking equally fine. + */ +const refusedDestination = (error: unknown): string | undefined => { + const property = (error instanceof JSONError ? error.data?.errors?.[0]?.property : null) ?? ''; + const port = /^checkout\.(\w+)\.custom_field_key$/.exec(property)?.[1]; + return port && isStripePort(port) ? DESTINATION_ERROR[port] : undefined; +}; + +export type TierCheckoutCollectionHandle = { + /** + * Check the configuration without persisting anything, painting errors inline. Meant to + * run alongside the tier form's own validation BEFORE either resource is written, so a + * refused save never leaves one of the two already committed. + */ + validate: () => boolean; + /** + * Persist the checkout configuration against the given tier — the one being edited, or + * the one a create just made, which is why the id arrives here rather than as a prop. + * Resolves true without a request when nothing changed. Resolves false when nothing was + * saved — invalid input (errors are shown inline) or a failed request (already handed + * to handleError) — so the caller can stop without an exception escaping the modal's + * onOk. + */ + save: (tierId: string) => Promise; +}; + +type TierCheckoutState = { + shipping: { + collect: boolean; + countriesMode: ShipsToMode; + allowedCountries: string[]; + addressFieldKey: string | null; + nameFieldKey: string | null; + }; + // Just a switch: the tax number stays on Stripe, so there is no destination to hold. + taxNumber: { collect: boolean }; + phone: { collect: boolean; customFieldKey: string | null }; +}; + +/** + * What the section would actually save, as state. Sub-choices behind a switched-off + * toggle stay in local state so flipping the toggle back restores them, but they are not + * part of any write — so they must not count toward dirtiness either, or a save that + * turned something off would compare its leftovers against a baseline that no longer + * holds them and read as unsaved forever. + */ +const effectiveState = (current: TierCheckoutState): TierCheckoutState => ({ + shipping: current.shipping.collect + ? { + ...current.shipping, + allowedCountries: + current.shipping.countriesMode === 'specific' ? current.shipping.allowedCountries : [], + } + : { + collect: false, + countriesMode: 'all', + allowedCountries: [], + addressFieldKey: null, + nameFieldKey: null, + }, + taxNumber: current.taxNumber, + phone: current.phone.collect ? current.phone : { collect: false, customFieldKey: null }, +}); + +const stateFromConfig = (config: TierCheckoutConfig | undefined): TierCheckoutState => { + const shipping = config?.shipping; + // Countries are a restriction, so a configuration that names none delivers everywhere. + const restrictedTo = shipping?.allowed_countries; + return { + shipping: { + collect: Boolean(shipping), + addressFieldKey: shipping?.address.custom_field_key ?? null, + nameFieldKey: shipping?.name.custom_field_key ?? null, + countriesMode: restrictedTo ? 'specific' : 'all', + allowedCountries: restrictedTo ?? [], + }, + taxNumber: { + collect: Boolean(config?.tax_number), + }, + phone: { + collect: Boolean(config?.phone), + customFieldKey: config?.phone?.custom_field_key ?? null, + }, + }; +}; + +/** The section's one card shell, shared by the configured state and the failed state. */ +function CheckoutCard({ children }: { children: ReactNode }) { + return ( +
+ Checkout + + {children} + +
+ ); +} + +/** A card row: plain-text label left, a control that prefers 256px but may shrink right. */ +function Row({ label, children }: { label: string; children: ReactNode }) { + return ( +
+
{label}
+
{children}
+
+ ); +} + +function CollectToggle({ + checked, + description, + id, + label, + onCheckedChange, +}: { + checked: boolean; + description: string; + id: string; + label: string; + onCheckedChange: (checked: boolean) => void; +}) { + return ( +
+ + {label} + + + {description} +
+ ); +} + +function DestinationRow({ + eligible, + error, + id, + label, + port, + value, + onChange, +}: { + eligible: Record; + error: string | undefined; + id: string; + label: string; + /** Names what is collected. What may hold it follows from the port table, not from here. */ + port: StripePort; + value: string | null; + onChange: (key: string) => void; +}) { + return ( + + + + {error && {error}} + + + ); +} + +const TierCheckoutCollection = forwardRef< + TierCheckoutCollectionHandle, + { + config: TierCheckoutConfig | undefined; + /** + * The configuration read failed on a Core that has the endpoint: the card's place is + * held with an explanation, because an absent section would read as "collection is + * off" to a publisher who came to check exactly that. The handle no-ops. + */ + failed?: boolean; + /** Fires when edits diverge from (or return to) the saved configuration. */ + onDirtyChange?: (dirty: boolean) => void; + } +>(({ config, failed, onDirtyChange }, ref) => { + const [state, setState] = useState(() => stateFromConfig(config)); + const [errors, setErrors] = useState({}); + const [countriesOpen, setCountriesOpen] = useState(false); + const { mutateAsync: editCheckoutConfig } = useEditTierCheckoutConfig(); + const handleError = useHandleError(); + + const { data: fieldsData } = useBrowseMemberCustomFields(); + const allFields = fieldsData?.members_custom_fields ?? []; + // What each collected value may be kept in is the server's rule, so it is read from the + // shared port table rather than restated here: anything wider would invite a pick the + // save then refuses, anything narrower would hide a field that would have been accepted. + // Cast because Object.entries widens the port keys back to string. + const eligible = Object.fromEntries( + Object.entries(PORT_FIELD).map(([port, wants]) => [ + port, + allFields.filter((field) => field.type === wants.type), + ]), + ) as Record; + + const selectedCountries = state.shipping.allowedCountries; + + // stateFromConfig builds every object literally and effectiveState rebuilds in that + // same shape, so key order is stable and stringify comparison is reliable. The mount is + // keyed by tier id, so `config` describes this state's origin for the whole lifetime — + // though the baseline recomputes when a save's invalidation refetches it. + const initialSerialized = useMemo( + () => JSON.stringify(effectiveState(stateFromConfig(config))), + [config], + ); + // A successful write makes the state it saved the baseline immediately, without waiting + // for a refetch to deliver it back — the create path depends on this, because a mount + // with no tier id has no config for any refetch to rebase it with. + const [savedSerialized, setSavedSerialized] = useState(null); + const dirty = + !failed && JSON.stringify(effectiveState(state)) !== (savedSerialized ?? initialSerialized); + + useEffect(() => { + onDirtyChange?.(dirty); + }, [dirty, onDirtyChange]); + + const buildErrors = (): ErrorMessages => { + const newErrors: ErrorMessages = {}; + // Only what the picker offers can be saved: a chosen key missing from the eligible + // list means the field was archived or deleted since — the server would refuse it, + // so it is surfaced here, next to a picker that would otherwise just look empty. + // Held off while the fields are still loading, when absence proves nothing. + const destinationError = (key: string | null, offered: MemberCustomField[]) => { + if (!key) { + return 'Choose where this should be kept'; + } + if (fieldsData && !offered.some((field) => field.key === key)) { + return 'This field is no longer available. Choose another'; + } + return undefined; + }; + if (state.shipping.collect) { + newErrors.shippingField = destinationError( + state.shipping.addressFieldKey, + eligible[STRIPE_PORT.shippingAddress], + ); + newErrors.shippingName = destinationError( + state.shipping.nameFieldKey, + eligible[STRIPE_PORT.shippingName], + ); + if (state.shipping.countriesMode === 'specific' && !state.shipping.allowedCountries.length) { + newErrors.shippingCountries = 'Choose at least one country you deliver to'; + } + } + if (state.phone.collect) { + // Two collections MAY share a destination: writes apply in a fixed order and the + // last wins, which is the designed behaviour for shared fields — so no distinctness + // rule here, deliberately. + newErrors.phoneField = destinationError( + state.phone.customFieldKey, + eligible[STRIPE_PORT.phone], + ); + } + return newErrors; + }; + + // The one judgement both handle methods share: paint what buildErrors found and say + // whether the state may be written. Untouched state passes without judgement — it + // saved once already, and re-judging it could block the whole tier save on a section + // the publisher never opened (e.g. a since-archived destination). Problems in stored + // config surface when it is edited. + const applyValidation = () => { + if (!dirty) { + setErrors({}); + return true; + } + const newErrors = buildErrors(); + setErrors(newErrors); + return !Object.values(newErrors).some(Boolean); + }; + + useImperativeHandle( + ref, + () => ({ + validate: applyValidation, + save: async (tierId: string) => { + if (!dirty) { + return true; + } + if (!applyValidation()) { + return false; + } + + try { + // Every block is stated so switching one off is written as collect: false + // rather than left alone by omission. + await editCheckoutConfig({ + tierId, + config: { + shipping: state.shipping.collect + ? { + collect: true, + // Everywhere is the absence of a list, never a copy of every country: + // that set moves, and a saved enumeration would silently become a + // restriction the day the processor adds one. + ...(state.shipping.countriesMode === 'specific' + ? { allowed_countries: state.shipping.allowedCountries } + : {}), + name: { custom_field_key: state.shipping.nameFieldKey! }, + address: { custom_field_key: state.shipping.addressFieldKey! }, + } + : { collect: false }, + tax_number: { collect: state.taxNumber.collect }, + phone: state.phone.collect + ? { + collect: true, + custom_field_key: state.phone.customFieldKey!, + } + : { collect: false }, + }, + }); + setSavedSerialized(JSON.stringify(effectiveState(state))); + return true; + } catch (error) { + const blamed = refusedDestination(error); + if (blamed) { + // Reported without a toast, the way the picker's own create does it: shown in + // place, but a refusal only the server can detect still has to reach error + // tracking rather than being swallowed by the field it lands on. + handleError(error, { withToast: false }); + setErrors((current) => ({ + ...current, + [blamed]: getErrorMessage(error, 'This field cannot be used here. Choose another'), + })); + return false; + } + handleError(error); + return false; + } + }, + }), + // buildErrors and dirty close over render-scope values, so everything they read has + // to invalidate the handle. + [dirty, editCheckoutConfig, fieldsData, handleError, state], + ); + + if (failed) { + return ( + + + Checkout collection settings could not be loaded, so they are not shown. Close and reopen + this tier to try again. + + + ); + } + + return ( + + + setState((current) => ({ + ...current, + shipping: { ...current.shipping, collect: checked }, + })) + } + /> + {state.shipping.collect && ( + <> + + + Ships to + + + + {state.shipping.countriesMode === 'specific' && ( + + + Select specific countries + + + + {selectedCountries.length + ? selectedCountries.map(countryName).join(', ') + : 'Select...'} + + + + { + setErrors((current) => ({ ...current, shippingCountries: undefined })); + setState((current) => ({ + ...current, + shipping: { ...current.shipping, allowedCountries: values }, + })); + }} + onClose={() => setCountriesOpen(false)} + /> + + + {errors.shippingCountries && {errors.shippingCountries}} + + + )} + { + setErrors((current) => ({ ...current, shippingField: undefined })); + setState((current) => ({ + ...current, + shipping: { ...current.shipping, addressFieldKey: key }, + })); + }} + /> + { + setErrors((current) => ({ ...current, shippingName: undefined })); + setState((current) => ({ + ...current, + shipping: { ...current.shipping, nameFieldKey: key }, + })); + }} + /> + + )} + + + + + setState((current) => ({ + ...current, + phone: { ...current.phone, collect: checked }, + })) + } + /> + {state.phone.collect && ( + { + setErrors((current) => ({ ...current, phoneField: undefined })); + setState((current) => ({ + ...current, + phone: { ...current.phone, customFieldKey: key }, + })); + }} + /> + )} + + + + + setState((current) => ({ + ...current, + taxNumber: { ...current.taxNumber, collect: checked }, + })) + } + /> + + ); +}); + +TierCheckoutCollection.displayName = 'TierCheckoutCollection'; + +export default TierCheckoutCollection; diff --git a/apps/admin/src/settings/membership/tiers/tier-detail-modal.tsx b/apps/admin/src/settings/membership/tiers/tier-detail-modal.tsx index eec7ffab0a1..f7d072744d3 100644 --- a/apps/admin/src/settings/membership/tiers/tier-detail-modal.tsx +++ b/apps/admin/src/settings/membership/tiers/tier-detail-modal.tsx @@ -1,4 +1,8 @@ import React, { useEffect, useRef } from 'react'; +import TierCheckoutCollection, { + type TierCheckoutCollectionHandle, +} from './tier-checkout-collection'; +import { useTierCheckoutCollection } from './use-tier-checkout-collection'; import TierDetailPreview from './tier-detail-preview'; import useCurrencyInput from '@/settings/hooks/use-currency-input'; import useSettingGroup from '@/settings/hooks/use-setting-group'; @@ -28,7 +32,7 @@ import { } from '@tryghost/shade/components'; import { type ErrorMessages, useForm, useHandleError } from '@tryghost/admin-x-framework/hooks'; import { Inline, Text } from '@tryghost/shade/primitives'; -import { LucideIcon } from '@tryghost/shade/utils'; +import { LucideIcon, cn } from '@tryghost/shade/utils'; import { useParams } from '@tryghost/admin-x-framework'; import { useSettingsNavigation } from '@/settings/hooks/use-settings-navigation'; import { SettingsModal } from '@tryghost/shade/patterns'; @@ -51,8 +55,14 @@ export type TierFormState = Partial> & { trial_days: string; }; -const TierDetailModalContent: React.FC<{ tier?: Tier }> = ({ tier }) => { +const TierDetailModalContent: React.FC<{ + tier?: Tier; + checkout: ReturnType; +}> = ({ tier, checkout }) => { const isFreeTier = tier?.type === 'free'; + // Both flags are already gated on a paid tier (saved or being created) inside the hook. + const checkoutVisible = checkout.enabled; + const checkoutFailed = checkout.failed; const [currencyOpen, setCurrencyOpen] = React.useState(false); const { updateRoute } = useSettingsNavigation(); @@ -88,7 +98,7 @@ const TierDetailModalContent: React.FC<{ tier?: Tier }> = ({ tier }) => { : undefined, }; - const { formState, saveState, updateForm, handleSave, errors, clearError, okProps } = + const { formState, saveState, updateForm, handleSave, validate, errors, clearError, okProps } = useForm({ initialState: { ...(tier || {}), @@ -122,7 +132,10 @@ const TierDetailModalContent: React.FC<{ tier?: Tier }> = ({ tier }) => { if (tier?.id) { await updateTier({ ...tier, ...values }); } else { - await createTier(values); + // The id the checkout save needs: a new tier has none until this create + // returns one, and onOk reads it from the ref right after. + const created = await createTier(values); + createdTierIdRef.current = created.tiers[0]?.id ?? null; } if (isFreeTier) { // If we changed the visibility, we also need to update Portal settings in some situations @@ -174,6 +187,18 @@ const TierDetailModalContent: React.FC<{ tier?: Tier }> = ({ tier }) => { canAddNewItem: (item) => !!item, }); const modalRef = useRef(null); + const checkoutCollectionRef = useRef(null); + const createdTierIdRef = useRef(null); + // The checkout section keeps its own form state, so it reports its dirtiness up for + // the modal's close confirmation to count. + const [checkoutDirty, setCheckoutDirty] = React.useState(false); + // If the section leaves the modal mid-session (its read failed), its unsaved edits + // leave with it — the flag must not stay armed for a section that no longer exists. + useEffect(() => { + if (!checkoutVisible) { + setCheckoutDirty(false); + } + }, [checkoutVisible]); const newBenefitInputRef = useRef(null); const addBenefit = () => { @@ -294,7 +319,7 @@ const TierDetailModalContent: React.FC<{ tier?: Tier }> = ({ tier }) => { ref={modalRef} buttonsDisabled={okProps.disabled} cancelLabel="Close" - dirty={saveState === 'unsaved'} + dirty={saveState === 'unsaved' || checkoutDirty} leftButton={leftButton} okLabel={okProps.label || 'Save'} okVariant={okProps.variant} @@ -306,7 +331,27 @@ const TierDetailModalContent: React.FC<{ tier?: Tier }> = ({ tier }) => { updateRoute('tiers'); }} onOk={async () => { - await handleSave({ fakeWhenUnchanged: true }); + // Two resources, one Save. The checkout section validates first so both error + // sets paint on one click; the tier's validation lives inside handleSave, which + // also flips the button to its Retry state on failure, exactly as it did before + // this section existed. + const checkoutValid = checkoutCollectionRef.current?.validate() ?? true; + if (!checkoutValid) { + validate(); + return; + } + // The tier is the primary resource, so it commits first: a tier failure leaves + // the checkout config untouched. A checkout failure — reported by its save + // returning false rather than throwing — leaves a saved tier and a Save that + // converges on retry, because the unchanged tier save is fake and the config + // save simply runs again. + if (!(await handleSave({ fakeWhenUnchanged: true }))) { + return; + } + const savedTierId = tier?.id ?? createdTierIdRef.current; + if (savedTierId) { + await checkoutCollectionRef.current?.save(savedTierId); + } }} >
@@ -512,7 +557,12 @@ const TierDetailModalContent: React.FC<{ tier?: Tier }> = ({ tier }) => {
Benefits - + `Reorder benefit${item ? `: ${item}` : ''}`} items={benefits.items} @@ -581,6 +631,16 @@ const TierDetailModalContent: React.FC<{ tier?: Tier }> = ({ tier }) => {
+ + {(checkoutVisible || checkoutFailed) && ( + + )}
@@ -604,13 +664,23 @@ const TierDetailModal: React.FC = () => { if (tierId) { tier = tiers?.find(({ id }) => id === tierId); + } - if (!tier) { - return null; - } + // Deferring the first paint until everything it shows is loaded is this modal's + // existing rule (the tier lookup above); the checkout configuration joins it so the + // Basic card never grows after it appears. Disabled (flag off, no Stripe, or a free + // tier) means nothing is fetched and nothing is waited on. + const checkout = useTierCheckoutCollection(tier, { creating: !tierId }); + + if (tierId && !tier) { + return null; + } + + if (!checkout.isReady) { + return null; } - return ; + return ; }; export default TierDetailModal; diff --git a/apps/admin/src/settings/membership/tiers/use-tier-checkout-collection.ts b/apps/admin/src/settings/membership/tiers/use-tier-checkout-collection.ts new file mode 100644 index 00000000000..9bd9beb4d82 --- /dev/null +++ b/apps/admin/src/settings/membership/tiers/use-tier-checkout-collection.ts @@ -0,0 +1,63 @@ +import { APIError } from '@tryghost/admin-x-framework/errors'; +import { type Tier } from '@tryghost/admin-x-framework/api/tiers'; +import { useFeatureFlag } from '@tryghost/admin-x-framework/hooks'; +import { checkStripeEnabled } from '@tryghost/admin-x-framework/api/settings'; +import { useBrowseTiersCheckoutConfig } from '@tryghost/admin-x-framework/api/tiers-checkout-config'; +import { useGlobalData } from '@/settings/providers/global-data-context'; + +/** + * The gate and the data for a tier's checkout collection, in one place. + * + * With the membersCustomFields flag or Stripe off, nothing is fetched and the tier modal + * renders exactly as it did before. The card itself additionally needs a paid tier — + * saved, or in the middle of being created. + * When the card is due, `isReady` lets the modal defer its first paint until the + * configuration is loaded — the same rule it already applies to the tier itself — so the + * card never grows after it appears. + * + * A failed read makes the checkout section unavailable, never the modal: an Admin can run + * against a Core without this endpoint (the flag alone is not a compatibility check), and + * a transient failure has to degrade the same way. There is no toast, because nothing + * here is actionable for the publisher — but the two failures read differently: a 404 is + * the feature not existing on this Core, so the card is simply absent, while any other + * settled failure is a load problem with the feature present, and `failed` lets the modal + * say so instead of silently looking like collection is off. + */ +export const useTierCheckoutCollection = ( + tier: Tier | undefined, + { creating = false }: { creating?: boolean } = {}, +) => { + const hasCustomFields = useFeatureFlag('membersCustomFields'); + const { config: globalConfig, settings } = useGlobalData(); + + // The read is tier-independent — one browse covers every tier — so it hangs only on + // the feature being on. That lets the tiers list warm it before any modal opens, and + // means a free-tier or new-tier modal costs at most one cached request. + const fetchWanted = hasCustomFields && checkStripeEnabled(settings || [], globalConfig || {}); + const { data, error, isError, isFetching } = useBrowseTiersCheckoutConfig({ + enabled: fetchWanted, + defaultErrorHandler: false, + }); + + // The card shows for a saved paid tier, and during creation — a new tier is paid by + // definition, and its configuration is held locally until the create supplies the id + // to save it against, the same conditions under which the price inputs show. + const sectionWanted = fetchWanted && (tier ? tier.type !== 'free' : creating); + + // Settled failure only: while a refetch after an earlier error is in flight the section + // is pending, not unavailable, so a card that is about to load holds the modal's paint + // instead of popping in after it. + const failed = isError && !isFetching; + const missingBackend = failed && error instanceof APIError && error.response?.status === 404; + + const enabled = sectionWanted && !failed; + + return { + enabled, + failed: sectionWanted && failed && !missingBackend, + isReady: !enabled || Boolean(data), + // The extra `?.` guards a malformed success response: the section degrades to its + // empty state instead of the modal crashing on a missing array. + config: data?.tiers_checkout_config?.find((entry) => entry.tier_id === tier?.id), + }; +}; diff --git a/apps/admin/src/settings/site/design-and-branding/global-settings.tsx b/apps/admin/src/settings/site/design-and-branding/global-settings.tsx index bd92f8eddd6..ada1a57f058 100644 --- a/apps/admin/src/settings/site/design-and-branding/global-settings.tsx +++ b/apps/admin/src/settings/site/design-and-branding/global-settings.tsx @@ -1,3 +1,4 @@ +import '@/settings/custom-fonts.css'; import ColorPickerField from '@/settings/components/color-picker-field'; import React, { useState } from 'react'; import UnsplashSelector from '@/settings/components/selectors/unsplash-selector'; diff --git a/apps/admin/src/shared/member-custom-fields/custom-field-picker.tsx b/apps/admin/src/shared/member-custom-fields/custom-field-picker.tsx new file mode 100644 index 00000000000..01875c605b7 --- /dev/null +++ b/apps/admin/src/shared/member-custom-fields/custom-field-picker.tsx @@ -0,0 +1,350 @@ +import CustomFieldIcon from '@/shared/member-custom-fields/custom-field-icon'; +import { + APIError, + HostLimitError, + JSONError, + ValidationError, + getErrorMessage, +} from '@tryghost/admin-x-framework/errors'; +import { + Button, + Combobox, + ComboboxContent, + ComboboxTrigger, + ComboboxValue, + Command, + CommandCheck, + CommandGroup, + CommandInput, + CommandItem, + CommandList, + Field, + FieldError, + FieldLabel, + Input, + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, + commandDefaultFilter, +} from '@tryghost/shade/components'; +import { CustomFieldTypeOption } from '@/shared/member-custom-fields/custom-field-type-option'; +import { LucideIcon } from '@tryghost/shade/utils'; +import { + type MemberCustomField, + useCreateMemberCustomField, +} from '@tryghost/admin-x-framework/api/member-custom-fields'; +import { useHandleError } from '@tryghost/admin-x-framework/hooks'; +import { useState } from 'react'; + +// cmdk scores an item's value and keywords as one joined string, and a match may run from +// one into the other. Identity here is the field key; the name is the keyword, and only the +// keyword is scored (the import's picker does the same, for the same reason). +function scoreByLabel(_value: string, query: string, keywords?: string[]): number { + return commandDefaultFilter((keywords ?? []).join(' '), query); +} + +export interface CustomFieldPickerProps { + id?: string; + /** Accessible name for the control; also labels the create form. */ + label: string; + /** The fields on offer. Callers filter to the types their context can accept. */ + fields: MemberCustomField[]; + /** Key of the selected field, or null for none. */ + value: string | null; + invalid?: boolean; + onChange: (key: string) => void; + /** + * The types a field created from this picker may have: one entry pins the type and the + * create form asks only for a name; several show a type choice among exactly these. + * Keep it within the types `fields` is filtered by — a creation outside that range + * would make a field this picker then refuses to list. Omit to hide the create action. + */ + createTypes?: MemberCustomField['type'][]; +} + +/** + * A single-select picker over custom fields, with creation in place. + * + * The shared control for any surface that maps something to a custom field (tier checkout + * collection today; anything in settings tomorrow). Options carry the field's type icon, + * and the list ends with the same green "+ Add custom field" the CSV import mapping uses — + * which also stands in for the empty state, since a search finding nothing is the strongest + * signal the field does not exist yet. + * + * Creation swaps the popover's content from the list to a small form, seeded from the + * search text. The same floating surface, so the page never grows or reflows under it — + * and the popover's focus handling carries over instead of being fought (the import's + * picker mounts its form outside and pays for it in focus juggling). The refusal handling + * follows the import mapping's: a name problem lands on the input, a ceiling or permission + * problem lands below the form and stops inviting retries. + */ +export function CustomFieldPicker({ + id, + label, + fields, + value, + invalid, + onChange, + createTypes, +}: CustomFieldPickerProps) { + const [open, setOpen] = useState(false); + const [creating, setCreating] = useState<{ initialName: string } | null>(null); + const [search, setSearch] = useState(''); + + const selected = fields.find((field) => field.key === value); + const canCreate = Boolean(createTypes?.length); + + // Programmatic closes bypass onOpenChange (the popover is controlled), so this owns the + // same cleanup: without it, the search text survives a selection and silently filters + // the reopened list — down to just the create action when nothing matches it. + const close = () => { + setOpen(false); + setCreating(null); + setSearch(''); + }; + + return ( + { + // Clicking away mid-create abandons it, same as Escape below. + if (next) { + setOpen(true); + } else { + close(); + } + }} + > + + + {selected ? ( + + + {selected.name} + + ) : ( + 'Select...' + )} + + + {/* No CommandEmpty is rendered on purpose: when a search matches nothing, the + forceMounted create action below is the answer, and an empty-state block would + just push it down (the CSV import's picker behaves the same way). */} + { + // Escape steps back: out of the form to the list, then out of the + // popover. Otherwise it would tear down both at once — and, worse, + // keep travelling to close whatever modal this sits in. + if (creating) { + event.preventDefault(); + setCreating(null); + } + }} + > + {creating && createTypes?.length ? ( + setCreating(null)} + onCreated={(key) => { + close(); + onChange(key); + }} + /> + ) : ( + + + + {/* Not rendered at all when there are no fields: an itemless + group still paints its padding, which stood as a blank band + between the search box and the create action. cmdk hides + the group by itself in the other empty case, a search that + filters every field out. */} + {fields.length > 0 && ( + + {fields.map((field) => ( + { + close(); + onChange(field.key); + }} + > + + {field.name} + {value === field.key && } + + ))} + + )} + {/* A row of the list rather than a footer, exactly as the CSV + import draws it: it scrolls with the fields, the arrow keys + reach it, and when a search matches nothing it is the one + item left — cmdk highlights it and Enter creates. Its own + group because forceMount on a group pins every item in it. */} + {canCreate && ( + + setCreating({ initialName: search.trim() })} + > + + Add custom field + + + )} + + + )} + + + ); +} + +/** + * The create form the popover swaps to. One offered type pins it and the form is name-only; + * several show a choice among exactly those. Refusal handling mirrors the import mapping. + */ +function CreateFieldInline({ + initialName, + label, + types, + onCancel, + onCreated, +}: { + initialName: string; + label: string; + types: MemberCustomField['type'][]; + onCancel: () => void; + onCreated: (key: string) => void; +}) { + const [name, setName] = useState(initialName); + const [typeId, setTypeId] = useState(types[0]); + const [nameError, setNameError] = useState(null); + const [saveError, setSaveError] = useState(null); + const [canRetry, setCanRetry] = useState(true); + const { mutateAsync: createField, isPending: isCreating } = useCreateMemberCustomField(); + const handleError = useHandleError(); + + const submit = async () => { + if (!name.trim()) { + setNameError('Enter a name for the field'); + return; + } + + try { + const response = await createField({ name: name.trim(), type: typeId }); + const field = response.members_custom_fields?.[0]; + if (!field) { + setSaveError('The field was created but could not be selected. Choose it from the list.'); + setCanRetry(false); + return; + } + onCreated(field.key); + } catch (error) { + const apiError = error instanceof JSONError ? error.data?.errors?.[0] : null; + if (error instanceof ValidationError && apiError?.property === 'name') { + setNameError(getErrorMessage(error, 'Invalid name')); + return; + } + // Shown inline below, so no toast — but still reported, the way the CSV import's + // picker does it, or unexpected failures on this path never reach error tracking. + if (!(error instanceof HostLimitError)) { + handleError(error, { withToast: false }); + } + setSaveError( + getErrorMessage( + error, + error instanceof APIError + ? error.message + : 'Could not create the custom field, please try again.', + ), + ); + setCanRetry(!(error instanceof HostLimitError) && apiError?.type !== 'NoPermissionError'); + } + }; + + return ( +
{ + if (event.key === 'Enter' && !isCreating && canRetry) { + event.preventDefault(); + void submit(); + } + }} + > + + + New custom field + + { + setName(event.target.value); + setNameError(null); + setSaveError(null); + setCanRetry(true); + }} + /> + {nameError && {nameError}} + + {/* Always shown so what is being created is never a surprise; with one offered + type it is disabled and pre-selected, the same way the field edit modal + states an unchangeable type. */} + + Type + + +
+ + +
+ {saveError && ( +

+ {saveError} +

+ )} +
+ ); +} diff --git a/apps/admin/test-utils/acceptance/tinybird.ts b/apps/admin/test-utils/acceptance/tinybird.ts index 821c96f1c4f..441b6db4395 100644 --- a/apps/admin/test-utils/acceptance/tinybird.ts +++ b/apps/admin/test-utils/acceptance/tinybird.ts @@ -47,19 +47,14 @@ export function webAnalyticsBootOverrides(): BootOverrides { }; } -let tokenSerial = 0; - /** * Serves the Admin API token request the Tinybird client makes before any - * pipe query. Each call mints a fresh token: the Tinybird client's SWR cache - * keys on the full pipe URL (token included) and outlives the render, so a - * repeated token would let one test's pipe responses satisfy the next test's - * queries without a request. + * pipe query. Pipe queries live on the render's own react-query client, so + * nothing outlives the test and a static token is fine. */ export function fakeTinybirdToken(): EndpointCapture { - tokenSerial += 1; return fakeAdminEndpoint('GET', '/tinybird/token/', { - tinybird: { token: `tinybird-test-token-${tokenSerial}` }, + tinybird: { token: 'tinybird-test-token' }, }); } diff --git a/apps/ember-admin/app/services/state-bridge.js b/apps/ember-admin/app/services/state-bridge.js index c7c8e94749c..8a57d0abb5b 100644 --- a/apps/ember-admin/app/services/state-bridge.js +++ b/apps/ember-admin/app/services/state-bridge.js @@ -22,6 +22,7 @@ const emberDataTypeMapping = { TagsResponseType: {type: 'tag'}, ThemesResponseType: {type: 'theme'}, TiersResponseType: {type: 'tier'}, + TiersCheckoutConfigResponseType: null, // tier checkout collection only exists in React admin UsersResponseType: {type: 'user'}, CustomThemeSettingsResponseType: null // invalidated by React theme activation; nothing to sync in Ember }; diff --git a/apps/shade/src/tokens.ts b/apps/shade/src/tokens.ts index aa10f1adf18..9099c40b178 100644 --- a/apps/shade/src/tokens.ts +++ b/apps/shade/src/tokens.ts @@ -10,7 +10,6 @@ export const SHADE_TOKEN_NAMES = [ 'animate-modal-in', 'animate-modal-in-from-right', 'animate-modal-in-reverse', - 'animate-setting-highlight-fade-out', 'animate-spin', 'animate-toaster-in', 'animate-toaster-out', @@ -257,33 +256,12 @@ export const SHADE_TOKEN_NAMES = [ 'ease-standard', 'focus-ring', 'font-body', - 'font-cardo', - 'font-chakra-petch', 'font-code', - 'font-fira-mono', - 'font-fira-sans', 'font-heading', - 'font-ibm-plex-serif', 'font-inherit', - 'font-inter', - 'font-jetbrains-mono', - 'font-lora', - 'font-manrope', - 'font-merriweather', 'font-mono', - 'font-noto-sans', - 'font-noto-serif', - 'font-nunito', - 'font-old-standard-tt', - 'font-poppins', - 'font-prata', - 'font-roboto', - 'font-rufina', 'font-sans', 'font-serif', - 'font-space-grotesk', - 'font-space-mono', - 'font-tenor-sans', 'foreground', 'input', 'input-group-radius', diff --git a/apps/shade/styles.css b/apps/shade/styles.css index 75857dc8197..0101f156183 100644 --- a/apps/shade/styles.css +++ b/apps/shade/styles.css @@ -1,25 +1,3 @@ -@import url(https://fonts.bunny.net/css?family=cardo:400,700); -@import url(https://fonts.bunny.net/css?family=manrope:300,500,700); -@import url(https://fonts.bunny.net/css?family=merriweather:300,700); -@import url(https://fonts.bunny.net/css?family=nunito:400,600,700); -@import url(https://fonts.bunny.net/css?family=old-standard-tt:400,700); -@import url(https://fonts.bunny.net/css?family=prata:400); -@import url(https://fonts.bunny.net/css?family=roboto:400,500,700); -@import url(https://fonts.bunny.net/css?family=rufina:400,500,700); -@import url(https://fonts.bunny.net/css?family=tenor-sans:400); -@import url(https://fonts.bunny.net/css?family=space-grotesk:700); -@import url(https://fonts.bunny.net/css?family=chakra-petch:400); -@import url(https://fonts.bunny.net/css?family=noto-sans:400,700); -@import url(https://fonts.bunny.net/css?family=poppins:400,700); -@import url(https://fonts.bunny.net/css?family=fira-sans:400,700); -@import url(https://fonts.bunny.net/css?family=inter:400,700); -@import url(https://fonts.bunny.net/css?family=noto-serif:400,700); -@import url(https://fonts.bunny.net/css?family=lora:400,700); -@import url(https://fonts.bunny.net/css?family=ibm-plex-serif:400,700); -@import url(https://fonts.bunny.net/css?family=space-mono:400,700); -@import url(https://fonts.bunny.net/css?family=fira-mono:400,700); -@import url(https://fonts.bunny.net/css?family=jetbrains-mono:400,700); - @import 'tailwindcss/theme.css'; @import './preflight.css'; @import 'tailwindcss/utilities.css'; @@ -76,15 +54,7 @@ } .dark .shade { - color: #fafafb; -} - -.dark .shade .gh-loading-orb-container { - background-color: #000000; -} - -.dark .shade .gh-loading-orb { - filter: invert(100%); + color: var(--foreground); } /* Suppress transitions/animations during a light/dark mode swap so the @@ -123,8 +93,3 @@ html.theme-switching *::after { .cm-tooltip-parent .cm-tooltip-autocomplete.cm-tooltip ul li:not([aria-selected]) { background: var(--background); } - -/* Prose classes are for formatting arbitrary HTML that comes from the API */ -.gh-prose-links a { - color: #30cf43; -} diff --git a/apps/shade/tailwind.theme.css b/apps/shade/tailwind.theme.css index c312fe3a4b0..710b77048c3 100644 --- a/apps/shade/tailwind.theme.css +++ b/apps/shade/tailwind.theme.css @@ -223,16 +223,6 @@ /* Typography */ /* ---------------------------------------------------------------------- */ - --font-cardo: Cardo; - --font-manrope: Manrope; - --font-merriweather: Merriweather; - --font-nunito: Nunito; - --font-tenor-sans: Tenor Sans; - --font-old-standard-tt: Old Standard TT; - --font-prata: Prata; - --font-roboto: Roboto; - --font-rufina: Rufina; - --font-inter: Inter; --font-body: var(--font-sans); --font-heading: var(--font-sans); --font-code: var(--font-mono); @@ -242,17 +232,6 @@ --font-serif: Georgia, serif; --font-mono: Consolas, Liberation Mono, Menlo, Courier, monospace; --font-inherit: inherit; - --font-space-grotesk: Space Grotesk; - --font-chakra-petch: Chakra Petch; - --font-noto-sans: Noto Sans; - --font-poppins: Poppins; - --font-fira-sans: Fira Sans; - --font-noto-serif: Noto Serif; - --font-lora: Lora; - --font-ibm-plex-serif: IBM Plex Serif; - --font-space-mono: Space Mono; - --font-fira-mono: Fira Mono; - --font-jetbrains-mono: JetBrains Mono; --tracking-tightest: -0.05em; --tracking-tighter: -0.025em; --tracking-tight: -0.01em; @@ -366,7 +345,6 @@ --animate-toaster-top-in: toasterTopIn 0.8s cubic-bezier(0.445, 0.05, 0.55, 0.95); --animate-fade-in: fadeIn 0.15s ease forwards; --animate-fade-out: fadeOut 0.15s ease forwards; - --animate-setting-highlight-fade-out: fadeOut 0.2s 1.4s ease forwards; --animate-modal-backdrop-in: fadeIn 0.15s ease forwards; --animate-modal-in: modalIn 0.25s ease forwards; --animate-modal-in-from-right: modalInFromRight 0.25s ease forwards; diff --git a/apps/shade/theme-variables.css b/apps/shade/theme-variables.css index 495169fdf72..52e3d5ebe81 100644 --- a/apps/shade/theme-variables.css +++ b/apps/shade/theme-variables.css @@ -85,7 +85,6 @@ --control-disabled-surface: var(--color-gray-100); --control-border: color-mix(in oklab, var(--color-gray-200), var(--color-gray-300)); --table-row-hover: var(--color-gray-50); - --members-sticky-hover-bg: var(--table-row-hover); --mobile-navbar-height: 64px; } @@ -126,7 +125,6 @@ --control-disabled-surface: var(--color-gray-950); --control-border: color-mix(in oklab, var(--color-gray-900) 70%, transparent); --table-row-hover: var(--color-sidebar-bg); - --members-sticky-hover-bg: var(--table-row-hover); --background: oklch(0.178 0.003 271); --foreground: var(--color-gray-200); diff --git a/e2e/package.json b/e2e/package.json index a98808a48aa..c2cb54b9033 100644 --- a/e2e/package.json +++ b/e2e/package.json @@ -36,6 +36,7 @@ "@eslint/js": "catalog:", "@faker-js/faker": "catalog:", "@playwright/test": "catalog:", + "@tryghost/checkout": "workspace:*", "@tryghost/custom-field-types": "workspace:*", "@tryghost/debug": "catalog:", "@tryghost/logging": "catalog:", diff --git a/e2e/scripts/probe-stripe-constraints.ts b/e2e/scripts/probe-stripe-constraints.ts index 344982937cd..56923bd18ee 100644 --- a/e2e/scripts/probe-stripe-constraints.ts +++ b/e2e/scripts/probe-stripe-constraints.ts @@ -1,14 +1,22 @@ import { asSessionCreateParams, provision, stripeClient } from './provision-stripe-environment.ts'; import { readFileSync } from 'node:fs'; +import type Stripe from 'stripe'; /** * Measures the checkout constraints the fake Stripe server enforces. * * The limits in `fake-stripe-server.ts` cannot be taken from Stripe's docs or its * published OpenAPI spec, because both disagree with the API: the spec carries no - * `maxItems` on `custom_fields`, states the `customer_update` rule only in prose, - * and marks `allowed_countries` required when the API does not. This script is how - * those limits were established, and how to re-establish them when Stripe moves one. + * `maxItems` on `custom_fields`, and states the `customer_update` rule only in prose. + * This script is how those limits were established, and how to re-establish them when + * Stripe moves one. + * + * Read what a probe reports, not whether it was accepted. Stripe's form encoding drops an + * empty object, so a parameter can disappear on the way out and the session is created + * without it — a success that collects nothing. This once produced the wrong conclusion + * here, that the API treats `allowed_countries` as optional where the spec marks it + * required. It does not: the list is the only key `shipping_address_collection` has, so + * leaving it out removes the whole parameter. * * Run: STRIPE_SECRET_KEY=sk_test_... pnpm --filter @tryghost/e2e stripe:probe */ @@ -42,10 +50,34 @@ const field = (overrides: Record = {}) => ({ ...overrides, }); +/** What the created session will actually ask a buyer for, which is the thing to read. */ +function collects(session: Stripe.Checkout.Session): string { + const asks: string[] = []; + const countries = session.shipping_address_collection?.allowed_countries; + if (countries) { + asks.push(`shipping_address (${countries.length} countries)`); + } + if (session.phone_number_collection?.enabled) { + asks.push('phone_number'); + } + if (session.tax_id_collection?.enabled) { + asks.push('tax_id'); + } + // Read through a cast: the API returns `custom_fields` on a session, but the types for + // the pinned version predate it and do not declare it. The probe exists to measure the + // API rather than the types, so the API is what it reads. + const questions = (session as { custom_fields?: unknown[] }).custom_fields; + if (questions?.length) { + asks.push(`custom_fields (${questions.length})`); + } + return asks.length ? asks.join(', ') : 'nothing'; +} + async function probe(name: string, params: Record): Promise { try { - await stripe.checkout.sessions.create(asSessionCreateParams(params)); + const session = await stripe.checkout.sessions.create(asSessionCreateParams(params)); log(` ACCEPTED ${name}`); + log(` collects ${collects(session)}`); } catch (error) { log(` REJECTED ${name}`); log(` ${(error as Error).message}`); @@ -92,10 +124,18 @@ async function main(): Promise { ...base, customer_update: { address: 'auto' }, }); + // Whether a session can ask for an address without naming countries — the shape an + // "everywhere" sentinel would need. Both of these are accepted and both collect nothing, + // because the empty object never reaches Stripe. There is no sentinel; a caller that + // means everywhere has to enumerate every country. await probe('shipping_address_collection without allowed_countries', { ...base, shipping_address_collection: {}, }); + await probe('shipping_address_collection with an empty allowed_countries', { + ...base, + shipping_address_collection: { allowed_countries: [] }, + }); // Whether the SDK's own `AllowedCountry` union can be trusted as the list Ghost enforces // when a publisher saves. The union is a published artefact, and published artefacts have diff --git a/e2e/tests/admin/members/import-custom-fields.test.ts b/e2e/tests/admin/members/import-custom-fields.test.ts index dc08a57dff3..5485d9efd23 100644 --- a/e2e/tests/admin/members/import-custom-fields.test.ts +++ b/e2e/tests/admin/members/import-custom-fields.test.ts @@ -18,12 +18,13 @@ import { usePerTestIsolation } from '@/helpers/playwright/isolation'; * two things only the browser exercises -- the export -> import loop end to end, and the * mapping step both auto-detecting an exported column and taking a hand-picked target. * - * Behind membersCustomFields, which gates the whole feature. + * Behind two flags: membersImportRedesign serves the mapping step this drives, and + * membersCustomFields is what puts custom fields into it. */ usePerTestIsolation(); test.describe('Ghost Admin - Members import with custom fields', () => { - test.use({ labs: { membersCustomFields: true } }); + test.use({ labs: { membersImportRedesign: true, membersCustomFields: true } }); test('an exported custom field value round-trips back through import, auto-mapped', async ({ page, diff --git a/ghost/core/core/server/services/automations/automations-repository.ts b/ghost/core/core/server/services/automations/automations-repository.ts index 422f7ba6b60..7a2dc20df22 100644 --- a/ghost/core/core/server/services/automations/automations-repository.ts +++ b/ghost/core/core/server/services/automations/automations-repository.ts @@ -66,7 +66,7 @@ export interface AutomationSummary { } export interface AutomationBrowseResult extends AutomationSummary { - stats: { + stats?: { last_run_created_at: Date | null; total_run_count: number; in_progress_run_count: number; diff --git a/ghost/core/core/server/services/content-import/import/importer.ts b/ghost/core/core/server/services/content-import/import/importer.ts index d870b854123..12c732d9b78 100644 --- a/ghost/core/core/server/services/content-import/import/importer.ts +++ b/ghost/core/core/server/services/content-import/import/importer.ts @@ -11,6 +11,7 @@ import type { PostsRepository, WrittenPost } from './post-repository'; import type { ImportRequest } from './schema'; import type { Clock, ImportRunStore, RowOutcome } from './store'; import type { PreparedImportSource } from './source'; +import { MediaInliningFailure, type PostMediaInlining } from './media'; export type { ImportRequest } from './schema'; @@ -60,6 +61,7 @@ interface ImporterDeps { getHtmlToLexical: () => HtmlToLexical; getMarkdownToHtml: () => MarkdownToHtml; getCleanHTML: () => CleanHTML; + createMediaInliner: () => PostMediaInlining; addJob: (job: { job: () => Promise; offloaded: boolean; name: string }) => void; report: FailureReporter; store: ImportRunStore; @@ -83,6 +85,7 @@ class ContentCSVImporter { private _getHtmlToLexical: () => HtmlToLexical; private _getMarkdownToHtml: () => MarkdownToHtml; private _getCleanHTML: () => CleanHTML; + private _createMediaInliner: () => PostMediaInlining; private _addJob: ImporterDeps['addJob']; private _report: FailureReporter; private _store: ImportRunStore; @@ -98,6 +101,7 @@ class ContentCSVImporter { getHtmlToLexical, getMarkdownToHtml, getCleanHTML, + createMediaInliner, addJob, report, store, @@ -112,6 +116,7 @@ class ContentCSVImporter { this._getHtmlToLexical = getHtmlToLexical; this._getMarkdownToHtml = getMarkdownToHtml; this._getCleanHTML = getCleanHTML; + this._createMediaInliner = createMediaInliner; this._addJob = addJob; this._report = report; this._store = store; @@ -186,9 +191,10 @@ class ContentCSVImporter { const htmlToLexical = this._getHtmlToLexical(); const markdownToHtml = this._getMarkdownToHtml(); const cleanHTML = this._getCleanHTML(); + const media = this._createMediaInliner(); let successfulWrites = 0; - let failedWrites = 0; - let firstWriteFailure: unknown; + let failedRows = 0; + let firstRowFailure: unknown; for (const [index, row] of rows.entries()) { const line = index + 2; @@ -212,6 +218,27 @@ class ContentCSVImporter { throw error; } + try { + await media.inline(data); + } catch (error) { + if (error instanceof MediaInliningFailure) { + if (failedRows === 0) { + firstRowFailure = error; + } + failedRows += 1; + this._store.record(runId, { + line, + title: row.title, + status: 'failed', + reason: messageOf(error), + mediaFailures: error.failures, + }); + continue; + } + + throw error; + } + let post: WrittenPost; let writeStatus: 'created' | 'updated'; let warnings: string[]; @@ -251,10 +278,10 @@ class ContentCSVImporter { warnings = result.warnings; successfulWrites += 1; } catch (error) { - if (failedWrites === 0) { - firstWriteFailure = error; + if (failedRows === 0) { + firstRowFailure = error; } - failedWrites += 1; + failedRows += 1; this._store.record(runId, { line, title: row.title, @@ -282,14 +309,14 @@ class ContentCSVImporter { this._store.record(runId, outcome); } - if (failedWrites > 0 && successfulWrites === 0) { + if (failedRows > 0 && successfulWrites === 0) { this._report( new errors.InternalServerError({ message: tpl(messages.allWritesFailed, { - count: failedWrites, - postNoun: failedWrites === 1 ? 'post' : 'posts', + count: failedRows, + postNoun: failedRows === 1 ? 'post' : 'posts', }), - err: firstWriteFailure, + err: firstRowFailure, }), ); } diff --git a/ghost/core/core/server/services/content-import/import/local-media-url.ts b/ghost/core/core/server/services/content-import/import/local-media-url.ts new file mode 100644 index 00000000000..becb1f031a0 --- /dev/null +++ b/ghost/core/core/server/services/content-import/import/local-media-url.ts @@ -0,0 +1,85 @@ +export interface LocalMediaUrlOptions { + siteUrl: string; + subdir: string; + assetBaseUrls: Array; +} + +const CONTENT_PATH_PREFIXES = ['/content/images', '/content/media', '/content/files']; + +function normalizedPathPrefix(value: string): string { + if (!value.startsWith('/')) { + return `/${value}`.replace(/\/+$/, ''); + } + + return value.replace(/\/+$/, ''); +} + +function pathHasPrefix(pathname: string, prefix: string): boolean { + return pathname === prefix || pathname.startsWith(`${prefix}/`); +} + +function isContentPath(pathname: string, subdirs: string[]): boolean { + const normalizedPathname = normalizedPathPrefix(pathname); + const prefixes = subdirs.flatMap((subdir) => { + const normalizedSubdir = normalizedPathPrefix(subdir); + return CONTENT_PATH_PREFIXES.map((prefix) => `${normalizedSubdir}${prefix}`); + }); + + return [...CONTENT_PATH_PREFIXES, ...prefixes].some((prefix) => + pathHasPrefix(normalizedPathname, prefix), + ); +} + +function parseUrl(value: string): URL | undefined { + try { + return new URL(value.startsWith('//') ? `https:${value}` : value); + } catch { + return undefined; + } +} + +function matchesBaseUrl(source: URL, baseUrl: string): boolean { + const base = parseUrl(baseUrl.trim()); + if (!base || source.host !== base.host) { + return false; + } + + const basePath = normalizedPathPrefix(base.pathname); + return basePath === '' || basePath === '/' || pathHasPrefix(source.pathname, basePath); +} + +/** + * Returns whether a media reference already belongs to this Ghost site or one + * of its configured asset hosts. This only classifies URLs; it never checks + * whether the referenced file exists. + */ +export function isLocalMediaUrl(sourceUrl: string, options: LocalMediaUrlOptions): boolean { + const value = sourceUrl.trim(); + if (value.startsWith('__GHOST_URL__')) { + return true; + } + + if (value.startsWith('/') && !value.startsWith('//')) { + return isContentPath(value, [options.subdir]); + } + + const source = parseUrl(value); + if (!source) { + return false; + } + + if ( + options.assetBaseUrls.some( + (baseUrl) => typeof baseUrl === 'string' && matchesBaseUrl(source, baseUrl), + ) + ) { + return true; + } + + const site = parseUrl(options.siteUrl); + if (!site || source.host !== site.host) { + return false; + } + + return isContentPath(source.pathname, [options.subdir, site.pathname]); +} diff --git a/ghost/core/core/server/services/content-import/import/media.ts b/ghost/core/core/server/services/content-import/import/media.ts new file mode 100644 index 00000000000..5ce70f2cf46 --- /dev/null +++ b/ghost/core/core/server/services/content-import/import/media.ts @@ -0,0 +1,329 @@ +import type { PostData } from './post-data'; +import type { ExternalMediaImporter, ExternalMediaImportResult } from '../../media-inliner/types'; + +const cheerio = require('cheerio'); + +export interface MediaFailure { + sourceUrl: string; + reason: string; +} + +export class MediaInliningFailure extends Error { + readonly failures: MediaFailure[]; + + constructor(failures: MediaFailure[]) { + const noun = failures.length === 1 ? 'file' : 'files'; + super(`Could not import ${failures.length} media ${noun}.`); + this.name = 'MediaInliningFailure'; + this.failures = failures; + } +} + +type JSONRecord = Record; + +const DIRECT_MEDIA_FIELDS: Record = { + image: ['src'], + audio: ['src', 'thumbnailSrc'], + video: ['src', 'thumbnailSrc', 'customThumbnailSrc'], + file: ['src'], + product: ['productImageSrc'], + header: ['backgroundImageSrc'], + signup: ['backgroundImageSrc'], + 'call-to-action': ['imageUrl'], + bookmark: ['metadata.icon', 'metadata.thumbnail'], + embed: ['metadata.thumbnail_url'], + 'before-after': ['beforeImage.src', 'afterImage.src'], +}; + +const HTML_MEDIA_FIELDS: Record = { + html: ['html'], + email: ['html'], + 'email-cta': ['html'], + toggle: ['heading', 'content'], + image: ['caption'], + gallery: ['caption'], + video: ['caption'], + product: ['productTitle', 'productDescription'], + header: ['header', 'subheader'], + bookmark: ['caption'], + codeblock: ['caption'], +}; + +const MARKDOWN_MEDIA_FIELDS: Record = { + markdown: ['markdown'], +}; + +const BLOCKED_MEDIA_DOMAINS = ['images.unsplash.com', 'gravatar.com']; + +function isRecord(value: unknown): value is JSONRecord { + return Boolean(value) && typeof value === 'object' && !Array.isArray(value); +} + +function isRemoteMediaUrl(value: string): boolean { + return /^(?:https?:)?\/\//i.test(value.trim()); +} + +export function isBlockedMediaUrl(value: string): boolean { + try { + const url = new URL(value.trim().replace(/^\/\//, 'https://')); + return BLOCKED_MEDIA_DOMAINS.some( + (domain) => url.hostname === domain || url.hostname.endsWith(`.${domain}`), + ); + } catch { + return false; + } +} + +function getPath(record: JSONRecord, path: string): unknown { + return path.split('.').reduce((value, part) => { + return isRecord(value) ? value[part] : undefined; + }, record); +} + +function setPath(record: JSONRecord, path: string, value: string): void { + const parts = path.split('.'); + const leaf = parts.pop() as string; + let target = record; + + for (const part of parts) { + target = target[part] as JSONRecord; + } + + target[leaf] = value; +} + +async function replaceAsync( + input: string, + pattern: RegExp, + replacer: (match: RegExpMatchArray) => Promise, +): Promise { + let result = ''; + let lastIndex = 0; + + for (const match of input.matchAll(pattern)) { + const index = match.index as number; + result += input.slice(lastIndex, index); + result += await replacer(match); + lastIndex = index + match[0].length; + } + + return result + input.slice(lastIndex); +} + +export interface PostMediaInlining { + inline(data: PostData): Promise; +} + +export class PostMediaInliner implements PostMediaInlining { + private _media: ExternalMediaImporter; + private _isLocalMediaUrl: (sourceUrl: string) => boolean; + private _cache = new Map>(); + private _failures = new Map(); + + constructor({ + media, + isLocalMediaUrl, + }: { + media: ExternalMediaImporter; + isLocalMediaUrl: (sourceUrl: string) => boolean; + }) { + this._media = media; + this._isLocalMediaUrl = isLocalMediaUrl; + } + + async inline(data: PostData): Promise { + this._failures = new Map(); + + if (data.feature_image) { + data.feature_image = await this.inlineUrl(data.feature_image); + } + if (data.posts_meta?.og_image) { + data.posts_meta.og_image = await this.inlineUrl(data.posts_meta.og_image); + } + if (data.posts_meta?.twitter_image) { + data.posts_meta.twitter_image = await this.inlineUrl(data.posts_meta.twitter_image); + } + if (data.lexical) { + data.lexical = await this.inlineLexical(data.lexical); + } + + if (this._failures.size > 0) { + throw new MediaInliningFailure([...this._failures.values()]); + } + } + + private async inlineUrl(sourceUrl: string): Promise { + if ( + !isRemoteMediaUrl(sourceUrl) || + isBlockedMediaUrl(sourceUrl) || + this._isLocalMediaUrl(sourceUrl) + ) { + return sourceUrl; + } + + let resultPromise = this._cache.get(sourceUrl); + if (!resultPromise) { + resultPromise = this._media.importUrl(sourceUrl); + this._cache.set(sourceUrl, resultPromise); + } + + const result = await resultPromise; + if (result.status === 'failed') { + this.recordFailure(sourceUrl, result.reason); + return sourceUrl; + } + + return result.storedUrl; + } + + private recordFailure(sourceUrl: string, reason: string): void { + if (!this._failures.has(sourceUrl)) { + this._failures.set(sourceUrl, { sourceUrl, reason }); + } + } + + private async inlineLexical(serializedLexical: string): Promise { + const lexical: unknown = JSON.parse(serializedLexical); + if (!isRecord(lexical) || !isRecord(lexical.root) || !Array.isArray(lexical.root.children)) { + return serializedLexical; + } + + await this.inlineLexicalChildren(lexical.root.children); + return JSON.stringify(lexical); + } + + private async inlineLexicalChildren(children: unknown[]): Promise { + for (const child of children) { + if (!isRecord(child)) { + continue; + } + + const type = typeof child.type === 'string' ? child.type : ''; + for (const path of DIRECT_MEDIA_FIELDS[type] ?? []) { + const value = getPath(child, path); + if (typeof value === 'string' && value) { + setPath(child, path, await this.inlineUrl(value)); + } + } + + if (type === 'gallery' && Array.isArray(child.images)) { + for (const image of child.images) { + if (isRecord(image) && typeof image.src === 'string' && image.src) { + image.src = await this.inlineUrl(image.src); + } + } + } + + for (const path of HTML_MEDIA_FIELDS[type] ?? []) { + const value = getPath(child, path); + if (typeof value === 'string' && value) { + setPath(child, path, await this.inlineHtml(value)); + } + } + + for (const path of MARKDOWN_MEDIA_FIELDS[type] ?? []) { + const value = getPath(child, path); + if (typeof value === 'string' && value) { + setPath(child, path, await this.inlineMarkdown(value)); + } + } + + if (Array.isArray(child.children)) { + await this.inlineLexicalChildren(child.children); + } + } + } + + private async inlineHtml(html: string): Promise { + const $ = cheerio.load(html, { decodeEntities: false }, false); + const attributes: Array<[string, string]> = [ + ['img[src]', 'src'], + ['img[data-src]', 'data-src'], + ['video[src]', 'src'], + ['video[poster]', 'poster'], + ['audio[src]', 'src'], + ['source[src]', 'src'], + ]; + + for (const [selector, attribute] of attributes) { + for (const element of $(selector).toArray()) { + const value = $(element).attr(attribute); + if (value) { + $(element).attr(attribute, await this.inlineUrl(value)); + } + } + } + + for (const element of $('[srcset]').toArray()) { + const value = $(element).attr('srcset'); + if (value) { + $(element).attr('srcset', await this.inlineSrcset(value)); + } + } + + for (const element of $('[style]').toArray()) { + const value = $(element).attr('style'); + if (value) { + $(element).attr('style', await this.inlineCss(value)); + } + } + + for (const element of $('style').toArray()) { + const value = $(element).html(); + if (value) { + $(element).html(await this.inlineCss(value)); + } + } + + return $.root().html() as string; + } + + private async inlineSrcset(srcset: string): Promise { + if (srcset.trimStart().startsWith('data:')) { + return srcset; + } + + const candidates = srcset.split(','); + const inlined: string[] = []; + for (const candidate of candidates) { + const match = candidate.match(/^(\s*)(\S+)(.*)$/s); + if (!match) { + inlined.push(candidate); + continue; + } + const [, leading, sourceUrl, descriptor] = match; + inlined.push(`${leading}${await this.inlineUrl(sourceUrl)}${descriptor}`); + } + return inlined.join(','); + } + + private inlineCss(css: string): Promise { + return replaceAsync(css, /url\(\s*(?:(["'])(.*?)\1|([^)'"\s][^)]*?))\s*\)/gi, async (match) => { + const sourceUrl = ((match[2] ?? match[3]) as string).trim(); + if (!sourceUrl) { + return match[0]; + } + const storedUrl = await this.inlineUrl(sourceUrl); + return match[0].replace(sourceUrl, storedUrl); + }); + } + + private async inlineMarkdown(markdown: string): Promise { + let result = await replaceAsync( + markdown, + /!\[[^\]]*]\(\s*(?:<([^>\s]+)>|((?:https?:)?\/\/[^\s)]+))(?=[\s)])/gi, + async (match) => { + const sourceUrl = (match[1] ?? match[2]) as string; + return match[0].replace(sourceUrl, await this.inlineUrl(sourceUrl)); + }, + ); + + result = await replaceAsync(result, /<(?:img|video|audio|source)\b[^>]*>/gi, async (match) => + this.inlineHtml(match[0]), + ); + + return replaceAsync(result, /]*>[\s\S]*?<\/style>/gi, async (match) => + this.inlineHtml(match[0]), + ); + } +} diff --git a/ghost/core/core/server/services/content-import/import/store.ts b/ghost/core/core/server/services/content-import/import/store.ts index a3c5c8a4318..cd90b838ec7 100644 --- a/ghost/core/core/server/services/content-import/import/store.ts +++ b/ghost/core/core/server/services/content-import/import/store.ts @@ -3,7 +3,7 @@ // replaces this. // skipped = the row was never attempted (the publisher can fix the file); -// failed = the write was attempted and lost. +// failed = row processing was attempted but did not produce a post. export type Clock = () => Date; export type RowStatus = 'created' | 'updated' | 'skipped' | 'failed'; @@ -15,6 +15,7 @@ export interface RowOutcome { title: string | null; status: RowStatus; reason?: string; + mediaFailures?: Array<{ sourceUrl: string; reason: string }>; warnings?: string[]; postId?: string; url?: string; diff --git a/ghost/core/core/server/services/content-import/index.ts b/ghost/core/core/server/services/content-import/index.ts index 5ef2ed4ce6a..76d14d09aa2 100644 --- a/ghost/core/core/server/services/content-import/index.ts +++ b/ghost/core/core/server/services/content-import/index.ts @@ -5,6 +5,8 @@ import readPostRows from './import/reader'; import { importRequestSchema, type ImportRequest } from './import/schema'; import { ImportRunStore } from './import/store'; import { prepareImportSource } from './import/source'; +import { PostMediaInliner } from './import/media'; +import { isLocalMediaUrl } from './import/local-media-url'; // The request is built from HTTP upload metadata, so it is validated at the // service boundary rather than trusted. @@ -25,6 +27,8 @@ function makeImporter(): ContentCSVImporter { const jobsService = require('../jobs'); const settingsCache = require('../../../shared/settings-cache'); const urlService = require('../url'); + const mediaInlinerService = require('../media-inliner'); + const config = require('../../../shared/config'); const ObjectID = require('bson-objectid').default; // Inline jobs never reach the job manager's Sentry handler, which is wired to the @@ -48,6 +52,20 @@ function makeImporter(): ContentCSVImporter { getHtmlToLexical: () => lexicalLib.htmlToLexicalConverter, getMarkdownToHtml: () => require('@tryghost/kg-markdown-html-renderer').render, getCleanHTML: () => require('@tryghost/mg-clean-html').cleanHTML, + createMediaInliner: () => + new PostMediaInliner({ + media: mediaInlinerService.getInstance(), + isLocalMediaUrl: (sourceUrl) => + isLocalMediaUrl(sourceUrl, { + siteUrl: config.getSiteUrl(), + subdir: config.getSubdir(), + assetBaseUrls: [ + config.get('urls:image'), + config.get('urls:media'), + config.get('urls:files'), + ], + }), + }), addJob: jobsService.addJob.bind(jobsService), report, store: new ImportRunStore(), diff --git a/ghost/core/core/server/services/stripe/services/checkout/completed-session.ts b/ghost/core/core/server/services/stripe/services/checkout/completed-session.ts index ed076fd5eb1..da9c6e6e746 100644 --- a/ghost/core/core/server/services/stripe/services/checkout/completed-session.ts +++ b/ghost/core/core/server/services/stripe/services/checkout/completed-session.ts @@ -1,5 +1,5 @@ import { z } from 'zod'; -import { STRIPE_PORTS, type StripePort } from './field-ports'; +import { STRIPE_PORTS, type StripePort } from '@tryghost/checkout'; /** * What Ghost reads off a completed session. diff --git a/ghost/core/core/server/services/stripe/services/checkout/session-options.ts b/ghost/core/core/server/services/stripe/services/checkout/session-options.ts index 775fd506906..22e7d68aa53 100644 --- a/ghost/core/core/server/services/stripe/services/checkout/session-options.ts +++ b/ghost/core/core/server/services/stripe/services/checkout/session-options.ts @@ -2,9 +2,10 @@ import logging from '@tryghost/logging'; import { MAX_CHECKOUT_CUSTOM_FIELDS, MAX_CHECKOUT_LABEL_LENGTH, + STRIPE_ALLOWED_COUNTRIES, isCheckoutEligible, type CheckoutEligibleFieldType, -} from './field-ports'; +} from '@tryghost/checkout'; import type { ResolvedCheckout, ResolvedQuestion } from '../../../tier-checkout-config'; /** @@ -118,15 +119,20 @@ export function stripeCheckoutCollectionOptions( } if (checkout.shipping) { - // The country list is what makes this reach Stripe at all. An empty - // `shipping_address_collection` form-encodes to nothing, so a request built that - // way carries no parameter and Stripe accepts it precisely because it was never - // asked to collect anything — which reads as success and collects no addresses. + // Ghost stores "everywhere" as no list, because the set of countries moves and a + // stored copy of it would quietly become a restriction. Stripe has no such sentinel: + // `allowed_countries` is the only key `shipping_address_collection` has, so a request + // that omits it carries no parameter at all, and Stripe accepts it precisely because + // it was never asked to collect anything — a session that succeeds and collects no + // address. So everywhere is expanded to every country here, at the one point that + // builds the request. // - // Defended again here rather than trusted from the settings screen: this is the - // checkout path, and a malformed configuration must cost the collection rather than - // throw inside a session build. - if (checkout.shipping.allowedCountries.length === 0) { + // An empty list is neither everywhere nor a real restriction, and would encode to the + // same absent parameter. Defended here rather than trusted from the settings screen: + // this is the checkout path, and a malformed configuration must cost the collection + // rather than throw inside a session build. + const allowedCountries = checkout.shipping.allowedCountries ?? [...STRIPE_ALLOWED_COUNTRIES]; + if (allowedCountries.length === 0) { logging.warn( { event: { name: 'stripe.checkout.collection_skipped' }, @@ -136,9 +142,7 @@ export function stripeCheckoutCollectionOptions( 'Skipping a Stripe checkout collection', ); } else { - options.shipping_address_collection = { - allowed_countries: checkout.shipping.allowedCountries, - }; + options.shipping_address_collection = { allowed_countries: allowedCountries }; } } diff --git a/ghost/core/core/server/services/tier-checkout-config/codec.ts b/ghost/core/core/server/services/tier-checkout-config/codec.ts index 2113c4b4cec..67e4462f608 100644 --- a/ghost/core/core/server/services/tier-checkout-config/codec.ts +++ b/ghost/core/core/server/services/tier-checkout-config/codec.ts @@ -7,20 +7,26 @@ import { CheckoutOptions } from './models'; const SEPARATOR = ','; export const optionsCodec = z.codec(DbCheckoutOptions, CheckoutOptions, { + // An absent list is everywhere, so the column is null rather than a copy of every + // country. An empty list is neither, and stays representable on purpose: nothing writes + // one, and the session builder refuses to collect against it rather than asking the + // processor for an address form it would never render. decode: (columns) => ({ - shippingAllowedCountries: splitList(columns.shipping_allowed_countries), + shippingAllowedCountries: + columns.shipping_allowed_countries === null + ? null + : splitList(columns.shipping_allowed_countries), taxNumber: columns.tax_number_collect, }), encode: (options) => ({ - shipping_allowed_countries: options.shippingAllowedCountries.length - ? joinList(options.shippingAllowedCountries) - : null, + shipping_allowed_countries: + options.shippingAllowedCountries === null ? null : joinList(options.shippingAllowedCountries), tax_number_collect: options.taxNumber, }), }); -function splitList(stored: string | null): string[] { - return stored ? stored.split(SEPARATOR).filter(Boolean) : []; +function splitList(stored: string): string[] { + return stored.split(SEPARATOR).filter(Boolean); } function joinList(values: string[]): string { diff --git a/ghost/core/core/server/services/tier-checkout-config/models.ts b/ghost/core/core/server/services/tier-checkout-config/models.ts index 22248675904..5eb3adaa6a0 100644 --- a/ghost/core/core/server/services/tier-checkout-config/models.ts +++ b/ghost/core/core/server/services/tier-checkout-config/models.ts @@ -13,8 +13,14 @@ export type CheckoutQuestion = z.infer; * 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()), + /** + * ISO 3166-1 alpha-2, or null for everywhere the processor ships. + * + * Null rather than a stored enumeration of every country, because that list moves: the + * day the processor adds one, a saved "everywhere" would silently be a restriction that + * excludes it, and nothing would say so. + */ + allowedCountries: z.array(z.string()).nullable(), nameCustomFieldKey: z.string(), addressCustomFieldKey: z.string(), }); @@ -51,7 +57,7 @@ export const emptyCheckoutConfig = (tierId: string): TierCheckoutConfig => ({ }); export const CheckoutOptions = z.object({ - shippingAllowedCountries: z.array(z.string()), + shippingAllowedCountries: z.array(z.string()).nullable(), taxNumber: z.boolean(), }); export type CheckoutOptions = 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 index 521a5f82855..d7e0e065c89 100644 --- a/ghost/core/core/server/services/tier-checkout-config/queries.ts +++ b/ghost/core/core/server/services/tier-checkout-config/queries.ts @@ -1,6 +1,6 @@ import type { Knex } from 'knex'; import { FIELD_STATUS } from '../members-custom-fields/schema'; -import { STRIPE_PORT } from '../stripe/services/checkout/field-ports'; +import { STRIPE_PORT } from '@tryghost/checkout'; import { DbCheckoutOptions } from './schema'; import type { CollectionRow, QuestionRow } from './codec'; diff --git a/ghost/core/core/server/services/tier-checkout-config/serializers.ts b/ghost/core/core/server/services/tier-checkout-config/serializers.ts index b958ffc147e..330cfa3c1b1 100644 --- a/ghost/core/core/server/services/tier-checkout-config/serializers.ts +++ b/ghost/core/core/server/services/tier-checkout-config/serializers.ts @@ -1,14 +1,14 @@ import { z } from 'zod'; -import { MAX_CHECKOUT_CUSTOM_FIELDS } from '../stripe/services/checkout/field-ports'; import { + MAX_CHECKOUT_CUSTOM_FIELDS, STRIPE_ALLOWED_COUNTRIES, isStripeAllowedCountry, -} from '../stripe/services/checkout/allowed-countries'; +} from '@tryghost/checkout'; import { TierCheckoutConfig } from './models'; // Every country Stripe will take, sent at once, was measured as accepted — so the only // ceiling is the list itself, and a request naming more than there are countries is naming -// something twice. +// something twice. A request that means all of them omits the list instead. const MAX_ALLOWED_COUNTRIES = STRIPE_ALLOWED_COUNTRIES.length; const QuestionInput = z.object({ @@ -60,10 +60,14 @@ export const CheckoutConfigInput = z.strictObject({ z.strictObject({ collect: z.literal(false) }), z.strictObject({ collect: z.literal(true), + // Absent means everywhere the processor ships. Empty is refused rather than + // read as everywhere: a publisher who cleared the list said something, and it + // was not "deliver worldwide". 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), + .max(MAX_ALLOWED_COUNTRIES) + .optional(), name: Destination, address: Destination, }), @@ -94,7 +98,8 @@ const CollectionResource = z.object({ const ShippingResource = z.object({ collect: z.literal(true), - allowed_countries: z.array(z.string()), + /** Absent means everywhere, the same way it does on the way in. */ + allowed_countries: z.array(z.string()).optional(), name: z.object({ custom_field_key: z.string() }), address: z.object({ custom_field_key: z.string() }), }); @@ -124,7 +129,9 @@ export const toCheckoutConfigResponse = z ? { shipping: { collect: true as const, - allowed_countries: config.shipping.allowedCountries, + ...(config.shipping.allowedCountries + ? { allowed_countries: config.shipping.allowedCountries } + : {}), name: { custom_field_key: config.shipping.nameCustomFieldKey }, address: { custom_field_key: config.shipping.addressCustomFieldKey }, }, diff --git a/ghost/core/core/server/services/tier-checkout-config/service.ts b/ghost/core/core/server/services/tier-checkout-config/service.ts index 45eaea32754..1417a23530b 100644 --- a/ghost/core/core/server/services/tier-checkout-config/service.ts +++ b/ghost/core/core/server/services/tier-checkout-config/service.ts @@ -7,11 +7,12 @@ import { DbCustomField, FIELD_STATUS } from '../members-custom-fields/schema'; import type { CustomField, RequestContext } from '../members-custom-fields'; import { MAX_CHECKOUT_LABEL_LENGTH, + PORT_FIELD, STRIPE_PORT, isCheckoutEligible, isStripePort, type StripePort, -} from '../stripe/services/checkout/field-ports'; +} from '@tryghost/checkout'; import { collectionRowCodec, optionsCodec, @@ -35,7 +36,6 @@ import { type ResolvedQuestion, type TierCheckoutConfig, } from './models'; -import { PORT_FIELD } from './destinations'; import { CheckoutConfigInput } from './serializers'; type FieldRow = Pick, 'key' | 'name' | 'type' | 'status'>; @@ -343,7 +343,9 @@ async function writeOptions( now: Date, ): Promise { const all = z.encode(optionsCodec, { - shippingAllowedCountries: stated.shipping?.collect ? stated.shipping.allowed_countries : [], + shippingAllowedCountries: stated.shipping?.collect + ? (stated.shipping.allowed_countries ?? null) + : null, taxNumber: stated.tax_number?.collect ?? false, }); diff --git a/ghost/core/core/shared/labs.js b/ghost/core/core/shared/labs.js index 888208f51e2..fe3ff0fd8d8 100644 --- a/ghost/core/core/shared/labs.js +++ b/ghost/core/core/shared/labs.js @@ -52,6 +52,7 @@ const PRIVATE_FEATURES = [ 'pictureImageFormats', 'getHelperDeduplication', 'membersCustomFields', + 'membersImportRedesign', 'paywallImprovements', 'tagDetailsReact', 'selfServeArchives', diff --git a/ghost/core/package.json b/ghost/core/package.json index 64badeefb61..7d71117bdc2 100644 --- a/ghost/core/package.json +++ b/ghost/core/package.json @@ -94,6 +94,7 @@ "@tryghost/api-framework": "catalog:", "@tryghost/bookshelf-plugins": "2.3.8", "@tryghost/brute-knex": "catalog:", + "@tryghost/checkout": "workspace:*", "@tryghost/color-utils": "catalog:", "@tryghost/config-url-helpers": "1.0.27", "@tryghost/custom-field-types": "workspace:*", diff --git a/ghost/core/test/e2e-api/admin/posts-importer.test.js b/ghost/core/test/e2e-api/admin/posts-importer.test.js index bb6a8c95ac0..d79d900aebb 100644 --- a/ghost/core/test/e2e-api/admin/posts-importer.test.js +++ b/ghost/core/test/e2e-api/admin/posts-importer.test.js @@ -11,9 +11,19 @@ const { } = require('../../utils/e2e-framework'); const { cacheInvalidateHeaderNotSet } = assertions; const path = require('path'); +const nock = require('nock'); const models = require('../../../core/server/models'); const jobsService = require('../../../core/server/services/jobs'); +const mediaInlinerService = require('../../../core/server/services/media-inliner'); +const { + PostMediaInliner, + isBlockedMediaUrl, +} = require('../../../core/server/services/content-import/import/media'); +const { + isLocalMediaUrl, +} = require('../../../core/server/services/content-import/import/local-media-url'); const adapterManager = require('../../../core/server/services/adapter-manager').default; +const urlUtils = require('../../../core/shared/url-utils').default; const { compress } = require('@tryghost/zip'); const sinon = require('sinon'); @@ -21,6 +31,7 @@ const csvPath = path.join(__dirname, '../../utils/fixtures/csv/valid-posts-impor // Test CSVs are written inline to a temp dir rather than committed as fixtures. let tmpDir; +let remoteImportedMediaUrls = []; const getImportedAssetPaths = () => [ path.join(adapterManager.getAdapter('storage:images').storagePath, 'csv-zip-photo.jpg'), path.join(adapterManager.getAdapter('storage:media').storagePath, 'csv-zip-movie.mp4'), @@ -57,6 +68,23 @@ const zipFile = async (name, files) => { return zipPath; }; +const cleanupRemoteImportedMedia = async () => { + const storageByDirectory = { + images: adapterManager.getAdapter('storage:images'), + media: adapterManager.getAdapter('storage:media'), + files: adapterManager.getAdapter('storage:files'), + }; + for (const mediaUrl of new Set(remoteImportedMediaUrls)) { + const absoluteUrl = urlUtils.transformReadyToAbsolute(mediaUrl); + const directory = new URL(absoluteUrl).pathname.match(/\/content\/(images|media|files)\//)?.[1]; + if (directory) { + const storage = storageByDirectory[directory]; + await storage.delete(storage.urlToPath(absoluteUrl)); + } + } + remoteImportedMediaUrls = []; +}; + describe('Posts Importer API', function () { let agent; @@ -74,6 +102,7 @@ describe('Posts Importer API', function () { // Each test logs in as a different role — reset the login rate limiter // so the repeated logins don't trip spam prevention await resetRateLimits(); + remoteImportedMediaUrls = []; await Promise.all(getImportedAssetPaths().map((filePath) => fs.rm(filePath, { force: true }))); }); @@ -81,8 +110,10 @@ describe('Posts Importer API', function () { // Every accepted upload schedules a background import — drain it so a job // doesn't run on into another test (or another file on this fork's DB) await jobsService.allSettled(); + await cleanupRemoteImportedMedia(); await Promise.all(getImportedAssetPaths().map((filePath) => fs.rm(filePath, { force: true }))); mockManager.restore(); + nock.cleanAll(); sinon.restore(); }); @@ -134,6 +165,255 @@ describe('Posts Importer API', function () { .expect(cacheInvalidateHeaderNotSet()); }); + it('downloads and stores referenced CSV media before creating the post', async function () { + await agent.loginAsOwner(); + const GIF1x1 = Buffer.from('R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==', 'base64'); + const [audioFixture, videoFixture, fileFixture] = await Promise.all([ + fs.readFile(path.join(__dirname, '../../utils/fixtures/media/sample.mp3')), + fs.readFile(path.join(__dirname, '../../utils/fixtures/media/sample_640x360.mp4')), + fs.readFile(path.join(__dirname, '../../utils/fixtures/files/test.pdf')), + ]); + const origin = 'https://csv-import-assets.example'; + const requests = [ + nock(origin).get('/body.jpg').reply(200, GIF1x1), + nock(origin).get('/og.jpg').reply(200, GIF1x1), + nock(origin).get('/twitter.jpg').reply(200, GIF1x1), + nock(origin).get('/audio.mp3').reply(200, audioFixture), + nock(origin).get('/video.mp4').reply(200, videoFixture), + nock(origin).get('/guide.pdf').reply(200, fileFixture), + ]; + const html = + `

` + + `
` + + `
` + + ``; + const csvValue = (value) => `"${value.replaceAll('"', '""')}"`; + const csv = [ + 'title,html,markdown,feature_image,og_image,twitter_image', + `Remote CSV media,${csvValue(html)},,${origin}/body.jpg,${origin}/og.jpg,${origin}/twitter.jpg`, + `Remote Markdown media,,${csvValue(`![Remote](${origin}/body.jpg)`)},,,`, + ].join('\n'); + const filePath = await csvFile('remote-media.csv', csv); + + await agent.post('posts/upload/').attach('postsfile', filePath).expectStatus(202); + await jobsService.allSettled(); + + for (const request of requests) { + assert.equal(request.isDone(), true, request.pendingMocks().join(', ')); + } + const post = await models.Post.findOne( + { title: 'Remote CSV media', status: 'all' }, + { withRelated: ['posts_meta'] }, + ); + assert.ok(post); + const lexical = JSON.parse(post.get('lexical')); + const nodeByType = (type) => lexical.root.children.find((node) => node.type === type); + const storedUrls = [ + nodeByType('image').src, + nodeByType('audio').src, + nodeByType('video').src, + nodeByType('file').src, + post.get('feature_image'), + post.related('posts_meta').get('og_image'), + post.related('posts_meta').get('twitter_image'), + ]; + const markdownPost = await models.Post.findOne({ + title: 'Remote Markdown media', + status: 'all', + }); + assert.ok(markdownPost); + const markdownLexical = JSON.parse(markdownPost.get('lexical')); + const markdownImage = markdownLexical.root.children.find((node) => node.type === 'image').src; + assert.equal(post.get('feature_image'), nodeByType('image').src); + assert.equal(markdownImage, nodeByType('image').src); + storedUrls.push(markdownImage); + remoteImportedMediaUrls.push(...storedUrls); + for (const storedUrl of storedUrls) { + const absoluteUrl = new URL(urlUtils.transformReadyToAbsolute(storedUrl)); + assert.equal(absoluteUrl.origin, new URL(urlUtils.getSiteUrl()).origin); + assert.match(absoluteUrl.pathname, /\/content\/(?:images|media|files)\//); + } + assert.doesNotMatch(post.get('html'), /csv-import-assets\.example/); + assert.doesNotMatch(markdownPost.get('html'), /csv-import-assets\.example/); + }); + + it('isolates unsupported referenced media to its CSV row', async function () { + await agent.loginAsOwner(); + const origin = 'https://csv-import-assets.example'; + const unsupportedFixture = await fs.readFile( + path.join(__dirname, '../../unit/server/services/media-inliner/test/fixtures/fixture.exe'), + ); + const request = nock(origin).get('/unsupported.exe').reply(200, unsupportedFixture); + const filePath = await csvFile( + 'unsupported-remote-media.csv', + `title,feature_image\nUnsupported remote media,${origin}/unsupported.exe\nContinues after media failure,\n`, + ); + + await agent.post('posts/upload/').attach('postsfile', filePath).expectStatus(202); + await jobsService.allSettled(); + + assert.equal(request.isDone(), true, request.pendingMocks().join(', ')); + const post = await models.Post.findOne({ title: 'Unsupported remote media', status: 'all' }); + assert.equal(post, null); + const continuedPost = await models.Post.findOne({ + title: 'Continues after media failure', + status: 'all', + }); + assert.ok(continuedPost); + }); + + it('preserves current-site media URLs without fetching or validating them', async function () { + await agent.loginAsOwner(); + const siteUrl = urlUtils.getSiteUrl().replace(/\/$/, ''); + const imageUrl = `${siteUrl}/content/images/already-stored.jpg`; + const unsplashUrl = 'https://images.unsplash.com/photo-123?fit=crop&w=1200'; + const gravatarUrl = 'https://www.gravatar.com/avatar/abc123?s=200'; + const importUrl = sinon.spy(mediaInlinerService.getInstance(), 'importUrl'); + const filePath = await csvFile( + 'local-media.csv', + [ + 'title,html,feature_image', + `Local media,"

",${imageUrl}`, + `Unsplash media,"

",${unsplashUrl}`, + `Gravatar media,"

",${gravatarUrl}`, + ].join('\n'), + ); + + await agent.post('posts/upload/').attach('postsfile', filePath).expectStatus(202); + await jobsService.allSettled(); + + sinon.assert.notCalled(importUrl); + const post = await models.Post.findOne({ title: 'Local media', status: 'all' }); + assert.ok(post); + assert.ok(post.get('feature_image').endsWith('/content/images/already-stored.jpg')); + assert.match(post.get('html'), /\/content\/images\/already-stored\.jpg/); + + for (const [title, sourceUrl] of [ + ['Unsplash media', unsplashUrl], + ['Gravatar media', gravatarUrl], + ]) { + const blockedPost = await models.Post.findOne({ title, status: 'all' }); + assert.ok(blockedPost); + assert.equal(blockedPost.get('feature_image'), sourceUrl); + assert.ok(blockedPost.get('html').includes(new URL(sourceUrl).hostname)); + } + }); + + it('classifies local media URL variants used by importer configuration', function () { + const options = { + siteUrl: 'https://example.com/blog/', + subdir: 'blog', + assetBaseUrls: [ + 'not a URL', + 'https://assets.example/c/site/', + 'https://root-assets.example/', + null, + ], + }; + + for (const sourceUrl of [ + '__GHOST_URL__/content/images/image.jpg', + '/content/media/video.mp4', + '/blog/content/files/guide.pdf', + '//example.com/blog/content/images/image.jpg', + 'https://assets.example/c/site/content/images/image.jpg', + 'https://root-assets.example/anything/image.jpg', + ]) { + assert.equal(isLocalMediaUrl(sourceUrl, options), true, sourceUrl); + } + + for (const sourceUrl of [ + '/blogger/content/images/image.jpg', + 'https://assets.example/c/site-other/image.jpg', + 'https://example.com/blog/content/images-other/image.jpg', + 'https://external.example/content/images/image.jpg', + 'not a URL', + ]) { + assert.equal(isLocalMediaUrl(sourceUrl, options), false, sourceUrl); + } + + assert.equal( + isLocalMediaUrl('https://example.com/content/images/image.jpg', { + ...options, + siteUrl: 'not a URL', + }), + false, + ); + }); + + it('rewrites media in raw Lexical HTML and Markdown card content', async function () { + const origin = 'https://raw-card-assets.example'; + const importUrl = sinon.stub().callsFake(async (sourceUrl) => ({ + status: 'stored', + sourceUrl, + storedUrl: sourceUrl.replace(origin, '__GHOST_URL__/content/images'), + })); + const inliner = new PostMediaInliner({ + media: { importUrl }, + isLocalMediaUrl: () => false, + }); + const data = { + feature_image: 'https://images.unsplash.com/photo-123', + lexical: JSON.stringify({ + root: { + children: [ + { + type: 'before-after', + beforeImage: { src: `${origin}/before.jpg` }, + afterImage: { src: `${origin}/after.jpg` }, + }, + { + type: 'html', + html: + `` + + `
` + + ``, + }, + { + type: 'markdown', + markdown: `![Remote](${origin}/markdown.jpg)`, + }, + ], + }, + }), + }; + + await inliner.inline(data); + + const lexical = JSON.parse(data.lexical); + assert.equal( + lexical.root.children[0].beforeImage.src, + '__GHOST_URL__/content/images/before.jpg', + ); + assert.equal(lexical.root.children[0].afterImage.src, '__GHOST_URL__/content/images/after.jpg'); + assert.doesNotMatch(lexical.root.children[1].html, /raw-card-assets\.example/); + assert.doesNotMatch(lexical.root.children[2].markdown, /raw-card-assets\.example/); + assert.equal(data.feature_image, 'https://images.unsplash.com/photo-123'); + assert.equal(isBlockedMediaUrl('https://gravatar.com/avatar/abc123'), true); + assert.equal(isBlockedMediaUrl('https://gravatar.com.evil.example/avatar/abc123'), false); + assert.equal(isBlockedMediaUrl('not a URL'), false); + assert.equal(importUrl.callCount, 8); + }); + + it('fails the run when media inlining throws an unexpected error', async function () { + await agent.loginAsOwner(); + const sourceUrl = 'https://unexpected-media-error.example/image.jpg'; + const importUrl = sinon + .stub(mediaInlinerService.getInstance(), 'importUrl') + .rejects(new Error('Unexpected media importer failure')); + const filePath = await csvFile( + 'unexpected-media-error.csv', + `title,feature_image\nUnexpected media failure,${sourceUrl}\n`, + ); + + await agent.post('posts/upload/').attach('postsfile', filePath).expectStatus(202); + await jobsService.allSettled(); + + sinon.assert.calledOnceWithExactly(importUrl, sourceUrl); + const post = await models.Post.findOne({ title: 'Unexpected media failure', status: 'all' }); + assert.equal(post, null); + }); + it('Imports the single mapped CSV inside a ZIP', async function () { await agent.loginAsOwner(); @@ -160,6 +440,7 @@ describe('Posts Importer API', function () { it('Stores and rewrites wrapped image, media, and file assets before importing posts', async function () { await agent.loginAsOwner(); + const importUrl = sinon.spy(mediaInlinerService.getInstance(), 'importUrl'); const csv = 'title,html,markdown,feature_image,og_image,twitter_image\n' + @@ -180,6 +461,8 @@ describe('Posts Importer API', function () { assert.equal(body.meta.total, 3); await jobsService.allSettled(); + sinon.assert.notCalled(importUrl); + for (const filePath of getImportedAssetPaths().slice(0, 3)) { assert.equal(await fs.stat(filePath).then(() => true), true, `${filePath} was stored`); } 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 index 699b4635dcb..692690f3433 100644 --- a/ghost/core/test/e2e-api/admin/tiers-checkout-config.test.ts +++ b/ghost/core/test/e2e-api/admin/tiers-checkout-config.test.ts @@ -455,10 +455,11 @@ describe('Tier Checkout Admin API', function () { ); }); - // 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 () { + // Turning it on is one decision; where the address lands is not something Ghost can + // guess. Where the parcel goes it can: no countries means everywhere. + it('still refuses to collect an address without saying where it lands', async function () { const body = await setCheckout({ shipping: { collect: true } }, 422); - assert.match(body.errors[0].context, /at least one country/); + assert.match(body.errors[0].context, /which custom field this is collected into/); }); // Ghost keeps no convention about where a collected value belongs, so a request that @@ -483,13 +484,30 @@ describe('Tier Checkout Admin API', function () { 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 () { + // Countries are a restriction, so naming none is not an incomplete request — it is a + // publisher who delivers everywhere. Stored as the absence of a list rather than a + // copy of every country, because that set moves: an enumeration saved today silently + // becomes a restriction the day the processor adds one. + it('delivers everywhere when the request names no countries', async function () { await createField({ name: 'Delivery address', type: 'address' }); - const body = await setCheckout(shipping({ allowed_countries: undefined }), 422); + await setCheckout(shipping({ allowed_countries: undefined })); + + const config = await readCheckout(); + assert.equal(config.shipping.collect, true); + assert.equal( + 'allowed_countries' in config.shipping, + false, + 'everywhere reads back as no list, the same way it was written', + ); + }); + + // Naming none and naming an empty list are different statements. A publisher who + // cleared the list said something, and it was not "deliver worldwide". + it('refuses an empty list of countries', async function () { + await createField({ name: 'Delivery address', type: 'address' }); + + const body = await setCheckout(shipping({ allowed_countries: [] }), 422); assert.match(body.errors[0].context, /at least one country/); }); @@ -643,17 +661,25 @@ describe('Tier Checkout Admin API', function () { }); // 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 () { + // resumption of the last one. Resuming without countries is therefore everywhere, and + // must not quietly inherit the list from before — a tier that once delivered only to + // GB would otherwise keep refusing everyone else, with nothing in the request saying so. + it('stops collecting, and does not inherit the old countries when it resumes', 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/); + await setCheckout(shipping({ allowed_countries: undefined })); + + const config = await readCheckout(); + assert.equal(config.shipping.collect, true); + assert.equal( + 'allowed_countries' in config.shipping, + false, + 'the GB it delivered to before came back with it', + ); }); }); diff --git a/ghost/core/test/e2e-api/members/create-stripe-checkout-session.test.js b/ghost/core/test/e2e-api/members/create-stripe-checkout-session.test.js index aca99996162..2268636910d 100644 --- a/ghost/core/test/e2e-api/members/create-stripe-checkout-session.test.js +++ b/ghost/core/test/e2e-api/members/create-stripe-checkout-session.test.js @@ -7,6 +7,7 @@ const { matchers, } = require('../../utils/e2e-framework'); const nock = require('nock'); +const { STRIPE_ALLOWED_COUNTRIES } = require('@tryghost/checkout'); const models = require('../../../core/server/models'); const membersService = require('../../../core/server/services/members'); const urlServiceUtils = require('../../utils/url-service-utils'); @@ -860,6 +861,46 @@ describe('Create Stripe Checkout Session', function () { assert.equal(sessionBody['shipping_address_collection[allowed_countries][1]'], 'IE'); }); + // Ghost stores "everywhere" as no countries at all, and Stripe has no way to say that: + // `allowed_countries` is the only key `shipping_address_collection` has, so a request + // that leaves it out carries no parameter, and Stripe creates a session that succeeds + // and collects no address. Measured against the live API, not read from the reference, + // which calls the list optional. So the expansion has to happen before the request — + // and this is what proves it did. + it('asks Stripe for every country when a tier delivers everywhere', async function () { + const { + body: { + members_custom_fields: [address], + }, + } = await adminAgent + .post('/members/custom_fields/') + .body({ members_custom_fields: [{ name: 'Delivery address', type: 'address' }] }); + + await adminAgent.put(`/tiers/${paidTier.id}/checkout_config/`).body({ + tiers_checkout_config: [ + { + shipping: { + collect: true, + name: { custom_field_key: 'shipping_name' }, + address: { custom_field_key: address.key }, + }, + }, + ], + }); + + const sessionBody = await startCheckout(); + + const sent = Object.keys(sessionBody).filter((key) => + key.startsWith('shipping_address_collection[allowed_countries]'), + ); + assert.equal( + sent.length, + STRIPE_ALLOWED_COUNTRIES.length, + 'a tier that delivers everywhere has to name every country Stripe ships to', + ); + assert.equal(sessionBody['shipping_address_collection[allowed_countries][0]'], 'AC'); + }); + // The safety property: a site that configured nothing sends what it always sent. // // `tax_id_collection` is excluded because automatic tax already sets it on this diff --git a/ghost/core/test/unit/server/services/automations/automations-repository.test.ts b/ghost/core/test/unit/server/services/automations/automations-repository.test.ts index e18153d0e71..646fbc9c3fd 100644 --- a/ghost/core/test/unit/server/services/automations/automations-repository.test.ts +++ b/ghost/core/test/unit/server/services/automations/automations-repository.test.ts @@ -728,7 +728,7 @@ describe('automations repository', function () { it('returns null for "last run created at" if the automation has no runs', async function () { const result = await repo.browse(); - assert(result.data.every((automation) => automation.stats.last_run_created_at === null)); + assert(result.data.every((automation) => automation.stats?.last_run_created_at === null)); }); it('returns the newest run creation time for the automation', async function () { @@ -741,7 +741,7 @@ describe('automations repository', function () { const browseResult = await repo.browse(); const automation = browseResult.data.find((candidate) => candidate.id === automationId); - assert(automation); + assert(automation?.stats); assert.deepEqual(automation.stats.last_run_created_at, latestRunCreatedAt); }); @@ -749,7 +749,7 @@ describe('automations repository', function () { it('returns zero for "total run count" if the automation has no runs', async function () { const result = await repo.browse(); - assert(result.data.every((automation) => automation.stats.total_run_count === 0)); + assert(result.data.every((automation) => automation.stats?.total_run_count === 0)); }); it('returns the number of runs for the automation', async function () { @@ -766,8 +766,8 @@ describe('automations repository', function () { const otherAutomation = browseResult.data.find( (candidate) => candidate.id === otherAutomationId, ); - assert(automation); - assert(otherAutomation); + assert(automation?.stats); + assert(otherAutomation?.stats); assert.equal(automation.stats.total_run_count, 3); assert.equal(otherAutomation.stats.total_run_count, 1); @@ -776,7 +776,7 @@ describe('automations repository', function () { it('returns zero for "in progress run count" if the automation has no runs', async function () { const result = await repo.browse(); - assert(result.data.every((automation) => automation.stats.in_progress_run_count === 0)); + assert(result.data.every((automation) => automation.stats?.in_progress_run_count === 0)); }); it('returns zero for "in progress run count" if none of the runs have pending steps', async function () { @@ -787,7 +787,7 @@ describe('automations repository', function () { const browseResult = await repo.browse(); const automation = browseResult.data.find((candidate) => candidate.id === automationId); - assert(automation); + assert(automation?.stats); assert.equal(automation.stats.in_progress_run_count, 0); }); @@ -819,8 +819,8 @@ describe('automations repository', function () { const otherAutomation = browseResult.data.find( (candidate) => candidate.id === otherAutomationId, ); - assert(automation); - assert(otherAutomation); + assert(automation?.stats); + assert(otherAutomation?.stats); assert.equal(automation.stats.in_progress_run_count, 2); assert.equal(otherAutomation.stats.in_progress_run_count, 1); diff --git a/ghost/core/test/unit/server/services/content-import/import/importer.test.ts b/ghost/core/test/unit/server/services/content-import/import/importer.test.ts index 15b1eb73cec..8c12ea41077 100644 --- a/ghost/core/test/unit/server/services/content-import/import/importer.test.ts +++ b/ghost/core/test/unit/server/services/content-import/import/importer.test.ts @@ -2,6 +2,7 @@ import assert from 'node:assert/strict'; import sinon from 'sinon'; import logging from '@tryghost/logging'; import ContentCSVImporter from '../../../../../../core/server/services/content-import/import/importer'; +import { MediaInliningFailure } from '../../../../../../core/server/services/content-import/import/media'; import { ImportRunStore } from '../../../../../../core/server/services/content-import/import/store'; import type { PostImportRow } from '../../../../../../core/server/services/content-import/import/row'; import type { PostData } from '../../../../../../core/server/services/content-import/import/post-data'; @@ -24,6 +25,8 @@ function harness(rows: PostImportRow[] = [row('First'), row('Second')]) { const updatedTitles = new Set(); const warningsByTitle = new Map(); const urlFailures = new Map(); + const inlineMedia = sinon.stub().resolves(); + const createMediaInliner = sinon.stub().callsFake(() => ({ inline: inlineMedia })); const store = new ImportRunStore(); let converterResolutions = 0; let markdownRendererResolutions = 0; @@ -68,6 +71,7 @@ function harness(rows: PostImportRow[] = [row('First'), row('Second')]) { getHtmlToLexical: () => htmlToLexicalFactory(), getMarkdownToHtml: () => markdownToHtmlFactory(), getCleanHTML: () => cleanHTMLFactory(), + createMediaInliner, addJob: (job: { name: string; offloaded: boolean; job: () => Promise }) => { jobs.push(job); }, @@ -125,6 +129,8 @@ function harness(rows: PostImportRow[] = [row('First'), row('Second')]) { updatedTitles, warningsByTitle, urlFailures, + inlineMedia, + createMediaInliner, store, setHtmlToLexicalFactory, setMarkdownToHtmlFactory, @@ -263,6 +269,10 @@ describe('ContentCSVImporter', function () { events.push('convert'); return (html: string) => ({ converted: html }); }); + h.inlineMedia.callsFake(async (data: PostData) => { + events.push('inline'); + assert.match(data.lexical ?? '', /unique\.jpg/); + }); const importer = new ContentCSVImporter({ ...h.deps, prepareSource: async () => ({ @@ -275,7 +285,7 @@ describe('ContentCSVImporter', function () { await importer.importCSV({ filePath: '/tmp/posts.zip', fileName: 'posts.zip' }); await h.jobs[0].job(); - assert.deepEqual(events.slice(0, 3), ['store', 'rewrite', 'convert']); + assert.deepEqual(events.slice(0, 4), ['store', 'rewrite', 'convert', 'inline']); assert.match(h.created[0].data.lexical ?? '', /unique\.jpg/); sinon.assert.calledOnce(cleanup); }); @@ -332,6 +342,132 @@ describe('ContentCSVImporter', function () { } }); + it('inlines built post data before opening the repository write transaction', async function () { + const h = harness([row('Inline order')]); + const events: string[] = []; + h.setHtmlToLexicalFactory(() => (html: string) => { + events.push('convert'); + return { converted: html }; + }); + h.inlineMedia.callsFake(async (data: PostData) => { + events.push('inline'); + data.feature_image = '__GHOST_URL__/content/images/inlined.jpg'; + }); + const write = h.deps.posts.write; + const importer = new ContentCSVImporter({ + ...h.deps, + posts: { + write: async (data, options, metadata) => { + events.push('write'); + assert.equal(data.feature_image, '__GHOST_URL__/content/images/inlined.jpg'); + return write(data, options, metadata); + }, + }, + }); + + await importer.importCSV({ filePath: '/tmp/posts.csv', fileName: 'posts.csv' }); + await h.jobs[0].job(); + + assert.deepEqual(events, ['convert', 'inline', 'write']); + }); + + it('creates an isolated media inliner for every import run', async function () { + const h = harness([row('Cached media')]); + + await h.importer.importCSV({ filePath: '/tmp/first.csv', fileName: 'first.csv' }); + await h.jobs[0].job(); + await h.importer.importCSV({ filePath: '/tmp/second.csv', fileName: 'second.csv' }); + await h.jobs[1].job(); + + sinon.assert.calledTwice(h.createMediaInliner); + assert.notEqual( + h.createMediaInliner.firstCall.returnValue, + h.createMediaInliner.secondCall.returnValue, + ); + }); + + it('records expected media failures against one row and imports the rest', async function () { + const h = harness([row('First'), row('Media failure'), row('Third')]); + const failure = new MediaInliningFailure([ + { sourceUrl: 'https://assets.test/missing.jpg', reason: 'Download failed.' }, + { sourceUrl: 'https://assets.test/broken.mp4', reason: 'Storage failed.' }, + ]); + h.inlineMedia.callsFake(async (data: PostData) => { + if (data.title === 'Media failure') { + throw failure; + } + }); + + await h.run(); + + assert.deepEqual( + h.created.map((call) => call.data.title), + ['First', 'Third'], + ); + assert.equal(h.inlineMedia.callCount, 3); + assert.deepEqual(h.reported, []); + assert.equal(h.store.get('run_test')?.status, 'complete'); + assert.deepEqual(h.store.get('run_test')?.rows[1], { + line: 3, + title: 'Media failure', + status: 'failed', + reason: 'Could not import 2 media files.', + mediaFailures: [ + { sourceUrl: 'https://assets.test/missing.jpg', reason: 'Download failed.' }, + { sourceUrl: 'https://assets.test/broken.mp4', reason: 'Storage failed.' }, + ], + }); + }); + + it('reports once when media failures prevent every post write', async function () { + const h = harness([row('First'), row('Second')]); + const failure = new MediaInliningFailure([ + { sourceUrl: 'https://assets.test/missing.jpg', reason: 'Download failed.' }, + ]); + h.inlineMedia.rejects(failure); + + await h.run(); + + assert.equal(h.created.length, 0); + assert.equal(h.reported.length, 1); + assert.equal( + (h.reported[0] as Error).message, + 'Content import failed to write all 2 attempted posts.', + ); + assert.match((h.reported[0] as Error).stack ?? '', /Could not import 1 media file/); + assert.equal(h.store.get('run_test')?.status, 'complete'); + assert.deepEqual( + h.store.get('run_test')?.rows.map((outcome) => outcome.status), + ['failed', 'failed'], + ); + }); + + it('stops the run when media preparation throws an unexpected error', async function () { + const h = harness([row('Media defect'), row('Never reached')]); + const failure = new Error('media importer defect'); + h.inlineMedia.rejects(failure); + + await h.run(); + + assert.equal(h.created.length, 0); + assert.equal(h.inlineMedia.callCount, 1); + assert.deepEqual(h.reported, [failure]); + assert.equal(h.store.get('run_test')?.status, 'failed'); + assert.equal(h.store.get('run_test')?.failureReason, failure.message); + }); + + it('does not inspect media for a row skipped during post-data validation', async function () { + const h = harness([ + { title: '', html: '

No title

', markdown: '', published_at: undefined }, + row('Valid row'), + ]); + + await h.run(); + + sinon.assert.calledOnce(h.inlineMedia); + assert.equal(h.inlineMedia.firstCall.args[0].title, 'Valid row'); + }); + it('forwards author and tag cells to the transactional write seam', async function () { const h = harness([ { diff --git a/ghost/core/test/unit/server/services/content-import/import/media.test.ts b/ghost/core/test/unit/server/services/content-import/import/media.test.ts new file mode 100644 index 00000000000..a5000c17440 --- /dev/null +++ b/ghost/core/test/unit/server/services/content-import/import/media.test.ts @@ -0,0 +1,728 @@ +import assert from 'node:assert/strict'; +import sinon from 'sinon'; +import { + isBlockedMediaUrl, + MediaInliningFailure, + PostMediaInliner, +} from '../../../../../../core/server/services/content-import/import/media'; +import { isLocalMediaUrl } from '../../../../../../core/server/services/content-import/import/local-media-url'; +import type { PostData } from '../../../../../../core/server/services/content-import/import/post-data'; +import type { ExternalMediaImportResult } from '../../../../../../core/server/services/media-inliner/types'; + +const postData = (overrides: Partial = {}): PostData => ({ + title: 'Media post', + slug: 'media-post', + status: 'published', + type: 'post', + visibility: 'public', + tags: [], + ...overrides, +}); + +function harness({ + localUrl = sinon.stub<[string], boolean>().returns(false), +}: { + localUrl?: sinon.SinonStub<[string], boolean>; +} = {}) { + const importUrl = sinon + .stub<[string], Promise>() + .callsFake(async (sourceUrl: string) => { + const fileName = new URL(sourceUrl.replace(/^\/\//, 'https://')).pathname.split('/').at(-1); + return { + status: 'stored', + sourceUrl, + storedUrl: `__GHOST_URL__/content/files/${fileName}`, + }; + }); + const inliner = new PostMediaInliner({ + media: { importUrl }, + isLocalMediaUrl: localUrl, + }); + + return { inliner, importUrl, localUrl }; +} + +describe('PostMediaInliner', function () { + afterEach(function () { + sinon.restore(); + }); + + it('describes a single failed media file', function () { + const error = new MediaInliningFailure([ + { sourceUrl: 'https://assets.test/image.jpg', reason: 'Download failed.' }, + ]); + + assert.equal(error.message, 'Could not import 1 media file.'); + }); + + it('inlines direct post image fields and every supported Lexical media card field', async function () { + const h = harness(); + const lexical = { + root: { + children: [ + { type: 'image', src: 'https://assets.test/image.jpg', href: 'https://example.com/' }, + { + type: 'gallery', + images: [ + { src: 'https://assets.test/gallery-one.jpg' }, + { src: 'https://assets.test/gallery-two.jpg' }, + ], + }, + { + type: 'audio', + src: 'https://assets.test/audio.mp3', + thumbnailSrc: 'https://assets.test/audio-cover.jpg', + }, + { + type: 'video', + src: 'https://assets.test/video.mp4', + thumbnailSrc: 'https://assets.test/video-cover.jpg', + customThumbnailSrc: 'https://assets.test/video-custom.jpg', + }, + { type: 'file', src: 'https://assets.test/guide.pdf' }, + { + type: 'product', + productImageSrc: 'https://assets.test/product.jpg', + productUrl: 'https://example.com/product', + }, + { + type: 'header', + backgroundImageSrc: 'https://assets.test/header.jpg', + buttonUrl: 'https://example.com/header-button', + }, + { type: 'signup', backgroundImageSrc: 'https://assets.test/signup.jpg' }, + { + type: 'call-to-action', + imageUrl: 'https://assets.test/cta.jpg', + buttonUrl: 'https://example.com/cta-button', + }, + { + type: 'bookmark', + url: 'https://example.com/bookmark', + metadata: { + icon: 'https://assets.test/bookmark-icon.ico', + thumbnail: 'https://assets.test/bookmark.jpg', + }, + }, + { + type: 'embed', + url: 'https://example.com/embed', + metadata: { thumbnail_url: 'https://assets.test/embed.jpg' }, + }, + { + type: 'before-after', + beforeImage: { src: 'https://assets.test/before.jpg' }, + afterImage: { src: 'https://assets.test/after.jpg' }, + }, + { + type: 'paragraph', + children: [{ type: 'image', src: 'https://assets.test/nested.jpg' }], + }, + ], + }, + }; + const data = postData({ + feature_image: 'https://assets.test/feature.jpg', + posts_meta: { + og_image: 'https://assets.test/og.jpg', + twitter_image: 'https://assets.test/twitter.jpg', + }, + lexical: JSON.stringify(lexical), + canonical_url: 'https://example.com/canonical', + codeinjection_head: '', + }); + + await h.inliner.inline(data); + + assert.equal(data.feature_image, '__GHOST_URL__/content/files/feature.jpg'); + assert.equal(data.posts_meta?.og_image, '__GHOST_URL__/content/files/og.jpg'); + assert.equal(data.posts_meta?.twitter_image, '__GHOST_URL__/content/files/twitter.jpg'); + const result = JSON.parse(data.lexical ?? '{}'); + const children = result.root.children; + assert.equal(children[0].src, '__GHOST_URL__/content/files/image.jpg'); + assert.equal(children[0].href, 'https://example.com/'); + assert.deepEqual( + children[1].images.map((image: { src: string }) => image.src), + [ + '__GHOST_URL__/content/files/gallery-one.jpg', + '__GHOST_URL__/content/files/gallery-two.jpg', + ], + ); + assert.equal(children[2].src, '__GHOST_URL__/content/files/audio.mp3'); + assert.equal(children[2].thumbnailSrc, '__GHOST_URL__/content/files/audio-cover.jpg'); + assert.equal(children[3].src, '__GHOST_URL__/content/files/video.mp4'); + assert.equal(children[3].thumbnailSrc, '__GHOST_URL__/content/files/video-cover.jpg'); + assert.equal(children[3].customThumbnailSrc, '__GHOST_URL__/content/files/video-custom.jpg'); + assert.equal(children[4].src, '__GHOST_URL__/content/files/guide.pdf'); + assert.equal(children[5].productImageSrc, '__GHOST_URL__/content/files/product.jpg'); + assert.equal(children[5].productUrl, 'https://example.com/product'); + assert.equal(children[6].backgroundImageSrc, '__GHOST_URL__/content/files/header.jpg'); + assert.equal(children[6].buttonUrl, 'https://example.com/header-button'); + assert.equal(children[7].backgroundImageSrc, '__GHOST_URL__/content/files/signup.jpg'); + assert.equal(children[8].imageUrl, '__GHOST_URL__/content/files/cta.jpg'); + assert.equal(children[8].buttonUrl, 'https://example.com/cta-button'); + assert.equal(children[9].metadata.icon, '__GHOST_URL__/content/files/bookmark-icon.ico'); + assert.equal(children[9].metadata.thumbnail, '__GHOST_URL__/content/files/bookmark.jpg'); + assert.equal(children[9].url, 'https://example.com/bookmark'); + assert.equal(children[10].metadata.thumbnail_url, '__GHOST_URL__/content/files/embed.jpg'); + assert.equal(children[10].url, 'https://example.com/embed'); + assert.equal(children[11].beforeImage.src, '__GHOST_URL__/content/files/before.jpg'); + assert.equal(children[11].afterImage.src, '__GHOST_URL__/content/files/after.jpg'); + assert.equal(children[12].children[0].src, '__GHOST_URL__/content/files/nested.jpg'); + assert.equal(data.canonical_url, 'https://example.com/canonical'); + assert.equal(data.codeinjection_head, ''); + assert.equal(h.importUrl.callCount, 22); + }); + + it('inlines media attributes, srcsets, and CSS URLs inside HTML fields', async function () { + const h = harness(); + const data = postData({ + lexical: JSON.stringify({ + root: { + children: [ + { + type: 'html', + html: + '' + + '' + + '' + + '
' + + '', + }, + ], + }, + }), + }); + + await h.inliner.inline(data); + + const html = JSON.parse(data.lexical ?? '{}').root.children[0].html; + for (const file of [ + 'image.jpg', + 'lazy.jpg', + 'small.jpg', + 'large.jpg', + 'video.mp4', + 'poster.jpg', + 'video.webm', + 'audio.mp3', + 'background.jpg', + 'style.jpg', + ]) { + assert.match(html, new RegExp(`__GHOST_URL__/content/files/${file.replace('.', '\\.')}`)); + } + assert.match(html, /href="https:\/\/example\.com\/keep"/); + assert.match(html, /small\.jpg 1x, __GHOST_URL__\/content\/files\/large\.jpg 2x/); + }); + + it('inlines image syntax and embedded media in Markdown cards without changing links', async function () { + const h = harness(); + const data = postData({ + lexical: JSON.stringify({ + root: { + children: [ + { + type: 'markdown', + markdown: + '![Photo](https://assets.test/markdown.jpg "Title")\n' + + '![Angle]()\n' + + '[Ordinary link](https://example.com/page)\n' + + '\n' + + '', + }, + ], + }, + }), + }); + + await h.inliner.inline(data); + + const markdown = JSON.parse(data.lexical ?? '{}').root.children[0].markdown; + assert.match(markdown, /__GHOST_URL__\/content\/files\/markdown\.jpg/); + assert.match(markdown, /__GHOST_URL__\/content\/files\/angle\.jpg/); + assert.match(markdown, /__GHOST_URL__\/content\/files\/embedded\.jpg/); + assert.match(markdown, /__GHOST_URL__\/content\/files\/markdown-style\.jpg/); + assert.match(markdown, /\[Ordinary link]\(https:\/\/example\.com\/page\)/); + }); + + it('preserves local, embedded, unsupported, and non-media URLs', async function () { + const h = harness(); + const lexical = JSON.stringify({ + root: { + children: [ + { type: 'image', src: '__GHOST_URL__/content/images/local.jpg' }, + { type: 'image', src: '/content/images/root-relative.jpg' }, + { type: 'image', src: 'data:image/gif;base64,AAAA' }, + { type: 'image', src: 'ftp://assets.test/legacy.jpg' }, + { type: 'link', url: 'https://assets.test/ordinary-link' }, + null, + ], + }, + }); + const data = postData({ + feature_image: 'data:image/gif;base64,AAAA', + posts_meta: {}, + lexical, + }); + + await h.inliner.inline(data); + + assert.equal(data.lexical, lexical); + sinon.assert.notCalled(h.importUrl); + }); + + it('preserves media hosted on blocked domains without importing or caching it', async function () { + const h = harness(); + const unsplashUrl = 'https://images.unsplash.com/photo-123?fit=crop&w=1200'; + const gravatarUrl = '//www.gravatar.com/avatar/abc123?s=200'; + const data = postData({ + feature_image: unsplashUrl, + posts_meta: { og_image: gravatarUrl }, + lexical: JSON.stringify({ + root: { + children: [ + { type: 'image', src: unsplashUrl }, + { type: 'image', src: gravatarUrl }, + ], + }, + }), + }); + + await h.inliner.inline(data); + + assert.equal(data.feature_image, unsplashUrl); + assert.equal(data.posts_meta?.og_image, gravatarUrl); + assert.equal(JSON.parse(data.lexical ?? '{}').root.children[0].src, unsplashUrl); + assert.equal(JSON.parse(data.lexical ?? '{}').root.children[1].src, gravatarUrl); + sinon.assert.notCalled(h.importUrl); + sinon.assert.notCalled(h.localUrl); + }); + + it('matches blocked domains without matching lookalike hostnames', function () { + for (const sourceUrl of [ + 'https://images.unsplash.com/image.jpg', + 'https://gravatar.com/avatar/abc123', + '//www.gravatar.com/avatar/abc123', + 'https://secure.gravatar.com/avatar/abc123', + 'https://cdn.images.unsplash.com/image.jpg', + ]) { + assert.equal(isBlockedMediaUrl(sourceUrl), true, sourceUrl); + } + + for (const sourceUrl of [ + 'https://images.unsplash.com.evil.example/image.jpg', + 'https://notgravatar.com/avatar/abc123', + 'https://gravatar.com.evil.example/avatar/abc123', + 'not a URL', + ]) { + assert.equal(isBlockedMediaUrl(sourceUrl), false, sourceUrl); + } + }); + + it('recognizes Ghost placeholders and root-relative content paths', function () { + const options = { + siteUrl: 'https://example.com/blog/', + subdir: 'blog', + assetBaseUrls: [], + }; + + for (const sourceUrl of [ + '__GHOST_URL__/content/images/image.jpg', + '__GHOST_URL__/anything', + '/content/images/image.jpg', + '/content/images', + '/content/media/video.mp4', + '/content/files/guide.pdf', + '/blog/content/images/image.jpg', + '/blog/content/media/video.mp4', + '/blog/content/files/guide.pdf', + ]) { + assert.equal(isLocalMediaUrl(sourceUrl, options), true, sourceUrl); + } + + assert.equal(isLocalMediaUrl('/content/images-other/image.jpg', options), false); + assert.equal(isLocalMediaUrl('/blogger/content/images/image.jpg', options), false); + }); + + it('recognizes current-site content URLs with configured subdirectories', function () { + const options = { + siteUrl: 'https://example.com/blog/', + subdir: '/blog', + assetBaseUrls: [], + }; + + for (const sourceUrl of [ + 'http://example.com/content/images/image.jpg', + '//example.com/blog/content/media/video.mp4', + 'https://example.com/blog/content/files/guide.pdf', + ]) { + assert.equal(isLocalMediaUrl(sourceUrl, options), true, sourceUrl); + } + + for (const sourceUrl of [ + 'https://example.com/about/image.jpg', + 'https://example.com/blog/content/images-other/image.jpg', + 'https://example.com.evil/content/images/image.jpg', + 'https://external.example/content/images/image.jpg', + ]) { + assert.equal(isLocalMediaUrl(sourceUrl, options), false, sourceUrl); + } + }); + + it('recognizes configured storage and CDN URL prefixes without near-matching', function () { + const options = { + siteUrl: 'https://example.com/', + subdir: '', + assetBaseUrls: [ + 'https://images.example/c/site/content/images/', + 'https://assets.example/c/site', + null, + undefined, + ], + }; + + for (const sourceUrl of [ + 'http://images.example/c/site/content/images/image.jpg', + '//assets.example/c/site/content/media/video.mp4', + ]) { + assert.equal(isLocalMediaUrl(sourceUrl, options), true, sourceUrl); + } + + for (const sourceUrl of [ + 'https://images.example/c/site/content/images-other/image.jpg', + 'https://assets.example/c/site-other/content/files/guide.pdf', + 'https://assets.example.evil/c/site/content/files/guide.pdf', + 'not a URL', + ]) { + assert.equal(isLocalMediaUrl(sourceUrl, options), false, sourceUrl); + } + }); + + it('does not import or cache URLs classified as local', async function () { + const sourceUrl = 'https://example.com/content/images/existing.jpg'; + const localUrl = sinon.stub<[string], boolean>(); + localUrl.onFirstCall().returns(true); + localUrl.onSecondCall().returns(false); + const h = harness({ localUrl }); + const first = postData({ feature_image: sourceUrl }); + const second = postData({ feature_image: sourceUrl }); + + await h.inliner.inline(first); + await h.inliner.inline(second); + + assert.equal(first.feature_image, sourceUrl); + assert.equal(second.feature_image, '__GHOST_URL__/content/files/existing.jpg'); + sinon.assert.calledTwice(localUrl); + sinon.assert.calledOnceWithExactly(h.importUrl, sourceUrl); + }); + + it('processes protocol-relative media URLs', async function () { + const h = harness(); + const data = postData({ feature_image: '//assets.test/protocol-relative.jpg' }); + + await h.inliner.inline(data); + + assert.equal(data.feature_image, '__GHOST_URL__/content/files/protocol-relative.jpg'); + sinon.assert.calledWithExactly(h.importUrl, '//assets.test/protocol-relative.jpg'); + }); + + it('reuses the same URL across fields in one row', async function () { + const h = harness(); + const sourceUrl = 'https://assets.test/shared.jpg'; + const data = postData({ + feature_image: sourceUrl, + posts_meta: { og_image: sourceUrl, twitter_image: sourceUrl }, + lexical: JSON.stringify({ + root: { children: [{ type: 'image', src: sourceUrl }] }, + }), + }); + + await h.inliner.inline(data); + + sinon.assert.calledOnceWithExactly(h.importUrl, sourceUrl); + assert.equal(data.feature_image, '__GHOST_URL__/content/files/shared.jpg'); + assert.equal(data.posts_meta?.og_image, '__GHOST_URL__/content/files/shared.jpg'); + assert.equal(data.posts_meta?.twitter_image, '__GHOST_URL__/content/files/shared.jpg'); + assert.equal( + JSON.parse(data.lexical ?? '{}').root.children[0].src, + '__GHOST_URL__/content/files/shared.jpg', + ); + }); + + it('reuses the same URL in the same field across rows', async function () { + const h = harness(); + const sourceUrl = 'https://assets.test/cross-row.jpg'; + const first = postData({ feature_image: sourceUrl }); + const second = postData({ feature_image: sourceUrl }); + + await h.inliner.inline(first); + await h.inliner.inline(second); + + sinon.assert.calledOnceWithExactly(h.importUrl, sourceUrl); + assert.equal(first.feature_image, '__GHOST_URL__/content/files/cross-row.jpg'); + assert.equal(second.feature_image, '__GHOST_URL__/content/files/cross-row.jpg'); + }); + + it('shares an in-flight import for simultaneous references', async function () { + const h = harness(); + const sourceUrl = 'https://assets.test/simultaneous.jpg'; + let resolveImport: (result: ExternalMediaImportResult) => void = () => {}; + const pendingImport = new Promise((resolve) => { + resolveImport = resolve; + }); + h.importUrl.returns(pendingImport); + const first = postData({ feature_image: sourceUrl }); + const second = postData({ feature_image: sourceUrl }); + + const firstInlining = h.inliner.inline(first); + const secondInlining = h.inliner.inline(second); + + sinon.assert.calledOnceWithExactly(h.importUrl, sourceUrl); + resolveImport({ + status: 'stored', + sourceUrl, + storedUrl: '__GHOST_URL__/content/files/simultaneous.jpg', + }); + await Promise.all([firstInlining, secondInlining]); + assert.equal(first.feature_image, '__GHOST_URL__/content/files/simultaneous.jpg'); + assert.equal(second.feature_image, '__GHOST_URL__/content/files/simultaneous.jpg'); + }); + + it('caches failed imports while reporting them for every affected row', async function () { + const h = harness(); + const sourceUrl = 'https://assets.test/missing.jpg'; + h.importUrl.resolves({ + status: 'failed', + sourceUrl, + stage: 'download', + reason: 'The media file could not be downloaded.', + }); + + for (const title of ['First affected row', 'Second affected row']) { + await assert.rejects( + h.inliner.inline(postData({ title, feature_image: sourceUrl })), + (error: unknown) => { + assert.ok(error instanceof MediaInliningFailure); + assert.deepEqual(error.failures, [ + { sourceUrl, reason: 'The media file could not be downloaded.' }, + ]); + return true; + }, + ); + } + + sinon.assert.calledOnceWithExactly(h.importUrl, sourceUrl); + }); + + it('keeps successful cached media when another URL makes the row fail', async function () { + const h = harness(); + const storedSource = 'https://assets.test/stored-before-failure.jpg'; + const failedSource = 'https://assets.test/failure-after-storage.jpg'; + h.importUrl.callsFake(async (sourceUrl: string) => { + if (sourceUrl === failedSource) { + return { + status: 'failed', + sourceUrl, + stage: 'storage', + reason: 'The media file could not be stored in Ghost.', + }; + } + return { + status: 'stored', + sourceUrl, + storedUrl: '__GHOST_URL__/content/images/stored-before-failure.jpg', + }; + }); + const failedRow = postData({ + feature_image: storedSource, + posts_meta: { og_image: failedSource }, + }); + + await assert.rejects(h.inliner.inline(failedRow), MediaInliningFailure); + const laterRow = postData({ feature_image: storedSource }); + await h.inliner.inline(laterRow); + + assert.equal(laterRow.feature_image, '__GHOST_URL__/content/images/stored-before-failure.jpg'); + assert.equal(h.importUrl.getCalls().filter((call) => call.args[0] === storedSource).length, 1); + }); + + it('keeps query strings and fragments in the exact cache key', async function () { + const h = harness(); + const sourceUrls = [ + 'https://assets.test/image.jpg', + 'https://assets.test/image.jpg?size=large', + 'https://assets.test/image.jpg#preview', + ]; + const data = postData({ + feature_image: sourceUrls[0], + posts_meta: { og_image: sourceUrls[1], twitter_image: sourceUrls[2] }, + }); + + await h.inliner.inline(data); + + assert.deepEqual( + h.importUrl.getCalls().map((call) => call.args[0]), + sourceUrls, + ); + }); + + it('does not share cached URLs across media-inliner instances', async function () { + const h = harness(); + const sourceUrl = 'https://assets.test/separate-imports.jpg'; + const nextImportInliner = new PostMediaInliner({ + media: { importUrl: h.importUrl }, + isLocalMediaUrl: h.localUrl, + }); + + await h.inliner.inline(postData({ feature_image: sourceUrl })); + await nextImportInliner.inline(postData({ feature_image: sourceUrl })); + + sinon.assert.calledTwice(h.importUrl); + }); + + it('collects every unique expected download, extraction, and storage failure', async function () { + const h = harness(); + h.importUrl.callsFake(async (sourceUrl: string) => { + if (sourceUrl.endsWith('/download-throws.jpg') || sourceUrl.endsWith('/download-null.jpg')) { + return { + status: 'failed', + sourceUrl, + stage: 'download', + reason: 'The media file could not be downloaded.', + } as const; + } + if (sourceUrl.endsWith('/unreadable.jpg')) { + return { + status: 'failed', + sourceUrl, + stage: 'extract', + reason: 'The downloaded media file could not be read.', + } as const; + } + if (sourceUrl.endsWith('/unsupported.exe')) { + return { + status: 'failed', + sourceUrl, + stage: 'unsupported', + reason: 'No configured storage accepts this media file.', + } as const; + } + if (sourceUrl.endsWith('/store-throws.jpg')) { + return { + status: 'failed', + sourceUrl, + stage: 'storage', + reason: 'The media file could not be stored in Ghost.', + } as const; + } + return { status: 'stored', sourceUrl, storedUrl: `local:${sourceUrl}` } as const; + }); + const data = postData({ + feature_image: 'https://assets.test/download-throws.jpg', + posts_meta: { + og_image: 'https://assets.test/download-null.jpg', + twitter_image: 'https://assets.test/unreadable.jpg', + }, + lexical: JSON.stringify({ + root: { + children: [ + { type: 'image', src: 'https://assets.test/unsupported.exe' }, + { type: 'image', src: 'https://assets.test/store-throws.jpg' }, + { type: 'image', src: 'https://assets.test/download-null.jpg' }, + ], + }, + }), + }); + + await assert.rejects(h.inliner.inline(data), (error: unknown) => { + assert.ok(error instanceof MediaInliningFailure); + assert.equal(error.message, 'Could not import 5 media files.'); + assert.deepEqual(error.failures, [ + { + sourceUrl: 'https://assets.test/download-throws.jpg', + reason: 'The media file could not be downloaded.', + }, + { + sourceUrl: 'https://assets.test/download-null.jpg', + reason: 'The media file could not be downloaded.', + }, + { + sourceUrl: 'https://assets.test/unreadable.jpg', + reason: 'The downloaded media file could not be read.', + }, + { + sourceUrl: 'https://assets.test/unsupported.exe', + reason: 'No configured storage accepts this media file.', + }, + { + sourceUrl: 'https://assets.test/store-throws.jpg', + reason: 'The media file could not be stored in Ghost.', + }, + ]); + return true; + }); + assert.equal(h.importUrl.callCount, 5, 'every unique reference is attempted before failing'); + }); + + it('propagates unexpected errors from the media importer', async function () { + const h = harness(); + const error = new Error('media importer defect'); + h.importUrl.rejects(error); + + await assert.rejects( + h.inliner.inline( + postData({ feature_image: 'https://assets.test/unexpected-import-error.jpg' }), + ), + (thrown) => thrown === error, + ); + }); + + it('preserves valid but empty or unrelated Lexical structures', async function () { + const h = harness(); + for (const lexical of ['null', '{}', '{"root":{}}', '{"root":{"children":[]}}']) { + const data = postData({ lexical }); + await h.inliner.inline(data); + assert.equal(data.lexical, lexical); + } + sinon.assert.notCalled(h.importUrl); + }); + + it('treats malformed Lexical JSON as an unexpected importer error', async function () { + const h = harness(); + const data = postData({ lexical: '{not-json' }); + + await assert.rejects(h.inliner.inline(data), SyntaxError); + }); + + it('preserves data-URI srcsets and empty CSS URLs', async function () { + const h = harness(); + const data = postData({ + lexical: JSON.stringify({ + root: { + children: [ + { + type: 'html', + html: + '' + + '' + + '
', + }, + { type: 'image', src: '' }, + { type: 'gallery', images: [null, {}, { src: '' }] }, + { type: 'bookmark' }, + { type: 'markdown', markdown: '' }, + { type: 42 }, + ], + }, + }), + }); + + await h.inliner.inline(data); + + const html = JSON.parse(data.lexical ?? '{}').root.children[0].html; + assert.match(html, /data:image\/svg\+xml;base64,AAAA 1x, data:image\/svg\+xml;base64,BBBB 2x/); + assert.match(html, /srcset=", __GHOST_URL__\/content\/files\/source\.jpg 2x"/); + assert.match(html, /background:url\(''\)/); + sinon.assert.calledOnce(h.importUrl); + }); +}); diff --git a/ghost/core/test/unit/server/services/stripe/allowed-countries.test.ts b/ghost/core/test/unit/server/services/stripe/allowed-countries.test.ts index cf8c8f71f45..58eb1e2ddc1 100644 --- a/ghost/core/test/unit/server/services/stripe/allowed-countries.test.ts +++ b/ghost/core/test/unit/server/services/stripe/allowed-countries.test.ts @@ -1,10 +1,7 @@ import fs from 'fs'; import path from 'path'; import { describe, it, assert } from 'vitest'; -import { - STRIPE_ALLOWED_COUNTRIES, - isStripeAllowedCountry, -} from '../../../../../core/server/services/stripe/services/checkout/allowed-countries'; +import { STRIPE_ALLOWED_COUNTRIES, isStripeAllowedCountry } from '@tryghost/checkout'; /** * The list was measured against the live API rather than taken from the SDK, because the diff --git a/ghost/core/test/utils/vitest-setup-db.ts b/ghost/core/test/utils/vitest-setup-db.ts index 482cce0681e..0799f0809ae 100644 --- a/ghost/core/test/utils/vitest-setup-db.ts +++ b/ghost/core/test/utils/vitest-setup-db.ts @@ -56,19 +56,22 @@ process.env.WEBHOOK_SECRET = process.env.WEBHOOK_SECRET || 'TEST_STRIPE_WEBHOOK_ // nothing exactly as a fresh name would. mysql keeps a random per-fork name: it // has no /tmp to bound (CI databases die with the job) and a random name sidesteps // the same stale-reuse hazard without a pre-boot DROP. -const poolSlot = parseInt(process.env.VITEST_POOL_ID || '', 10); -const sqliteId = Number.isInteger(poolSlot) - ? `pool_${poolSlot}` - : crypto.randomBytes(4).toString('hex'); -const sqliteBase = process.env.database__connection__filename; -process.env.database__connection__filename = sqliteBase - ? `${sqliteBase.replace(/\.db$/i, '')}-${sqliteId}.db` - : `/tmp/ghost-test-${sqliteId}.db`; -const mysqlId = crypto.randomBytes(4).toString('hex'); -const mysqlBase = process.env.database__connection__database; -process.env.database__connection__database = mysqlBase - ? `${mysqlBase}_${mysqlId}` - : `ghost_testing_${mysqlId}`; +if (process.env.NODE_ENV.includes('mysql')) { + const mysqlId = crypto.randomBytes(4).toString('hex'); + const mysqlBase = process.env.database__connection__database; + process.env.database__connection__database = mysqlBase + ? `${mysqlBase}_${mysqlId}` + : `ghost_testing_${mysqlId}`; +} else { + const poolSlot = parseInt(process.env.VITEST_POOL_ID || '', 10); + const sqliteId = Number.isInteger(poolSlot) + ? `pool_${poolSlot}` + : crypto.randomBytes(4).toString('hex'); + const sqliteBase = process.env.database__connection__filename; + process.env.database__connection__filename = sqliteBase + ? `${sqliteBase.replace(/\.db$/i, '')}-${sqliteId}.db` + : `/tmp/ghost-test-${sqliteId}.db`; +} // Delete this slot's leftover sqlite file (+ sidecars) before Ghost loads, so a // reused pool name boots from a clean slate — see the note above. SQLITE LEG ONLY: diff --git a/packages/checkout/README.md b/packages/checkout/README.md new file mode 100644 index 00000000000..f9bb6f08501 --- /dev/null +++ b/packages/checkout/README.md @@ -0,0 +1,62 @@ +# @tryghost/checkout + +What Ghost's Stripe Checkout can collect, and where it lands. + +Three parties have to agree on this and none of them can import another's source: +Ghost Core builds the checkout session and validates what a publisher saves, Admin +offers the publisher only what that save would accept, and the end-to-end harness +models Stripe. A divergence between the first two is a setting that looks saved and +collects nothing. + +## What is in here + +- `STRIPE_ALLOWED_COUNTRIES` / `isStripeAllowedCountry` — the countries Stripe + Checkout accepts in `shipping_address_collection`. +- `STRIPE_PORTS` / `STRIPE_PORT` / `isStripePort` — the names Stripe returns + collected values under. +- `PORT_FIELD` — what each port supplies, and the custom field type that can hold it. +- `MAX_CHECKOUT_CUSTOM_FIELDS`, `MAX_CHECKOUT_LABEL_LENGTH`, + `CHECKOUT_ELIGIBLE_FIELD_TYPES` / `isCheckoutEligible` — the caps Stripe enforces + on checkout questions. + +## Measured, not read + +Every value here was established by `e2e/scripts/probe-stripe-constraints.ts` against +the live API at Ghost's pinned version, because the published artefacts disagree with +it: the OpenAPI spec carries no `maxItems` on `custom_fields` and states the +`customer_update` rule only in prose, and the SDK's `AllowedCountry` union omits `SD`, +which the API accepts. Re-probe before changing a value; do not read a new one off the +documentation. + +Two consequences of the country list are worth knowing before touching it. A code +Stripe rejects fails the whole session create, so it is checked when a publisher +chooses a country rather than when a member tries to buy. And there is no sentinel for +"everywhere": `allowed_countries` is the only key `shipping_address_collection` has, so +omitting or emptying it removes the parameter and the checkout silently collects no +address. Any internal "all countries" representation has to be expanded to the full list +before the session is built. + +Two guards live outside this package, where the things they compare live: +`ghost/core/test/unit/server/services/stripe/allowed-countries.test.ts` holds the list +against the Stripe SDK that Ghost pins, and against the separate copy the end-to-end +harness keeps. That copy is deliberate — a fake that shared this list could never catch +Ghost offering a country Stripe refuses. + +## Develop + +This is a workspace package in the Ghost monorepo. From the repo root: + +```bash +pnpm --filter @tryghost/checkout build # compile to build/ with tsc (ESM) +pnpm --filter @tryghost/checkout test # type-check + unit tests +``` + +In-monorepo consumers resolve this package via the `source` export condition (raw +`src/*.ts`, no build needed in dev/test). Production and any published tarball use the +compiled `build/` output. + +This package is ESM-only and compiled with `tsc` (`module: nodenext`). Relative imports +in `src/` must carry an explicit extension; write the real `.ts` one and `tsc` rewrites +it to `.js` on emit. `ghost/core` is CommonJS and consumes this through `require(esm)`, +which forbids top-level `await` anywhere in the module graph — keep module-level +initialization synchronous. diff --git a/packages/checkout/eslint.config.mjs b/packages/checkout/eslint.config.mjs new file mode 100644 index 00000000000..67e677f5105 --- /dev/null +++ b/packages/checkout/eslint.config.mjs @@ -0,0 +1,3 @@ +import { nodeLibConfig } from '@internal/cfg-eslint'; + +export default nodeLibConfig(); diff --git a/packages/checkout/package.json b/packages/checkout/package.json new file mode 100644 index 00000000000..f8cfefb1b99 --- /dev/null +++ b/packages/checkout/package.json @@ -0,0 +1,61 @@ +{ + "name": "@tryghost/checkout", + "version": "0.0.0", + "private": true, + "description": "Measured Stripe Checkout constraints: the countries it ships to, the values it returns, and the custom field types those land in", + "license": "MIT", + "author": "Ghost Foundation", + "repository": { + "type": "git", + "url": "git+https://github.com/TryGhost/Ghost.git", + "directory": "packages/checkout" + }, + "files": [ + "build" + ], + "type": "module", + "main": "build/index.js", + "types": "build/index.d.ts", + "exports": { + ".": { + "source": "./src/index.ts", + "types": "./build/index.d.ts", + "default": "./build/index.js" + } + }, + "scripts": { + "build": "tsc", + "test:unit": "NODE_ENV=testing vitest run --coverage", + "test": "pnpm run '/^test:/'", + "test:types": "tsc --noEmit -p test/tsconfig.json", + "lint:code": "eslint src/ --cache", + "lint:test": "eslint test/ --cache", + "lint": "pnpm run '/^lint:/'" + }, + "dependencies": { + "@tryghost/custom-field-types": "workspace:*" + }, + "devDependencies": { + "@internal/cfg-eslint": "workspace:*", + "@internal/cfg-typescript": "workspace:*", + "@internal/cfg-vitest": "workspace:*", + "@types/node": "catalog:", + "@typescript/native": "catalog:", + "@vitest/coverage-v8": "catalog:", + "eslint": "catalog:", + "typescript": "catalog:", + "vitest": "catalog:" + }, + "ghostPackage": { + "goldenPath": "compliant" + }, + "nx": { + "targets": { + "build": { + "outputs": [ + "{projectRoot}/build" + ] + } + } + } +} diff --git a/ghost/core/core/server/services/stripe/services/checkout/allowed-countries.ts b/packages/checkout/src/allowed-countries.ts similarity index 100% rename from ghost/core/core/server/services/stripe/services/checkout/allowed-countries.ts rename to packages/checkout/src/allowed-countries.ts diff --git a/ghost/core/core/server/services/tier-checkout-config/destinations.ts b/packages/checkout/src/destinations.ts similarity index 92% rename from ghost/core/core/server/services/tier-checkout-config/destinations.ts rename to packages/checkout/src/destinations.ts index 877cb094c5e..659cfb398f8 100644 --- a/ghost/core/core/server/services/tier-checkout-config/destinations.ts +++ b/packages/checkout/src/destinations.ts @@ -1,5 +1,5 @@ import type { FieldType } from '@tryghost/custom-field-types'; -import type { StripePort } from '../stripe/services/checkout/field-ports'; +import type { StripePort } from './field-ports.ts'; /** * What a port supplies, and what to call a field made to hold it. The type is a rule about diff --git a/ghost/core/core/server/services/stripe/services/checkout/field-ports.ts b/packages/checkout/src/field-ports.ts similarity index 100% rename from ghost/core/core/server/services/stripe/services/checkout/field-ports.ts rename to packages/checkout/src/field-ports.ts diff --git a/packages/checkout/src/index.ts b/packages/checkout/src/index.ts new file mode 100644 index 00000000000..afadf572717 --- /dev/null +++ b/packages/checkout/src/index.ts @@ -0,0 +1,28 @@ +/** + * What Ghost's Stripe Checkout can collect, and where it lands. + * + * Shared because three parties have to agree on it and cannot import each other's source: + * Ghost Core builds the session and validates what a publisher saves, Admin offers the + * publisher only what the save would accept, and a divergence between them is a setting + * that looks saved and collects nothing. + * + * Every value here was measured against the live Stripe API at Ghost's pinned version by + * `e2e/scripts/probe-stripe-constraints.ts`, not read from the docs or the SDK — both have + * disagreed with the API. Re-measure before changing one. + */ + +export { STRIPE_ALLOWED_COUNTRIES, isStripeAllowedCountry } from './allowed-countries.ts'; +export type { StripeAllowedCountry } from './allowed-countries.ts'; + +export { + CHECKOUT_ELIGIBLE_FIELD_TYPES, + MAX_CHECKOUT_CUSTOM_FIELDS, + MAX_CHECKOUT_LABEL_LENGTH, + STRIPE_PORT, + STRIPE_PORTS, + isCheckoutEligible, + isStripePort, +} from './field-ports.ts'; +export type { CheckoutEligibleFieldType, StripePort } from './field-ports.ts'; + +export { PORT_FIELD } from './destinations.ts'; diff --git a/packages/checkout/test/index.test.ts b/packages/checkout/test/index.test.ts new file mode 100644 index 00000000000..f6852a0a9e9 --- /dev/null +++ b/packages/checkout/test/index.test.ts @@ -0,0 +1,71 @@ +import { describe, it, assert } from 'vitest'; +import { + CHECKOUT_ELIGIBLE_FIELD_TYPES, + MAX_CHECKOUT_CUSTOM_FIELDS, + MAX_CHECKOUT_LABEL_LENGTH, + PORT_FIELD, + STRIPE_ALLOWED_COUNTRIES, + STRIPE_PORT, + STRIPE_PORTS, + isCheckoutEligible, + isStripeAllowedCountry, + isStripePort, +} from '../src/index.ts'; + +describe('allowed countries', function () { + it('accepts a country Stripe ships to', function () { + assert.ok(isStripeAllowedCountry('GB')); + // Absent from a general ISO country list, accepted by Stripe. + assert.ok(isStripeAllowedCountry('XK')); + assert.ok(isStripeAllowedCountry('ZZ')); + // Omitted by the pinned SDK's own union, accepted by the live API. + assert.ok(isStripeAllowedCountry('SD')); + }); + + it('refuses one it does not', function () { + // The usual slip for GB. Two letters, looks like a country, and Stripe refuses it. + assert.equal(isStripeAllowedCountry('UK'), false); + // Sanctioned, so present in a general country list and refused by Stripe. + assert.equal(isStripeAllowedCountry('KP'), false); + assert.equal(isStripeAllowedCountry('IR'), false); + }); + + it('carries no duplicates', function () { + assert.equal(new Set(STRIPE_ALLOWED_COUNTRIES).size, STRIPE_ALLOWED_COUNTRIES.length); + }); +}); + +describe('ports', function () { + it('recognises the names Stripe returns values under', function () { + for (const port of STRIPE_PORTS) { + assert.ok(isStripePort(port)); + } + assert.equal(isStripePort('email'), false); + }); + + it('names a field for every port, of a type that can hold what it returns', function () { + // Stripe returns a structured address for the address, plain text for the rest, so a + // port left out here would be collected into whatever a request happened to name. + assert.deepEqual(Object.keys(PORT_FIELD).sort(), [...STRIPE_PORTS].sort()); + assert.equal(PORT_FIELD[STRIPE_PORT.shippingAddress].type, 'address'); + assert.equal(PORT_FIELD[STRIPE_PORT.shippingName].type, 'short_text'); + assert.equal(PORT_FIELD[STRIPE_PORT.phone].type, 'short_text'); + }); +}); + +describe('checkout questions', function () { + it('can be asked in a type Stripe renders', function () { + for (const type of CHECKOUT_ELIGIBLE_FIELD_TYPES) { + assert.ok(isCheckoutEligible(type)); + } + // No Stripe equivalent: an address is collected through its own parameter. + assert.equal(isCheckoutEligible('address'), false); + // Stripe's text input caps shorter than this type allows. + assert.equal(isCheckoutEligible('long_text'), false); + }); + + it('states the caps Stripe enforces', function () { + assert.equal(MAX_CHECKOUT_CUSTOM_FIELDS, 3); + assert.equal(MAX_CHECKOUT_LABEL_LENGTH, 50); + }); +}); diff --git a/packages/checkout/test/tsconfig.json b/packages/checkout/test/tsconfig.json new file mode 100644 index 00000000000..1ac47d8d2d0 --- /dev/null +++ b/packages/checkout/test/tsconfig.json @@ -0,0 +1,8 @@ +{ + "extends": "../tsconfig.json", + "compilerOptions": { + "rootDir": "..", + "noEmit": true + }, + "include": ["../src/**/*", "**/*"] +} diff --git a/packages/checkout/tsconfig.json b/packages/checkout/tsconfig.json new file mode 100644 index 00000000000..9ae7d2cc13b --- /dev/null +++ b/packages/checkout/tsconfig.json @@ -0,0 +1,8 @@ +{ + "extends": "@internal/cfg-typescript/esm.json", + "compilerOptions": { + "rootDir": "src", + "outDir": "build" + }, + "include": ["src/**/*"] +} diff --git a/packages/checkout/vitest.config.ts b/packages/checkout/vitest.config.ts new file mode 100644 index 00000000000..fec5312f658 --- /dev/null +++ b/packages/checkout/vitest.config.ts @@ -0,0 +1,4 @@ +import { createVitestConfig } from '@internal/cfg-vitest'; + +// Pass overrides to tune coverage thresholds, setupFiles, etc. +export default createVitestConfig(); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1e88aeeccc5..b209b6c69b3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -787,6 +787,9 @@ importers: '@tryghost/admin-x-framework': specifier: workspace:* version: link:../admin-x-framework + '@tryghost/checkout': + specifier: workspace:* + version: link:../../packages/checkout '@tryghost/color-utils': specifier: 'catalog:' version: 0.2.20 @@ -1011,9 +1014,6 @@ importers: '@tanstack/react-query': specifier: 'catalog:' version: 5.101.4(react@18.3.1) - '@tinybirdco/charts': - specifier: 0.3.0 - version: 0.3.0(react-dom@18.3.1(react@18.3.1))(react@18.3.1) '@tryghost/custom-field-types': specifier: workspace:* version: link:../../packages/custom-field-types @@ -2152,6 +2152,9 @@ importers: '@playwright/test': specifier: 'catalog:' version: 1.61.1 + '@tryghost/checkout': + specifier: workspace:* + version: link:../packages/checkout '@tryghost/custom-field-types': specifier: workspace:* version: link:../packages/custom-field-types @@ -2269,6 +2272,9 @@ importers: '@tryghost/brute-knex': specifier: 'catalog:' version: 3.2.2(better-sqlite3@12.11.1)(express@4.22.2(supports-color@10.2.2))(mysql2@3.22.5(@types/node@22.20.1))(supports-color@10.2.2) + '@tryghost/checkout': + specifier: workspace:* + version: link:../../packages/checkout '@tryghost/color-utils': specifier: 'catalog:' version: 0.2.20 @@ -4045,6 +4051,40 @@ importers: specifier: 'catalog:' version: 4.1.10(@opentelemetry/api@1.9.1)(@types/node@22.20.1)(@vitest/browser-playwright@4.1.10)(@vitest/coverage-v8@4.1.10)(@vitest/ui@4.1.10)(jsdom@30.0.1(@noble/hashes@1.8.0))(msw@2.14.6(@types/node@22.20.1)(@typescript/typescript6@6.0.2))(vite@8.1.3(@types/node@22.20.1)(esbuild@0.28.2)(jiti@2.7.0)(terser@5.48.0)(tsx@4.23.12)(yaml@2.9.0)) + packages/checkout: + dependencies: + '@tryghost/custom-field-types': + specifier: workspace:* + version: link:../custom-field-types + devDependencies: + '@internal/cfg-eslint': + specifier: workspace:* + version: link:../../configs/eslint + '@internal/cfg-typescript': + specifier: workspace:* + version: link:../../configs/typescript + '@internal/cfg-vitest': + specifier: workspace:* + version: link:../../configs/vitest + '@types/node': + specifier: 'catalog:' + version: 22.20.1 + '@typescript/native': + specifier: 'catalog:' + version: typescript@7.0.2 + '@vitest/coverage-v8': + specifier: 'catalog:' + version: 4.1.10(@vitest/browser@4.1.10)(vitest@4.1.10) + eslint: + specifier: 'catalog:' + version: 9.39.4(jiti@2.7.0)(supports-color@10.2.2) + typescript: + specifier: 'catalog:' + version: '@typescript/typescript6@6.0.2' + vitest: + specifier: 'catalog:' + version: 4.1.10(@opentelemetry/api@1.9.1)(@types/node@22.20.1)(@vitest/browser-playwright@4.1.10)(@vitest/coverage-v8@4.1.10)(@vitest/ui@4.1.10)(jsdom@30.0.1(@noble/hashes@1.8.0))(msw@2.14.6(@types/node@22.20.1)(@typescript/typescript6@6.0.2))(vite@8.1.3(@types/node@22.20.1)(esbuild@0.28.2)(jiti@2.7.0)(terser@5.48.0)(tsx@4.23.12)(yaml@2.9.0)) + packages/custom-field-types: dependencies: zod: @@ -9201,12 +9241,6 @@ packages: '@textlint/types@15.8.0': resolution: {integrity: sha512-Anhc6y5736YIsvqae0U6k0YmB2M/QVHkEeOv2aydAn/WIkdI69dCOiDbe3/+RagS3qstFTSFWJzNRA2lUjv19w==} - '@tinybirdco/charts@0.3.0': - resolution: {integrity: sha512-nL/SswRPO8VBOjRBxpJ9o4O6dWCgvs2xcsYiucA7AR4dTTewydTy+2bPFBxkubuX/LFce9FLOGWwFy8yOIi37g==} - peerDependencies: - react: ^18.2.0 - react-dom: ^18.2.0 - '@tiptap/core@2.27.2': resolution: {integrity: sha512-ABL1N6eoxzDzC1bYvkMbvyexHacszsKdVPYqhl5GwHLOvpZcv9VE9QaKwDILTyz5voCA0lGcAAXZp+qnXOk5lQ==} peerDependencies: @@ -13339,9 +13373,6 @@ packages: ecdsa-sig-formatter@1.0.11: resolution: {integrity: sha512-nagl3RYrbNv6kQkeJIpt6NJZy8twLB/2vtz6yN9Z4vRKHN4/QZJIEbqohALSgwKdnksuY3k5Addp5lg8sVoVcQ==} - echarts@5.6.0: - resolution: {integrity: sha512-oTbVTsXfKuEhxftHqL5xprgLoc0k7uScAwtryCgWF6hPYFLRwOUHiFmHGCBKP5NPFNkDVopOieyUqYGH8Fa3kA==} - editions@1.3.4: resolution: {integrity: sha512-gzao+mxnYDzIysXKMQi/+M1mjy/rjestjg6OPoYTtI+3Izp23oiGZitsl9lPDPiTGXbcSIk1iJWhliSaglxnUg==} engines: {node: '>=0.8'} @@ -21315,11 +21346,6 @@ packages: engines: {node: '>=16'} hasBin: true - swr@2.4.2: - resolution: {integrity: sha512-ej644Y2bvkIajfR32KGeSSdBXQW+ScjGjkybZgSE7kFpk9eGnV44XY9FJylXi+W75pavSX1PVNB57W5EbhGIYw==} - peerDependencies: - react: ^16.11.0 || ^17.0.0 || ^18.0.0 || ^19.0.0 - symbol-tree@3.2.4: resolution: {integrity: sha512-9QNk5KwDF+Bvz+PyObkmSYjI5ksVUYtjW7AU22r2NKcfLJcXp96hkDWU3+XndOsUb+AQ9QhfzfCT2O+CNWT5Tw==} @@ -21738,9 +21764,6 @@ packages: tslib@1.14.1: resolution: {integrity: sha512-Xni35NKzjgMrwevysHTCArtLDpPvye8zV/0E4EyYn43P7/7qvQwPh9BGkHewbMulVntbigmcT7rdX3BNo9wRJg==} - tslib@2.3.0: - resolution: {integrity: sha512-N82ooyxVNm6h1riLCoyS9e3fuJ3AMG2zIZs2Gd1ATcSFjSA23Q0fzjjZeh0jbJvWVDZ0cJT8yaNNaaXHzueNjg==} - tslib@2.8.1: resolution: {integrity: sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==} @@ -22921,9 +22944,6 @@ packages: zod@4.4.3: resolution: {integrity: sha512-ytENFjIJFl2UwYglde2jchW2Hwm4GJFLDiSXWdTrJQBIN9Fcyp7n4DhxJEiWNAJMV1/BqWfW/kkg71UDcHJyTQ==} - zrender@5.6.1: - resolution: {integrity: sha512-OFXkDJKcrlx5su2XbzJvj/34Q3m6PvyCZkVPHGYpcCJ52ek4U/ymZyfuV1nKE23AyBJ51E/6Yr0mhZ7xGTO4ag==} - zustand@4.5.7: resolution: {integrity: sha512-CHOUy7mu3lbD6o6LJLfllpjkzhHXSBlX8B9+qPddUsIfeF5S/UZ5q0kmCsnRqT1UHFQZchNFDDzMbQsuesHWlw==} engines: {node: '>=12.7.0'} @@ -28694,13 +28714,6 @@ snapshots: dependencies: '@textlint/ast-node-types': 15.8.0 - '@tinybirdco/charts@0.3.0(react-dom@18.3.1(react@18.3.1))(react@18.3.1)': - dependencies: - echarts: 5.6.0 - react: 18.3.1 - react-dom: 18.3.1(react@18.3.1) - swr: 2.4.2(react@18.3.1) - '@tiptap/core@2.27.2(@tiptap/pm@2.27.2)': dependencies: '@tiptap/pm': 2.27.2 @@ -34505,11 +34518,6 @@ snapshots: dependencies: safe-buffer: 5.2.1 - echarts@5.6.0: - dependencies: - tslib: 2.3.0 - zrender: 5.6.1 - editions@1.3.4: {} editions@6.22.0: @@ -45647,12 +45655,6 @@ snapshots: picocolors: 1.1.1 sax: 1.6.0 - swr@2.4.2(react@18.3.1): - dependencies: - dequal: 2.0.3 - react: 18.3.1 - use-sync-external-store: 1.6.0(react@18.3.1) - symbol-tree@3.2.4: {} symlink-or-copy@1.3.1: {} @@ -46167,8 +46169,6 @@ snapshots: tslib@1.14.1: {} - tslib@2.3.0: {} - tslib@2.8.1: {} tsscmp@1.0.6: {} @@ -47650,10 +47650,6 @@ snapshots: zod@4.4.3: {} - zrender@5.6.1: - dependencies: - tslib: 2.3.0 - zustand@4.5.7(@types/react@18.3.31)(react@18.3.1): dependencies: use-sync-external-store: 1.6.0(react@18.3.1)