diff --git a/apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx b/apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx index 17df2c245bb..cb2be80ef8d 100644 --- a/apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx +++ b/apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx @@ -44,10 +44,6 @@ const features: Feature[] = [{ description: 'Preview the next version of the admin interface', flag: 'adminForward' }, { - title: 'Domain Warmup', - description: 'Enable custom sending domain warmup for gradual email volume increases', - flag: 'domainWarmup' -},{ title: 'Updated theme translation (beta)', description: 'Enable theme translation using i18next instead of the old translation package.', flag: 'themeTranslation' diff --git a/apps/admin/src/layout/app-sidebar/hooks/use-featurebase.ts b/apps/admin/src/layout/app-sidebar/hooks/use-featurebase.ts index 6020bda8247..3e239836156 100644 --- a/apps/admin/src/layout/app-sidebar/hooks/use-featurebase.ts +++ b/apps/admin/src/layout/app-sidebar/hooks/use-featurebase.ts @@ -1,116 +1,139 @@ -import {useCallback, useEffect, useRef} from 'react'; +import {useCallback, useEffect, useRef, useState} from 'react'; import {getFeaturebaseToken} from '@tryghost/admin-x-framework'; import {useBrowseConfig} from '@tryghost/admin-x-framework/api/config'; -import {useCurrentUser} from '@tryghost/admin-x-framework/api/current-user'; import {useFeatureFlag} from '@/hooks/use-feature-flag'; import {useUserPreferences} from '@/hooks/user-preferences'; +import {deferred, type Deferred} from '@/utils/deferred'; type FeaturebaseCallback = (err: unknown, data?: unknown) => void; type FeaturebaseFunction = (action: string, options: Record, callback?: FeaturebaseCallback) => void; declare global { interface Window { - Featurebase?: FeaturebaseFunction & {q?: unknown[]}; + Featurebase?: FeaturebaseFunction; } } const SDK_URL = 'https://do.featurebase.app/js/sdk.js'; +const DEFAULT_BOARD = 'Feature Request'; +let featurebaseSDKPromise: Promise | null = null; function loadFeaturebaseSDK(): Promise { - return new Promise((resolve, reject) => { - const existingScript = document.querySelector(`script[src="${SDK_URL}"]`); - if (existingScript) { - resolve(); - return; - } + if (!featurebaseSDKPromise) { + featurebaseSDKPromise = new Promise((resolve, reject) => { + const existingScript = document.querySelector(`script[src="${SDK_URL}"]`); + if (existingScript) { + resolve(); + return; + } - const script = document.createElement('script'); - script.src = SDK_URL; - script.onload = () => resolve(); - script.onerror = (event) => { - script.remove(); - const error = new Error(`[Featurebase] Failed to load SDK from ${SDK_URL}`, {cause: event}); - console.error(error); - reject(error); - }; - document.head.appendChild(script); - - // Set up the queue function while script loads - if (typeof window.Featurebase !== 'function') { - window.Featurebase = function (...args: unknown[]) { - (window.Featurebase!.q = window.Featurebase!.q || []).push(args); - } as FeaturebaseFunction & {q?: unknown[]}; - } - }); + const script = document.createElement('script'); + script.src = SDK_URL; + script.onload = () => resolve(); + script.onerror = (event) => { + script.remove(); + featurebaseSDKPromise = null; // Allow retry on next interaction + const error = new Error(`[Featurebase] Failed to load SDK from ${SDK_URL}`, {cause: event}); + console.error(error); + reject(error); + }; + document.head.appendChild(script); + }); + } + return featurebaseSDKPromise; } interface Featurebase { openFeedbackWidget: (options?: {board?: string}) => void; + preloadFeedbackWidget: () => void; } +/** + * Hook for lazy-loading and interacting with the Featurebase feedback widget. + * + * The SDK and authentication token are NOT fetched on mount. Instead, loading + * is deferred until user interaction (hover/focus/click on the Feedback button). + * This improves initial page load performance. + */ export function useFeaturebase(): Featurebase { - const {data: currentUser} = useCurrentUser(); const {data: config} = useBrowseConfig(); const {data: preferences} = useUserPreferences(); const featureFlagEnabled = useFeatureFlag('featurebaseFeedback'); - const isInitializingRef = useRef(false); - const initializedWithRef = useRef<{theme: string; token: string} | null>(null); + const [shouldLoad, setShouldLoad] = useState(false); - const featurebaseConfig = config?.config.featurebase; - const featurebaseOrg = featurebaseConfig?.organization; - const featurebaseEnabled = !!(featureFlagEnabled && featurebaseConfig?.enabled); + const {organization, enabled} = config?.config.featurebase ?? {}; + const featurebaseEnabled = !!(featureFlagEnabled && enabled); const theme = preferences?.nightShift ? 'dark' : 'light'; + // Token is only fetched once shouldLoad becomes true (on user interaction) const {data: tokenData} = getFeaturebaseToken({ - enabled: featurebaseEnabled + enabled: featurebaseEnabled && shouldLoad }); const token = tokenData?.featurebase?.token; useEffect(() => { - if (!featurebaseEnabled || !featurebaseOrg || !currentUser || !token) { - return; + if (shouldLoad) { + loadFeaturebaseSDK().catch((err) => { + console.error('[Featurebase] Failed to load SDK:', err); + }); } + }, [shouldLoad]); - const initializedWith = initializedWithRef.current; - const initializedWithSameData = initializedWith && initializedWith.theme === theme && initializedWith.token === token; - - if (isInitializingRef.current || initializedWithSameData) { + const deferredInitRef = useRef>(deferred()); + useEffect(() => { + if (!shouldLoad || !organization || !token) { return; } - - isInitializingRef.current = true; - - loadFeaturebaseSDK().then(() => { + void featurebaseSDKPromise?.then(() => { window.Featurebase?.('initialize_feedback_widget', { - organization: featurebaseOrg, + organization, theme, - defaultBoard: 'Feature Request', + defaultBoard: DEFAULT_BOARD, featurebaseJwt: token }, (err) => { - isInitializingRef.current = false; - if (err) { console.error('[Featurebase] Failed to initialize widget:', err); - initializedWithRef.current = null; + deferredInitRef.current.reject(err); + + // reset so we can retry on next interaction + deferredInitRef.current = deferred(); + setShouldLoad(false); } else { - initializedWithRef.current = {theme, token}; + deferredInitRef.current.resolve(); } }); - }).catch(() => { - isInitializingRef.current = false; - initializedWithRef.current = null; }); - }, [featurebaseEnabled, featurebaseOrg, currentUser, token, theme]); + }, [organization, theme, token, shouldLoad]); + + /** + * Called on hover/focus to start loading SDK + fetching token in advance. + * This makes the widget open faster when the user actually clicks. + */ + const preloadFeedbackWidget = useCallback(() => { + if (!featurebaseEnabled) { + return; + } + // Trigger SDK loading and initialization via effects above + setShouldLoad(true); + }, [featurebaseEnabled]); const openFeedbackWidget = useCallback((options?: {board?: string}) => { - window.postMessage({ - target: 'FeaturebaseWidget', - data: { - action: 'openFeedbackWidget', - ...(options?.board && {setBoard: options.board}) - } - }, '*'); - }, []); + if (!featurebaseEnabled) { + return; + } + + setShouldLoad(true); + + void deferredInitRef.current.promise.then(() => { + window.postMessage({ + target: 'FeaturebaseWidget', + data: { + action: 'openFeedbackWidget', + ...(options?.board && {setBoard: options.board}) + } + }, '*'); + }); + }, [featurebaseEnabled]); - return {openFeedbackWidget}; + return {openFeedbackWidget, preloadFeedbackWidget}; } diff --git a/apps/admin/src/layout/app-sidebar/nav-ghost-pro.tsx b/apps/admin/src/layout/app-sidebar/nav-ghost-pro.tsx index c7daf6d7a48..4f95bbc4194 100644 --- a/apps/admin/src/layout/app-sidebar/nav-ghost-pro.tsx +++ b/apps/admin/src/layout/app-sidebar/nav-ghost-pro.tsx @@ -16,7 +16,7 @@ function NavGhostPro({ ...props }: React.ComponentProps) { const { data: currentUser } = useCurrentUser(); const { data: config } = useBrowseConfig(); const featurebaseFeedbackFlag = useFeatureFlag('featurebaseFeedback'); - const { openFeedbackWidget } = useFeaturebase(); + const { openFeedbackWidget, preloadFeedbackWidget } = useFeaturebase(); if (!currentUser) { return null; @@ -44,7 +44,7 @@ function NavGhostPro({ ...props }: React.ComponentProps) { )} {showFeedback && ( - + Feedback diff --git a/apps/admin/src/utils/deferred.test.ts b/apps/admin/src/utils/deferred.test.ts new file mode 100644 index 00000000000..e7e995e24df --- /dev/null +++ b/apps/admin/src/utils/deferred.test.ts @@ -0,0 +1,28 @@ +import {describe, expect, it} from 'vitest'; +import {deferred} from './deferred'; + +describe('deferred', () => { + it('returns a promise with externally accessible resolve and reject functions', () => { + const d = deferred(); + + expect(d.promise).toBeInstanceOf(Promise); + expect(typeof d.resolve).toBe('function'); + expect(typeof d.reject).toBe('function'); + }); + + it('resolve function resolves the promise', async () => { + const d = deferred(); + + d.resolve('test value'); + + await expect(d.promise).resolves.toBe('test value'); + }); + + it('reject function rejects the promise', async () => { + const d = deferred(); + + d.reject(new Error('test error')); + + await expect(d.promise).rejects.toThrow('test error'); + }); +}); diff --git a/apps/admin/src/utils/deferred.ts b/apps/admin/src/utils/deferred.ts new file mode 100644 index 00000000000..20e912536e3 --- /dev/null +++ b/apps/admin/src/utils/deferred.ts @@ -0,0 +1,10 @@ +export const deferred = function deferred() { + let resolve!: (value: T) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((_resolve, _reject) => { + resolve = _resolve; + reject = _reject; + }); + return {promise, resolve, reject}; +}; +export type Deferred = ReturnType>; diff --git a/ghost/admin/app/components/editor/email-size-warning.js b/ghost/admin/app/components/editor/email-size-warning.js index e6a775bd934..0d085ee800f 100644 --- a/ghost/admin/app/components/editor/email-size-warning.js +++ b/ghost/admin/app/components/editor/email-size-warning.js @@ -14,8 +14,10 @@ export default class EmailSizeWarningComponent extends Component { get isEnabled() { return this.settings.editorDefaultEmailRecipients !== 'disabled' && this.args.post - && !this.args.post.email - && !this.args.post.isNew; + && !this.args.post.isNew + && !this.args.post.hasEmail + && !this.args.post.isPublished + && !this.args.post.isPage; } constructor() { diff --git a/ghost/admin/app/services/email-size-warning.js b/ghost/admin/app/services/email-size-warning.js index c6fba919297..fab0d38608c 100644 --- a/ghost/admin/app/services/email-size-warning.js +++ b/ghost/admin/app/services/email-size-warning.js @@ -20,7 +20,6 @@ export default class EmailSizeWarningService extends Service { @inject config; - _newsletter = null; _lastPostId = null; _lastUpdatedAt = null; @@ -51,18 +50,6 @@ export default class EmailSizeWarningService extends Service { return this._fetchTask.perform(post); } - async _loadNewsletter() { - if (!this._newsletter) { - const newsletters = await this.store.query('newsletter', { - filter: 'status:active', - order: 'sort_order DESC', - limit: 1 - }); - this._newsletter = newsletters.firstObject; - } - return this._newsletter; - } - _calculateLinkRewritingAdjustment(html) { const contentStartMarker = ''; const contentEndMarker = ''; @@ -105,17 +92,11 @@ export default class EmailSizeWarningService extends Service { @task *_fetchTask(post) { - yield this._loadNewsletter(); - if (!this._newsletter) { - return {overLimit: false, emailSizeKb: null}; - } - try { const url = new URL( this.ghostPaths.url.api('/email_previews/posts', post.id), window.location.href ); - url.searchParams.set('newsletter', this._newsletter.slug); const response = yield this.ajax.request(url.href); const [emailPreview] = response.email_previews; diff --git a/ghost/admin/app/styles/layouts/member-activity.css b/ghost/admin/app/styles/layouts/member-activity.css index a5275b9d2b5..3d9f24c7358 100644 --- a/ghost/admin/app/styles/layouts/member-activity.css +++ b/ghost/admin/app/styles/layouts/member-activity.css @@ -136,6 +136,10 @@ padding-bottom: 12px; } +.gh-members-activity .gh-list-scrolling tbody tr:hover > .gh-list-data { + background: var(--whitegrey-l2); +} + .gh-members-activity .gh-list h3 { margin-bottom: 4px; } diff --git a/ghost/core/core/server/services/email-address/email-address-service.ts b/ghost/core/core/server/services/email-address/email-address-service.ts index 4620523e1f4..6f477966e02 100644 --- a/ghost/core/core/server/services/email-address/email-address-service.ts +++ b/ghost/core/core/server/services/email-address/email-address-service.ts @@ -15,10 +15,6 @@ export type EmailAddressesValidation = { export type EmailAddressType = 'from' | 'replyTo'; -type LabsService = { - isSet: (flag: string) => boolean -} - type GetAddressOptions = { useFallbackAddress: boolean } @@ -30,7 +26,6 @@ export class EmailAddressService { #getFallbackDomain: () => string | null; #getFallbackEmail: () => EmailAddress | null; #isValidEmailAddress: (email: string) => boolean; - #labs: LabsService; constructor(dependencies: { getManagedEmailEnabled: () => boolean, @@ -38,9 +33,7 @@ export class EmailAddressService { getFallbackDomain: () => string | null, getDefaultEmail: () => EmailAddress, getFallbackEmail: () => string | null, - isValidEmailAddress: (email: string) => boolean, - labs: LabsService - + isValidEmailAddress: (email: string) => boolean }) { this.#getManagedEmailEnabled = dependencies.getManagedEmailEnabled; this.#getSendingDomain = dependencies.getSendingDomain; @@ -54,7 +47,6 @@ export class EmailAddressService { return EmailAddressParser.parse(fallbackAddress); }; this.#isValidEmailAddress = dependencies.isValidEmailAddress; - this.#labs = dependencies.labs; } get sendingDomain(): string | null { @@ -117,7 +109,7 @@ export class EmailAddressService { } // Case: use fallback address when warming up custom domain - if (this.#labs.isSet('domainWarmup') && options.useFallbackAddress) { + if (options.useFallbackAddress) { const fallbackEmail = this.fallbackEmail; if (fallbackEmail) { if (!fallbackEmail.name) { diff --git a/ghost/core/core/server/services/email-service/domain-warming-service.ts b/ghost/core/core/server/services/email-service/domain-warming-service.ts index d8628132cd6..1f69d297d47 100644 --- a/ghost/core/core/server/services/email-service/domain-warming-service.ts +++ b/ghost/core/core/server/services/email-service/domain-warming-service.ts @@ -1,7 +1,3 @@ -type LabsService = { - isSet: (flag: string) => boolean; -}; - type ConfigService = { get: (key: string) => string | undefined; } @@ -29,17 +25,14 @@ const DefaultWarmupOptions: WarmupVolumeOptions = { export class DomainWarmingService { #emailModel: EmailModel; - #labs: LabsService; #config: ConfigService; #warmupConfig: WarmupVolumeOptions; constructor(dependencies: { models: {Email: EmailModel}; - labs: LabsService; config: ConfigService; }) { this.#emailModel = dependencies.models.Email; - this.#labs = dependencies.labs; this.#config = dependencies.config; this.#warmupConfig = DefaultWarmupOptions; @@ -49,12 +42,6 @@ export class DomainWarmingService { * @returns Whether the domain warming feature is enabled */ isEnabled(): boolean { - const hasLabsFlag = this.#labs.isSet('domainWarmup'); - - if (!hasLabsFlag) { - return false; - } - const fallbackDomain = this.#config.get('hostSettings:managedEmail:fallbackDomain'); const fallbackAddress = this.#config.get('hostSettings:managedEmail:fallbackAddress'); diff --git a/ghost/core/core/server/services/email-service/email-service-wrapper.js b/ghost/core/core/server/services/email-service/email-service-wrapper.js index fa1bd7e10ac..0381c9de00e 100644 --- a/ghost/core/core/server/services/email-service/email-service-wrapper.js +++ b/ghost/core/core/server/services/email-service/email-service-wrapper.js @@ -106,7 +106,6 @@ class EmailServiceWrapper { const domainWarmingService = new DomainWarmingService({ models: {Email}, - labs, config: configService }); diff --git a/ghost/core/core/server/services/lib/mailgun-client.js b/ghost/core/core/server/services/lib/mailgun-client.js index 04ada8fb129..3df0a659480 100644 --- a/ghost/core/core/server/services/lib/mailgun-client.js +++ b/ghost/core/core/server/services/lib/mailgun-client.js @@ -7,14 +7,12 @@ const errors = require('@tryghost/errors'); module.exports = class MailgunClient { #config; #settings; - #labs; static DEFAULT_BATCH_SIZE = 1000; - constructor({config, settings, labs}) { + constructor({config, settings}) { this.#config = config; this.#settings = settings; - this.#labs = labs; } /** @@ -194,13 +192,10 @@ module.exports = class MailgunClient { #getDomainsToFetch(mailgunConfig) { const domains = [mailgunConfig.domain]; - // Check if domain warming is enabled - if (this.#labs.isSet('domainWarmup')) { - const fallbackDomain = this.#config.get('hostSettings:managedEmail:fallbackDomain'); - if (fallbackDomain && fallbackDomain !== mailgunConfig.domain) { - domains.push(fallbackDomain); - logging.info(`[MailgunClient] Domain warming enabled, fetching from both primary (${mailgunConfig.domain}) and fallback (${fallbackDomain}) domains`); - } + const fallbackDomain = this.#config.get('hostSettings:managedEmail:fallbackDomain'); + if (fallbackDomain && fallbackDomain !== mailgunConfig.domain) { + domains.push(fallbackDomain); + logging.info(`[MailgunClient] Domain warming enabled, fetching from both primary (${mailgunConfig.domain}) and fallback (${fallbackDomain}) domains`); } return domains; diff --git a/ghost/core/core/shared/labs.js b/ghost/core/core/shared/labs.js index 962c1e3cec3..9f1104fb93d 100644 --- a/ghost/core/core/shared/labs.js +++ b/ghost/core/core/shared/labs.js @@ -52,7 +52,6 @@ const PRIVATE_FEATURES = [ 'emailUniqueid', 'welcomeEmails', 'adminForward', - 'domainWarmup', 'themeTranslation', 'commentModeration', 'commentPermalinks', diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snap index 187e79fc435..41728dc089a 100644 --- a/ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snap +++ b/ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snap @@ -18,7 +18,6 @@ Object { "contentVisibility": true, "contentVisibilityAlpha": true, "customFonts": true, - "domainWarmup": true, "editorExcerpt": true, "emailCustomization": true, "emailUniqueid": true, diff --git a/ghost/core/test/integration/services/email-service/domain-warming.test.js b/ghost/core/test/integration/services/email-service/domain-warming.test.js index 0a3ede1ef5f..15c92ebc5cf 100644 --- a/ghost/core/test/integration/services/email-service/domain-warming.test.js +++ b/ghost/core/test/integration/services/email-service/domain-warming.test.js @@ -3,13 +3,11 @@ const models = require('../../../../core/server/models'); const sinon = require('sinon'); const assert = require('assert/strict'); const jobManager = require('../../../../core/server/services/jobs/job-service'); -const labs = require('../../../../core/shared/labs'); const configUtils = require('../../../utils/config-utils'); describe('Domain Warming Integration Tests', function () { let agent; let clock; - let labsStub; before(async function () { const agents = await agentProvider.getAgentsWithFrontend(); @@ -93,14 +91,6 @@ describe('Domain Warming Integration Tests', function () { mockManager.mockMailgun(); mockManager.mockStripe(); - // Enable the domain warming labs flag - labsStub = sinon.stub(labs, 'isSet').callsFake((key) => { - if (key === 'domainWarmup') { - return true; - } - return false; - }); - // Set required config values for domain warming configUtils.set('hostSettings:managedEmail:fallbackDomain', 'fallback.example.com'); configUtils.set('hostSettings:managedEmail:fallbackAddress', 'noreply@fallback.example.com'); @@ -111,10 +101,7 @@ describe('Domain Warming Integration Tests', function () { clock.restore(); clock = null; } - if (labsStub) { - labsStub.restore(); - labsStub = null; - } + mockManager.restore(); await configUtils.restore(); await jobManager.allSettled(); @@ -262,9 +249,9 @@ describe('Domain Warming Integration Tests', function () { } }); - it('sends all emails via fallback when domain warming is disabled', async function () { - labsStub.restore(); - labsStub = sinon.stub(labs, 'isSet').returns(false); + it('does not warm up when fallback domain and address are not set', async function () { + configUtils.set('hostSettings:managedEmail:fallbackDomain', null); + configUtils.set('hostSettings:managedEmail:fallbackAddress', null); await createMembers(50, 'nowarmup'); diff --git a/ghost/core/test/unit/frontend/helpers/__snapshots__/ghost-head.test.js.snap b/ghost/core/test/unit/frontend/helpers/__snapshots__/ghost-head.test.js.snap index 4f6adae0afb..0e78caf590b 100644 --- a/ghost/core/test/unit/frontend/helpers/__snapshots__/ghost-head.test.js.snap +++ b/ghost/core/test/unit/frontend/helpers/__snapshots__/ghost-head.test.js.snap @@ -1097,57 +1097,6 @@ Object { } `; -exports[`{{ghost_head}} helper includes tinybird tracker script when config is set does not include tracker script when neither trafficAnalytics nor trafficAnalyticsTracking is set 1 1`] = ` -Object { - "rendered": " - - - - - - - - - - - - - - - - - - - - - - - ", -} -`; - exports[`{{ghost_head}} helper includes tinybird tracker script when config is set does not include tracker script when preview is set 1 1`] = ` Object { "rendered": " @@ -1173,57 +1122,6 @@ Object { } `; -exports[`{{ghost_head}} helper includes tinybird tracker script when config is set does not include tracker script when trafficAnalytics is not set 1 1`] = ` -Object { - "rendered": " - - - - - - - - - - - - - - - - - - - - - - - ", -} -`; - exports[`{{ghost_head}} helper includes tinybird tracker script when config is set does not include tracker script when web analytics is not enabled 1 1`] = ` Object { "rendered": " @@ -1483,213 +1381,6 @@ Object { } `; -exports[`{{ghost_head}} helper includes tinybird tracker script when config is set includes tracker script 1 1`] = ` -Object { - "rendered": " - - - - - - - - - - - - - - - - - - - - - - - - ", -} -`; - -exports[`{{ghost_head}} helper includes tinybird tracker script when config is set includes tracker script when both trafficAnalytics and trafficAnalyticsTracking are set 1 1`] = ` -Object { - "rendered": " - - - - - - - - - - - - - - - - - - - - - - - - ", -} -`; - -exports[`{{ghost_head}} helper includes tinybird tracker script when config is set includes tracker script when trafficAnalytics is set 1 1`] = ` -Object { - "rendered": " - - - - - - - - - - - - - - - - - - - - - - - ", -} -`; - -exports[`{{ghost_head}} helper includes tinybird tracker script when config is set includes tracker script when trafficAnalyticsTracking is set 1 1`] = ` -Object { - "rendered": " - - - - - - - - - - - - - - - - - - - - - - - - ", -} -`; - exports[`{{ghost_head}} helper includes tinybird tracker script when config is set includes tracker script when web analytics is enabled 1 1`] = ` Object { "rendered": " diff --git a/ghost/core/test/unit/server/services/email-address/email-address-service.test.ts b/ghost/core/test/unit/server/services/email-address/email-address-service.test.ts index e9da2ad364c..169412ed534 100644 --- a/ghost/core/test/unit/server/services/email-address/email-address-service.test.ts +++ b/ghost/core/test/unit/server/services/email-address/email-address-service.test.ts @@ -1,20 +1,7 @@ import assert from 'assert/strict'; -import sinon from 'sinon'; import {EmailAddressService} from '../../../../../core/server/services/email-address/email-address-service'; describe('EmailAddressService', function () { - let labsStub: any; - - beforeEach(function () { - labsStub = { - isSet: sinon.stub() - }; - }); - - afterEach(function () { - sinon.restore(); - }); - // Helper to create service config with overrides const createConfig = (overrides: any = {}) => ({ getManagedEmailEnabled: () => true, @@ -23,7 +10,6 @@ describe('EmailAddressService', function () { getDefaultEmail: () => ({address: 'noreply@ghost.org', name: 'Ghost'}), getFallbackEmail: () => 'fallback@fallback.example.com', isValidEmailAddress: () => true, - labs: labsStub, ...overrides }); @@ -33,8 +19,7 @@ describe('EmailAddressService', function () { }; describe('getAddress with fallback domain', function () { - it('uses fallback address when domainWarmup flag is enabled and useFallbackAddress is true', function () { - labsStub.isSet.withArgs('domainWarmup').returns(true); + it('uses fallback address when useFallbackAddress is true', function () { const service = createService(); const result = service.getAddress({ @@ -48,7 +33,6 @@ describe('EmailAddressService', function () { }); it('does not use fallback address when useFallbackAddress is false', function () { - labsStub.isSet.withArgs('domainWarmup').returns(true); const service = createService(); const result = service.getAddress({ @@ -60,22 +44,7 @@ describe('EmailAddressService', function () { assert.equal(result.replyTo, undefined); }); - it('does not use fallback address when domainWarmup flag is disabled', function () { - labsStub.isSet.withArgs('domainWarmup').returns(false); - const service = createService(); - - const result = service.getAddress({ - from: {address: 'custom@custom.example.com', name: 'Custom Sender'} - }, {useFallbackAddress: true}); - - // Should use the original address when flag is disabled - assert.equal(result.from.address, 'custom@custom.example.com'); - assert.equal(result.from.name, 'Custom Sender'); - assert.equal(result.replyTo, undefined); - }); - it('does not use fallback address when fallback email is not configured', function () { - labsStub.isSet.withArgs('domainWarmup').returns(true); const service = createService({ getFallbackEmail: () => null }); @@ -91,7 +60,6 @@ describe('EmailAddressService', function () { }); it('preserves existing replyTo when using fallback address', function () { - labsStub.isSet.withArgs('domainWarmup').returns(true); const service = createService(); const result = service.getAddress({ @@ -106,7 +74,6 @@ describe('EmailAddressService', function () { }); it('sets fallback from name to default email name when preferred from has no name', function () { - labsStub.isSet.withArgs('domainWarmup').returns(true); const service = createService(); const result = service.getAddress({ @@ -119,7 +86,6 @@ describe('EmailAddressService', function () { }); it('preserves fallback email name when already set', function () { - labsStub.isSet.withArgs('domainWarmup').returns(true); const service = createService({ getFallbackEmail: () => '"Fallback Sender" ' }); diff --git a/ghost/core/test/unit/server/services/email-service/domain-warming-service.test.ts b/ghost/core/test/unit/server/services/email-service/domain-warming-service.test.ts index 1eb7378ae44..81781759a2e 100644 --- a/ghost/core/test/unit/server/services/email-service/domain-warming-service.test.ts +++ b/ghost/core/test/unit/server/services/email-service/domain-warming-service.test.ts @@ -4,9 +4,6 @@ import sinon from 'sinon'; import assert from 'assert/strict'; describe('Domain Warming Service', function () { - let labs: { - isSet: sinon.SinonStub; - }; let config: { get: sinon.SinonStub; }; @@ -16,10 +13,6 @@ describe('Domain Warming Service', function () { let clock: sinon.SinonFakeTimers; beforeEach(function () { - labs = { - isSet: sinon.stub().returns(false) - }; - config = { get: sinon.stub().returns(undefined) }; @@ -41,7 +34,6 @@ describe('Domain Warming Service', function () { it('should instantiate with required dependencies', function () { const service = new DomainWarmingService({ models: {Email}, - labs, config }); assert.ok(service); @@ -49,27 +41,11 @@ describe('Domain Warming Service', function () { }); describe('isEnabled', function () { - it('should return false when domainWarmup flag is not set', function () { - labs.isSet.withArgs('domainWarmup').returns(false); - const service = new DomainWarmingService({ - models: {Email}, - labs, - config - }); - - const result = service.isEnabled(); - assert.equal(result, false); - sinon.assert.calledOnce(labs.isSet); - sinon.assert.calledWith(labs.isSet, 'domainWarmup'); - }); - - it('should return false when domainWarmup flag is set but fallback domain is missing', function () { - labs.isSet.withArgs('domainWarmup').returns(true); + it('should return false when fallback domain is missing', function () { config.get.withArgs('hostSettings:managedEmail:fallbackDomain').returns(undefined); config.get.withArgs('hostSettings:managedEmail:fallbackAddress').returns('noreply@fallback.com'); const service = new DomainWarmingService({ models: {Email}, - labs, config }); @@ -77,13 +53,11 @@ describe('Domain Warming Service', function () { assert.equal(result, false); }); - it('should return false when domainWarmup flag is set but fallback address is missing', function () { - labs.isSet.withArgs('domainWarmup').returns(true); + it('should return false when fallback address is missing', function () { config.get.withArgs('hostSettings:managedEmail:fallbackDomain').returns('fallback.example.com'); config.get.withArgs('hostSettings:managedEmail:fallbackAddress').returns(undefined); const service = new DomainWarmingService({ models: {Email}, - labs, config }); @@ -91,13 +65,11 @@ describe('Domain Warming Service', function () { assert.equal(result, false); }); - it('should return true when domainWarmup flag is set and fallback config is present', function () { - labs.isSet.withArgs('domainWarmup').returns(true); + it('should return true when fallback config is present', function () { config.get.withArgs('hostSettings:managedEmail:fallbackDomain').returns('fallback.example.com'); config.get.withArgs('hostSettings:managedEmail:fallbackAddress').returns('noreply@fallback.com'); const service = new DomainWarmingService({ models: {Email}, - labs, config }); @@ -121,7 +93,6 @@ describe('Domain Warming Service', function () { const service = new DomainWarmingService({ models: {Email}, - labs, config }); @@ -139,7 +110,6 @@ describe('Domain Warming Service', function () { const service = new DomainWarmingService({ models: {Email}, - labs, config }); @@ -158,7 +128,6 @@ describe('Domain Warming Service', function () { const service = new DomainWarmingService({ models: {Email}, - labs, config }); @@ -178,7 +147,6 @@ describe('Domain Warming Service', function () { const service = new DomainWarmingService({ models: {Email}, - labs, config }); @@ -197,7 +165,6 @@ describe('Domain Warming Service', function () { const service = new DomainWarmingService({ models: {Email}, - labs, config }); @@ -215,7 +182,6 @@ describe('Domain Warming Service', function () { const service = new DomainWarmingService({ models: {Email}, - labs, config }); @@ -231,7 +197,6 @@ describe('Domain Warming Service', function () { const service = new DomainWarmingService({ models: {Email}, - labs, config }); @@ -272,7 +237,6 @@ describe('Domain Warming Service', function () { const service = new DomainWarmingService({ models: {Email: EmailModel}, - labs, config }); @@ -292,7 +256,6 @@ describe('Domain Warming Service', function () { const service = new DomainWarmingService({ models: {Email}, - labs, config }); @@ -311,7 +274,6 @@ describe('Domain Warming Service', function () { const service = new DomainWarmingService({ models: {Email}, - labs, config }); diff --git a/ghost/core/test/unit/server/services/lib/mailgun-client.test.js b/ghost/core/test/unit/server/services/lib/mailgun-client.test.js index e606ea8bbb4..130b2df4351 100644 --- a/ghost/core/test/unit/server/services/lib/mailgun-client.test.js +++ b/ghost/core/test/unit/server/services/lib/mailgun-client.test.js @@ -31,13 +31,12 @@ const createBatchCounter = (customHandler) => { }; describe('MailgunClient', function () { - let config, settings, labs; + let config, settings; beforeEach(function () { // options objects that can be stubbed or spied config = {get() {}}; settings = {get() {}}; - labs = {isSet() {}}; }); afterEach(function () { @@ -55,7 +54,7 @@ describe('MailgunClient', function () { batchSize: 1000 }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); assert(typeof mailgunClient.getBatchSize() === 'number'); }); @@ -71,7 +70,7 @@ describe('MailgunClient', function () { targetDeliveryWindow: 300 }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); assert.equal(mailgunClient.getTargetDeliveryWindow(), 300); }); @@ -86,7 +85,7 @@ describe('MailgunClient', function () { batchSize: 1000 }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); assert.equal(mailgunClient.getTargetDeliveryWindow(), 0); }); @@ -101,7 +100,7 @@ describe('MailgunClient', function () { batchSize: 1000, targetDeliveryWindow: 'invalid' }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); assert.equal(mailgunClient.getTargetDeliveryWindow(), 0); }); @@ -116,7 +115,7 @@ describe('MailgunClient', function () { batchSize: 1000, targetDeliveryWindow: -3000 }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); assert.equal(mailgunClient.getTargetDeliveryWindow(), 0); }); @@ -131,7 +130,7 @@ describe('MailgunClient', function () { batchSize: 1000 }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); assert.equal(mailgunClient.isConfigured(), true); }); @@ -141,12 +140,12 @@ describe('MailgunClient', function () { settingsStub.withArgs('mailgun_domain').returns('settingsdomain.com'); settingsStub.withArgs('mailgun_base_url').returns('https://example.com/v3'); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); assert.equal(mailgunClient.isConfigured(), true); }); it('cannot configure Mailgun if config/settings missing', function () { - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); assert.equal(mailgunClient.isConfigured(), false); }); @@ -163,7 +162,7 @@ describe('MailgunClient', function () { 'Content-Type': 'application/json' }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); await mailgunClient.fetchEvents(MAILGUN_OPTIONS, () => {}); settingsStub.withArgs('mailgun_api_key').returns('settingsApiKey2'); @@ -213,7 +212,7 @@ describe('MailgunClient', function () { 'Content-Type': 'application/json' }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); await mailgunClient.fetchEvents(MAILGUN_OPTIONS, () => {}); assert.equal(configApiMock.isDone(), true); @@ -222,7 +221,7 @@ describe('MailgunClient', function () { describe('send()', function () { it('does not send if not configured', async function () { - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); const response = await mailgunClient.send({}, {}, []); assert.equal(response, null); @@ -272,7 +271,7 @@ describe('MailgunClient', function () { 'Content-Type': 'application/json' }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); const response = await mailgunClient.send(message, recipientData, []); assert(response.id === 'message-id'); assert(sendMock.isDone()); @@ -307,7 +306,7 @@ describe('MailgunClient', function () { } }; - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); await assert.rejects(mailgunClient.send(message, recipientData, [])); }); @@ -351,7 +350,7 @@ describe('MailgunClient', function () { 'Content-Type': 'application/json' }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); const response = await mailgunClient.send(message, recipientData, []); assert(response.id === 'message-id'); assert(sendMock.isDone()); @@ -396,7 +395,7 @@ describe('MailgunClient', function () { 'Content-Type': 'application/json' }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); const response = await mailgunClient.send(message, recipientData, []); assert(response.id === 'message-id'); assert(sendMock.isDone()); @@ -441,7 +440,7 @@ describe('MailgunClient', function () { 'Content-Type': 'application/json' }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); const response = await mailgunClient.send(message, recipientData, []); assert(response.id === 'message-id'); assert(sendMock.isDone()); @@ -486,7 +485,7 @@ describe('MailgunClient', function () { 'Content-Type': 'application/json' }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); const response = await mailgunClient.send(message, recipientData, []); assert(response.id === 'message-id'); assert(sendMock.isDone()); @@ -531,7 +530,7 @@ describe('MailgunClient', function () { 'Content-Type': 'application/json' }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); const response = await mailgunClient.send(message, recipientData, []); assert(response.id === 'message-id'); assert(sendMock.isDone()); @@ -576,7 +575,7 @@ describe('MailgunClient', function () { 'Content-Type': 'application/json' }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); const response = await mailgunClient.send(message, recipientData, []); assert(response.id === 'message-id'); assert(sendMock.isDone()); @@ -621,7 +620,7 @@ describe('MailgunClient', function () { 'Content-Type': 'application/json' }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); const response = await mailgunClient.send(message, recipientData, []); assert(response.id === 'message-id'); assert(sendMock.isDone()); @@ -666,7 +665,7 @@ describe('MailgunClient', function () { 'Content-Type': 'application/json' }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); const response = await mailgunClient.send(message, recipientData, []); assert(response.id === 'message-id'); assert(sendMock.isDone()); @@ -676,7 +675,7 @@ describe('MailgunClient', function () { describe('fetchEvents()', function () { it('does not fetch if not configured', async function () { const counter = createBatchCounter(); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); await mailgunClient.fetchEvents(MAILGUN_OPTIONS, counter.batchHandler); assert.equal(counter.events, 0); assert.equal(counter.batches, 0); @@ -717,7 +716,7 @@ describe('MailgunClient', function () { const counter = createBatchCounter(); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); await mailgunClient.fetchEvents(MAILGUN_OPTIONS, counter.batchHandler); assert.equal(firstPageMock.isDone(), true); @@ -764,7 +763,7 @@ describe('MailgunClient', function () { const maxEvents = 3; - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); await mailgunClient.fetchEvents(MAILGUN_OPTIONS, counter.batchHandler, {maxEvents}); assert.equal(counter.batches, 2); @@ -810,7 +809,7 @@ describe('MailgunClient', function () { const maxEvents = 3; - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); await mailgunClient.fetchEvents(MAILGUN_OPTIONS, counter.batchHandler, {maxEvents}); assert.equal(counter.batches, 1); @@ -856,7 +855,7 @@ describe('MailgunClient', function () { throw new Error('test error'); }); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); await assert.rejects(mailgunClient.fetchEvents(MAILGUN_OPTIONS, counter.batchHandler), /test error/); assert.equal(counter.batches, 1); @@ -900,7 +899,7 @@ describe('MailgunClient', function () { const batchHandler = sinon.spy(); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); await mailgunClient.fetchEvents(MAILGUN_OPTIONS, batchHandler); assert.equal(firstPageMock.isDone(), true); @@ -927,7 +926,7 @@ describe('MailgunClient', function () { } }; - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); const result = mailgunClient.normalizeEvent(event); assert.deepEqual(result, { @@ -989,7 +988,7 @@ describe('MailgunClient', function () { timestamp: 1614275662 }; - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); const result = mailgunClient.normalizeEvent(event); assert.deepEqual(result, { @@ -1061,7 +1060,7 @@ describe('MailgunClient', function () { timestamp: 1614275662 }; - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); const result = mailgunClient.normalizeEvent(event); assert.deepEqual(result, { @@ -1082,8 +1081,6 @@ describe('MailgunClient', function () { }); describe('fetchEvents() - Domain Warming', function () { - let labsStub; - // Helper to setup config with domain warming const setupDomainWarmingConfig = (domainWarmingEnabled = true, fallbackDomain = 'fallback.com') => { const configStub = sinon.stub(config, 'get'); @@ -1095,11 +1092,8 @@ describe('MailgunClient', function () { }, batchSize: 1000 }); - configStub.withArgs('hostSettings:managedEmail:fallbackDomain').returns(fallbackDomain); - - // Stub the labs object that's passed to the constructor - labsStub = sinon.stub(labs, 'isSet'); - labsStub.withArgs('domainWarmup').returns(domainWarmingEnabled); + configStub.withArgs('hostSettings:managedEmail:fallbackDomain').returns( + domainWarmingEnabled ? fallbackDomain : null); return configStub; }; @@ -1117,7 +1111,7 @@ describe('MailgunClient', function () { nock('https://api.mailgun.net').get('/v3/fallback.com/events/all-2-next').query(MAILGUN_OPTIONS).replyWithFile(200, `${__dirname}/fixtures/empty.json`); const counter = createBatchCounter(); - const mailgunClient = new MailgunClient({config, settings, labs}); + const mailgunClient = new MailgunClient({config, settings}); await mailgunClient.fetchEvents(MAILGUN_OPTIONS, counter.batchHandler); assert.equal(primaryMock.isDone(), true); @@ -1134,7 +1128,7 @@ describe('MailgunClient', function () { const fallbackMock = nock('https://api.mailgun.net').get('/v3/fallback.com/events').query(MAILGUN_OPTIONS).replyWithFile(200, `${__dirname}/fixtures/all-2.json`); const counter = createBatchCounter(); - await new MailgunClient({config, settings, labs}).fetchEvents(MAILGUN_OPTIONS, counter.batchHandler); + await new MailgunClient({config, settings}).fetchEvents(MAILGUN_OPTIONS, counter.batchHandler); assert.equal(primaryMock.isDone(), true); assert.equal(fallbackMock.isDone(), false); @@ -1148,7 +1142,7 @@ describe('MailgunClient', function () { nock('https://api.mailgun.net').get('/v3/primary.com/events/all-1-next').query(MAILGUN_OPTIONS).replyWithFile(200, `${__dirname}/fixtures/empty.json`); const counter = createBatchCounter(); - await new MailgunClient({config, settings, labs}).fetchEvents(MAILGUN_OPTIONS, counter.batchHandler); + await new MailgunClient({config, settings}).fetchEvents(MAILGUN_OPTIONS, counter.batchHandler); assert.equal(primaryMock.isDone(), true); assert.equal(counter.events, 4); @@ -1161,7 +1155,7 @@ describe('MailgunClient', function () { nock('https://api.mailgun.net').get('/v3/primary.com/events/all-1-next').query(MAILGUN_OPTIONS).replyWithFile(200, `${__dirname}/fixtures/empty.json`); const counter = createBatchCounter(); - await new MailgunClient({config, settings, labs}).fetchEvents(MAILGUN_OPTIONS, counter.batchHandler); + await new MailgunClient({config, settings}).fetchEvents(MAILGUN_OPTIONS, counter.batchHandler); assert.equal(primaryMock.isDone(), true); assert.equal(counter.events, 4); @@ -1174,7 +1168,7 @@ describe('MailgunClient', function () { const fallbackMock = nock('https://api.mailgun.net').get('/v3/fallback.com/events').query(MAILGUN_OPTIONS).replyWithFile(200, `${__dirname}/fixtures/all-2.json`); await assert.rejects( - new MailgunClient({config, settings, labs}).fetchEvents(MAILGUN_OPTIONS, () => {}) + new MailgunClient({config, settings}).fetchEvents(MAILGUN_OPTIONS, () => {}) ); assert.equal(fallbackMock.isDone(), false); @@ -1190,7 +1184,7 @@ describe('MailgunClient', function () { nock('https://api.mailgun.net').get('/v3/fallback.com/events/all-1-next').query(MAILGUN_OPTIONS).replyWithFile(200, `${__dirname}/fixtures/empty.json`); const counter = createBatchCounter(); - await new MailgunClient({config, settings, labs}).fetchEvents(MAILGUN_OPTIONS, counter.batchHandler); + await new MailgunClient({config, settings}).fetchEvents(MAILGUN_OPTIONS, counter.batchHandler); assert.equal(counter.batches, 3); assert(counter.events >= 8);