From d4542a1567342c2fc937f8c454148d184904fd4c Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Wed, 3 Jun 2026 07:43:27 +0100 Subject: [PATCH 1/8] Removed unused relations from llms (#28332) These relations were only used by the lazy router. This is a short-term change to reduce the amount of work needed to generate llms files and verify whether it improves latency in the wild. The llms entry fetches only use the post/page fields needed to build the index, full text output, and markdown URLs. Loading tags and authors here adds extra relation work without changing the generated llms output, so this removes those relations from both `/llms.txt` and `/llms-full.txt`. If this proves to improve latency in the wild, then further refactors will be needed for this to work with LazyRouter. --- .../core/frontend/services/llms/service.js | 6 +-- .../frontend/services/llms/service.test.js | 40 +++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/ghost/core/core/frontend/services/llms/service.js b/ghost/core/core/frontend/services/llms/service.js index d20f433259f..daebdc7b773 100644 --- a/ghost/core/core/frontend/services/llms/service.js +++ b/ghost/core/core/frontend/services/llms/service.js @@ -199,8 +199,7 @@ function createLlmsService({settingsCache, labs, config, urlServiceFacade, urlUt limit: 'all', order: type === 'post' ? 'published_at desc' : 'id asc', filter: `status:published+visibility:public+type:${type}`, - columns: ['id', 'title', 'slug', 'custom_excerpt', 'plaintext', 'published_at', 'type'], - withRelated: ['tags', 'authors'] + columns: ['id', 'title', 'slug', 'custom_excerpt', 'plaintext', 'published_at', 'type'] }); const entries = page.data.map((model) => { @@ -222,8 +221,7 @@ function createLlmsService({settingsCache, labs, config, urlServiceFacade, urlUt page: pageNum, order: type === 'post' ? 'published_at desc' : 'id asc', filter: `status:published+visibility:public+type:${type}`, - columns: ['id', 'title', 'slug', 'html', 'plaintext', 'custom_excerpt', 'updated_at', 'published_at', 'created_at', 'type'], - withRelated: ['tags', 'authors'] + columns: ['id', 'title', 'slug', 'html', 'plaintext', 'custom_excerpt', 'updated_at', 'published_at', 'created_at', 'type'] }); const entries = result.data.map((model) => { diff --git a/ghost/core/test/unit/frontend/services/llms/service.test.js b/ghost/core/test/unit/frontend/services/llms/service.test.js index 4de44e23bf9..a9322b5ea42 100644 --- a/ghost/core/test/unit/frontend/services/llms/service.test.js +++ b/ghost/core/test/unit/frontend/services/llms/service.test.js @@ -230,6 +230,46 @@ describe('Unit: frontend/services/llms/service', function () { assert.ok(callCount >= 4, `Expected at least 4 DB calls (2 per getLlmsTxt), got ${callCount}`); }); + it('does not load post relations for llms.txt index entries', async function () { + const calls = []; + const models = { + Post: { + findPage: async function (options) { + calls.push(options); + return {data: []}; + } + } + }; + + const service = createService({models, urlMap: {}}); + + await service.getLlmsTxt(); + + assert.equal(calls.length, 2); + assert.equal(calls[0].withRelated, undefined); + assert.equal(calls[1].withRelated, undefined); + }); + + it('does not load post relations for llms-full.txt entries', async function () { + const calls = []; + const models = { + Post: { + findPage: async function (options) { + calls.push(options); + return {data: []}; + } + } + }; + + const service = createService({models, urlMap: {}}); + + await service.getLlmsFullTxt(); + + assert.equal(calls.length, 2); + assert.equal(calls[0].withRelated, undefined); + assert.equal(calls[1].withRelated, undefined); + }); + describe('fetchPublicEntry', function () { it('calls the correct controller for pages vs posts', async function () { const calls = []; From c0e382175ff42924de44b1e789c0a5136eb0848b Mon Sep 17 00:00:00 2001 From: Peter Zimon Date: Wed, 3 Jun 2026 11:13:25 +0200 Subject: [PATCH 2/8] Added context menu component to Shade (#28339) ref https://linear.app/ghost/issue/DES-1401/add-context-menu-component-to-shade Adds a reusable Shade `ContextMenu` component backed by `@radix-ui/react-context-menu`, matching the compound component shape of the existing Shade dropdown menu. ## Details - Added `ContextMenu` primitives for trigger, content, items, checkbox/radio items, labels, separators, shortcuts, groups, portals, and submenus. - Added a Storybook story covering grouped actions, submenu behavior, checked states, disabled state, shortcuts, icons, and destructive item styling. - Exported the component from `@tryghost/shade/components`. - Added the Radix context menu dependency through the workspace catalog and lockfile. - Added Shade's direct `@typescript-eslint/eslint-plugin` dev dependency so the lint hook resolves the same ESLint 8-compatible TypeScript ESLint package as the parser. ## Validation - `pnpm --filter @tryghost/shade test:types` - `pnpm --filter @tryghost/shade build` - `pnpm install --lockfile-only --ignore-scripts` - Pre-commit lint-staged hook for the changed Shade TS/TSX files ref https://linear.app/ghost/issue/DES-1401/add-context-menu-comp-shade --- apps/shade/package.json | 2 + apps/shade/src/components.ts | 2 + .../components/ui/context-menu.stories.tsx | 86 ++++++++ apps/shade/src/components/ui/context-menu.tsx | 202 ++++++++++++++++++ pnpm-lock.yaml | 39 ++++ pnpm-workspace.yaml | 2 + 6 files changed, 333 insertions(+) create mode 100644 apps/shade/src/components/ui/context-menu.stories.tsx create mode 100644 apps/shade/src/components/ui/context-menu.tsx diff --git a/apps/shade/package.json b/apps/shade/package.json index b120dd6efa2..184dc5ce723 100644 --- a/apps/shade/package.json +++ b/apps/shade/package.json @@ -89,6 +89,7 @@ "@testing-library/react": "catalog:", "@types/node": "catalog:", "@types/react-world-flags": "1.6.0", + "@typescript-eslint/eslint-plugin": "catalog:", "@typescript-eslint/parser": "catalog:", "@vitejs/plugin-react": "catalog:", "@vitest/coverage-v8": "catalog:", @@ -119,6 +120,7 @@ "@radix-ui/react-alert-dialog": "1.1.15", "@radix-ui/react-avatar": "catalog:", "@radix-ui/react-checkbox": "catalog:", + "@radix-ui/react-context-menu": "catalog:", "@radix-ui/react-dialog": "1.1.15", "@radix-ui/react-dropdown-menu": "2.1.16", "@radix-ui/react-hover-card": "1.1.15", diff --git a/apps/shade/src/components.ts b/apps/shade/src/components.ts index 07e7548e196..826f5021fa5 100644 --- a/apps/shade/src/components.ts +++ b/apps/shade/src/components.ts @@ -11,6 +11,7 @@ export * from './components/ui/card'; export * from './components/ui/chart'; export * from './components/ui/checkbox'; export * from './components/ui/command'; +export * from './components/ui/context-menu'; export * from './components/ui/data-list'; export * from './components/ui/dialog'; export * from './components/ui/dropdown-menu'; @@ -50,6 +51,7 @@ export * from './components/ui/tooltip'; export * from './components/ui/trend-badge'; export type {DropdownMenuCheckboxItemProps as DropdownMenuCheckboxItemProps} from '@radix-ui/react-dropdown-menu'; +export type {ContextMenuCheckboxItemProps as ContextMenuCheckboxItemProps} from '@radix-ui/react-context-menu'; export {IconComponents as Icon} from './components/ui/icon'; diff --git a/apps/shade/src/components/ui/context-menu.stories.tsx b/apps/shade/src/components/ui/context-menu.stories.tsx new file mode 100644 index 00000000000..98a90a455f8 --- /dev/null +++ b/apps/shade/src/components/ui/context-menu.stories.tsx @@ -0,0 +1,86 @@ +import type {Meta, StoryObj} from '@storybook/react-vite'; +import {Archive, Copy, Pencil, Share, Trash} from 'lucide-react'; +import {ContextMenu, ContextMenuCheckboxItem, ContextMenuContent, ContextMenuGroup, ContextMenuItem, ContextMenuLabel, ContextMenuPortal, ContextMenuRadioGroup, ContextMenuRadioItem, ContextMenuSeparator, ContextMenuShortcut, ContextMenuSub, ContextMenuSubContent, ContextMenuSubTrigger, ContextMenuTrigger} from './context-menu'; + +const meta = { + title: 'Components / Context menu', + component: ContextMenu, + tags: ['autodocs'], + parameters: { + docs: { + description: { + component: 'Right-click menu for object-specific actions. Use when commands are secondary to the main surface and need pointer or long-press access.' + } + } + } +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +const TriggerBox = () => ( + + Right click or long press + +); + +export const Default: Story = { + parameters: { + docs: { + description: { + story: 'Includes groups, separators, submenus, disabled items, checkbox and radio states, destructive styling, icons, and keyboard shortcuts.' + } + } + }, + args: { + children: [ + , + + Post actions + + + + + Edit + E + + + + Duplicate + D + + + + Share + + + + + Show in list + Pin to top + + + + + + Move to + + + + + Drafts + Archive + + + + + Locked action + + + + Delete + + + ] + } +}; diff --git a/apps/shade/src/components/ui/context-menu.tsx b/apps/shade/src/components/ui/context-menu.tsx new file mode 100644 index 00000000000..b6756d42a06 --- /dev/null +++ b/apps/shade/src/components/ui/context-menu.tsx @@ -0,0 +1,202 @@ +import * as React from 'react'; +import * as ContextMenuPrimitive from '@radix-ui/react-context-menu'; +import {Check, ChevronRight, Circle} from 'lucide-react'; + +import {cn} from '@/lib/utils'; +import {SHADE_APP_NAMESPACES} from '@/shade-app'; + +const ContextMenu = ContextMenuPrimitive.Root; + +const ContextMenuTrigger = ContextMenuPrimitive.Trigger; + +const ContextMenuGroup = ContextMenuPrimitive.Group; + +const ContextMenuPortal = ContextMenuPrimitive.Portal; + +const ContextMenuSub = ContextMenuPrimitive.Sub; + +const ContextMenuRadioGroup = ContextMenuPrimitive.RadioGroup; + +const ContextMenuSubTrigger = React.forwardRef< + React.ElementRef, + React.ComponentPropsWithoutRef & { + inset?: boolean + } +>(({className, inset, children, ...props}, ref) => ( + + {children} + + +)); +ContextMenuSubTrigger.displayName = ContextMenuPrimitive.SubTrigger.displayName; + +const ContextMenuSubContent = React.forwardRef< + React.ElementRef, + React.ComponentPropsWithoutRef +>(({className, ...props}, ref) => ( +
+ +
+)); +ContextMenuSubContent.displayName = ContextMenuPrimitive.SubContent.displayName; + +const ContextMenuContent = React.forwardRef< + React.ElementRef, + React.ComponentPropsWithoutRef +>(({className, ...props}, ref) => ( + +
+ +
+
+)); +ContextMenuContent.displayName = ContextMenuPrimitive.Content.displayName; + +const ContextMenuItem = React.forwardRef< + React.ElementRef, + React.ComponentPropsWithoutRef & { + inset?: boolean, + variant?: 'default' | 'destructive' + } +>(({className, inset, variant = 'default', ...props}, ref) => ( + svg]:size-4 [&>svg]:shrink-0', + variant === 'destructive' && 'text-destructive focus:bg-destructive/10 focus:text-destructive', + inset && 'pl-8', + className + )} + {...props} + /> +)); +ContextMenuItem.displayName = ContextMenuPrimitive.Item.displayName; + +const ContextMenuCheckboxItem = React.forwardRef< + React.ElementRef, + React.ComponentPropsWithoutRef +>(({className, children, checked, ...props}, ref) => ( + + + + + + + {children} + +)); +ContextMenuCheckboxItem.displayName = ContextMenuPrimitive.CheckboxItem.displayName; + +const ContextMenuRadioItem = React.forwardRef< + React.ElementRef, + React.ComponentPropsWithoutRef +>(({className, children, ...props}, ref) => ( + + + + + + + {children} + +)); +ContextMenuRadioItem.displayName = ContextMenuPrimitive.RadioItem.displayName; + +const ContextMenuLabel = React.forwardRef< + React.ElementRef, + React.ComponentPropsWithoutRef & { + inset?: boolean + } +>(({className, inset, ...props}, ref) => ( + +)); +ContextMenuLabel.displayName = ContextMenuPrimitive.Label.displayName; + +const ContextMenuSeparator = React.forwardRef< + React.ElementRef, + React.ComponentPropsWithoutRef +>(({className, ...props}, ref) => ( + +)); +ContextMenuSeparator.displayName = ContextMenuPrimitive.Separator.displayName; + +const ContextMenuShortcut = ({ + className, + ...props +}: React.HTMLAttributes) => { + return ( + + ); +}; +ContextMenuShortcut.displayName = 'ContextMenuShortcut'; + +export { + ContextMenu, + ContextMenuTrigger, + ContextMenuContent, + ContextMenuItem, + ContextMenuCheckboxItem, + ContextMenuRadioItem, + ContextMenuLabel, + ContextMenuSeparator, + ContextMenuShortcut, + ContextMenuGroup, + ContextMenuPortal, + ContextMenuSub, + ContextMenuSubContent, + ContextMenuSubTrigger, + ContextMenuRadioGroup +}; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1f8889520ab..acf4ff0875f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -24,6 +24,9 @@ catalogs: '@radix-ui/react-checkbox': specifier: 1.3.3 version: 1.3.3 + '@radix-ui/react-context-menu': + specifier: 2.2.16 + version: 2.2.16 '@radix-ui/react-form': specifier: 0.1.8 version: 0.1.8 @@ -129,6 +132,9 @@ catalogs: '@types/validator': specifier: 13.15.10 version: 13.15.10 + '@typescript-eslint/eslint-plugin': + specifier: 8.49.0 + version: 8.49.0 '@typescript-eslint/parser': specifier: 8.49.0 version: 8.49.0 @@ -1425,6 +1431,9 @@ importers: '@radix-ui/react-checkbox': specifier: 'catalog:' version: 1.3.3(@types/react-dom@18.3.7(@types/react@18.3.29))(@types/react@18.3.29)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) + '@radix-ui/react-context-menu': + specifier: 'catalog:' + version: 2.2.16(@types/react-dom@18.3.7(@types/react@18.3.29))(@types/react@18.3.29)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) '@radix-ui/react-dialog': specifier: 1.1.15 version: 1.1.15(@types/react-dom@18.3.7(@types/react@18.3.29))(@types/react@18.3.29)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) @@ -1549,6 +1558,9 @@ importers: '@types/react-world-flags': specifier: 1.6.0 version: 1.6.0 + '@typescript-eslint/eslint-plugin': + specifier: 'catalog:' + version: 8.49.0(@typescript-eslint/parser@8.49.0(eslint@8.57.1)(typescript@5.9.3))(eslint@8.57.1)(typescript@5.9.3) '@typescript-eslint/parser': specifier: 'catalog:' version: 8.49.0(eslint@8.57.1)(typescript@5.9.3) @@ -6241,6 +6253,19 @@ packages: '@types/react': optional: true + '@radix-ui/react-context-menu@2.2.16': + resolution: {integrity: sha512-O8morBEW+HsVG28gYDZPTrT9UUovQUlJue5YO836tiTJhuIWBm/zQHc7j388sHWtdH/xUZurK9olD2+pcqx5ww==} + peerDependencies: + '@types/react': '*' + '@types/react-dom': '*' + react: ^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc + react-dom: ^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc + peerDependenciesMeta: + '@types/react': + optional: true + '@types/react-dom': + optional: true + '@radix-ui/react-context@1.1.2': resolution: {integrity: sha512-jCi/QKUM2r1Ju5a3J64TH2A5SpKAgh0LpknyqdQ4m6DCV0xJ2HG1xARRwNGPQfi1SLdLWZ1OJz6F4OMBBNiGJA==} peerDependencies: @@ -25858,6 +25883,20 @@ snapshots: optionalDependencies: '@types/react': 18.3.29 + '@radix-ui/react-context-menu@2.2.16(@types/react-dom@18.3.7(@types/react@18.3.29))(@types/react@18.3.29)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)': + dependencies: + '@radix-ui/primitive': 1.1.3 + '@radix-ui/react-context': 1.1.2(@types/react@18.3.29)(react@18.3.1) + '@radix-ui/react-menu': 2.1.16(@types/react-dom@18.3.7(@types/react@18.3.29))(@types/react@18.3.29)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) + '@radix-ui/react-primitive': 2.1.3(@types/react-dom@18.3.7(@types/react@18.3.29))(@types/react@18.3.29)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) + '@radix-ui/react-use-callback-ref': 1.1.1(@types/react@18.3.29)(react@18.3.1) + '@radix-ui/react-use-controllable-state': 1.2.2(@types/react@18.3.29)(react@18.3.1) + react: 18.3.1 + react-dom: 18.3.1(react@18.3.1) + optionalDependencies: + '@types/react': 18.3.29 + '@types/react-dom': 18.3.7(@types/react@18.3.29) + '@radix-ui/react-context@1.1.2(@types/react@18.3.29)(react@18.3.1)': dependencies: react: 18.3.1 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 5c25ce92910..bd4bfa2bff7 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -31,6 +31,7 @@ catalog: '@playwright/test': 1.60.0 '@radix-ui/react-avatar': 1.1.11 '@radix-ui/react-checkbox': 1.3.3 + '@radix-ui/react-context-menu': 2.2.16 '@radix-ui/react-form': 0.1.8 '@radix-ui/react-popover': 1.1.15 '@radix-ui/react-separator': 1.1.8 @@ -67,6 +68,7 @@ catalog: '@types/react': 18.3.29 '@types/react-dom': 18.3.7 '@types/validator': 13.15.10 + '@typescript-eslint/eslint-plugin': 8.49.0 '@typescript-eslint/parser': 8.49.0 '@vitejs/plugin-react': 4.7.0 '@vitest/coverage-v8': 4.1.7 From d6b88b281d355e04858ec6a45318557991af0a19 Mon Sep 17 00:00:00 2001 From: Sag Date: Wed, 3 Jun 2026 13:28:39 +0200 Subject: [PATCH 3/8] Added extra DOM validation for AP notifications (#28338) ref https://linear.app/ghost/issue/SC-45/ ref https://linear.app/ghost/issue/SC-35/ no issue - Added DOMPurify sanitization for `dangerouslySetInnerHTML` usages - The content is already sanitised on the server side. However, as best practice, we avoid trusting any server-provided content and sanitise it again on the frontend before rendering --- apps/activitypub/package.json | 2 +- .../src/views/notifications/notifications.tsx | 6 +- .../test/acceptance/dom-validation.test.ts | 71 ++++++ .../unit/utils/content-formatters.test.ts | 207 +++++++++++++----- 4 files changed, 232 insertions(+), 54 deletions(-) diff --git a/apps/activitypub/package.json b/apps/activitypub/package.json index cd41e1103f0..a8c5509bf3b 100644 --- a/apps/activitypub/package.json +++ b/apps/activitypub/package.json @@ -1,6 +1,6 @@ { "name": "@tryghost/activitypub", - "version": "3.1.32", + "version": "3.1.33", "license": "MIT", "repository": { "type": "git", diff --git a/apps/activitypub/src/views/notifications/notifications.tsx b/apps/activitypub/src/views/notifications/notifications.tsx index 0a3b0957649..acdebf11587 100644 --- a/apps/activitypub/src/views/notifications/notifications.tsx +++ b/apps/activitypub/src/views/notifications/notifications.tsx @@ -17,7 +17,7 @@ import {Notification, isApiError} from '@src/api/activitypub'; import {handleProfileClick} from '@utils/handle-profile-click'; import {renderFeedAttachment} from '@components/feed/feed-item'; import {renderTimestamp} from '@src/utils/render-timestamp'; -import {stripHtml} from '@src/utils/content-formatters'; +import {sanitizeHtml, stripHtml} from '@src/utils/content-formatters'; import {useNavigateWithBasePath} from '@src/hooks/use-navigate-with-base-path'; import {useNotificationsForUser} from '@hooks/use-activity-pub-queries'; @@ -192,7 +192,7 @@ const ProfileLinkedContent: React.FC<{ return (
@@ -436,7 +436,7 @@ const Notifications: React.FC = () => { (group.type !== 'reply' && group.type !== 'mention' ?
{group.post?.type === 'article' && group.post?.title && <>{group.post.title} — } - +
: <>
diff --git a/apps/activitypub/test/acceptance/dom-validation.test.ts b/apps/activitypub/test/acceptance/dom-validation.test.ts index f96e83d0a1e..15ee970f21a 100644 --- a/apps/activitypub/test/acceptance/dom-validation.test.ts +++ b/apps/activitypub/test/acceptance/dom-validation.test.ts @@ -232,4 +232,75 @@ test.describe('DOM validation for rendered AP content', async () => { const excerptText = await excerptElement.textContent(); expect(excerptText).toContain('">Data + VBScript + Encoded JavaScript + `); + + const links = renderHtml(result).querySelectorAll('a'); + + expect(links).toHaveLength(5); + links.forEach((link) => { + expect(link.getAttribute('href')).toBeNull(); + }); + }); + + it('strips unsupported tags while keeping anchor text readable', function () { + const result = sanitizeStrippedHtml(` +

Hello bold safe link

+ + Image + + + `); + + const div = renderHtml(result); + + expect(div.querySelector('p')).toBeNull(); + expect(div.querySelector('strong')).toBeNull(); + expect(div.querySelector('script')).toBeNull(); + expect(div.querySelector('img')).toBeNull(); + expect(div.querySelector('svg')).toBeNull(); + expect(div.querySelector('iframe')).toBeNull(); + expect(div.querySelector('a')?.textContent).toBe('safe link'); + expect(div.textContent).toContain('Hello bold safe link'); + }); + + it('returns plain text when stripHtml has no exclusions', function () { + const result = sanitizeHtml(stripHtml(` +

Hello safe link

+
+ Bold + `)); + + const div = renderHtml(result); + + expect(div.querySelector('a')).toBeNull(); + expect(div.querySelector('br')).toBeNull(); + expect(div.textContent).toBe('Hello safe link Bold'); + }); }); }); From fd134be69c442f11e2e2a29eb720330e5e4a500d Mon Sep 17 00:00:00 2001 From: Jonatan Svennberg Date: Wed, 3 Jun 2026 13:43:07 +0200 Subject: [PATCH 4/8] =?UTF-8?q?=F0=9F=8E=A8=20Add=20archived=20tiers=20to?= =?UTF-8?q?=20tier=20filter=20(#28189)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ref https://linear.app/ghost/issue/BER-3588/add-archived-tiers-to-tiers-filter Members can keep archived paid tiers, so the filter needs to preserve those options while separating them from active tiers. --- .../filter-sources/use-tier-value-source.ts | 47 ++++- .../members/components/members-filters.tsx | 7 +- .../members/use-member-filter-fields.test.ts | 17 ++ .../unit/hooks/use-tier-value-source.test.tsx | 171 ++++++++++++++++++ 4 files changed, 232 insertions(+), 10 deletions(-) create mode 100644 apps/posts/test/unit/hooks/use-tier-value-source.test.tsx diff --git a/apps/posts/src/hooks/filter-sources/use-tier-value-source.ts b/apps/posts/src/hooks/filter-sources/use-tier-value-source.ts index 3081ab0c08c..23b0effe487 100644 --- a/apps/posts/src/hooks/filter-sources/use-tier-value-source.ts +++ b/apps/posts/src/hooks/filter-sources/use-tier-value-source.ts @@ -1,15 +1,54 @@ -import {FilterOption, ValueSource} from '@tryghost/shade/patterns'; import {createLocalValueSource} from './create-local-value-source'; +import {getActiveTiers, getArchivedTiers, useBrowseTiers} from '@tryghost/admin-x-framework/api/tiers'; +import {useMemo} from 'react'; +import type {FilterOption, ValueSource} from '@tryghost/shade/patterns'; +import type {Tier} from '@tryghost/admin-x-framework/api/tiers'; + +interface TierValueSource { + valueSource: ValueSource; + // True once more than one paid tier (active or archived) exists, i.e. when + // filtering members by tier is meaningful. Stays false until tiers load. + hasMultipleTiers: boolean; +} + +function toTierFilterOption(tier: Tier): FilterOption { + return { + value: tier.id, + label: tier.active ? tier.name : `${tier.name} (archived)`, + detail: tier.slug + }; +} + +// Active tiers first, then archived; each group keeps the order returned by the API. +function buildTierFilterOptions(tiers: Tier[]): FilterOption[] { + return [ + ...getActiveTiers(tiers).map(toTierFilterOption), + ...getArchivedTiers(tiers).map(toTierFilterOption) + ]; +} + +export function useTierValueSource(): TierValueSource { + // The tiers endpoint returns every match in a single response, so no paging or + // limit is needed; `type:paid` keeps free/complimentary tiers out of the filter. + const {data: tiersData, isLoading} = useBrowseTiers({ + searchParams: {filter: 'type:paid'} + }); + + const tiers = useMemo(() => tiersData?.tiers ?? [], [tiersData?.tiers]); + const options = useMemo(() => buildTierFilterOptions(tiers), [tiers]); + const hasMultipleTiers = tiers.length > 1; -export function useTierValueSource(options: FilterOption[] = []): ValueSource { const useLocalTierValueSource = createLocalValueSource, string>({ id: 'posts.tiers.local', useItems: () => ({ data: options, - isLoading: false + isLoading }), toOption: option => option }); - return useLocalTierValueSource(); + return { + valueSource: useLocalTierValueSource(), + hasMultipleTiers + }; } diff --git a/apps/posts/src/views/members/components/members-filters.tsx b/apps/posts/src/views/members/components/members-filters.tsx index d9a41e2c233..f6d4eccd566 100644 --- a/apps/posts/src/views/members/components/members-filters.tsx +++ b/apps/posts/src/views/members/components/members-filters.tsx @@ -13,7 +13,6 @@ import {getSettingValue, useBrowseSettings} from '@tryghost/admin-x-framework/ap import {getSiteTimezone} from '@src/utils/get-site-timezone'; import {useBrowseNewsletters} from '@tryghost/admin-x-framework/api/newsletters'; import {useBrowseOffers} from '@tryghost/admin-x-framework/api/offers'; -import {useBrowseTiers} from '@tryghost/admin-x-framework/api/tiers'; import {useEmailPostValueSource} from '@src/hooks/filter-sources/use-email-post-value-source'; import {useLabelValueSource} from '@src/hooks/filter-sources/use-label-value-source'; import {usePostResourceValueSource} from '@src/hooks/filter-sources/use-post-resource-value-source'; @@ -55,7 +54,6 @@ const MembersFilters: React.FC = ({ activeView, iconOnly = false }) => { - const {data: tiersData} = useBrowseTiers({searchParams: {limit: '100'}}); const {data: offersData} = useBrowseOffers({}); const {data: newslettersData} = useBrowseNewsletters({searchParams: {limit: '100'}}); const {data: settingsData} = useBrowseSettings({}); @@ -68,11 +66,8 @@ const MembersFilters: React.FC = ({ const emailTrackClicks = getSettingValue(settings, 'email_track_clicks') === true; const siteTimezone = getSiteTimezone(settings); - const tiers = tiersData?.tiers || []; const newsletters = newslettersData?.newsletters || []; const offers = useMemo(() => offersData?.offers ?? EMPTY_OFFERS, [offersData?.offers]); - const activePaidTiers = tiers.filter(tier => tier.type === 'paid' && tier.active); - const hasMultipleTiers = activePaidTiers.length > 1; const offersOptions = useMemo(() => { return buildOfferOptions(offers); @@ -98,7 +93,7 @@ const MembersFilters: React.FC = ({ const postValueSource = usePostResourceValueSource(); const emailValueSource = useEmailPostValueSource(); const labelValueSource = useLabelValueSource(); - const tierValueSource = useTierValueSource(activePaidTiers.map(tier => ({value: tier.id, label: tier.name, detail: tier.slug}))); + const {valueSource: tierValueSource, hasMultipleTiers} = useTierValueSource(); const filterFields = useMemberFilterFields({ newsletters, diff --git a/apps/posts/src/views/members/use-member-filter-fields.test.ts b/apps/posts/src/views/members/use-member-filter-fields.test.ts index 93fb8da4b24..1d9de40f7d0 100644 --- a/apps/posts/src/views/members/use-member-filter-fields.test.ts +++ b/apps/posts/src/views/members/use-member-filter-fields.test.ts @@ -171,6 +171,23 @@ describe('useMemberFilterFields', () => { expect(statusField?.options?.map(o => o.value)).toEqual(['paid', 'free', 'comped', 'gift']); }); + it('includes the membership tier filter when multiple paid tiers are available', () => { + const {result} = renderHook(() => useMemberFilterFields({ + paidMembersEnabled: true, + hasMultipleTiers: true, + tierValueSource, + siteTimezone: 'UTC' + })); + + const subscriptionFields = result.current.find(group => group.group === 'Subscription')?.fields ?? []; + const tierField = subscriptionFields.find(field => field.key === 'tier_id'); + + expect(subscriptionFields.map(field => field.key)).toContain('tier_id'); + expect(tierField).toMatchObject({ + valueSource: tierValueSource + }); + }); + it('hydrates grouped retention offers on the offer field', () => { const {result} = renderHook(() => useMemberFilterFields({ paidMembersEnabled: true, diff --git a/apps/posts/test/unit/hooks/use-tier-value-source.test.tsx b/apps/posts/test/unit/hooks/use-tier-value-source.test.tsx new file mode 100644 index 00000000000..d7fc140ad7b --- /dev/null +++ b/apps/posts/test/unit/hooks/use-tier-value-source.test.tsx @@ -0,0 +1,171 @@ +import {beforeEach, describe, expect, it, vi} from 'vitest'; +import {renderHook} from '@testing-library/react'; +import {useTierValueSource} from '@src/hooks/filter-sources/use-tier-value-source'; +import type {Tier} from '@tryghost/admin-x-framework/api/tiers'; + +const {mockUseBrowseTiers} = vi.hoisted(() => ({ + mockUseBrowseTiers: vi.fn() +})); + +vi.mock('@tryghost/admin-x-framework/api/tiers', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useBrowseTiers: mockUseBrowseTiers + }; +}); + +function tier(overrides: Partial): Tier { + return { + id: 'tier-id', + name: 'Tier', + description: null, + slug: 'tier', + active: true, + type: 'paid', + welcome_page_url: null, + created_at: '2024-01-01T00:00:00.000Z', + updated_at: '2024-01-01T00:00:00.000Z', + visibility: 'public', + benefits: [], + trial_days: 0, + ...overrides + }; +} + +function mockTiersResponse({ + tiers = [], + isLoading = false +}: { + tiers?: Tier[]; + isLoading?: boolean; +} = {}) { + mockUseBrowseTiers.mockReturnValue({ + data: {tiers}, + isLoading + }); +} + +describe('useTierValueSource', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('fetches every paid tier in a single request', () => { + mockTiersResponse(); + + renderHook(() => useTierValueSource()); + + expect(mockUseBrowseTiers).toHaveBeenCalledWith({searchParams: {filter: 'type:paid'}}); + }); + + it('handles the first render before any tiers data has loaded', () => { + mockUseBrowseTiers.mockReturnValue({data: undefined, isLoading: true}); + + const {result} = renderHook(() => { + const source = useTierValueSource(); + return { + hasMultipleTiers: source.hasMultipleTiers, + state: source.valueSource.useOptions({query: '', selectedValues: []}) + }; + }); + + expect(result.current.hasMultipleTiers).toBe(false); + expect(result.current.state.options).toEqual([]); + expect(result.current.state.isInitialLoad).toBe(true); + }); + + it('exposes active tiers before archived tiers, labelling archived ones', () => { + mockTiersResponse({ + tiers: [ + tier({id: 'archived', name: 'Archived Gold', slug: 'archived-gold', active: false}), + tier({id: 'active', name: 'Active Gold', slug: 'active-gold', active: true}) + ] + }); + + const {result} = renderHook(() => { + const {valueSource} = useTierValueSource(); + return valueSource.useOptions({query: '', selectedValues: []}); + }); + + expect(result.current.options).toEqual([ + { + value: 'active', + label: 'Active Gold', + detail: 'active-gold' + }, + { + value: 'archived', + label: 'Archived Gold (archived)', + detail: 'archived-gold' + } + ]); + }); + + it('counts paid tiers when deciding if the tier filter is available', () => { + const cases = [ + { + tiers: [tier({id: 'active', active: true}), tier({id: 'archived', active: false})], + expected: true + }, + { + tiers: [tier({id: 'archived-1', active: false}), tier({id: 'archived-2', active: false})], + expected: true + }, + { + tiers: [tier({id: 'archived', active: false})], + expected: false + }, + { + tiers: [], + expected: false + } + ]; + + for (const testCase of cases) { + mockTiersResponse({tiers: testCase.tiers}); + + const {result} = renderHook(() => useTierValueSource()); + + expect(result.current.hasMultipleTiers).toBe(testCase.expected); + } + }); + + it('searches local tier options by archived label text', () => { + mockTiersResponse({ + tiers: [ + tier({id: 'active-tier', name: 'Active Gold', slug: 'active-gold', active: true}), + tier({id: 'archived-tier', name: 'Archived Gold', slug: 'archived-gold', active: false}) + ] + }); + + const {result} = renderHook(() => { + const {valueSource} = useTierValueSource(); + return valueSource.useOptions({query: 'archived', selectedValues: []}); + }); + + expect(result.current.options).toEqual([ + { + value: 'archived-tier', + label: 'Archived Gold (archived)', + detail: 'archived-gold' + } + ]); + }); + + it('reports the initial load state while tiers are loading', () => { + mockTiersResponse({tiers: [], isLoading: true}); + + const {result} = renderHook(() => { + const source = useTierValueSource(); + return { + hasMultipleTiers: source.hasMultipleTiers, + state: source.valueSource.useOptions({query: '', selectedValues: []}) + }; + }); + + expect(result.current.hasMultipleTiers).toBe(false); + expect(result.current.state.options).toEqual([]); + expect(result.current.state.isInitialLoad).toBe(true); + }); +}); From aad73cb8e135087b12be5d4c49fb98cd6a316682 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Wed, 3 Jun 2026 09:13:12 -0500 Subject: [PATCH 5/8] Started inserting automation run when member signs up (#28263) towards https://linear.app/ghost/issue/NY-1286 ref #28120 **I recommend [reviewing this with whitespace changes disabled](https://github.com/TryGhost/Ghost/pull/28263/changes?w=1).** When a member signs up, we should trigger an automation run and possibly insert a new row. Nothing happens yet when these rows are inserted. That will be done in a future change. This is development-only; triggering is a no-op in production. I tested this by: - Adding automated tests. - Verified that the old member welcome email is still sent on member signup. - Creating this temporary endpoint and verifying that the results looked expected after member signup: ```js router.get('/automations/database-dump', (_req, res) => { const db = getTemporaryFakeAutomationsDatabase(); res.json({ automation_runs: db.prepare('SELECT * FROM automation_runs').all(), automation_run_steps: db.prepare('SELECT * FROM automation_run_steps').all() }); }); ``` --- .../services/automations/automations-api.ts | 26 + .../automations/automations-repository.ts | 5 + .../fake-database-automations-repository.ts | 127 ++++ ...database.js => temporary-fake-database.ts} | 33 +- .../server/services/gifts/gift-service.ts | 15 +- .../members/members-api/members-api.js | 2 + .../repositories/member-repository.js | 81 ++- .../automations-repository.test.ts | 205 +++++++ .../services/gifts/gift-service.test.ts | 25 +- .../repositories/member-repository.test.js | 573 ++++++++++-------- 10 files changed, 779 insertions(+), 313 deletions(-) rename ghost/core/core/server/services/automations/{temporary-fake-database.js => temporary-fake-database.ts} (92%) create mode 100644 ghost/core/test/unit/server/services/automations/automations-repository.test.ts diff --git a/ghost/core/core/server/services/automations/automations-api.ts b/ghost/core/core/server/services/automations/automations-api.ts index 61e555cc347..a87499031ae 100644 --- a/ghost/core/core/server/services/automations/automations-api.ts +++ b/ghost/core/core/server/services/automations/automations-api.ts @@ -6,10 +6,12 @@ import type {DatabaseSync} from 'node:sqlite'; import {z} from 'zod'; import {createFakeDatabaseAutomationsRepository} from './fake-database-automations-repository'; import type { + AutomationsRepository, EditAutomationData } from './automations-repository'; const domainEvents = require('@tryghost/domain-events'); +const labs = require('../../../shared/labs'); const StartAutomationsPollEvent = require('./events/start-automations-poll-event'); const temporaryFakeAutomationsDatabase = require('./temporary-fake-database'); @@ -238,6 +240,30 @@ export function requestPoll() { domainEvents.dispatch(StartAutomationsPollEvent.create()); } +type TriggerOptions = Parameters[0] & { + event: 'member_sign_up'; +}; +export async function trigger(options: TriggerOptions) { + if (options.event !== 'member_sign_up') { + throw new errors.IncorrectUsageError({ + message: 'Member signup is the only supported event right now. More may be added later' + }); + } + + const isAllowedEnvironment = ( + process.env.NODE_ENV === 'development' || + process.env.NODE_ENV?.startsWith('testing') + ); + const shouldTrigger = isAllowedEnvironment && labs.isSet('automations'); + if (!shouldTrigger) { + return; + } + + await repository.trigger(options); + + requestPoll(); +} + export function _resetTestDatabase() { if (process.env.NODE_ENV?.startsWith('testing')) { testDatabase = null; diff --git a/ghost/core/core/server/services/automations/automations-repository.ts b/ghost/core/core/server/services/automations/automations-repository.ts index 6e5b16e64d4..bc9e3a184b9 100644 --- a/ghost/core/core/server/services/automations/automations-repository.ts +++ b/ghost/core/core/server/services/automations/automations-repository.ts @@ -66,4 +66,9 @@ export interface AutomationsRepository { browse(): Promise>; getById(id: string): Promise; edit(id: string, data: EditAutomationData): Promise; + trigger(options: { + memberEmail: string; + memberId: string; + memberStatus: 'free' | 'paid'; + }): Promise; } diff --git a/ghost/core/core/server/services/automations/fake-database-automations-repository.ts b/ghost/core/core/server/services/automations/fake-database-automations-repository.ts index d4b2e7dc996..9f2d9a42114 100644 --- a/ghost/core/core/server/services/automations/fake-database-automations-repository.ts +++ b/ghost/core/core/server/services/automations/fake-database-automations-repository.ts @@ -2,6 +2,7 @@ import errors from '@tryghost/errors'; import tpl from '@tryghost/tpl'; import ObjectId from 'bson-objectid'; import type {DatabaseSync} from 'node:sqlite'; +import {MEMBER_WELCOME_EMAIL_SLUGS} from '../member-welcome-emails/constants'; import type { Automation, AutomationAction, @@ -12,6 +13,8 @@ import type { Page } from './automations-repository'; +const HOUR_MS = 60 * 60 * 1000; + const messages = { invalidAutomationActionRevision: 'Automation action "{actionId}" of type "{actionType}" is missing required revision field "{field}".', conflictingAutomationActionId: 'Automation action "{actionId}" already exists and cannot be inserted.', @@ -44,6 +47,14 @@ interface EdgeRow { target_action_id: string; } +type NextActionRevisionRow = { + automation_id: string; + action_id: string; + automation_action_revision_id: string; + type: 'wait' | 'send_email'; + wait_hours: number | null; +}; + export function createFakeDatabaseAutomationsRepository({ getDatabase }: { @@ -98,6 +109,16 @@ export function createFakeDatabaseAutomationsRepository({ return buildAutomation(database, updatedAutomation); }); + }, + + async trigger(options: { + memberEmail: string; + memberId: string; + memberStatus: 'free' | 'paid'; + }): Promise { + const database = getDatabase(); + + return withTransaction(database, () => trigger(database, options)); } }; } @@ -115,6 +136,112 @@ function withTransaction(database: DatabaseSync, operation: () => T): T { } } +function trigger(database: DatabaseSync, { + memberEmail, + memberId, + memberStatus +}: Readonly<{ + memberEmail: string; + memberId: string; + memberStatus: 'free' | 'paid'; +}>): void { + const firstAction = findFirstActionRevision(database, memberStatus); + if (!firstAction) { + return; + } + + const now = new Date(); + const nowString = now.toISOString(); + + const readyAt = getReadyAtForAction(firstAction, now); + + const run = { + id: ObjectId().toHexString(), + created_at: nowString, + updated_at: nowString, + automation_id: firstAction.automation_id, + member_id: memberId, + member_email: memberEmail + }; + + database.prepare(` + INSERT INTO automation_runs + (id, created_at, updated_at, automation_id, member_id, member_email) VALUES + (:id, :created_at, :updated_at, :automation_id, :member_id, :member_email) + `).run(run); + database.prepare(` + INSERT INTO automation_run_steps + (id, created_at, updated_at, automation_run_id, automation_action_revision_id, ready_at) VALUES + (:id, :created_at, :updated_at, :automation_run_id, :automation_action_revision_id, :ready_at) + `).run({ + id: ObjectId().toHexString(), + created_at: nowString, + updated_at: nowString, + automation_run_id: run.id, + automation_action_revision_id: firstAction.automation_action_revision_id, + ready_at: readyAt.toISOString() + }); +} + +function findFirstActionRevision(database: DatabaseSync, memberStatus: 'free' | 'paid'): NextActionRevisionRow | null { + const automationSlug: NonNullable = MEMBER_WELCOME_EMAIL_SLUGS[memberStatus]; + + const row = database.prepare(` + SELECT + automation.id AS automation_id, + actions.id AS action_id, + revisions.id AS automation_action_revision_id, + actions.type AS type, + revisions.wait_hours AS wait_hours + FROM automations automation + INNER JOIN automation_actions actions ON actions.automation_id = automation.id + INNER JOIN automation_action_revisions revisions ON revisions.action_id = actions.id + WHERE automation.slug = ? + AND automation.status = 'active' + AND actions.deleted_at IS NULL + AND NOT EXISTS ( + SELECT 1 + FROM automation_action_edges edge + INNER JOIN automation_actions source_actions ON source_actions.id = edge.source_action_id + AND source_actions.deleted_at IS NULL + WHERE edge.target_action_id = actions.id + ) + AND revisions.created_at = ( + SELECT MAX(created_at) + FROM automation_action_revisions + WHERE action_id = actions.id + ) + ORDER BY actions.created_at, actions.id + LIMIT 1 + `).get(automationSlug) as NextActionRevisionRow | undefined; + + return row ?? null; +} + +function getReadyAtForAction( + action: Pick, + now: Readonly +): Date { + switch (action.type) { + case 'wait': { + const waitHours = requireValue({ + ...action, + id: action.action_id + }, 'wait_hours'); + const waitMs = waitHours * HOUR_MS; + return new Date(now.getTime() + waitMs); + } + case 'send_email': + return now; + default: { + const _exhaustive: never = action.type; + throw new errors.IncorrectUsageError({ + message: `Unexpected action type ${_exhaustive}` + }); + } + } +} + function loadAutomation(database: DatabaseSync, automationId: string): AutomationRow | null { const automation = database.prepare(` SELECT id, slug, name, status, created_at, updated_at diff --git a/ghost/core/core/server/services/automations/temporary-fake-database.js b/ghost/core/core/server/services/automations/temporary-fake-database.ts similarity index 92% rename from ghost/core/core/server/services/automations/temporary-fake-database.js rename to ghost/core/core/server/services/automations/temporary-fake-database.ts index 0d09019a6ba..b1c3f96b3c1 100644 --- a/ghost/core/core/server/services/automations/temporary-fake-database.js +++ b/ghost/core/core/server/services/automations/temporary-fake-database.ts @@ -10,16 +10,16 @@ * migration once we're sure this schema is correct. */ -const errors = require('@tryghost/errors'); -const ObjectId = require('bson-objectid').default; +import * as errors from '@tryghost/errors'; +import ObjectId from 'bson-objectid'; +import type {DatabaseSync} from 'node:sqlite'; -/** - * @returns {import('node:sqlite').DatabaseSync} - */ -function createTemporaryFakeAutomationsDatabase() { - const {DatabaseSync} = require('node:sqlite'); +export function createTemporaryFakeAutomationsDatabase(): DatabaseSync { + // We want to do this import dynamically. + // eslint-disable-next-line @typescript-eslint/no-require-imports + const sqlite = require('node:sqlite'); - const database = new DatabaseSync(':memory:'); + const database = new sqlite.DatabaseSync(':memory:'); database.exec('PRAGMA foreign_keys = ON;'); const id = () => ObjectId().toHexString(); @@ -98,10 +98,10 @@ CREATE TABLE automation_run_steps ( automation_run_id TEXT NOT NULL REFERENCES automation_runs(id), automation_action_revision_id TEXT NOT NULL REFERENCES automation_action_revisions(id), ready_at TEXT NOT NULL, - step_attempts INTEGER NOT NULL, + step_attempts INTEGER NOT NULL DEFAULT 0, started_at TEXT, finished_at TEXT, - status TEXT NOT NULL, + status TEXT NOT NULL DEFAULT 'pending', locked_by TEXT, locked_at TEXT ) STRICT; @@ -312,13 +312,9 @@ CREATE TABLE automation_run_steps ( return database; } -/** @type {null | import('node:sqlite').DatabaseSync} */ -let cachedDatabase = null; +let cachedDatabase: DatabaseSync | null = null; -/** - * @returns {import('node:sqlite').DatabaseSync} - */ -function getTemporaryFakeAutomationsDatabase() { +export function getTemporaryFakeAutomationsDatabase(): DatabaseSync { if (process.env.NODE_ENV !== 'development') { throw new errors.IncorrectUsageError({ message: 'Fake automations database should only be used in development' @@ -326,7 +322,4 @@ function getTemporaryFakeAutomationsDatabase() { } cachedDatabase ??= createTemporaryFakeAutomationsDatabase(); return cachedDatabase; -} - -exports.createTemporaryFakeAutomationsDatabase = createTemporaryFakeAutomationsDatabase; -exports.getTemporaryFakeAutomationsDatabase = getTemporaryFakeAutomationsDatabase; +} \ No newline at end of file diff --git a/ghost/core/core/server/services/gifts/gift-service.ts b/ghost/core/core/server/services/gifts/gift-service.ts index 8e04794cdfb..df225553609 100644 --- a/ghost/core/core/server/services/gifts/gift-service.ts +++ b/ghost/core/core/server/services/gifts/gift-service.ts @@ -6,7 +6,6 @@ import type {GiftRepository} from './gift-repository'; import type {GiftReminderScheduler} from './gift-reminder-scheduler'; import tpl from '@tryghost/tpl'; import {GIFT_REMINDER_FLOOR_DAYS, GIFT_REMINDER_LEAD_DAYS} from './constants'; -import {MEMBER_WELCOME_EMAIL_SLUGS} from '../member-welcome-emails/constants'; const MS_PER_DAY = 24 * 60 * 60 * 1000; const GIFT_REMINDER_LEAD_MS = GIFT_REMINDER_LEAD_DAYS * MS_PER_DAY; @@ -38,7 +37,12 @@ interface MemberModel { interface MemberRepository { get(filter: Record, options?: Record): Promise; update(data: Record, options?: Record): Promise; - enqueueWelcomeEmailRun(memberId: string, slug: string, options?: Record): Promise; + triggerMemberSignupAutomation( + memberId: string, + memberEmail: string, + memberStatus: 'free' | 'paid', + options?: Record + ): Promise; } type Tier = { @@ -306,7 +310,12 @@ export class GiftService { await this.deps.giftRepository.update(redeemed, {transacting}); // Gift members receive the paid welcome email, as they receive access to paid content - await this.deps.memberRepository.enqueueWelcomeEmailRun(memberId, MEMBER_WELCOME_EMAIL_SLUGS.paid, {transacting}); + await this.deps.memberRepository.triggerMemberSignupAutomation( + memberId, + member.get('email'), + 'paid', + {transacting} + ); return {redeemed, member}; }; diff --git a/ghost/core/core/server/services/members/members-api/members-api.js b/ghost/core/core/server/services/members/members-api/members-api.js index ff5085e0100..d2c5d3c6558 100644 --- a/ghost/core/core/server/services/members/members-api/members-api.js +++ b/ghost/core/core/server/services/members/members-api/members-api.js @@ -19,6 +19,7 @@ const WellKnownController = require('./controllers/well-known-controller'); const {EmailSuppressedEvent} = require('../../email-suppression-list/email-suppression-list'); const MagicLink = require('../../lib/magic-link/magic-link'); const DomainEvents = require('@tryghost/domain-events'); +const automationsApi = require('../../automations/automations-api'); module.exports = function MembersAPI({ tokenConfig: { @@ -104,6 +105,7 @@ module.exports = function MembersAPI({ tokenService, newslettersService, productRepository, + automationsApi, Automation, WelcomeEmailAutomationRun, Member, diff --git a/ghost/core/core/server/services/members/members-api/repositories/member-repository.js b/ghost/core/core/server/services/members/members-api/repositories/member-repository.js index a3dfbdfa419..bec03ddfef0 100644 --- a/ghost/core/core/server/services/members/members-api/repositories/member-repository.js +++ b/ghost/core/core/server/services/members/members-api/repositories/member-repository.js @@ -11,6 +11,7 @@ const crypto = require('crypto'); const hasActiveOffer = require('../utils/has-active-offer'); const StartAutomationsPollEvent = require('../../../automations/events/start-automations-poll-event'); const {MEMBER_WELCOME_EMAIL_SLUGS} = require('../../../member-welcome-emails/constants'); +/** @import * as automationsApi from '../../../automations/automations-api' */ const messages = { noStripeConnection: 'Cannot {action} without a Stripe Connection', @@ -66,6 +67,7 @@ module.exports = class MemberRepository { * @param {any} deps.offersAPI * @param {ITokenService} deps.tokenService * @param {any} deps.newslettersService + * @param {Pick} deps.automationsApi * @param {any} deps.Automation * @param {any} deps.WelcomeEmailAutomationRun */ @@ -87,6 +89,7 @@ module.exports = class MemberRepository { offersAPI, tokenService, newslettersService, + automationsApi, Automation, WelcomeEmailAutomationRun }) { @@ -107,6 +110,7 @@ module.exports = class MemberRepository { this._offersAPI = offersAPI; this.tokenService = tokenService; this._newslettersService = newslettersService; + this._automationsApi = automationsApi; this._Automation = Automation; this._WelcomeEmailAutomationRun = WelcomeEmailAutomationRun; @@ -173,28 +177,40 @@ module.exports = class MemberRepository { return nickname && nickname.toLowerCase() === 'complimentary'; } + /** + * @param {string} memberId + * @param {string} memberEmail + * @param {'free' | 'paid'} memberStatus + * @returns {Promise} + */ + async #triggerMemberSignupAutomation(memberId, memberEmail, memberStatus) { + // TODO(NY-1311) When moving to real tables, we should insert the new + // rows in a new transaction. + await this._automationsApi.trigger({ + event: 'member_sign_up', + memberId, + memberEmail, + memberStatus + }); + } + /** * Looks up the active welcome email automation for the given slug and enqueues a * `WelcomeEmailAutomationRun` for the member. Dispatches `StartAutomationsPollEvent` - * so the poll picks it up. Returns the created run, or null if there is no active - * automation/email for that slug. - * - * Callers are responsible for any eligibility gating (member status, source, etc.) - * before calling this — this helper just looks up + inserts + dispatches. Pass - * `options.transacting` to run the insert inside an existing transaction; the - * dispatch is automatically deferred until that transaction commits. + * so the poll picks it up. * * @param {string} memberId - * @param {string} slug automation slug, see MEMBER_WELCOME_EMAIL_SLUGS - * @param {object} [options] bookshelf options (transacting, context, etc.) + * @param {'free' | 'paid'} memberStatus + * @param {object} options + * @returns {Promise} */ - async enqueueWelcomeEmailRun(memberId, slug, options = {}) { + async #triggerMemberSignupLegacyAutomation(memberId, memberStatus, options) { if (!this._Automation || !this._WelcomeEmailAutomationRun) { - return null; + return; } const automation = await this._Automation.findOne( - {slug}, + {slug: MEMBER_WELCOME_EMAIL_SLUGS[memberStatus]}, {...options, withRelated: ['welcomeEmailAutomatedEmail']} ); const email = automation?.related('welcomeEmailAutomatedEmail'); @@ -206,10 +222,10 @@ module.exports = class MemberRepository { ); if (!isActive) { - return null; + return; } - const run = await this._WelcomeEmailAutomationRun.add({ + await this._WelcomeEmailAutomationRun.add({ welcome_email_automation_id: automation.id, member_id: memberId, next_welcome_email_automated_email_id: email.id, @@ -220,8 +236,25 @@ module.exports = class MemberRepository { }, options); this.dispatchEvent(StartAutomationsPollEvent.create(), options); + } - return run; + /** + * Trigger an automation for member signup. + * + * Callers are responsible for any eligibility gating (member status, source, etc.) + * before calling this. + * + * @param {string} memberId + * @param {string} memberEmail + * @param {'free' | 'paid'} memberStatus + * @param {object} bookshelfOptions + * @returns {Promise} + */ + async triggerMemberSignupAutomation(memberId, memberEmail, memberStatus, bookshelfOptions) { + await Promise.all([ + this.#triggerMemberSignupAutomation(memberId, memberEmail, memberStatus), + this.#triggerMemberSignupLegacyAutomation(memberId, memberStatus, bookshelfOptions) + ]); } /** @@ -437,7 +470,12 @@ module.exports = class MemberRepository { labels }, {...memberAddOptions, transacting}); - await this.enqueueWelcomeEmailRun(newMember.id, MEMBER_WELCOME_EMAIL_SLUGS.free, {transacting}); + await this.triggerMemberSignupAutomation( + newMember.id, + newMember.get('email'), + 'free', + {transacting} + ); return newMember; }; @@ -1527,8 +1565,8 @@ module.exports = class MemberRepository { const context = options?.context || {}; const source = this._resolveContextSource(context); - // Enqueue paid welcome email if: - // 1. The source is allowed to send welcome emails + // Enqueue automation if: + // 1. The source is allowed to trigger automations // 2. The member status changed to 'paid' // 3. The previous status wasn't 'gift', as gift members already received the paid welcome email on redemption if ( @@ -1536,7 +1574,12 @@ module.exports = class MemberRepository { updatedMember.get('status') === 'paid' && updatedMember._previousAttributes.status !== 'gift' ) { - await this.enqueueWelcomeEmailRun(memberModel.id, MEMBER_WELCOME_EMAIL_SLUGS.paid, options); + await this.triggerMemberSignupAutomation( + memberModel.id, + memberModel.get('email'), + 'paid', + options + ); } } } 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 new file mode 100644 index 00000000000..1c2e5312398 --- /dev/null +++ b/ghost/core/test/unit/server/services/automations/automations-repository.test.ts @@ -0,0 +1,205 @@ +import assert from 'node:assert/strict'; +import {AutomationsRepository} from '../../../../../core/server/services/automations/automations-repository'; +import {createTemporaryFakeAutomationsDatabase} from '../../../../../core/server/services/automations/temporary-fake-database'; +import {createFakeDatabaseAutomationsRepository} from '../../../../../core/server/services/automations/fake-database-automations-repository'; +import type {DatabaseSync, SQLInputValue} from 'node:sqlite'; + +const addHours = (dateCol: unknown, hours: number): Date => { + assert(typeof dateCol === 'string', 'Expected date column to be a string'); + const start = new Date(dateCol).valueOf(); + const delta = hours * 60 * 60 * 1000; + return new Date(start + delta); +}; + +// These tests are partly coupled to the *fake* repository. We should be able to +// modify it once we have the real repository. +describe('automations repository', function () { + let database: DatabaseSync; + let repo: AutomationsRepository; + + const getRunByMemberEmail = (email: string) => ( + database!.prepare(` + SELECT + automation_runs.*, + automations.slug AS automation_slug + FROM automation_runs + INNER JOIN automations ON automations.id = automation_runs.automation_id + WHERE automation_runs.member_email = ? + `).get(email) + ); + + const getStepByRunId = (runId: SQLInputValue) => ( + database!.prepare(` + SELECT + automation_run_steps.*, + automation_actions.id AS action_id, + automation_actions.type AS action_type, + automation_action_revisions.wait_hours AS wait_hours, + automation_action_revisions.email_subject AS email_subject + FROM automation_run_steps + INNER JOIN automation_action_revisions ON automation_action_revisions.id = automation_run_steps.automation_action_revision_id + INNER JOIN automation_actions ON automation_actions.id = automation_action_revisions.action_id + WHERE automation_run_steps.automation_run_id = ? + `).get(runId) + ); + + const getAutomationBySlug = async (slug: string) => { + const automationSummaries = await repo.browse(); + const automationSummary = automationSummaries.data.find(automation => automation.slug === slug); + assert(automationSummary); + const automation = await repo.getById(automationSummary.id); + assert(automation); + return automation; + }; + + const getRunCountByAutomationId = (automationId: SQLInputValue) => { + const result = database!.prepare(` + SELECT COUNT(*) AS count + FROM automation_runs + WHERE automation_id = ? + `).get(automationId); + return result?.count; + }; + + beforeEach(function () { + database = createTemporaryFakeAutomationsDatabase(); + repo = createFakeDatabaseAutomationsRepository({ + getDatabase: () => database + }); + }); + + afterEach(function () { + database.close(); + }); + + describe('trigger', function () { + it('can trigger an automation for a free member', async function () { + await repo.trigger({ + memberEmail: 'free@example.com', + memberId: 'member_123', + memberStatus: 'free' + }); + + const run = getRunByMemberEmail('free@example.com'); + assert(run); + assert.equal(run.member_email, 'free@example.com'); + assert.equal(run.member_id, 'member_123'); + assert.equal(run.automation_slug, 'member-welcome-email-free'); + assert.equal(run.created_at, run.updated_at); + + const step = getStepByRunId(run.id); + assert(step); + assert.equal(step.automation_run_id, run.id); + assert.equal(step.action_type, 'wait'); + assert.equal(step.wait_hours, 48); + assert.equal(step.created_at, run.created_at); + assert.equal(step.updated_at, run.updated_at); + assert.equal(step.ready_at, addHours(run.created_at, 48).toISOString()); + assert.equal(step.step_attempts, 0); + assert.equal(step.started_at, null); + assert.equal(step.finished_at, null); + assert.equal(step.status, 'pending'); + assert.equal(step.locked_by, null); + assert.equal(step.locked_at, null); + }); + + it('can trigger an automation for a paid member', async function () { + await repo.trigger({ + memberEmail: 'paid@example.com', + memberId: 'member_123', + memberStatus: 'paid' + }); + + const run = getRunByMemberEmail('paid@example.com'); + assert(run); + assert.equal(run.automation_slug, 'member-welcome-email-paid'); + + const step = getStepByRunId(run.id); + assert(step); + assert.equal(step.automation_run_id, run.id); + assert.equal(step.action_type, 'wait'); + }); + + it('inserts the first non-deleted step', async function () { + const automation = await getAutomationBySlug('member-welcome-email-free'); + await repo.edit(automation.id, { + status: 'active', + actions: [ + { + id: 'wait-action-to-delete', + type: 'wait', + data: {wait_hours: 72} + }, + { + id: 'main-wait-action', + type: 'wait', + data: {wait_hours: 24} + } + ], + edges: [{ + source_action_id: 'wait-action-to-delete', + target_action_id: 'main-wait-action' + }] + }); + await repo.edit(automation.id, { + status: 'active', + actions: [ + { + id: 'main-wait-action', + type: 'wait', + data: {wait_hours: 24} + } + ], + edges: [] + }); + + await repo.trigger({ + memberEmail: 'free@example.com', + memberId: 'member_123', + memberStatus: 'free' + }); + + const run = getRunByMemberEmail('free@example.com'); + assert(run); + + const step = getStepByRunId(run.id); + assert(step); + assert.equal(step.action_id, 'main-wait-action'); + }); + + it('does not trigger an automation for an inactive automation', async function () { + const freeAutomation = await getAutomationBySlug('member-welcome-email-free'); + await repo.edit(freeAutomation.id, { + ...freeAutomation, + status: 'inactive' + }); + + await repo.trigger({ + memberEmail: 'inactive-free@example.com', + memberId: 'member_123', + memberStatus: 'free' + }); + + assert.equal(getRunByMemberEmail('inactive-free@example.com'), undefined); + assert.equal(getRunCountByAutomationId(freeAutomation.id), 0); + }); + + it('does not trigger an automation for an automation with no actions', async function () { + const freeAutomation = await getAutomationBySlug('member-welcome-email-free'); + await repo.edit(freeAutomation.id, { + status: 'active', + actions: [], + edges: [] + }); + + await repo.trigger({ + memberEmail: 'free-no-actions@example.com', + memberId: 'member_123', + memberStatus: 'free' + }); + + assert.equal(getRunByMemberEmail('free-no-actions@example.com'), undefined); + assert.equal(getRunCountByAutomationId(freeAutomation.id), 0); + }); + }); +}); diff --git a/ghost/core/test/unit/server/services/gifts/gift-service.test.ts b/ghost/core/test/unit/server/services/gifts/gift-service.test.ts index fc41f3a64bf..352352bedb9 100644 --- a/ghost/core/test/unit/server/services/gifts/gift-service.test.ts +++ b/ghost/core/test/unit/server/services/gifts/gift-service.test.ts @@ -48,7 +48,7 @@ describe('GiftService', function () { let memberRepository: { get: sinon.SinonStub; update: sinon.SinonStub; - enqueueWelcomeEmailRun: sinon.SinonStub; + triggerMemberSignupAutomation: sinon.SinonStub; }; let staffServiceEmails: { notifyGiftPurchased: sinon.SinonStub; @@ -101,7 +101,7 @@ describe('GiftService', function () { return Promise.resolve({id: 'member_1', get: memberGet}); }), update: sinon.stub().resolves(undefined), - enqueueWelcomeEmailRun: sinon.stub().resolves(undefined) + triggerMemberSignupAutomation: sinon.stub().resolves(undefined) }; staffServiceEmails = { notifyGiftPurchased: sinon.stub(), @@ -1265,7 +1265,7 @@ describe('GiftService', function () { sinon.assert.notCalled(staffServiceEmails.notifyGiftSubscriptionStarted); }); - it('enqueues the paid welcome email run for a new gift signup', async function () { + it('triggers the paid member signup automation for a new gift signup', async function () { const gift = buildGift(); const memberGet = sinon.stub(); memberGet.withArgs('status').returns('gift'); @@ -1279,14 +1279,15 @@ describe('GiftService', function () { await service.redeem('gift-token', 'member_1', {newMember: true}); sinon.assert.calledOnceWithExactly( - memberRepository.enqueueWelcomeEmailRun, + memberRepository.triggerMemberSignupAutomation, 'member_1', - 'member-welcome-email-paid', + 'member@example.com', + 'paid', {transacting: 'trx'} ); }); - it('enqueues the paid welcome email run when an existing free member redeems a gift', async function () { + it('triggers the paid member signup automation when an existing free member redeems a gift', async function () { const gift = buildGift(); const memberGet = sinon.stub(); memberGet.withArgs('status').returns('free'); @@ -1300,14 +1301,15 @@ describe('GiftService', function () { await service.redeem('gift-token', 'member_1'); sinon.assert.calledOnceWithExactly( - memberRepository.enqueueWelcomeEmailRun, + memberRepository.triggerMemberSignupAutomation, 'member_1', - 'member-welcome-email-paid', + 'member@example.com', + 'paid', {transacting: 'trx'} ); }); - it('passes the external transaction through to the welcome email enqueue', async function () { + it('passes the external transaction through to the member signup automation trigger', async function () { const gift = buildGift(); const memberGet = sinon.stub(); memberGet.withArgs('status').returns('free'); @@ -1322,9 +1324,10 @@ describe('GiftService', function () { await service.redeem('gift-token', 'member_1', {transacting: externalTrx}); sinon.assert.calledOnceWithExactly( - memberRepository.enqueueWelcomeEmailRun, + memberRepository.triggerMemberSignupAutomation, 'member_1', - 'member-welcome-email-paid', + 'member@example.com', + 'paid', {transacting: externalTrx} ); }); diff --git a/ghost/core/test/unit/server/services/members/members-api/repositories/member-repository.test.js b/ghost/core/test/unit/server/services/members/members-api/repositories/member-repository.test.js index 23832589119..96383dbaf6c 100644 --- a/ghost/core/test/unit/server/services/members/members-api/repositories/member-repository.test.js +++ b/ghost/core/test/unit/server/services/members/members-api/repositories/member-repository.test.js @@ -22,6 +22,7 @@ describe('MemberRepository', function () { let WelcomeEmailAutomationRun; let newslettersService; let offersAPI; + let automationsApi; let productRepository; let stripeAPIService; let tokenService; @@ -47,6 +48,7 @@ describe('MemberRepository', function () { WelcomeEmailAutomationRun, newslettersService, offersAPI, + automationsApi, productRepository, stripeAPIService, tokenService, @@ -164,6 +166,10 @@ describe('MemberRepository', function () { }) }; + automationsApi = { + trigger: sinon.stub().resolves() + }; + productRepository = { get: sinon.stub().resolves({ get: sinon.stub().returns(), @@ -1704,150 +1710,164 @@ describe('MemberRepository', function () { }); }); - describe('create - automation run integration', function () { - it('creates automation run for free member signup (free welcome email)', async function () { - const repo = buildRepo({ - Member, - Outbox, - WelcomeEmailAutomationRun, - MemberStatusEvent, - MemberSubscribeEventModel: MemberSubscribeEvent, - newslettersService, - Automation, - OfferRedemption: mockOfferRedemption - }); - + describe('create - automation integration', function () { + it('triggers an automation event for free signup', async function () { + const repo = buildRepo(); await repo.create({email: 'test@example.com', name: 'Test Member'}, {}); - sinon.assert.calledOnce(WelcomeEmailAutomationRun.add); - const runCall = WelcomeEmailAutomationRun.add.firstCall.args[0]; - assert.equal(runCall.welcome_email_automation_id, 'automation_id_free'); - assert.equal(runCall.member_id, 'member_id_123'); - assert.equal(runCall.next_welcome_email_automated_email_id, 'automated_email_id_free'); - assert.ok(runCall.ready_at); - assert.equal(runCall.step_started_at, null); - assert.equal(runCall.step_attempts, 0); - assert.equal(runCall.exit_reason, null); + sinon.assert.calledOnceWithExactly(automationsApi.trigger, { + event: 'member_sign_up', + memberId: 'member_id_123', + memberEmail: 'test@example.com', + memberStatus: 'free' + }); }); - it('does not create automation run for disallowed sources', async function () { - const repo = buildRepo({ - Member, - Outbox, - WelcomeEmailAutomationRun, - MemberStatusEvent, - MemberSubscribeEventModel: MemberSubscribeEvent, - newslettersService, - Automation, - OfferRedemption: mockOfferRedemption - }); + describe('legacy automations', function () { + it('creates automation run for free member signup (free welcome email)', async function () { + const repo = buildRepo({ + Member, + Outbox, + WelcomeEmailAutomationRun, + MemberStatusEvent, + MemberSubscribeEventModel: MemberSubscribeEvent, + newslettersService, + Automation, + OfferRedemption: mockOfferRedemption + }); - const disallowedSources = [ - {name: 'import', context: {import: true}}, - {name: 'admin', context: {user: true}}, - {name: 'api', context: {api_key: true}} - ]; + await repo.create({email: 'test@example.com', name: 'Test Member'}, {}); + + sinon.assert.calledOnce(WelcomeEmailAutomationRun.add); + const runCall = WelcomeEmailAutomationRun.add.firstCall.args[0]; + assert.equal(runCall.welcome_email_automation_id, 'automation_id_free'); + assert.equal(runCall.member_id, 'member_id_123'); + assert.equal(runCall.next_welcome_email_automated_email_id, 'automated_email_id_free'); + assert.ok(runCall.ready_at); + assert.equal(runCall.step_started_at, null); + assert.equal(runCall.step_attempts, 0); + assert.equal(runCall.exit_reason, null); + }); + + it('does not create automation run for disallowed sources', async function () { + const repo = buildRepo({ + Member, + Outbox, + WelcomeEmailAutomationRun, + MemberStatusEvent, + MemberSubscribeEventModel: MemberSubscribeEvent, + newslettersService, + Automation, + OfferRedemption: mockOfferRedemption + }); - for (const source of disallowedSources) { - WelcomeEmailAutomationRun.add.resetHistory(); - await repo.create({email: 'test@example.com', name: 'Test Member'}, {context: source.context}); - sinon.assert.notCalled(WelcomeEmailAutomationRun.add); - } - }); + const disallowedSources = [ + {name: 'import', context: {import: true}}, + {name: 'admin', context: {user: true}}, + {name: 'api', context: {api_key: true}} + ]; - it('passes transaction to automation run creation', async function () { - const repo = buildRepo({ - Member, - Outbox, - WelcomeEmailAutomationRun, - MemberStatusEvent, - MemberSubscribeEventModel: MemberSubscribeEvent, - newslettersService, - Automation, - OfferRedemption: mockOfferRedemption + for (const source of disallowedSources) { + WelcomeEmailAutomationRun.add.resetHistory(); + await repo.create({email: 'test@example.com', name: 'Test Member'}, {context: source.context}); + sinon.assert.notCalled(WelcomeEmailAutomationRun.add); + } }); - await repo.create({email: 'test@example.com', name: 'Test Member'}, {}); + it('passes transaction to automation run creation', async function () { + const repo = buildRepo({ + Member, + Outbox, + WelcomeEmailAutomationRun, + MemberStatusEvent, + MemberSubscribeEventModel: MemberSubscribeEvent, + newslettersService, + Automation, + OfferRedemption: mockOfferRedemption + }); - const runOptions = WelcomeEmailAutomationRun.add.firstCall.args[1]; - assert.ok(runOptions.transacting); - }); + await repo.create({email: 'test@example.com', name: 'Test Member'}, {}); - it('does NOT create automation run when welcome email is inactive', async function () { - Automation.findOne.resolves({ - get: sinon.stub().callsFake((key) => { - const data = {status: 'inactive'}; - return data[key]; - }), - related: sinon.stub().callsFake((relation) => { - assert.equal(relation, 'welcomeEmailAutomatedEmail'); - return { - get: sinon.stub().callsFake((key) => { - const data = {lexical: '{"root":{}}'}; - return data[key]; - }) - }; - }) + const runOptions = WelcomeEmailAutomationRun.add.firstCall.args[1]; + assert.ok(runOptions.transacting); }); - const repo = buildRepo({ - Member, - Outbox, - WelcomeEmailAutomationRun, - MemberStatusEvent, - MemberSubscribeEventModel: MemberSubscribeEvent, - newslettersService, - Automation, - OfferRedemption: mockOfferRedemption - }); + it('does NOT create automation run when welcome email is inactive', async function () { + Automation.findOne.resolves({ + get: sinon.stub().callsFake((key) => { + const data = {status: 'inactive'}; + return data[key]; + }), + related: sinon.stub().callsFake((relation) => { + assert.equal(relation, 'welcomeEmailAutomatedEmail'); + return { + get: sinon.stub().callsFake((key) => { + const data = {lexical: '{"root":{}}'}; + return data[key]; + }) + }; + }) + }); - await repo.create({email: 'test@example.com', name: 'Test Member'}, {}); + const repo = buildRepo({ + Member, + Outbox, + WelcomeEmailAutomationRun, + MemberStatusEvent, + MemberSubscribeEventModel: MemberSubscribeEvent, + newslettersService, + Automation, + OfferRedemption: mockOfferRedemption + }); - sinon.assert.notCalled(WelcomeEmailAutomationRun.add); - }); + await repo.create({email: 'test@example.com', name: 'Test Member'}, {}); - it('does NOT create automation run when member is signing up for a paid subscription (stripeCustomer is present)', async function () { - const repo = buildRepo({ - Member, - Outbox, - WelcomeEmailAutomationRun, - MemberStatusEvent, - MemberSubscribeEventModel: MemberSubscribeEvent, - newslettersService, - Automation, - OfferRedemption: mockOfferRedemption + sinon.assert.notCalled(WelcomeEmailAutomationRun.add); }); - // Stub linkSubscription to avoid needing all the stripe-related mocks - sinon.stub(repo, 'linkSubscription').resolves(); - sinon.stub(repo, 'upsertCustomer').resolves(); + it('does NOT create automation run when member is signing up for a paid subscription (stripeCustomer is present)', async function () { + const repo = buildRepo({ + Member, + Outbox, + WelcomeEmailAutomationRun, + MemberStatusEvent, + MemberSubscribeEventModel: MemberSubscribeEvent, + newslettersService, + Automation, + OfferRedemption: mockOfferRedemption + }); - // Create a member with a stripeCustomer (i.e., signing up for paid subscription) - await repo.create({ - email: 'test@example.com', - name: 'Test Member', - stripeCustomer: { - id: 'cus_123', - name: 'Test Member', + // Stub linkSubscription to avoid needing all the stripe-related mocks + sinon.stub(repo, 'linkSubscription').resolves(); + sinon.stub(repo, 'upsertCustomer').resolves(); + + // Create a member with a stripeCustomer (i.e., signing up for paid subscription) + await repo.create({ email: 'test@example.com', - subscriptions: { - data: [{ - id: 'sub_123', - customer: 'cus_123', - status: 'active' - }] + name: 'Test Member', + stripeCustomer: { + id: 'cus_123', + name: 'Test Member', + email: 'test@example.com', + subscriptions: { + data: [{ + id: 'sub_123', + customer: 'cus_123', + status: 'active' + }] + } } - } - }, {}); + }, {}); - // The free welcome email should NOT be sent when stripeCustomer is present - sinon.assert.notCalled(WelcomeEmailAutomationRun.add); - sinon.assert.notCalled(Automation.findOne); - sinon.assert.notCalled(Member.transaction); + // The free welcome email should NOT be sent when stripeCustomer is present + sinon.assert.notCalled(WelcomeEmailAutomationRun.add); + sinon.assert.notCalled(Automation.findOne); + sinon.assert.notCalled(Member.transaction); + }); }); }); - describe('linkSubscription - automation run integration', function () { + describe('linkSubscription - automation integration', function () { let subscriptionData; beforeEach(function () { @@ -1998,7 +2018,7 @@ describe('MemberRepository', function () { sinon.restore(); }); - it('creates automation run when member status changes to paid', async function () { + it('triggers an automation event for paid signup', async function () { Member.edit.resolves({ attributes: {status: 'paid'}, _previousAttributes: {status: 'free'}, @@ -2008,20 +2028,7 @@ describe('MemberRepository', function () { }) }); - const repo = buildRepo({ - Member, - Outbox, - WelcomeEmailAutomationRun, - MemberPaidSubscriptionEvent, - StripeCustomerSubscription, - MemberProductEvent, - MemberStatusEvent, - stripeAPIService, - productRepository, - Automation, - OfferRedemption: mockOfferRedemption - }); - + const repo = buildRepo(); sinon.stub(repo, 'getSubscriptionByStripeID').resolves(null); await repo.linkSubscription({ @@ -2034,51 +2041,41 @@ describe('MemberRepository', function () { context: {} }); - sinon.assert.calledOnce(WelcomeEmailAutomationRun.add); - const runCall = WelcomeEmailAutomationRun.add.firstCall.args[0]; - assert.equal(runCall.welcome_email_automation_id, 'automation_id_paid'); - assert.equal(runCall.member_id, 'member_id_123'); - assert.equal(runCall.next_welcome_email_automated_email_id, 'automated_email_id_paid'); - assert.ok(runCall.ready_at); - assert.equal(runCall.step_started_at, null); - assert.equal(runCall.step_attempts, 0); - assert.equal(runCall.exit_reason, null); - }); - - it('does NOT create automation run for disallowed sources', async function () { - Member.edit.resolves({ - attributes: {status: 'paid'}, - _previousAttributes: {status: 'free'}, - get: sinon.stub().callsFake((key) => { - const data = {status: 'paid'}; - return data[key]; - }) + sinon.assert.calledOnceWithExactly(automationsApi.trigger, { + event: 'member_sign_up', + memberId: 'member_id_123', + memberEmail: 'test@example.com', + memberStatus: 'paid' }); + }); - const repo = buildRepo({ - Member, - Outbox, - WelcomeEmailAutomationRun, - MemberPaidSubscriptionEvent, - StripeCustomerSubscription, - MemberProductEvent, - MemberStatusEvent, - stripeAPIService, - productRepository, - Automation, - OfferRedemption: mockOfferRedemption - }); + describe('legacy automations', function () { + it('creates automation run when member status changes to paid', async function () { + Member.edit.resolves({ + attributes: {status: 'paid'}, + _previousAttributes: {status: 'free'}, + get: sinon.stub().callsFake((key) => { + const data = {status: 'paid'}; + return data[key]; + }) + }); - sinon.stub(repo, 'getSubscriptionByStripeID').resolves(null); + const repo = buildRepo({ + Member, + Outbox, + WelcomeEmailAutomationRun, + MemberPaidSubscriptionEvent, + StripeCustomerSubscription, + MemberProductEvent, + MemberStatusEvent, + stripeAPIService, + productRepository, + Automation, + OfferRedemption: mockOfferRedemption + }); - const disallowedSources = [ - {name: 'import', context: {import: true}}, - {name: 'admin', context: {user: true}}, - {name: 'api', context: {api_key: true}} - ]; + sinon.stub(repo, 'getSubscriptionByStripeID').resolves(null); - for (const source of disallowedSources) { - WelcomeEmailAutomationRun.add.resetHistory(); await repo.linkSubscription({ id: 'member_id_123', subscription: subscriptionData @@ -2086,106 +2083,162 @@ describe('MemberRepository', function () { transacting: { executionPromise: Promise.resolve() }, - context: source.context + context: {} }); - sinon.assert.notCalled(WelcomeEmailAutomationRun.add); - } - }); - it('does NOT create automation run when paid welcome email is inactive', async function () { - Member.edit.resolves({ - attributes: {status: 'paid'}, - _previousAttributes: {status: 'free'}, - get: sinon.stub().callsFake((key) => { - const data = {status: 'paid'}; - return data[key]; - }) - }); + sinon.assert.calledOnce(WelcomeEmailAutomationRun.add); + const runCall = WelcomeEmailAutomationRun.add.firstCall.args[0]; + assert.equal(runCall.welcome_email_automation_id, 'automation_id_paid'); + assert.equal(runCall.member_id, 'member_id_123'); + assert.equal(runCall.next_welcome_email_automated_email_id, 'automated_email_id_paid'); + assert.ok(runCall.ready_at); + assert.equal(runCall.step_started_at, null); + assert.equal(runCall.step_attempts, 0); + assert.equal(runCall.exit_reason, null); + }); + + it('does NOT create automation run for disallowed sources', async function () { + Member.edit.resolves({ + attributes: {status: 'paid'}, + _previousAttributes: {status: 'free'}, + get: sinon.stub().callsFake((key) => { + const data = {status: 'paid'}; + return data[key]; + }) + }); - Automation.findOne.resolves({ - id: 'automation_id_paid', - get: sinon.stub().callsFake((key) => { - const data = {status: 'inactive'}; - return data[key]; - }), - related: sinon.stub().callsFake((relation) => { - assert.equal(relation, 'welcomeEmailAutomatedEmail'); - return { - id: 'automated_email_id_paid', - get: sinon.stub().callsFake((key) => { - const data = {lexical: '{"root":{}}'}; - return data[key]; - }) - }; - }) - }); + const repo = buildRepo({ + Member, + Outbox, + WelcomeEmailAutomationRun, + MemberPaidSubscriptionEvent, + StripeCustomerSubscription, + MemberProductEvent, + MemberStatusEvent, + stripeAPIService, + productRepository, + Automation, + OfferRedemption: mockOfferRedemption + }); - const repo = buildRepo({ - Member, - Outbox, - WelcomeEmailAutomationRun, - MemberPaidSubscriptionEvent, - StripeCustomerSubscription, - MemberProductEvent, - MemberStatusEvent, - stripeAPIService, - productRepository, - Automation, - OfferRedemption: mockOfferRedemption + sinon.stub(repo, 'getSubscriptionByStripeID').resolves(null); + + const disallowedSources = [ + {name: 'import', context: {import: true}}, + {name: 'admin', context: {user: true}}, + {name: 'api', context: {api_key: true}} + ]; + + for (const source of disallowedSources) { + WelcomeEmailAutomationRun.add.resetHistory(); + await repo.linkSubscription({ + id: 'member_id_123', + subscription: subscriptionData + }, { + transacting: { + executionPromise: Promise.resolve() + }, + context: source.context + }); + sinon.assert.notCalled(WelcomeEmailAutomationRun.add); + } }); - sinon.stub(repo, 'getSubscriptionByStripeID').resolves(null); + it('does NOT create automation run when paid welcome email is inactive', async function () { + Member.edit.resolves({ + attributes: {status: 'paid'}, + _previousAttributes: {status: 'free'}, + get: sinon.stub().callsFake((key) => { + const data = {status: 'paid'}; + return data[key]; + }) + }); - await repo.linkSubscription({ - id: 'member_id_123', - subscription: subscriptionData - }, { - transacting: { - executionPromise: Promise.resolve() - }, - context: {} - }); + Automation.findOne.resolves({ + id: 'automation_id_paid', + get: sinon.stub().callsFake((key) => { + const data = {status: 'inactive'}; + return data[key]; + }), + related: sinon.stub().callsFake((relation) => { + assert.equal(relation, 'welcomeEmailAutomatedEmail'); + return { + id: 'automated_email_id_paid', + get: sinon.stub().callsFake((key) => { + const data = {lexical: '{"root":{}}'}; + return data[key]; + }) + }; + }) + }); - sinon.assert.notCalled(WelcomeEmailAutomationRun.add); - }); + const repo = buildRepo({ + Member, + Outbox, + WelcomeEmailAutomationRun, + MemberPaidSubscriptionEvent, + StripeCustomerSubscription, + MemberProductEvent, + MemberStatusEvent, + stripeAPIService, + productRepository, + Automation, + OfferRedemption: mockOfferRedemption + }); - it('does NOT create automation run when previous status was "gift" (already received paid welcome at redemption)', async function () { - Member.edit.resolves({ - attributes: {status: 'paid'}, - _previousAttributes: {status: 'gift'}, - get: sinon.stub().callsFake((key) => { - const data = {status: 'paid'}; - return data[key]; - }) - }); + sinon.stub(repo, 'getSubscriptionByStripeID').resolves(null); - const repo = buildRepo({ - Member, - Outbox, - WelcomeEmailAutomationRun, - MemberPaidSubscriptionEvent, - StripeCustomerSubscription, - MemberProductEvent, - MemberStatusEvent, - stripeAPIService, - productRepository, - Automation, - OfferRedemption: mockOfferRedemption + await repo.linkSubscription({ + id: 'member_id_123', + subscription: subscriptionData + }, { + transacting: { + executionPromise: Promise.resolve() + }, + context: {} + }); + + sinon.assert.notCalled(WelcomeEmailAutomationRun.add); }); - sinon.stub(repo, 'getSubscriptionByStripeID').resolves(null); + it('does NOT create automation run when previous status was "gift" (already received paid welcome at redemption)', async function () { + Member.edit.resolves({ + attributes: {status: 'paid'}, + _previousAttributes: {status: 'gift'}, + get: sinon.stub().callsFake((key) => { + const data = {status: 'paid'}; + return data[key]; + }) + }); - await repo.linkSubscription({ - id: 'member_id_123', - subscription: subscriptionData - }, { - transacting: { - executionPromise: Promise.resolve() - }, - context: {} - }); + const repo = buildRepo({ + Member, + Outbox, + WelcomeEmailAutomationRun, + MemberPaidSubscriptionEvent, + StripeCustomerSubscription, + MemberProductEvent, + MemberStatusEvent, + stripeAPIService, + productRepository, + Automation, + OfferRedemption: mockOfferRedemption + }); - sinon.assert.notCalled(WelcomeEmailAutomationRun.add); + sinon.stub(repo, 'getSubscriptionByStripeID').resolves(null); + + await repo.linkSubscription({ + id: 'member_id_123', + subscription: subscriptionData + }, { + transacting: { + executionPromise: Promise.resolve() + }, + context: {} + }); + + sinon.assert.notCalled(WelcomeEmailAutomationRun.add); + }); }); }); From bdcd83e691ca3e9b4cea578087df5036c98ae9d2 Mon Sep 17 00:00:00 2001 From: Peter Zimon Date: Wed, 3 Jun 2026 16:56:48 +0200 Subject: [PATCH 6/8] Automation UX improvements (#28337) ref https://linear.app/ghost/issue/NY-1318/small-automation-ui-delights This PR collects a set of small UX improvements for the Automations editor canvas and step settings experience. The changes are intentionally scoped to the React automation editor UI in `apps/posts` and focus on making canvas navigation, step insertion, node actions, and email-step editing feel more direct and polished. ## What changed ### Canvas zoom controls - Added compact custom zoom controls to the bottom-left of the React Flow canvas. - The controls are horizontal: zoom out, current zoom percentage, zoom in. - Clicking the zoom percentage opens a centered drop-up menu with preset zoom levels and `Fit to view`. - Zoom actions use React Flow viewport helpers with animation for smoother transitions. - Disabled the default React Flow double-click zoom because it conflicts with node interactions. ### Add-step connectors and picker behavior - Restyled the in-between-node add-step affordance so it matches the larger tail add button more closely. - Kept connector lines visible and stable on hover, while showing the `+` button only when the pointer is over the connector hit area. - Adjusted connector color and hidden handle sizing to remove visual gaps between nodes and connector lines. - Changed the in-between-node step picker to open above the `+` button, with wider menu sizing and stronger shadow. - Tweaked picker menu hover styling to a lighter gray. ### New-step feedback - Added a more visible pop/fade animation when a new automation step is created. - Newly inserted steps are automatically selected so their settings sidebar opens immediately after insertion. ### Wait-step settings control - Replaced the wait-days input/select pair with a Shade `InputGroup`. - The input group now shows the number and fixed `days` label on the left, with compact `-` and `+` steppers on the right. - Stepper buttons clamp values between `1` and `30` days and respect invalid manual input. - Kept manual input validation intact. ### Node context menus - Added right-click context menus to automation nodes using the new Shade `ContextMenu` components from `main`. - The trigger node has `Edit settings` only. - Action nodes have `Edit settings` and `Delete`. - Email action nodes additionally have `Edit email body` and `Preview`, separated from `Delete` with a menu separator. - `Edit settings` opens the existing right settings sidebar. - `Delete` uses the existing action deletion path, preserving chain reconnection behavior. - `Edit email body` opens the email editor modal directly without opening or flashing the sidebar. - `Preview` opens the same email editor modal with the Preview tab active by default. ### Email node shortcuts - Double-clicking a send-email node now opens the email editor modal directly. - Double-clicking trigger, wait, or tail nodes does nothing special. - Canvas double-click zoom is disabled so this interaction is no longer hijacked by React Flow. ### Sidebar typography - Changed the automation settings sidebar heading from semibold to medium weight to better match the surrounding hierarchy. ## Implementation notes for reviewers - Most changes are contained in `apps/posts/src/views/Automations/components/automation-canvas.tsx`. - Connector/add-step styling lives in `add-step-edge.tsx` and `step-picker.tsx`. - The email modal now accepts `initialMode?: 'edit' | 'preview'` so callers can open directly into Preview without duplicating modal behavior. - Context menu entries are built from a local menu-entry model so future node types can add node-specific actions without changing the node shell rendering. - Menu click/pointer events are stopped at the context-menu content to avoid React Flow treating menu-item clicks as node clicks. - Right-click opening and email-menu actions avoid selecting the node, which prevents the sidebar from flashing open behind the modal. ## Files touched - `apps/posts/src/views/Automations/components/automation-canvas.tsx` - `apps/posts/src/views/Automations/components/add-step-edge.tsx` - `apps/posts/src/views/Automations/components/step-picker.tsx` - `apps/posts/src/views/Automations/components/email-modal/email-content-modal.tsx` - `apps/posts/test/unit/views/automations/automation-editor.test.tsx` ## Validation - `pnpm --filter @tryghost/posts test:unit -- test/unit/views/automations/automation-editor.test.tsx` - `pnpm --filter @tryghost/posts lint` - Passes with the existing unrelated warning in `apps/posts/src/hooks/use-post-success-modal.ts` about `site?.icon` in a `useMemo` dependency array. ## Out of scope - No API, model, database, or migration changes. - No automation schema changes. - No changes to public apps. - No new shared Shade components beyond consuming the new `ContextMenu` component that was merged into `main`. --------- Co-authored-by: Troy Ciesco --- .../Automations/components/add-step-edge.tsx | 28 +- .../components/automation-canvas.tsx | 422 +++++++++++++++--- .../email-modal/email-content-modal.tsx | 23 +- .../Automations/components/step-picker.tsx | 4 +- .../src/views/Automations/components/types.ts | 1 + .../automations/automation-editor.test.tsx | 312 ++++++++++++- 6 files changed, 706 insertions(+), 84 deletions(-) create mode 100644 apps/posts/src/views/Automations/components/types.ts diff --git a/apps/posts/src/views/Automations/components/add-step-edge.tsx b/apps/posts/src/views/Automations/components/add-step-edge.tsx index de167fdc507..e5890f77ce1 100644 --- a/apps/posts/src/views/Automations/components/add-step-edge.tsx +++ b/apps/posts/src/views/Automations/components/add-step-edge.tsx @@ -12,9 +12,8 @@ export type AddStepEdgeData = { onPick: (type: StepPickerType, anchor: {sourceId: string; targetId: string}) => void; }; -const INSERT_BUTTON_CLASSES = 'border-transparent bg-green-500 text-white shadow-sm hover:bg-green-600'; +const INSERT_BUTTON_CLASSES = 'border-dashed border-border-default bg-surface-page text-text-secondary shadow-sm hover:border-border-strong'; const DEFAULT_EDGE_STROKE = 'var(--xy-edge-stroke)'; -const HOVERED_EDGE_STROKE = 'var(--color-green-500)'; const AddStepEdge: React.FC = ({ id, @@ -27,7 +26,8 @@ const AddStepEdge: React.FC = ({ data }) => { const [open, setOpen] = useState(false); - const [hovered, setHovered] = useState(false); + const [edgeHovered, setEdgeHovered] = useState(false); + const [labelHovered, setLabelHovered] = useState(false); const edgeData = data as AddStepEdgeData | undefined; const [path, labelX, labelY] = getSmoothStepPath({ @@ -39,10 +39,6 @@ const AddStepEdge: React.FC = ({ targetPosition }); - const edgeStyle: React.CSSProperties = { - stroke: hovered || open ? HOVERED_EDGE_STROKE : DEFAULT_EDGE_STROKE - }; - if (!edgeData) { return ; } @@ -52,12 +48,12 @@ const AddStepEdge: React.FC = ({ edgeData.onPick(type, {sourceId: edgeData.sourceId, targetId: edgeData.targetId}); }; - const visible = open || hovered; + const visible = open || edgeHovered || labelHovered; const button = ( ); @@ -86,7 +82,7 @@ const AddStepEdge: React.FC = ({ control = ( {button} - + @@ -95,18 +91,18 @@ const AddStepEdge: React.FC = ({ return ( setHovered(true)} - onMouseLeave={() => setHovered(false)} + onMouseEnter={() => setEdgeHovered(true)} + onMouseLeave={() => setEdgeHovered(false)} > - +
setHovered(true)} - onMouseLeave={() => setHovered(false)} + onMouseEnter={() => setLabelHovered(true)} + onMouseLeave={() => setLabelHovered(false)} > {/* Wider hit zone so the + becomes visible when the cursor is near the edge midpoint. */}
diff --git a/apps/posts/src/views/Automations/components/automation-canvas.tsx b/apps/posts/src/views/Automations/components/automation-canvas.tsx index 7dcd33e0329..9cfa5bdfa3a 100644 --- a/apps/posts/src/views/Automations/components/automation-canvas.tsx +++ b/apps/posts/src/views/Automations/components/automation-canvas.tsx @@ -4,9 +4,10 @@ import EmailContentModal from './email-modal/email-content-modal'; import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; import StepPicker, {type StepPickerType} from './step-picker'; import {AutomationAction, AutomationDetail, AutomationSendEmailAction, AutomationWaitAction, InsertActionAnchor, MAX_AUTOMATION_ACTIONS, insertSendEmailAction, insertWaitAction, removeAction, updateSendEmailAction, updateWaitAction} from '@tryghost/admin-x-framework/api/automations'; -import {Background, BackgroundVariant, Edge, Handle, Node, NodeProps, Position, ReactFlow} from '@xyflow/react'; -import {Banner, Button, Checkbox, Input, Label, LoadingIndicator, Popover, PopoverContent, PopoverTrigger, Select, SelectTrigger, Tooltip, TooltipContent, TooltipProvider, TooltipTrigger} from '@tryghost/shade/components'; +import {Background, BackgroundVariant, Controls, Edge, Handle, Node, NodeProps, Position, ReactFlow, useReactFlow, useViewport} from '@xyflow/react'; +import {Banner, Button, Checkbox, ContextMenu, ContextMenuContent, ContextMenuItem, ContextMenuSeparator, ContextMenuTrigger, DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuSeparator, DropdownMenuShortcut, DropdownMenuTrigger, Field, FieldError, FieldLabel, Input, InputGroup, InputGroupAddon, InputGroupButton, InputGroupInput, InputGroupText, Label, LoadingIndicator, Popover, PopoverContent, PopoverTrigger, Select, SelectTrigger, Tooltip, TooltipContent, TooltipProvider, TooltipTrigger} from '@tryghost/shade/components'; import {LucideIcon, cn, formatNumber} from '@tryghost/shade/utils'; +import type {EmailModalMode} from './types'; const MAX_WAIT_DAYS = 30; const WHOLE_NUMBER_PATTERN = /^\d+$/; @@ -16,8 +17,11 @@ const NODE_WIDTH = 256; const NODE_COLUMN_CENTER_X = NODE_X + (NODE_WIDTH / 2); const NODE_GAP_Y = 180; const INITIAL_VIEWPORT_Y = 40; +const VIEWPORT_ANIMATION_DURATION = 180; +const NODE_ENTER_ANIMATION_DURATION = 250; const DISABLED_REASON = `Limit of ${formatNumber(MAX_AUTOMATION_ACTIONS)} steps reached`; const DEFAULT_EDGE_STROKE = 'var(--xy-edge-stroke)'; +const ZOOM_PRESETS = [1.5, 1, 0.75, 0.5, 0.25]; // React Flow node IDs for the trigger and tail nodes. The canvas builds the visual graph using // these; they are not action IDs and never reach the API. @@ -39,7 +43,24 @@ type StepNodeDisplayData = { value?: string; }; +type NodeContextMenuItem = { + icon?: React.ElementType; + label: string; + onSelect: () => void; + type?: 'item'; + variant?: 'default' | 'destructive'; +}; + +type NodeContextMenuSeparator = { + id: string; + type: 'separator'; +}; + +type NodeContextMenuEntry = NodeContextMenuItem | NodeContextMenuSeparator; + type StepNodeData = StepNodeDisplayData & { + contextMenuItems: NodeContextMenuEntry[]; + isNew: boolean; selected: boolean; onSelect: () => void; }; @@ -56,32 +77,83 @@ type TailFlowNode = Node; type AutomationFlowNode = StepFlowNode | TailFlowNode; const HIDDEN_HANDLE_STYLE: React.CSSProperties = { + background: 'transparent', + border: 'none', + height: 0, + minHeight: 0, + minWidth: 0, opacity: 0, pointerEvents: 'none', - background: 'transparent', - border: 'none' + width: 0 }; const HiddenHandle: React.FC<{type: 'source' | 'target'; position: Position}> = ({type, position}) => ( ); -const NodeShell: React.FC> = ({children, className, data}) => ( - -); +const NodeShell: React.FC> = ({children, className, data}) => { + const ignoreNextClickRef = useRef(false); + + return ( + { + if (!open) { + ignoreNextClickRef.current = false; + } + }}> + + + + event.stopPropagation()} + onPointerDown={event => event.stopPropagation()} + > + {data.contextMenuItems.map((item) => { + if (item.type === 'separator') { + return ; + } + const Icon = item.icon; + return ( + + {Icon && } + {item.label} + + ); + })} + + + ); +}; const StepNodeContent: React.FC<{data: StepNodeData}> = ({data}) => { const Icon = data.icon; @@ -161,7 +233,7 @@ const TailNode: React.FC> = ({data}) => { - + @@ -178,6 +250,90 @@ const edgeTypes = { 'add-step-edge': AddStepEdge }; +const AutomationCanvasControls: React.FC = () => { + const [open, setOpen] = useState(false); + const {fitView, zoomIn, zoomOut, zoomTo} = useReactFlow(); + const {zoom} = useViewport(); + const animationOptions = {duration: VIEWPORT_ANIMATION_DURATION}; + const zoomPercent = Math.round(zoom * 100); + + const handleZoomTo = (nextZoom: number) => { + setOpen(false); + void zoomTo(nextZoom, animationOptions); + }; + + const handleFitView = () => { + setOpen(false); + void fitView(animationOptions); + }; + + return ( + + + + + + + + {ZOOM_PRESETS.map((preset) => { + const presetPercent = Math.round(preset * 100); + const isSelected = Math.abs(zoom - preset) < 0.01; + return ( + handleZoomTo(preset)}> + {formatNumber(presetPercent)}% + {isSelected && ( + + + + )} + + ); + })} + + + Fit to view + + + + + + ); +}; + export const formatWait = (hours: number): string => { if (hours <= 0) { throw new Error('Wait time must be a positive number of hours.'); @@ -202,6 +358,60 @@ const buildActionData = (action: AutomationAction): StepNodeDisplayData => { } }; +const buildNodeContextMenuItems = ({ + canDelete = false, + canEditEmailBody = false, + onDelete, + onEditEmailBody, + onPreviewEmail, + onSelectStep, + stepId +}: { + canDelete?: boolean; + canEditEmailBody?: boolean; + onDelete?: (deleteStepId: string) => void; + onEditEmailBody?: (editEmailBodyStepId: string, mode?: EmailModalMode) => void; + onPreviewEmail?: (previewEmailStepId: string) => void; + onSelectStep: (nextStepId: string) => void; + stepId: string; +}): NodeContextMenuEntry[] => { + const items: NodeContextMenuEntry[] = [{ + icon: LucideIcon.Settings2, + label: 'Edit settings', + onSelect: () => onSelectStep(stepId) + }]; + + if (canEditEmailBody && onEditEmailBody) { + items.push({ + icon: LucideIcon.Pencil, + label: 'Edit email body', + onSelect: () => onEditEmailBody(stepId) + }); + } + + if (canEditEmailBody && onPreviewEmail) { + items.push({ + icon: LucideIcon.Eye, + label: 'Preview', + onSelect: () => onPreviewEmail(stepId) + }); + } + + if (canDelete && onDelete) { + if (canEditEmailBody) { + items.push({id: 'before-delete', type: 'separator'}); + } + items.push({ + icon: LucideIcon.Trash2, + label: 'Delete', + onSelect: () => onDelete(stepId), + variant: 'destructive' + }); + } + + return items; +}; + // Returns the actions of `automation` ordered along the chain from the head. Throws on malformed // data (cycle, branch, or disconnected nodes). The canvas wraps its render tree in an Error // Boundary that catches these and renders the same "Couldn't load automation" banner. @@ -246,12 +456,16 @@ const getInitialActionOrder = (automation: AutomationDetail): AutomationAction[] type BuildGraphParams = { automation: AutomationDetail; disabled: boolean; + onDelete: (stepId: string) => void; + onEditEmailBody: (stepId: string, mode?: EmailModalMode) => void; + onPreviewEmail: (stepId: string) => void; onPick: (type: StepPickerType, anchor: CanvasAnchor) => void; onSelectStep: (stepId: string) => void; + newStepId: string | null; selectedStepId: string | null; } -const buildGraph = ({automation, disabled, onPick, onSelectStep, selectedStepId}: BuildGraphParams): {nodes: AutomationFlowNode[]; edges: Edge[]} => { +const buildGraph = ({automation, disabled, onDelete, onEditEmailBody, onPick, onPreviewEmail, onSelectStep, newStepId, selectedStepId}: BuildGraphParams): {nodes: AutomationFlowNode[]; edges: Edge[]} => { const ordered = getInitialActionOrder(automation); const baseNodeProps = { draggable: false, @@ -273,7 +487,12 @@ const buildGraph = ({automation, disabled, onPick, onSelectStep, selectedStepId} type: 'trigger', position: {x: NODE_X, y: 0}, data: { + contextMenuItems: buildNodeContextMenuItems({ + onSelectStep, + stepId: TRIGGER_CANVAS_ID + }), icon: LucideIcon.Zap, + isNew: false, label: 'Trigger', value: 'Member signs up', selected: selectedStepId === TRIGGER_CANVAS_ID, @@ -290,6 +509,16 @@ const buildGraph = ({automation, disabled, onPick, onSelectStep, selectedStepId} position: {x: NODE_X, y: NODE_GAP_Y * (index + 1)}, data: { ...buildActionData(action), + contextMenuItems: buildNodeContextMenuItems({ + canDelete: true, + canEditEmailBody: action.type === 'send_email', + onDelete, + onEditEmailBody, + onPreviewEmail, + onSelectStep, + stepId: action.id + }), + isNew: newStepId === action.id, selected: selectedStepId === action.id, onSelect: () => onSelectStep(action.id) }, @@ -441,11 +670,13 @@ const getStepSidebarDetail = ({automation, stepId, onDelete, onUpdateWait, onUpd } }; -const SidebarField: React.FC<{label: string; children: React.ReactNode}> = ({label, children}) => ( - + ); const ReadOnlySelect: React.FC<{value: string}> = ({value}) => ( @@ -508,8 +739,6 @@ const WaitSidebarBody: React.FC<{ const days = Number(daysText); const isValid = getValidWaitDays(daysText) !== null; - const unitLabel = days === 1 ? 'Day' : 'Days'; - const updateWaitDays = (nextDays: number) => { const nextHours = nextDays * 24; if (nextHours !== action.data.wait_hours) { @@ -517,6 +746,17 @@ const WaitSidebarBody: React.FC<{ } }; + const stepWaitDays = (direction: -1 | 1) => { + const currentDays = getValidWaitDays(daysText); + if (currentDays === null) { + return; + } + + const nextDays = Math.min(MAX_WAIT_DAYS, Math.max(1, currentDays + direction)); + setDaysText(String(nextDays)); + updateWaitDays(nextDays); + }; + const handleChange = (event: React.ChangeEvent) => { const nextDaysText = event.target.value; setDaysText(nextDaysText); @@ -530,21 +770,47 @@ const WaitSidebarBody: React.FC<{ return (
- -
- + + - -
+ {days === 1 ? 'day' : 'days'} + + stepWaitDays(-1)} + > + + + = MAX_WAIT_DAYS} + size='icon-xs' + title='Increase wait by one day' + onClick={() => stepWaitDays(1)} + > + + + + {!isValid && ( - + Enter a whole number between 1 and {formatNumber(MAX_WAIT_DAYS)} days. - + )}
@@ -561,8 +827,9 @@ const SendEmailSidebarBody: React.FC<{ onDelete: () => void; }> = ({action, onUpdateSubject, onEditEmail, onDelete}) => (
- + onUpdateSubject(e.target.value)} @@ -610,7 +877,7 @@ const StepSidebarContent: React.FC<{detail: StepSidebarDetail}> = ({detail}) =>
{detail.label} -

{detail.title}

+

{detail.title}

@@ -666,7 +933,6 @@ type AutomationCanvasProps = { type SelectedStep = { id: string; - isEditingEmail: boolean; }; const insertActionByType = { @@ -675,6 +941,9 @@ const insertActionByType = { }; const AutomationCanvas: React.FC = ({automation, isLoading, isError, onChange}) => { + const [newStepId, setNewStepId] = useState(null); + const [emailModalMode, setEmailModalMode] = useState('edit'); + const [emailModalStepId, setEmailModalStepId] = useState(null); const [selectedStep, setSelectedStep] = useState(null); const selectedStepId = selectedStep?.id ?? null; @@ -688,14 +957,30 @@ const AutomationCanvas: React.FC = ({automation, isLoadin const apiAnchor = toApiAnchor(anchor); const insertAction = insertActionByType[type]; const next = insertAction({detail: automation, anchor: apiAnchor}); + const insertedAction = next.actions.find(action => !automation.actions.some(existingAction => existingAction.id === action.id)); + setNewStepId(insertedAction?.id ?? null); + if (insertedAction) { + setSelectedStep({id: insertedAction.id}); + } onChange(next); }, [automation, onChange]); + useEffect(() => { + if (!newStepId) { + return; + } + const timeout = window.setTimeout(() => { + setNewStepId(null); + }, NODE_ENTER_ANIMATION_DURATION); + return () => window.clearTimeout(timeout); + }, [newStepId]); + const handleDelete = useCallback((actionId: string) => { if (!automation) { return; } const next = removeAction({detail: automation, actionId}); + setEmailModalStepId(currentId => (currentId === actionId ? null : currentId)); setSelectedStep(null); onChange(next); }, [automation, onChange]); @@ -718,12 +1003,23 @@ const AutomationCanvas: React.FC = ({automation, isLoadin onChange(updateSendEmailAction({detail: automation, actionId, emailSubject: subject, emailLexical: action.data.email_lexical})); }, [automation, onChange]); - const handleEditEmail = (actionId: string) => { - setSelectedStep({id: actionId, isEditingEmail: true}); - }; + const handleEditEmail = useCallback((actionId: string, mode: EmailModalMode = 'edit') => { + setEmailModalMode(mode); + setEmailModalStepId(actionId); + }, []); - const emailModalAction = selectedStep?.isEditingEmail && automation - ? automation.actions.find((action): action is AutomationSendEmailAction => action.id === selectedStep.id && action.type === 'send_email') + const handleContextMenuEditEmail = useCallback((actionId: string, mode: EmailModalMode = 'edit') => { + setSelectedStep(null); + setEmailModalMode(mode); + setEmailModalStepId(actionId); + }, []); + + const handleContextMenuPreviewEmail = useCallback((actionId: string) => { + handleContextMenuEditEmail(actionId, 'preview'); + }, [handleContextMenuEditEmail]); + + const emailModalAction = emailModalStepId && automation + ? automation.actions.find((action): action is AutomationSendEmailAction => action.id === emailModalStepId && action.type === 'send_email') : undefined; const initialViewport = useRef(getInitialViewport(window.innerWidth)); @@ -735,11 +1031,15 @@ const AutomationCanvas: React.FC = ({automation, isLoadin return buildGraph({ automation, disabled: automation.actions.length >= MAX_AUTOMATION_ACTIONS, + onDelete: handleDelete, + onEditEmailBody: handleContextMenuEditEmail, onPick: handlePick, - onSelectStep: id => setSelectedStep({id, isEditingEmail: false}), + onPreviewEmail: handleContextMenuPreviewEmail, + onSelectStep: id => setSelectedStep({id}), + newStepId, selectedStepId }); - }, [automation, handlePick, selectedStepId]); + }, [automation, handleContextMenuEditEmail, handleContextMenuPreviewEmail, handleDelete, handlePick, newStepId, selectedStepId]); const sidebarDetail = automation ? getStepSidebarDetail({ automation, @@ -754,11 +1054,20 @@ const AutomationCanvas: React.FC = ({automation, isLoadin }, []); const closeEmailModal = () => { - if (!emailModalAction) { + setEmailModalStepId(null); + setEmailModalMode('edit'); + }; + + const handleNodeDoubleClick = useCallback((event: React.MouseEvent, node: AutomationFlowNode) => { + event.stopPropagation(); + if (!automation || node.id === TAIL_CANVAS_ID || node.id === TRIGGER_CANVAS_ID) { return; } - setSelectedStep({id: emailModalAction.id, isEditingEmail: false}); - }; + const action = automation.actions.find(item => item.id === node.id); + if (action?.type === 'send_email') { + handleEditEmail(action.id); + } + }, [automation, handleEditEmail]); if (isLoading) { return ( @@ -787,7 +1096,7 @@ const AutomationCanvas: React.FC = ({automation, isLoadin return (
= ({automation, isLoadin nodesFocusable={false} nodeTypes={nodeTypes} proOptions={{hideAttribution: true}} + zoomOnDoubleClick={false} zoomOnScroll={false} panOnScroll - onNodeClick={(_, node) => { + onNodeClick={(event, node) => { + if (event.button !== 0) { + return; + } if (node.id !== TAIL_CANVAS_ID) { - setSelectedStep({id: node.id, isEditingEmail: false}); + setSelectedStep({id: node.id}); } }} + onNodeDoubleClick={handleNodeDoubleClick} onPaneClick={clearDetail} > + {emailModalAction && automation && ( { onChange(updateSendEmailAction({detail: automation, actionId: emailModalAction.id, emailSubject: subject, emailLexical: lexical})); - setSelectedStep({id: emailModalAction.id, isEditingEmail: false}); + closeEmailModal(); }} /> )} diff --git a/apps/posts/src/views/Automations/components/email-modal/email-content-modal.tsx b/apps/posts/src/views/Automations/components/email-modal/email-content-modal.tsx index 4ffbb0ef695..66324831f83 100644 --- a/apps/posts/src/views/Automations/components/email-modal/email-content-modal.tsx +++ b/apps/posts/src/views/Automations/components/email-modal/email-content-modal.tsx @@ -10,6 +10,7 @@ import {useBrowseAutomatedEmails, usePreviewWelcomeEmail} from '@tryghost/admin- import {useEmailPreview} from './use-email-preview'; import {useEmailSenderDetails} from './use-sender-details'; import {useForm, useHandleError} from '@tryghost/admin-x-framework/hooks'; +import type {EmailModalMode} from '../types'; interface EmailPreviewModalContentProps { title: string; @@ -80,21 +81,21 @@ const EmailPreviewBody: React.FC = ({children, className} ); export interface EmailContentModalProps { - initialSubject: string; initialLexical: string; + initialMode?: EmailModalMode; + initialSubject: string; onClose: () => void; onSave: (data: {subject: string; lexical: string}) => void; } -type PreviewMode = 'edit' | 'preview'; - -const EmailContentModal: React.FC = ({initialSubject, initialLexical, onClose, onSave}) => { +const EmailContentModal: React.FC = ({initialMode = 'edit', initialSubject, initialLexical, onClose, onSave}) => { const {mutateAsync: previewWelcomeEmail} = usePreviewWelcomeEmail(); const {data: automatedEmailsData} = useBrowseAutomatedEmails(); const [showTestDropdown, setShowTestDropdown] = useState(false); - const [mode, setMode] = useState('edit'); + const [mode, setMode] = useState(initialMode); const [previewSubjectOverride, setPreviewSubjectOverride] = useState(null); const [confirmDiscardOpen, setConfirmDiscardOpen] = useState(false); + const hasEnteredInitialPreview = useRef(false); const dropdownRef = useRef(null); const normalizedLexical = useRef(initialLexical || ''); const hasEditorBeenFocused = useRef(false); @@ -129,6 +130,14 @@ const EmailContentModal: React.FC = ({initialSubject, in setErrors }); + useEffect(() => { + if (initialMode !== 'preview' || hasEnteredInitialPreview.current) { + return; + } + hasEnteredInitialPreview.current = true; + enterPreview(formState); + }, [enterPreview, formState, initialMode]); + const isDirty = saveState === 'unsaved'; // Single close funnel: Esc, overlay click, and the Close button all route here. @@ -183,7 +192,7 @@ const EmailContentModal: React.FC = ({initialSubject, in }; }, []); - const handleModeChange = useCallback((nextMode: PreviewMode) => { + const handleModeChange = useCallback((nextMode: EmailModalMode) => { setMode(nextMode); if (nextMode === 'preview') { @@ -241,7 +250,7 @@ const EmailContentModal: React.FC = ({initialSubject, in data-testid='email-mode-toggle' value={mode} variant='segmented-sm' - onValueChange={value => value && handleModeChange(value as PreviewMode)} + onValueChange={value => value && handleModeChange(value as EmailModalMode)} > Email content diff --git a/apps/posts/src/views/Automations/components/step-picker.tsx b/apps/posts/src/views/Automations/components/step-picker.tsx index 47360d510c8..99ea95614dc 100644 --- a/apps/posts/src/views/Automations/components/step-picker.tsx +++ b/apps/posts/src/views/Automations/components/step-picker.tsx @@ -16,7 +16,7 @@ interface PickerOptionProps { const PickerOption: React.FC = ({icon: Icon, label, description, onClick}) => ( @@ -30,6 +32,13 @@ const mockEditMutation = { isLoading: false, variables: undefined as {id: string; status: 'active' | 'inactive'} | undefined }; +const mockReactFlow = { + fitView: vi.fn(), + zoomIn: vi.fn(), + zoomOut: vi.fn(), + zoomTo: vi.fn() +}; +let mockViewportZoom = 1; vi.mock('@tryghost/admin-x-framework/api/automations', async () => { const actual = await vi.importActual( @@ -48,11 +57,14 @@ type StubEdge = {id: string; source: string; target: string; type?: string; data type StubReactFlowProps = { nodes: StubNode[]; edges?: StubEdge[]; + children?: React.ReactNode; className?: string; nodeTypes?: Record>; edgeTypes?: Record>; onNodeClick?: (event: React.MouseEvent, node: StubNode) => void; + onNodeDoubleClick?: (event: React.MouseEvent, node: StubNode) => void; onPaneClick?: (event: React.MouseEvent) => void; + zoomOnDoubleClick?: boolean; }; type NodeRenderProps = {id: string; data: Record; type: string}; type EdgeRenderProps = {id: string; data: Record; sourceX: number; sourceY: number; targetX: number; targetY: number; sourcePosition: string; targetPosition: string}; @@ -61,8 +73,8 @@ vi.mock('@xyflow/react', async () => { const actual = await vi.importActual('@xyflow/react'); return { ...actual, - ReactFlow: ({nodes, edges, className, nodeTypes, edgeTypes, onNodeClick, onPaneClick}: StubReactFlowProps) => ( -
+ ReactFlow: ({nodes, edges, children, className, nodeTypes, edgeTypes, onNodeClick, onNodeDoubleClick, onPaneClick, zoomOnDoubleClick}: StubReactFlowProps) => ( +
{nodes.map((node) => { const nodeType = node.type ?? 'default'; const Custom = nodeTypes?.[nodeType]; @@ -75,6 +87,10 @@ vi.mock('@xyflow/react', async () => { event.stopPropagation(); onNodeClick?.(event, node); }} + onDoubleClick={(event) => { + event.stopPropagation(); + onNodeDoubleClick?.(event, node); + }} > {Custom ? : null}
@@ -101,13 +117,28 @@ vi.mock('@xyflow/react', async () => { ); })} + {children}
), Background: () => null, + Controls: ({children, className, showFitView, showInteractive, showZoom, style}: {children?: React.ReactNode; className?: string; showFitView?: boolean; showInteractive?: boolean; showZoom?: boolean; style?: React.CSSProperties}) => ( +
+ {children} +
+ ), Handle: () => null, BaseEdge: () => null, EdgeLabelRenderer: ({children}: {children: React.ReactNode}) => <>{children}, - getSmoothStepPath: () => ['M 0 0', 0, 0] + getSmoothStepPath: () => ['M 0 0', 0, 0], + useReactFlow: () => mockReactFlow, + useViewport: () => ({x: 0, y: 0, zoom: mockViewportZoom}) }; }); @@ -160,6 +191,11 @@ describe('AutomationEditor', () => { beforeEach(() => { mockUseReadAutomation.mockReset(); mockEditMutation.mutate.mockReset(); + mockReactFlow.fitView.mockReset(); + mockReactFlow.zoomIn.mockReset(); + mockReactFlow.zoomOut.mockReset(); + mockReactFlow.zoomTo.mockReset(); + mockViewportZoom = 1; mockEditMutation.isLoading = false; mockEditMutation.variables = undefined; }); @@ -220,6 +256,84 @@ describe('AutomationEditor', () => { ]); }); + it('renders styled canvas zoom controls without the interaction toggle', () => { + mockUseReadAutomation.mockReturnValue({ + data: {automations: [automationDetail]}, + isLoading: false, + isError: false + }); + + renderEditor(); + + expect(screen.getByTestId('react-flow-mock')).toHaveAttribute('data-zoom-on-double-click', 'false'); + const controls = screen.getByTestId('react-flow-controls'); + expect(controls).toHaveAttribute('data-show-interactive', 'false'); + expect(controls).toHaveAttribute('data-show-fit-view', 'false'); + expect(controls).toHaveAttribute('data-show-zoom', 'false'); + expect(controls).toHaveStyle({bottom: '24px', left: '24px'}); + expect(controls).toHaveClass('overflow-hidden', 'rounded-md'); + expect(screen.getByRole('button', {name: 'Zoom out'})).toBeInTheDocument(); + expect(screen.getByRole('button', {name: 'Zoom level 100%'})).toHaveTextContent('100%'); + expect(screen.getByRole('button', {name: 'Zoom in'})).toBeInTheDocument(); + }); + + it('animates viewport changes from the custom canvas controls', () => { + mockUseReadAutomation.mockReturnValue({ + data: {automations: [automationDetail]}, + isLoading: false, + isError: false + }); + + renderEditor(); + + fireEvent.click(screen.getByRole('button', {name: 'Zoom in'})); + fireEvent.click(screen.getByRole('button', {name: 'Zoom out'})); + + expect(mockReactFlow.zoomIn).toHaveBeenCalledWith({duration: 180}); + expect(mockReactFlow.zoomOut).toHaveBeenCalledWith({duration: 180}); + }); + + it('opens a zoom preset menu from the canvas controls', () => { + mockViewportZoom = 0.75; + mockUseReadAutomation.mockReturnValue({ + data: {automations: [automationDetail]}, + isLoading: false, + isError: false + }); + + renderEditor(); + + fireEvent.pointerDown(screen.getByRole('button', {name: 'Zoom level 75%'}), {button: 0, ctrlKey: false}); + + expect(screen.getByRole('menuitem', {name: '150%'})).toBeInTheDocument(); + expect(screen.getByRole('menuitem', {name: '100%'})).toBeInTheDocument(); + expect(screen.getByRole('menuitem', {name: '75%'})).toBeInTheDocument(); + expect(screen.getByRole('menuitem', {name: '50%'})).toBeInTheDocument(); + expect(screen.getByRole('menuitem', {name: '25%'})).toBeInTheDocument(); + expect(screen.getByRole('menuitem', {name: 'Fit to view'})).toBeInTheDocument(); + expect(screen.getByRole('menuitem', {name: '75%'}).querySelector('svg')).toBeInTheDocument(); + }); + + it('animates zoom preset and fit view menu selections', () => { + mockUseReadAutomation.mockReturnValue({ + data: {automations: [automationDetail]}, + isLoading: false, + isError: false + }); + + renderEditor(); + + fireEvent.pointerDown(screen.getByRole('button', {name: 'Zoom level 100%'}), {button: 0, ctrlKey: false}); + fireEvent.click(screen.getByRole('menuitem', {name: '150%'})); + + expect(mockReactFlow.zoomTo).toHaveBeenCalledWith(1.5, {duration: 180}); + + fireEvent.pointerDown(screen.getByRole('button', {name: 'Zoom level 100%'}), {button: 0, ctrlKey: false}); + fireEvent.click(screen.getByRole('menuitem', {name: 'Fit to view'})); + + expect(mockReactFlow.fitView).toHaveBeenCalledWith({duration: 180}); + }); + it('opens a read-only sidebar for the trigger step', () => { mockUseReadAutomation.mockReturnValue({ data: {automations: [automationDetail]}, @@ -242,6 +356,129 @@ describe('AutomationEditor', () => { expect(within(sidebar).queryByRole('button', {name: /Edit/})).not.toBeInTheDocument(); }); + it('opens step properties from the node right-click menu', async () => { + mockUseReadAutomation.mockReturnValue({ + data: {automations: [automationDetail]}, + isLoading: false, + isError: false + }); + + renderEditor(); + + const waitStep = screen.getByRole('button', {name: 'Wait: 1 day'}); + fireEvent.contextMenu(waitStep, {clientX: 12, clientY: 12}); + fireEvent.click(await screen.findByRole('menuitem', {name: 'Edit settings'})); + + expect(waitStep).toHaveAttribute('aria-pressed', 'true'); + const sidebar = screen.getByRole('complementary', {name: 'Step details'}); + expect(within(sidebar).getByRole('heading', {name: '1 day'})).toBeInTheDocument(); + expect(within(sidebar).getByText('Wait for')).toBeInTheDocument(); + }); + + it('selects a node after its context menu is dismissed', async () => { + mockUseReadAutomation.mockReturnValue({ + data: {automations: [automationDetail]}, + isLoading: false, + isError: false + }); + + renderEditor(); + + const waitStep = screen.getByRole('button', {name: 'Wait: 1 day'}); + fireEvent.contextMenu(waitStep); + expect(await screen.findByRole('menuitem', {name: 'Edit settings'})).toBeInTheDocument(); + + fireEvent.keyDown(document, {key: 'Escape'}); + await waitFor(() => { + expect(screen.queryByRole('menuitem', {name: 'Edit settings'})).not.toBeInTheDocument(); + }); + + fireEvent.click(waitStep); + + expect(waitStep).toHaveAttribute('aria-pressed', 'true'); + expect(screen.getByRole('complementary', {name: 'Step details'})).toHaveAttribute('data-state', 'open'); + }); + + it('shows delete in action node menus but not the trigger node menu', async () => { + mockUseReadAutomation.mockReturnValue({ + data: {automations: [automationDetail]}, + isLoading: false, + isError: false + }); + + renderEditor(); + + fireEvent.contextMenu(screen.getByRole('button', {name: 'Trigger: Member signs up'})); + expect(await screen.findByRole('menuitem', {name: 'Edit settings'})).toBeInTheDocument(); + expect(screen.queryByRole('menuitem', {name: 'Delete'})).not.toBeInTheDocument(); + fireEvent.keyDown(document, {key: 'Escape'}); + + const waitStep = screen.getByRole('button', {name: 'Wait: 1 day'}); + fireEvent.contextMenu(waitStep); + fireEvent.click(await screen.findByRole('menuitem', {name: 'Delete'})); + + expect(screen.queryByRole('button', {name: 'Wait: 1 day'})).not.toBeInTheDocument(); + expect(screen.getByRole('button', {name: 'Send email: Welcome to The Blueprint'})).toBeInTheDocument(); + }); + + it('opens the email editor from the send email node right-click menu', async () => { + mockUseReadAutomation.mockReturnValue({ + data: {automations: [automationDetail]}, + isLoading: false, + isError: false + }); + + renderEditor(); + + const emailStep = screen.getByRole('button', {name: 'Send email: Welcome to The Blueprint'}); + fireEvent.contextMenu(emailStep); + fireEvent.click(await screen.findByRole('menuitem', {name: 'Edit email body'})); + + expect(await screen.findByTestId('email-content-modal')).toBeInTheDocument(); + expect(screen.queryByRole('complementary', {name: 'Step details'})).not.toBeInTheDocument(); + expect(emailStep).toHaveAttribute('aria-pressed', 'false'); + expect(screen.getByTestId('modal-initial-mode')).toHaveTextContent('edit'); + expect(screen.getByTestId('modal-initial-subject')).toHaveTextContent('Welcome to The Blueprint'); + expect(screen.getByTestId('modal-initial-lexical')).toHaveTextContent('{"root":{"children":[]}}'); + }); + + it('opens the email editor preview from the send email node right-click menu', async () => { + mockUseReadAutomation.mockReturnValue({ + data: {automations: [automationDetail]}, + isLoading: false, + isError: false + }); + + renderEditor(); + + const emailStep = screen.getByRole('button', {name: 'Send email: Welcome to The Blueprint'}); + fireEvent.contextMenu(emailStep); + fireEvent.click(await screen.findByRole('menuitem', {name: 'Preview'})); + + expect(await screen.findByTestId('email-content-modal')).toBeInTheDocument(); + expect(screen.queryByRole('complementary', {name: 'Step details'})).not.toBeInTheDocument(); + expect(emailStep).toHaveAttribute('aria-pressed', 'false'); + expect(screen.getByTestId('modal-initial-mode')).toHaveTextContent('preview'); + }); + + it('opens the email editor from an email node double-click without opening the sidebar', async () => { + mockUseReadAutomation.mockReturnValue({ + data: {automations: [automationDetail]}, + isLoading: false, + isError: false + }); + + renderEditor(); + + const emailStep = screen.getByRole('button', {name: 'Send email: Welcome to The Blueprint'}); + fireEvent.doubleClick(emailStep); + + expect(await screen.findByTestId('email-content-modal')).toBeInTheDocument(); + expect(screen.queryByRole('complementary', {name: 'Step details'})).not.toBeInTheDocument(); + expect(emailStep).toHaveAttribute('aria-pressed', 'false'); + expect(screen.getByTestId('modal-initial-mode')).toHaveTextContent('edit'); + }); + it('shows paid member eligibility for the paid welcome automation trigger', () => { mockUseReadAutomation.mockReturnValue({ data: { @@ -459,6 +696,34 @@ describe('AutomationEditor', () => { expect(mutateCall.actions).toContainEqual({id: 'action-wait', type: 'wait', data: {wait_hours: 72}}); }); + it('increments and decrements the wait step from the day input group buttons', () => { + mockUseReadAutomation.mockReturnValue({ + data: {automations: [automationDetail]}, + isLoading: false, + isError: false + }); + + renderEditor(); + + fireEvent.click(screen.getByRole('button', {name: 'Wait: 1 day'})); + let sidebar = screen.getByRole('complementary', {name: 'Step details'}); + expect(within(sidebar).getByLabelText('Wait for')).toHaveValue('1'); + expect(within(sidebar).getByRole('button', {name: 'Decrease wait by one day'})).toBeDisabled(); + + fireEvent.click(within(sidebar).getByRole('button', {name: 'Increase wait by one day'})); + + expect(screen.getByRole('button', {name: 'Wait: 2 days'})).toBeInTheDocument(); + sidebar = screen.getByRole('complementary', {name: 'Step details'}); + expect(within(sidebar).getByDisplayValue('2')).toBeInTheDocument(); + + fireEvent.click(within(sidebar).getByRole('button', {name: 'Decrease wait by one day'})); + + expect(screen.getByRole('button', {name: 'Wait: 1 day'})).toBeInTheDocument(); + sidebar = screen.getByRole('complementary', {name: 'Step details'}); + expect(within(sidebar).getByDisplayValue('1')).toBeInTheDocument(); + expect(within(sidebar).getByRole('button', {name: 'Decrease wait by one day'})).toBeDisabled(); + }); + it('rejects non-decimal wait editor values', () => { mockUseReadAutomation.mockReturnValue({ data: {automations: [automationDetail]}, @@ -476,6 +741,7 @@ describe('AutomationEditor', () => { fireEvent.change(waitInput, {target: {value}}); expect(waitInput).toHaveAttribute('aria-invalid', 'true'); + expect(waitInput).toHaveAttribute('aria-describedby', 'automation-wait-days-error'); expect(within(sidebar).getByText('Enter a whole number between 1 and 30 days.')).toBeInTheDocument(); expect(screen.getByRole('button', {name: 'Published'})).toBeDisabled(); } @@ -881,7 +1147,9 @@ describe('AutomationEditor', () => { fireEvent.click(within(picker).getByText('Wait')); // The new step renders with the default 24h wait ("1 day") at the end of the chain. - expect(screen.getByText('1 day')).toBeInTheDocument(); + const insertedNode = screen.getByRole('button', {name: 'Wait: 1 day'}); + expect(insertedNode).toHaveAttribute('aria-pressed', 'true'); + expect(screen.getAllByText('1 day')).toHaveLength(2); // Adding a step flips the editor into a dirty state. expect(screen.getByRole('button', {name: 'Publish changes'})).toBeEnabled(); }); @@ -900,7 +1168,11 @@ describe('AutomationEditor', () => { fireEvent.click(within(picker).getByText('Email')); // The new send_email step renders with the placeholder subject. - expect(screen.getByText('Untitled email')).toBeInTheDocument(); + const insertedNode = screen.getByRole('button', {name: 'Send email: Untitled email'}); + expect(screen.getAllByText('Untitled email')).toHaveLength(2); + expect(insertedNode).toHaveClass('animate-in'); + expect(insertedNode).toHaveClass('zoom-in-90'); + expect(insertedNode).toHaveAttribute('aria-pressed', 'true'); expect(screen.getByRole('button', {name: 'Publish changes'})).toBeEnabled(); }); @@ -932,6 +1204,34 @@ describe('AutomationEditor', () => { expect(edgePairs).toContainEqual([insertedId, 'action-email']); }); + it('keeps the in-edge + button visible after leaving the button while still hovering the edge', () => { + mockUseReadAutomation.mockReturnValue({ + data: {automations: [automationDetail]}, + isLoading: false, + isError: false + }); + + renderEditor(); + + const edge = screen.getByTestId('react-flow-mock-edges').querySelector('[data-edge-id="e-action-wait-action-email"]'); + const edgeGroup = edge?.querySelector('g'); + const button = screen.getByTestId('add-step-button-action-wait-action-email'); + const labelHitZone = button.closest('.pointer-events-auto'); + + expect(edgeGroup).toBeInTheDocument(); + expect(labelHitZone).toBeInTheDocument(); + + fireEvent.mouseEnter(edgeGroup!); + expect(button).toHaveClass('opacity-100'); + + fireEvent.mouseEnter(labelHitZone!); + fireEvent.mouseLeave(labelHitZone!, {relatedTarget: edgeGroup}); + expect(button).toHaveClass('opacity-100'); + + fireEvent.mouseLeave(edgeGroup!); + expect(button).toHaveClass('opacity-0'); + }); + it('deletes a wait step and reconnects the chain', () => { mockUseReadAutomation.mockReturnValue({ data: {automations: [automationDetail]}, From 4f485adb3a590463d80b77c9eec2d3093208e011 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Wed, 3 Jun 2026 09:57:27 -0500 Subject: [PATCH 7/8] Disabled Secretlint false positive in settings helper (#28341) ref https://github.com/TryGhost/Ghost/pull/28325/changes#r3344367234 --- .../core/server/services/settings-helpers/settings-helpers.js | 1 + 1 file changed, 1 insertion(+) diff --git a/ghost/core/core/server/services/settings-helpers/settings-helpers.js b/ghost/core/core/server/services/settings-helpers/settings-helpers.js index 76476c88fa5..e963fd06435 100644 --- a/ghost/core/core/server/services/settings-helpers/settings-helpers.js +++ b/ghost/core/core/server/services/settings-helpers/settings-helpers.js @@ -40,6 +40,7 @@ class SettingsHelpers { throw new errors.IncorrectUsageError({message: tpl(messages.incorrectKeyType)}); } + // secretlint-disable-next-line @secretlint/secretlint-rule-pattern const secretKey = this.settingsCache.get(`stripe_${type === 'connect' ? 'connect_' : ''}secret_key`); const publicKey = this.settingsCache.get(`stripe_${type === 'connect' ? 'connect_' : ''}publishable_key`); From 2cdffab8cea7e2f475ab43f7574232e0ff2d57a7 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Wed, 3 Jun 2026 10:21:47 -0500 Subject: [PATCH 8/8] Improved member repository subscription argument type (#28342) no ref Before this change, an argument to `linkSubscription` was `any`. Now it's a "proper" type. --- .../members/members-api/repositories/member-repository.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ghost/core/core/server/services/members/members-api/repositories/member-repository.js b/ghost/core/core/server/services/members/members-api/repositories/member-repository.js index bec03ddfef0..57eda19fc45 100644 --- a/ghost/core/core/server/services/members/members-api/repositories/member-repository.js +++ b/ghost/core/core/server/services/members/members-api/repositories/member-repository.js @@ -11,6 +11,7 @@ const crypto = require('crypto'); const hasActiveOffer = require('../utils/has-active-offer'); const StartAutomationsPollEvent = require('../../../automations/events/start-automations-poll-event'); const {MEMBER_WELCOME_EMAIL_SLUGS} = require('../../../member-welcome-emails/constants'); +/** @import {Knex} from 'knex' */ /** @import * as automationsApi from '../../../automations/automations-api' */ const messages = { @@ -1059,7 +1060,8 @@ module.exports = class MemberRepository { * @param {Object} data.subscription * @param {string | null} [data.offerId] * @param {import('../../../member-attribution/attribution-builder').AttributionResource} [data.attribution] - * @param {*} options + * @param {object} [options] + * @param {Knex.Transaction} [options.transacting] * @returns */ async linkSubscription(data, options = {}) {