From b79666a3a6e87747472117b2a748f67ce6d03c5c Mon Sep 17 00:00:00 2001 From: louisghost Date: Tue, 15 Sep 2026 13:13:35 +0200 Subject: [PATCH 01/11] Added nullable posts.auto_excerpt and posts.reading_time columns (#30587) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Adds nullable `posts.auto_excerpt` and `posts.reading_time` columns (DDL only, `{ algorithm: 'auto' }`) so automatic excerpt and reading time can later be stored instead of recomputed on every Posts API response. - Updates `schema.js` and the schema integrity hash to match. - Strips `auto_excerpt` from post serializers so it stays internal; `reading_time` remains the existing public API field and is still computed at read time in this PR. - Also strips the new columns from member activity-feed attribution and avoids leaking null `reading_time` into webhook payloads before population exists. - Migration lives in `6.64/` (after `v6.63.0`); bumps Core + ember-admin to `6.64.0-rc.0`. No write-path population, backfill, or stored read path yet — those land in follow-up PRs. ## Test plan - [x] Confirm migration adds both columns on MySQL (`pnpm knex-migrator migrate` for 6.64) and rolls back cleanly - [x] `pnpm test:single test/unit/server/data/schema/integrity.test.js` - [x] Spot-check Content/Admin post responses: no `auto_excerpt` key; `reading_time` / `excerpt` unchanged from today - [x] Migration review checklist on the PR --------- Co-authored-by: Cursor --- .../utils/serializers/output/utils/clean.js | 2 ++ .../serializers/output/utils/extra-attrs.js | 3 +++ ...sts-auto-excerpt-and-reading-time-columns.js | 17 +++++++++++++++++ ghost/core/core/server/data/schema/schema.js | 6 ++++++ .../repositories/event-repository.js | 4 ++++ .../output/utils/extra-attrs.test.js | 8 ++++++++ .../unit/server/data/schema/integrity.test.js | 2 +- 7 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 ghost/core/core/server/data/migrations/versions/6.64/2026-09-08-11-01-12-add-posts-auto-excerpt-and-reading-time-columns.js diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/utils/clean.js b/ghost/core/core/server/api/endpoints/utils/serializers/output/utils/clean.js index 0466b577abc..3807fbfd0da 100644 --- a/ghost/core/core/server/api/endpoints/utils/serializers/output/utils/clean.js +++ b/ghost/core/core/server/api/endpoints/utils/serializers/output/utils/clean.js @@ -140,6 +140,8 @@ const post = (attrs, frame) => { delete attrs.author; delete attrs.type; delete attrs.newsletter_id; + // Internal stored metadata — not part of the public Posts/Pages API yet + delete attrs.auto_excerpt; return attrs; }; diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/utils/extra-attrs.js b/ghost/core/core/server/api/endpoints/utils/serializers/output/utils/extra-attrs.js index 0bfd66417ba..a34420a1f4b 100644 --- a/ghost/core/core/server/api/endpoints/utils/serializers/output/utils/extra-attrs.js +++ b/ghost/core/core/server/api/endpoints/utils/serializers/output/utils/extra-attrs.js @@ -64,6 +64,9 @@ module.exports.forPost = (options, model, attrs) => { } // 4. Add `reading_time` if no columns were requested, or if `reading_time` was requested via `columns` + // reading_time is also a DB column now; drop the raw value so we only expose it when + // computed below (avoids leaking `reading_time: null` into APIs/webhooks). + delete attrs.reading_time; if (noColumnsRequested || columnsIncludesReadingTime) { if (attrs.html) { let additionalImages = 0; diff --git a/ghost/core/core/server/data/migrations/versions/6.64/2026-09-08-11-01-12-add-posts-auto-excerpt-and-reading-time-columns.js b/ghost/core/core/server/data/migrations/versions/6.64/2026-09-08-11-01-12-add-posts-auto-excerpt-and-reading-time-columns.js new file mode 100644 index 00000000000..7777b9b59f1 --- /dev/null +++ b/ghost/core/core/server/data/migrations/versions/6.64/2026-09-08-11-01-12-add-posts-auto-excerpt-and-reading-time-columns.js @@ -0,0 +1,17 @@ +const { combineNonTransactionalMigrations, createAddColumnMigration } = require('../../utils'); + +const addPostsColumn = (name, definition) => + createAddColumnMigration('posts', name, definition, { algorithm: 'auto' }); + +module.exports = combineNonTransactionalMigrations( + addPostsColumn('auto_excerpt', { + type: 'string', + maxlength: 500, + nullable: true, + }), + addPostsColumn('reading_time', { + type: 'integer', + unsigned: true, + nullable: true, + }), +); diff --git a/ghost/core/core/server/data/schema/schema.js b/ghost/core/core/server/data/schema/schema.js index 4242135c08b..84b8ee7c96a 100644 --- a/ghost/core/core/server/data/schema/schema.js +++ b/ghost/core/core/server/data/schema/schema.js @@ -184,6 +184,12 @@ module.exports = { nullable: true, validations: { isLength: { max: 300 } }, }, + auto_excerpt: { + type: 'string', + maxlength: 500, + nullable: true, + }, + reading_time: { type: 'integer', unsigned: true, nullable: true }, codeinjection_head: { type: 'text', maxlength: 65535, nullable: true }, codeinjection_foot: { type: 'text', maxlength: 65535, nullable: true }, custom_template: { type: 'string', maxlength: 100, nullable: true }, diff --git a/ghost/core/core/server/services/members/members-api/repositories/event-repository.js b/ghost/core/core/server/services/members/members-api/repositories/event-repository.js index 5a2ab78ec27..e6ad5c6da4b 100644 --- a/ghost/core/core/server/services/members/members-api/repositories/event-repository.js +++ b/ghost/core/core/server/services/members/members-api/repositories/event-repository.js @@ -428,6 +428,8 @@ module.exports = class EventRepository { delete json.postAttribution?.mobiledoc; delete json.postAttribution?.lexical; delete json.postAttribution?.plaintext; + delete json.postAttribution?.auto_excerpt; + delete json.postAttribution?.reading_time; const createdWithStatus = json.signupStatusEvent?.to_status ?? null; delete json.signupStatusEvent; return { @@ -488,6 +490,8 @@ module.exports = class EventRepository { delete json.postAttribution?.mobiledoc; delete json.postAttribution?.lexical; delete json.postAttribution?.plaintext; + delete json.postAttribution?.auto_excerpt; + delete json.postAttribution?.reading_time; return { type: 'donation_event', data: { diff --git a/ghost/core/test/unit/api/canary/utils/serializers/output/utils/extra-attrs.test.js b/ghost/core/test/unit/api/canary/utils/serializers/output/utils/extra-attrs.test.js index acb5e4ccae6..09c3cfc8728 100644 --- a/ghost/core/test/unit/api/canary/utils/serializers/output/utils/extra-attrs.test.js +++ b/ghost/core/test/unit/api/canary/utils/serializers/output/utils/extra-attrs.test.js @@ -94,5 +94,13 @@ describe('Unit: endpoints/utils/serializers/output/utils/extra-attrs', function ); assert.equal(Object.prototype.hasOwnProperty.call(attrs, 'reading_time'), true); }); + + it('does not leak null reading_time from the database when html is absent', function () { + const attrs = { + reading_time: null, + }; + extraAttrsUtil.forPost({}, model, attrs); + assert.equal(Object.prototype.hasOwnProperty.call(attrs, 'reading_time'), false); + }); }); }); diff --git a/ghost/core/test/unit/server/data/schema/integrity.test.js b/ghost/core/test/unit/server/data/schema/integrity.test.js index 7530b75762c..475d7fc7824 100644 --- a/ghost/core/test/unit/server/data/schema/integrity.test.js +++ b/ghost/core/test/unit/server/data/schema/integrity.test.js @@ -37,7 +37,7 @@ const parseYaml = require('../../../../../core/server/services/route-settings/ya */ describe('DB version integrity', function () { // Only these variables should need updating - const currentSchemaHash = 'f8167b5e21aac21f007f008e6e60a332'; + const currentSchemaHash = '5309c65de16da6833fc799828233fa7e'; const currentFixturesHash = '5718e0d4eb037f159c312369e949829a'; const currentSettingsHash = '6ea42a00cca61a1ba87f66eb6e25a78a'; const currentRoutesHash = 'd8c25fa01bf6d22a2bcb05ba0de70dc1'; From 533fd14c8dbf6714a3ecde6b234e7d0011f3db83 Mon Sep 17 00:00:00 2001 From: Jonatan Svennberg Date: Tue, 15 Sep 2026 15:03:04 +0200 Subject: [PATCH 02/11] Added ingestion lag to the email analytics job completion logs (#30735) ref https://linear.app/ghost/issue/BER-3911 The opened-events lag warning measured the age of the last processed event, which only moves when events arrive, so quiet sites warned for days. The `job.completed` log for the opened and delivery pipelines now carries `lag_seconds`: the time between now and the last point up to which all events have been fetched and processed. It is omitted until a fetch has succeeded in the current process. The warning and its threshold are removed and the missing-events sweep has no lag entry because its window trails the delivery pipeline by design. Also fixes the email debug screen's status poll stopping on a failed request. Co-authored-by: Ricardo Pinto --- .../ember-admin/app/components/posts/debug.js | 4 +- .../email-analytics-service-wrapper.ts | 71 +++++----- ghost/core/core/shared/config/defaults.json | 3 +- .../admin/__snapshots__/emails.test.js.snap | 40 ++++++ ghost/core/test/e2e-api/admin/emails.test.js | 13 ++ .../email-analytics-service-wrapper.test.ts | 123 ++++++++++++++---- .../email-analytics-service.test.ts | 16 +++ 7 files changed, 201 insertions(+), 69 deletions(-) diff --git a/apps/ember-admin/app/components/posts/debug.js b/apps/ember-admin/app/components/posts/debug.js index cb9a41c863f..1efc5586cfc 100644 --- a/apps/ember-admin/app/components/posts/debug.js +++ b/apps/ember-admin/app/components/posts/debug.js @@ -244,9 +244,9 @@ export default class Debug extends Component { async fetchAnalyticsStatus() { try { if (this._fetchAnalyticsStatus.isRunning) { - return this._fetchAnalyticsStatus.last; + return await this._fetchAnalyticsStatus.last; } - return this._fetchAnalyticsStatus.perform(); + return await this._fetchAnalyticsStatus.perform(); } catch (e) { // Skip } diff --git a/ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.ts b/ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.ts index 9c6afb8d4bf..f3a5cb3dcca 100644 --- a/ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.ts +++ b/ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.ts @@ -121,6 +121,7 @@ export class EmailAnalyticsServiceWrapper { jobType: string, fetchResult: EmailAnalyticsFetchResult, totalDurationMs: number, + lagSeconds: number | null = null, ): void { const config = this.#getConfig(); @@ -150,6 +151,7 @@ export class EmailAnalyticsServiceWrapper { const logMessage = [ `[Background Job] ${this.#backgroundJobName} processed ${jobType} | ${this.#logPrefix}`, `${eventCount} events in ${(totalDurationMs / 1000).toFixed(1)}s (${throughput.toFixed(2)} events/s)`, + ...(lagSeconds === null ? [] : [`Lag: ${(lagSeconds / 60).toFixed(1)}m`]), `Mode: ${batchMode}`, `Timings: API ${(apiPollingTimeMs / 1000).toFixed(1)}s (${apiPercent}%) / Processing ${(processingTimeMs / 1000).toFixed(1)}s (${processingPercent}%) / Aggregation ${(aggregationTimeMs / 1000).toFixed(1)}s (${aggregationPercent}%) [Email ${(emailAggregationTimeMs / 1000).toFixed(1)}s / Member ${(memberAggregationTimeMs / 1000).toFixed(1)}s]`, `Events: opened=${result.opened} delivered=${result.delivered} failed=${result.permanentFailed + result.temporaryFailed} unprocessable=${result.unprocessable}`, @@ -163,6 +165,7 @@ export class EmailAnalyticsServiceWrapper { task: jobType, event_count: eventCount, duration_ms: totalDurationMs, + ...(lagSeconds === null ? {} : { lag_seconds: lagSeconds }), }, }, logMessage, @@ -194,53 +197,45 @@ export class EmailAnalyticsServiceWrapper { } } - async fetchLatestOpenedEvents({ - maxEvents = Infinity, - }: { maxEvents?: number } = {}): Promise { - const config = this.#getConfig(); - - const beginTimestamp = await this.service.getLastOpenedEventTimestamp(); - const lagMinutes = (Date.now() - beginTimestamp.getTime()) / 60000; - const lagThreshold = config.get('emailAnalytics:openedJobLagWarningMinutes'); - - // NOTE: We only update the begin timestamp when we process events, so there's cases where we can have a false positive - // - Ghost or Mailgun outages - // - Lack of actual email activity - if (lagThreshold && lagMinutes > lagThreshold) { - logging.warn( - `${this.#logPrefix} Opened events processing is ${lagMinutes.toFixed(1)} minutes behind (threshold: ${lagThreshold})`, - ); - } - + async #fetchAndLog( + jobType: string, + fetch: () => Promise, + lagPipeline?: 'latestOpened' | 'latest', + ): Promise { const fetchStartedAt = Date.now(); - const fetchResult = await this.service.fetchLatestOpenedEvents({ maxEvents }); + const fetchResult = await fetch(); const totalDuration = Date.now() - fetchStartedAt; - this._logJobCompletion('latest-opened', fetchResult, totalDuration); + // Lag is read after the fetch so a clean run counts as caught up + const lagSeconds = lagPipeline ? this.service.getStatus()[lagPipeline].lagSeconds : null; + this._logJobCompletion(jobType, fetchResult, totalDuration, lagSeconds); return fetchResult.eventCount; } - async fetchLatestNonOpenedEvents({ + async fetchLatestOpenedEvents({ maxEvents = Infinity, }: { maxEvents?: number } = {}): Promise { - const fetchStartedAt = Date.now(); - const fetchResult = await this.service.fetchLatestNonOpenedEvents({ maxEvents }); - const totalDuration = Date.now() - fetchStartedAt; - - this._logJobCompletion('latest', fetchResult, totalDuration); + return this.#fetchAndLog( + 'latest-opened', + () => this.service.fetchLatestOpenedEvents({ maxEvents }), + 'latestOpened', + ); + } - return fetchResult.eventCount; + async fetchLatestNonOpenedEvents({ + maxEvents = Infinity, + }: { maxEvents?: number } = {}): Promise { + return this.#fetchAndLog( + 'latest', + () => this.service.fetchLatestNonOpenedEvents({ maxEvents }), + 'latest', + ); } async fetchMissing({ maxEvents = Infinity }: { maxEvents?: number } = {}): Promise { - const fetchStartedAt = Date.now(); - const fetchResult = await this.service.fetchMissing({ maxEvents }); - const totalDuration = Date.now() - fetchStartedAt; - - this._logJobCompletion('missing', fetchResult, totalDuration); - - return fetchResult.eventCount; + // The missing-events sweep trails the delivery pipeline by design, so it has no lag of its own + return this.#fetchAndLog('missing', () => this.service.fetchMissing({ maxEvents })); } async fetchScheduled({ maxEvents }: { maxEvents: number }): Promise { @@ -248,13 +243,7 @@ export class EmailAnalyticsServiceWrapper { return 0; } - const fetchStartedAt = Date.now(); - const fetchResult = await this.service.fetchScheduled({ maxEvents }); - const totalDuration = Date.now() - fetchStartedAt; - - this._logJobCompletion('scheduled', fetchResult, totalDuration); - - return fetchResult.eventCount; + return this.#fetchAndLog('scheduled', () => this.service.fetchScheduled({ maxEvents })); } async startFetch(): Promise { diff --git a/ghost/core/core/shared/config/defaults.json b/ghost/core/core/shared/config/defaults.json index d9f5dcfa1b0..683b863f152 100644 --- a/ghost/core/core/shared/config/defaults.json +++ b/ghost/core/core/shared/config/defaults.json @@ -305,8 +305,7 @@ "enabled": false, "threshold": 500 } - }, - "openedJobLagWarningMinutes": 30 + } }, "linkClickTrackingCacheMemberUuid": false, "backgroundJobs": { diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/emails.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/emails.test.js.snap index 1b35d37ba76..f75e024af6c 100644 --- a/ghost/core/test/e2e-api/admin/__snapshots__/emails.test.js.snap +++ b/ghost/core/test/e2e-api/admin/__snapshots__/emails.test.js.snap @@ -734,6 +734,46 @@ Object { } `; +exports[`Emails API Can read the analytics status 1: [body] 1`] = ` +Object { + "latest": Object { + "fetchedThrough": null, + "jobName": "email-analytics-latest-others", + "lagSeconds": null, + "running": false, + }, + "latestOpened": Object { + "fetchedThrough": null, + "jobName": "email-analytics-latest-opened", + "lagSeconds": null, + "running": false, + }, + "missing": Object { + "fetchedThrough": null, + "jobName": "email-analytics-missing", + "lagSeconds": null, + "running": false, + }, + "scheduled": Object { + "jobName": "email-analytics-scheduled", + "running": false, + }, +} +`; + +exports[`Emails API Can read the analytics status 2: [headers] 1`] = ` +Object { + "access-control-allow-origin": "http://127.0.0.1:2369", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "397", + "content-type": "application/json; charset=utf-8", + "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Accept-Version, Origin, Accept-Encoding", + "x-powered-by": "Express", +} +`; + exports[`Emails API Can read the sending status of a failed email 1: [body] 1`] = ` Object { "email_statuses": Array [ diff --git a/ghost/core/test/e2e-api/admin/emails.test.js b/ghost/core/test/e2e-api/admin/emails.test.js index 15cf341d6ea..1ec0ae1d778 100644 --- a/ghost/core/test/e2e-api/admin/emails.test.js +++ b/ghost/core/test/e2e-api/admin/emails.test.js @@ -203,6 +203,19 @@ describe('Emails API', function () { mockManager.assert.emittedEvent('email.edited'); }); + it('Can read the analytics status', async function () { + // The analytics job is never scheduled under test, so the pipelines are in their initial + // state: nothing running and lag unknown until a fetch has succeeded in this process + await agent + .get(`emails/${fixtureManager.get('emails', 0).id}/analytics/`) + .expectStatus(200) + .matchBodySnapshot() + .matchHeaderSnapshot({ + 'content-version': anyContentVersion, + etag: anyEtag, + }); + }); + it('Can browse email batches', async function () { await agent .get(`emails/${fixtureManager.get('emails', 0).id}/batches/`) diff --git a/ghost/core/test/unit/server/services/email-analytics/email-analytics-service-wrapper.test.ts b/ghost/core/test/unit/server/services/email-analytics/email-analytics-service-wrapper.test.ts index 382f7925a0c..b8482e9e6c6 100644 --- a/ghost/core/test/unit/server/services/email-analytics/email-analytics-service-wrapper.test.ts +++ b/ghost/core/test/unit/server/services/email-analytics/email-analytics-service-wrapper.test.ts @@ -2,6 +2,7 @@ import assert from 'node:assert/strict'; import sinon from 'sinon'; import logging from '@tryghost/logging'; import { EmailAnalyticsServiceWrapper } from '../../../../../core/server/services/email-analytics/email-analytics-service-wrapper'; +import type { EmailAnalyticsFetchResult } from '../../../../../core/server/services/email-analytics/email-analytics-service'; import { EventProcessingResult } from '../../../../../core/server/services/email-analytics/event-processing-result'; import { Queries } from '../../../../../core/server/services/email-analytics/lib/queries'; @@ -21,20 +22,11 @@ describe('EmailAnalyticsServiceWrapper', function () { sinon.restore(); }); - function logLatestOpenedJob(logName: string) { + function initWrapper(logName: string, configOverrides: Record = {}) { const wrapper = new EmailAnalyticsServiceWrapper({ logName }); wrapper.init({ config: { - get: (key) => { - switch (key) { - case 'emailAnalytics:metrics:openThroughput:enabled': - return true; - case 'emailAnalytics:metrics:openThroughput:threshold': - return 0; - default: - return undefined; - } - }, + get: (key?: string) => (key ? configOverrides[key] : undefined), }, domainEvents: { subscribe: sinon.stub(), @@ -66,19 +58,28 @@ describe('EmailAnalyticsServiceWrapper', function () { metric: metricStub, }, }); - wrapper._logJobCompletion( - 'latest-opened', - { - eventCount: 10, - apiPollingTimeMs: 500, - processingTimeMs: 1000, - aggregationTimeMs: 500, - emailAggregationTimeMs: 300, - memberAggregationTimeMs: 200, - result: new EventProcessingResult(), - }, - 2000, - ); + return wrapper; + } + + function createFetchResult(overrides: Partial = {}) { + return { + eventCount: 10, + apiPollingTimeMs: 500, + processingTimeMs: 1000, + aggregationTimeMs: 500, + emailAggregationTimeMs: 300, + memberAggregationTimeMs: 200, + result: new EventProcessingResult(), + ...overrides, + }; + } + + function logLatestOpenedJob(logName: string) { + const wrapper = initWrapper(logName, { + 'emailAnalytics:metrics:openThroughput:enabled': true, + 'emailAnalytics:metrics:openThroughput:threshold': 0, + }); + wrapper._logJobCompletion('latest-opened', createFetchResult(), 2000); return wrapper; } @@ -175,6 +176,80 @@ describe('EmailAnalyticsServiceWrapper', function () { ); }); + function jobCompletionLogs(infoLog: sinon.SinonStub) { + return infoLog.args.filter( + ([payload]) => + (payload as { system?: { event?: string } })?.system?.event === 'job.completed', + ); + } + + it('includes the pipeline lag in job completion logs', function () { + const infoLog = sinon.stub(logging, 'info'); + const wrapper = initWrapper('newsletters'); + + wrapper._logJobCompletion('latest', createFetchResult(), 2000, 330); + + sinon.assert.calledWith( + infoLog, + sinon.match({ + system: sinon.match({ event: 'job.completed', task: 'latest', lag_seconds: 330 }), + }), + sinon.match(' | Lag: 5.5m | '), + ); + }); + + it('leaves lag out of job completion logs when it is not known', function () { + const infoLog = sinon.stub(logging, 'info'); + + logLatestOpenedJob('newsletters'); + + const [payload, message] = jobCompletionLogs(infoLog)[0]; + assert.equal('lag_seconds' in payload.system, false); + assert.doesNotMatch(message, /Lag:/); + }); + + it('logs the opened and delivery pipelines with the lag measured after their fetch', async function () { + const infoLog = sinon.stub(logging, 'info'); + const wrapper = initWrapper('newsletters'); + const fetchResult = createFetchResult({ eventCount: 1 }); + const fetchStubs = [ + sinon.stub(wrapper.service, 'fetchLatestOpenedEvents').resolves(fetchResult), + sinon.stub(wrapper.service, 'fetchLatestNonOpenedEvents').resolves(fetchResult), + sinon.stub(wrapper.service, 'fetchMissing').resolves(fetchResult), + ]; + const pipeline = (jobName: string, lagSeconds: number) => ({ + running: false, + jobName, + fetchedThrough: null, + lagSeconds, + }); + const statusStub = sinon.stub(wrapper.service, 'getStatus').returns({ + latestOpened: pipeline('email-analytics-latest-opened', 60), + latest: pipeline('email-analytics-latest-others', 120), + missing: pipeline('email-analytics-missing', 1800), + scheduled: { running: false, jobName: 'email-analytics-scheduled' }, + }); + + await wrapper.fetchLatestOpenedEvents(); + await wrapper.fetchLatestNonOpenedEvents(); + await wrapper.fetchMissing(); + + const completions = jobCompletionLogs(infoLog).map(([payload]) => [ + payload.system.task, + payload.system.lag_seconds, + ]); + // The missing-events sweep trails the delivery pipeline by design, so its lag is not logged + assert.deepEqual(completions, [ + ['latest-opened', 60], + ['latest', 120], + ['missing', undefined], + ]); + // Read after each fetch, so a clean run counts as caught up + sinon.assert.calledTwice(statusStub); + assert.ok(statusStub.firstCall.calledAfter(fetchStubs[0].firstCall)); + assert.ok(statusStub.secondCall.calledAfter(fetchStubs[1].firstCall)); + }); + it('skips opened event polling when the cursor seed has no opened column', async function () { const wrapper = new EmailAnalyticsServiceWrapper({ logName: 'gifts' }); wrapper.init({ diff --git a/ghost/core/test/unit/server/services/email-analytics/email-analytics-service.test.ts b/ghost/core/test/unit/server/services/email-analytics/email-analytics-service.test.ts index 0bc026008dd..6f8b6d86909 100644 --- a/ghost/core/test/unit/server/services/email-analytics/email-analytics-service.test.ts +++ b/ghost/core/test/unit/server/services/email-analytics/email-analytics-service.test.ts @@ -207,6 +207,22 @@ describe('EmailAnalyticsService', function () { assert.equal(service.getStatus().latest.lagSeconds, 120); }); + it('keeps progress unknown when the first fetch after a restart fails', async function () { + const service = createService({ + queries: { + getLastEventTimestamp: sinon.stub().resolves(new Date(Date.now() - 2 * 24 * 60 * 60_000)), + }, + fetchEvents: sinon.stub().rejects(new Error('Mailgun unavailable')), + }); + + await assert.rejects(service.fetchLatestNonOpenedEvents(), /Mailgun unavailable/); + + // The restored cursor is the last processed event, which can be days old on a quiet + // site, so it is not treated as progress + assert.equal(service.getStatus().latest.fetchedThrough, null); + assert.equal(service.getStatus().latest.lagSeconds, null); + }); + it('does not publish progress before processing and final aggregation succeed', async function () { const processor = createStubEventProcessor(); const fetchEvents = sinon.stub().callsFake(async () => { From 1788373efcdc66a755c617cdf2bd181a6f438883 Mon Sep 17 00:00:00 2001 From: Peter Zimon Date: Tue, 15 Sep 2026 15:55:52 +0200 Subject: [PATCH 03/11] Added React member activity behind a private experiment (#30734) Migrated the global and per-member Activity screens to React behind the private `membersActivityReact` experiment. The existing `/members-activity` URL, member selection, event filters, permissions, and navigation remain compatible with Ember. Default enablement and Ember removal are separate rollout steps. The screen uses Shade and the existing member event parser, with full-page financial details and cleaned click URLs. Historical email previews prefer stored HTML/subject and use a post preview only when a valid post identity is available. The editor preview and existing five-event member detail feed retain their behavior. Pagination uses bounded requests to finish each timestamp boundary per event type before moving backward. It works with the existing API and avoids dropping same-timestamp events. Loading, empty, failure, and retry states include accessible announcements. The Ember handoff preserves React filters across reloads without briefly rewriting the URL. Related: [PLA-285](https://linear.app/ghost/issue/PLA-285/migrate-members-activity-to-react). ### Validation - Full Admin and admin-x-framework unit suites passed; Admin typecheck passed. - 17 Activity browser acceptance tests passed; 15 route-access/fallback acceptance tests passed. - Existing Ember Activity tests passed (13); Core Labs/config/settings tests passed. - Real-browser tests verified actual React/Ember ownership, member/profile navigation, and filtered reloads. A real API fixture verified all 75 same-timestamp signups plus one older signup appear exactly once. URL override persistence and clearing back to Ember after reload also passed. - Independent slice reviews and final Standards/Spec reviews completed; findings addressed. - `pnpm check` passed formatting and repository lint. Its Core test phase failed in unchanged cron/date and gift-image/email-renderer tests. The two date assertions pass with `TZ=UTC`; isolated gift-image tests still time out, with a local Fontconfig configuration error. Admin and framework tests passed within the full check. ### Manual testing 1. Open `/ghost/#/members-activity?labs=membersActivityReact`, or enable **React member activity** in Labs' private features. 2. Search/select a member, open their profile, then use **View all member activity**. Toggle filters, reload, and use Back/Forward. 3. Scroll a long feed and check subscription/donation/gift values and post links. Open an email preview, switch desktop/mobile, and close with Escape. 4. Disable the persisted flag if enabled, visit `/ghost/#/members-activity?labs=`, and reload to verify the Ember fallback. See [the Activity README](https://github.com/TryGhost/Ghost/blob/codex/pla-285-member-activity-react/apps/admin/src/members/activity/README.md) for behavior and pagination details. - [x] Read and followed the Contributor Guide - [x] Explained the change - [x] Added automated regression tests --------- Co-authored-by: Steve Larson <9larsons@gmail.com> --- .../src/api/member-activity-pagination.ts | 200 +++++++++ .../src/api/member-activity.ts | 72 ++++ apps/admin-x-framework/src/api/members.ts | 8 +- .../api/member-activity-pagination.test.ts | 268 ++++++++++++ .../src/layout/app-sidebar/nav-content.tsx | 3 +- .../member-activity-gate.acceptance.test.tsx | 29 ++ apps/admin/src/member-activity-gate.tsx | 9 + .../activity/activity-email-preview-data.ts | 68 ++++ .../activity/activity-email-preview.test.tsx | 175 ++++++++ .../activity/activity-email-preview.tsx | 214 ++++++++++ .../members/activity/activity-event.test.ts | 200 +++++++++ .../src/members/activity/activity-event.ts | 95 +++++ .../members/activity/activity-filters.test.ts | 78 ++++ .../src/members/activity/activity-filters.ts | 150 +++++++ .../activity/activity-member-search.tsx | 117 ++++++ .../src/members/activity/activity-row.tsx | 139 +++++++ .../member-activity.acceptance.test.tsx | 381 ++++++++++++++++++ .../activity/member-activity.screen.ts | 38 ++ .../src/members/activity/member-activity.tsx | 373 +++++++++++++++++ apps/admin/src/members/api.ts | 1 + .../members/detail/member-activity-feed.tsx | 19 +- apps/admin/src/members/detail/member-event.ts | 1 + .../src/posts/analytics/utils/link-helpers.ts | 28 +- .../src/route-access.acceptance.test.tsx | 17 + apps/admin/src/routes.tsx | 14 +- .../advanced/labs/private-features.tsx | 5 + apps/admin/src/shared/clean-tracked-url.ts | 22 + .../app/routes/members-activity.js | 43 ++ apps/ember-admin/app/services/feature.js | 1 + .../admin/members/activity-navigation.test.ts | 84 ++++ .../admin/members/activity-pagination.test.ts | 88 ++++ .../members/activity-session-override.test.ts | 28 ++ ghost/core/core/shared/labs.js | 1 + .../src/selectors/member-activity.ts | 8 + 34 files changed, 2936 insertions(+), 41 deletions(-) create mode 100644 apps/admin-x-framework/src/api/member-activity-pagination.ts create mode 100644 apps/admin-x-framework/src/api/member-activity.ts create mode 100644 apps/admin-x-framework/test/unit/api/member-activity-pagination.test.ts create mode 100644 apps/admin/src/member-activity-gate.acceptance.test.tsx create mode 100644 apps/admin/src/member-activity-gate.tsx create mode 100644 apps/admin/src/members/activity/activity-email-preview-data.ts create mode 100644 apps/admin/src/members/activity/activity-email-preview.test.tsx create mode 100644 apps/admin/src/members/activity/activity-email-preview.tsx create mode 100644 apps/admin/src/members/activity/activity-event.test.ts create mode 100644 apps/admin/src/members/activity/activity-event.ts create mode 100644 apps/admin/src/members/activity/activity-filters.test.ts create mode 100644 apps/admin/src/members/activity/activity-filters.ts create mode 100644 apps/admin/src/members/activity/activity-member-search.tsx create mode 100644 apps/admin/src/members/activity/activity-row.tsx create mode 100644 apps/admin/src/members/activity/member-activity.acceptance.test.tsx create mode 100644 apps/admin/src/members/activity/member-activity.screen.ts create mode 100644 apps/admin/src/members/activity/member-activity.tsx create mode 100644 apps/admin/src/shared/clean-tracked-url.ts create mode 100644 e2e/tests/admin/members/activity-navigation.test.ts create mode 100644 e2e/tests/admin/members/activity-pagination.test.ts create mode 100644 e2e/tests/admin/members/activity-session-override.test.ts create mode 100644 packages/testing/test-data/src/selectors/member-activity.ts diff --git a/apps/admin-x-framework/src/api/member-activity-pagination.ts b/apps/admin-x-framework/src/api/member-activity-pagination.ts new file mode 100644 index 00000000000..9b59bbb75c1 --- /dev/null +++ b/apps/admin-x-framework/src/api/member-activity-pagination.ts @@ -0,0 +1,200 @@ +import { escapeNqlString } from '@tryghost/nql-string'; +import type { MemberActivityEvent, MemberActivityFeedResponseType } from './members'; + +type OlderCursor = { kind: 'older'; before?: string }; +type BoundaryCursor = { + kind: 'boundary'; + timestamp: string; + seen: string[]; + completedTypes: string[]; + currentType?: string; + beforeId?: string; +}; + +export type MemberActivityCursor = OlderCursor | BoundaryCursor; +export interface MemberActivityPage extends MemberActivityFeedResponseType { + nextCursor?: MemberActivityCursor; +} + +type ReadEvents = (params: Record) => Promise; + +const eventKey = (event: MemberActivityEvent) => `${event.type}:${event.data.id}`; + +// Older Core filters this source as "complained" but returns "complaint". +// Include both spellings so this also works after the server fixes the alias. +const filterTypes = (type: string): string[] => + type === 'email_complaint_event' ? ['email_complaint_event', 'email_complained_event'] : [type]; + +function eventTimestamp(event: MemberActivityEvent): string { + const timestamp = event.data.created_at; + if (!timestamp || !Number.isFinite(Date.parse(timestamp))) { + throw new Error('Member activity returned an event without a valid timestamp.'); + } + // SQLite compares Ghost's second-precision timestamps as text. Omit a zero + // fraction to match those rows, but retain nonzero milliseconds for precision. + return new Date(timestamp) + .toISOString() + .replace('T', ' ') + .replace(/(?:\.000)?Z$/, ''); +} + +function validateEvents(events: MemberActivityEvent[]): void { + for (const event of events) { + eventTimestamp(event); + if (typeof event.data.id !== 'string' || !event.data.id || !event.type) { + throw new Error('Member activity returned an event without a valid identity.'); + } + } +} + +function hasMore(response: MemberActivityFeedResponseType, limit: number): boolean { + const total = response.meta?.pagination.total; + // The total also catches a server applying a lower page limit. Never silently + // treat a clamped response as the end of the feed. + return typeof total === 'number' + ? total > response.events.length + : response.events.length >= limit; +} + +/** + * The older events API has no cursor or offset and merges several event tables. + * Its same-timestamp ordering is not globally ID-ordered, so adding id < lastId + * to the merged feed would still skip events. Complete its final timestamp one + * event type at a time, where the server's ID-descending order is unambiguous. + * Discover types through the API instead of maintaining a second event catalog. + * + * Requests and returned pages stay bounded by limit. Cursor memory is bounded + * by one page's initial boundary identities and the number of event types, + * regardless of the size of a newsletter send. Shared recipient IDs belonging + * to distinct email event types remain distinct events. + */ +export async function loadMemberActivityPage({ + read, + filter = '', + limit, + cursor = { kind: 'older' }, + signal, +}: { + read: ReadEvents; + filter?: string; + limit: number; + cursor?: MemberActivityCursor; + signal?: AbortSignal; +}): Promise { + if (!Number.isInteger(limit) || limit < 1) { + throw new Error('Member activity page size must be a positive integer.'); + } + // A failed request must leave the previous page's cursor untouched so a retry + // starts from exactly the same position. + let state: MemberActivityCursor = + cursor.kind === 'boundary' + ? { ...cursor, seen: [...cursor.seen], completedTypes: [...cursor.completedTypes] } + : { ...cursor }; + const events: MemberActivityEvent[] = []; + + const request = async (parts: string[], requestLimit: number) => { + signal?.throwIfAborted(); + const response = await read({ + // Core requires event-type constraints at the root AND level. Wrapping + // the base filter in parentheses would make mixed type/member filters fail. + filter: [filter, ...parts].filter(Boolean).join('+'), + limit: String(requestLimit), + }); + // useFetchApi owns the in-flight network request. Stop any following drain + // requests when React Query cancels this query after navigation/filtering. + signal?.throwIfAborted(); + validateEvents(response.events); + if ( + response.events.length > requestLimit || + (!response.events.length && hasMore(response, requestLimit)) + ) { + throw new Error('Member activity pagination did not make progress.'); + } + return response; + }; + + while (events.length < limit) { + const remaining = limit - events.length; + if (state.kind === 'older') { + const before = state.before; + const response = await request( + before ? [`data.created_at:<${escapeNqlString(before)}`] : [], + remaining, + ); + if (before && response.events.some((event) => eventTimestamp(event) >= before)) { + throw new Error('Member activity returned events outside the requested time range.'); + } + events.push(...response.events); + if (!hasMore(response, remaining)) { + return { events, meta: response.meta }; + } + const timestamp = eventTimestamp(response.events[response.events.length - 1]); + state = { + kind: 'boundary', + timestamp, + seen: response.events.filter((event) => eventTimestamp(event) === timestamp).map(eventKey), + completedTypes: [], + }; + if (events.length === limit) { + return { events, meta: response.meta, nextCursor: state }; + } + continue; + } + + const timestampFilter = `data.created_at:${escapeNqlString(state.timestamp)}`; + if (!state.currentType) { + const typeFilter = state.completedTypes.length + ? [`type:-[${state.completedTypes.flatMap(filterTypes).map(escapeNqlString).join(',')}]`] + : []; + const discovery = await request([timestampFilter, ...typeFilter], 1); + const event = discovery.events[0]; + if ( + !event || + (!state.completedTypes.length && discovery.meta?.pagination.total === state.seen.length) + ) { + state = { kind: 'older', before: state.timestamp }; + continue; + } + if (eventTimestamp(event) !== state.timestamp || state.completedTypes.includes(event.type)) { + throw new Error('Member activity returned events outside the requested type or timestamp.'); + } + state.currentType = event.type; + } + + const beforeId = state.beforeId; + const currentType = state.currentType; + const timestamp = state.timestamp; + const response = await request( + [ + timestampFilter, + `type:[${filterTypes(currentType).map(escapeNqlString).join(',')}]`, + ...(beforeId ? [`id:<${escapeNqlString(beforeId)}`] : []), + ], + remaining, + ); + if ( + response.events.some( + (event) => + event.type !== currentType || + eventTimestamp(event) !== timestamp || + (beforeId && String(event.data.id) >= beforeId), + ) + ) { + throw new Error('Member activity returned events outside the requested cursor.'); + } + const seen = new Set(state.seen); + events.push(...response.events.filter((event) => !seen.has(eventKey(event)))); + + if (hasMore(response, remaining)) { + // The single-type response is ID-descending. Use the minimum defensively + // so the cursor advances even if an older server returns a shuffled page. + state.beforeId = response.events.map((event) => String(event.data.id)).sort()[0]; + } else { + state.completedTypes.push(state.currentType); + state.currentType = undefined; + state.beforeId = undefined; + } + } + + return { events, nextCursor: state }; +} diff --git a/apps/admin-x-framework/src/api/member-activity.ts b/apps/admin-x-framework/src/api/member-activity.ts new file mode 100644 index 00000000000..854858c4aef --- /dev/null +++ b/apps/admin-x-framework/src/api/member-activity.ts @@ -0,0 +1,72 @@ +import { useEffect, useMemo } from 'react'; +import { useInfiniteQuery } from '@tanstack/react-query'; +import { escapeNqlString } from '@tryghost/nql-string'; +import useHandleError from '../hooks/use-handle-error'; +import { apiUrl, useFetchApi } from '../utils/api/fetch-api'; +import { loadMemberActivityPage, type MemberActivityCursor } from './member-activity-pagination'; +import type { MemberActivityFeedResponseType } from './members'; + +export interface BrowseMemberActivityOptions { + memberId?: string; + excludedEvents?: string[]; + limit?: number; + enabled?: boolean; + defaultErrorHandler?: boolean; +} + +/** Full, paginated activity feed. The member detail preview keeps its five-row query. */ +export function useBrowseMemberActivityFeed({ + memberId, + excludedEvents = [], + limit = 50, + enabled = true, + defaultErrorHandler = true, +}: BrowseMemberActivityOptions = {}) { + const fetchApi = useFetchApi(); + const handleError = useHandleError(); + // Accept the Activity page's domain filters, not arbitrary NQL. Core splits + // type selectors from row filters, so root-OR expressions cannot safely be + // combined with a timestamp or per-type pagination cursor. + const baseFilter = [ + excludedEvents.length && `type:-[${excludedEvents.map(escapeNqlString).join(',')}]`, + memberId && `data.member_id:${escapeNqlString(memberId)}`, + ] + .filter(Boolean) + .join('+'); + const result = useInfiniteQuery({ + queryKey: [ + 'MemberActivityFeedResponseType', + apiUrl('/members/events/', { filter: baseFilter, limit: String(limit) }), + 'timeline', + ], + enabled, + initialPageParam: { kind: 'older' } as MemberActivityCursor, + queryFn: ({ pageParam, signal }) => + loadMemberActivityPage({ + read: (params) => + fetchApi(apiUrl('/members/events/', params)), + filter: baseFilter, + limit, + cursor: pageParam, + signal, + }), + getNextPageParam: (page) => page.nextCursor, + }); + const data = useMemo( + () => + result.data && { + events: result.data.pages.flatMap((page) => page.events), + meta: result.data.pages[0]?.meta, + isEnd: !result.data.pages[result.data.pages.length - 1]?.nextCursor, + }, + [result.data], + ); + + useEffect(() => { + if (result.error && defaultErrorHandler) { + handleError(result.error); + } + }, [result.error, handleError, defaultErrorHandler]); + + return { ...result, data }; +} diff --git a/apps/admin-x-framework/src/api/members.ts b/apps/admin-x-framework/src/api/members.ts index fe1ade0350a..e3108006af1 100644 --- a/apps/admin-x-framework/src/api/members.ts +++ b/apps/admin-x-framework/src/api/members.ts @@ -13,6 +13,8 @@ import { useCurrentUser } from './current-user'; import { canManageMembers } from './users'; import { FREE_SEGMENT, PAID_SEGMENT } from '../utils/recipient-filter'; +export { useBrowseMemberActivityFeed, type BrowseMemberActivityOptions } from './member-activity'; + export type MemberLabel = { id: string; name: string; @@ -857,11 +859,11 @@ const MEMBER_ACTIVITY_LIMIT = '20'; // last event of the previous page (events are ordered created_at desc). // // KNOWN LIMITATION: the cursor is `created_at`-only, without the id tie-breaker -// Ember's version added (`+id:<''`). Two events emitted in the same +// required for reliable pagination. Two events emitted in the same // second on a page boundary can be skipped from the paginated list. The current // consumer (`MemberActivityFeed` in `apps/admin`) only fetches 5 events and -// never calls `fetchNextPage`, so this is not exploitable today; add an id -// secondary cursor before another screen starts paginating. +// never calls `fetchNextPage`. Paginated consumers must use +// useBrowseMemberActivityFeed, which drains timestamp boundaries by event type. function memberEventsCursor(events: MemberActivityEvent[]): string | undefined { const createdAt = events[events.length - 1]?.data?.created_at; if (!createdAt) { diff --git a/apps/admin-x-framework/test/unit/api/member-activity-pagination.test.ts b/apps/admin-x-framework/test/unit/api/member-activity-pagination.test.ts new file mode 100644 index 00000000000..1a09f32430f --- /dev/null +++ b/apps/admin-x-framework/test/unit/api/member-activity-pagination.test.ts @@ -0,0 +1,268 @@ +import { describe, expect, it, vi } from 'vitest'; +import { + loadMemberActivityPage, + type MemberActivityCursor, +} from '../../../src/api/member-activity-pagination'; +import type { MemberActivityEvent, MemberActivityFeedResponseType } from '../../../src/api/members'; + +const timestamp = '2026-09-14T10:00:00.000Z'; +const olderTimestamp = '2026-09-14T09:59:59.000Z'; + +function event( + id: number, + type = 'email_delivered_event', + createdAt = timestamp, + memberId = 'member-1', +): MemberActivityEvent { + return { + type, + data: { id: String(id).padStart(24, '0'), created_at: createdAt, member_id: memberId }, + }; +} + +const identity = (value: MemberActivityEvent) => `${value.type}:${value.data.id}`; +const serverFilterType = (value: MemberActivityEvent) => + value.type === 'email_complaint_event' ? 'email_complained_event' : value.type; + +/** A legacy server: only filter+limit, timestamp order, and per-type ID order. + * Mixed types deliberately use type priority ahead of ID, as Core does. */ +function legacyServer( + source: MemberActivityEvent[], + { metadata = true, maxLimit = Infinity, textTimestamps = false } = {}, +) { + return vi.fn( + async ({ filter, limit }: Record): Promise => { + let matching = [...source]; + for (const match of filter.matchAll(/data\.created_at:(<)?'([^']+)'/g)) { + const cursor = textTimestamps ? match[2] : Date.parse(match[2].replace(' ', 'T') + 'Z'); + matching = matching.filter((value) => { + // SQLite compares the second-precision text Ghost stores, without + // converting the filter value to a date as MySQL does. + const stored = textTimestamps + ? value.data.created_at!.replace('T', ' ').replace(/\.000Z$/, '') + : Date.parse(value.data.created_at!); + return match[1] ? stored < cursor : stored === cursor; + }); + } + for (const match of filter.matchAll(/type:-\[([^\]]+)\]/g)) { + const excluded = match[1].replaceAll("'", '').split(','); + matching = matching.filter((value) => !excluded.includes(serverFilterType(value))); + } + for (const match of filter.matchAll(/type:'([^']+)'/g)) { + matching = matching.filter((value) => serverFilterType(value) === match[1]); + } + for (const match of filter.matchAll(/type:\[([^\]]+)\]/g)) { + const included = match[1].replaceAll("'", '').split(','); + matching = matching.filter((value) => included.includes(serverFilterType(value))); + } + for (const match of filter.matchAll(/data\.member_id:'([^']+)'/g)) { + matching = matching.filter((value) => value.data.member_id === match[1]); + } + for (const match of filter.matchAll(/(?:^|\+)id:<'([^']+)'/g)) { + matching = matching.filter((value) => String(value.data.id) < match[1]); + } + matching.sort( + (a, b) => + Date.parse(b.data.created_at!) - Date.parse(a.data.created_at!) || + a.type.localeCompare(b.type) || + String(b.data.id).localeCompare(String(a.data.id)), + ); + const pageLimit = Math.min(Number(limit), maxLimit); + return { + events: matching.slice(0, pageLimit), + ...(metadata + ? { + meta: { + pagination: { + limit: pageLimit, + total: matching.length, + page: 1, + pages: Math.ceil(matching.length / pageLimit), + next: null, + prev: null, + }, + }, + } + : {}), + }; + }, + ); +} + +async function browseAll( + read: ReturnType, + options: { limit?: number; filter?: string } = {}, +) { + let cursor: MemberActivityCursor | undefined; + const events: MemberActivityEvent[] = []; + for (let pageCount = 0; pageCount < 1000; pageCount++) { + const page = await loadMemberActivityPage({ read, limit: 50, ...options, cursor }); + expect(page.events.length).toBeLessThanOrEqual(options.limit ?? 50); + events.push(...page.events); + cursor = page.nextCursor; + if (!cursor) { + return events; + } + } + throw new Error('Pagination failed to finish.'); +} + +describe('member activity pagination against the legacy endpoint', () => { + it('returns every event in a large same-second send with bounded requests and no duplicate identities', async () => { + const source = [ + ...Array.from({ length: 2500 }, (_, index) => event(index + 1)), + ...Array.from({ length: 60 }, (_, index) => + event(index + 2501, 'email_delivered_event', olderTimestamp), + ), + ]; + const read = legacyServer(source); + const result = await browseAll(read); + + expect(result.map(identity)).toEqual( + [...source] + .sort( + (a, b) => + Date.parse(b.data.created_at!) - Date.parse(a.data.created_at!) || + String(b.data.id).localeCompare(String(a.data.id)), + ) + .map(identity), + ); + expect(new Set(result.map(identity)).size).toBe(source.length); + expect(read.mock.calls.every(([params]) => Number(params.limit) <= 50)).toBe(true); + expect(Math.max(...read.mock.calls.map(([params]) => params.filter.length))).toBeLessThan(250); + }); + + it('keeps events of different types with the same recipient ID and discovers unknown types', async () => { + const source = [ + ...Array.from({ length: 80 }, (_, index) => event(index + 1, 'email_delivered_event')), + ...Array.from({ length: 80 }, (_, index) => event(index + 1, 'email_opened_event')), + ...Array.from({ length: 70 }, (_, index) => event(index + 1, 'future_server_event')), + event(900, 'signup_event', olderTimestamp), + ]; + const result = await browseAll(legacyServer(source), { limit: 7 }); + + expect(result.map(identity).sort()).toEqual(source.map(identity).sort()); + expect(result[result.length - 1]).toEqual(source[source.length - 1]); + expect(new Set(result.map(identity)).size).toBe(source.length); + }); + + it('preserves exclusions and member constraints for every boundary request', async () => { + const source = [ + ...Array.from({ length: 70 }, (_, index) => event(index + 1)), + ...Array.from({ length: 70 }, (_, index) => event(index + 1, 'email_opened_event')), + ...Array.from({ length: 70 }, (_, index) => + event(index + 1, 'email_delivered_event', timestamp, 'member-2'), + ), + ]; + const filter = "type:-[email_opened_event]+data.member_id:'member-1'"; + const read = legacyServer(source); + const result = await browseAll(read, { limit: 10, filter }); + + expect(result.map(identity).sort()).toEqual(source.slice(0, 70).map(identity).sort()); + expect(read.mock.calls.every(([params]) => params.filter.startsWith(filter))).toBe(true); + }); + + it('drains spam complaints when Core uses a different type name in its filter', async () => { + const source = [ + ...Array.from({ length: 70 }, (_, index) => event(index + 1, 'email_complaint_event')), + ...Array.from({ length: 70 }, (_, index) => event(index + 1, 'email_opened_event')), + ]; + const result = await browseAll(legacyServer(source), { limit: 7 }); + expect(result.map(identity).sort()).toEqual(source.map(identity).sort()); + }); + + it('does not require pagination metadata from an older backend', async () => { + const source = Array.from({ length: 105 }, (_, index) => event(index + 1)); + expect( + (await browseAll(legacyServer(source, { metadata: false }))).map(identity).sort(), + ).toEqual(source.map(identity).sort()); + }); + + it('continues when a server clamps the requested page size', async () => { + const source = Array.from({ length: 105 }, (_, index) => event(index + 1)); + expect((await browseAll(legacyServer(source, { maxLimit: 7 }))).map(identity).sort()).toEqual( + source.map(identity).sort(), + ); + }); + + it.each([1, 75])( + 'paginates SQLite text timestamps with %s events at the boundary', + async (boundaryCount) => { + const source = [ + ...Array.from({ length: boundaryCount }, (_, index) => event(index + 1)), + event(100, 'login_event', olderTimestamp), + ]; + const result = await browseAll(legacyServer(source, { textTimestamps: true }), { + limit: Math.min(boundaryCount, 50), + }); + + expect(result.map(identity).sort()).toEqual(source.map(identity).sort()); + expect(result[result.length - 1]).toEqual(source[source.length - 1]); + expect(new Set(result.map(identity)).size).toBe(source.length); + }, + ); + + it('keeps sub-second timestamp precision across page boundaries', async () => { + const source = [ + event(3, 'login_event', '2026-09-14T10:00:00.900Z'), + event(2, 'login_event', '2026-09-14T10:00:00.500Z'), + event(1, 'login_event', '2026-09-14T10:00:00.100Z'), + event(0, 'login_event', '2026-09-14T10:00:00.000Z'), + ]; + expect(await browseAll(legacyServer(source), { limit: 1 })).toEqual(source); + }); + + it('retries a failed boundary request from the unchanged cursor', async () => { + const source = Array.from({ length: 125 }, (_, index) => event(index + 1)); + const read = legacyServer(source); + const first = await loadMemberActivityPage({ read, limit: 50 }); + const savedCursor = JSON.stringify(first.nextCursor); + const reliableRead = read.getMockImplementation()!; + read.mockImplementationOnce(reliableRead).mockRejectedValueOnce(new Error('Connection lost')); + + await expect( + loadMemberActivityPage({ read, limit: 50, cursor: first.nextCursor }), + ).rejects.toThrow('Connection lost'); + expect(JSON.stringify(first.nextCursor)).toBe(savedCursor); + const second = await loadMemberActivityPage({ read, limit: 50, cursor: first.nextCursor }); + const third = await loadMemberActivityPage({ read, limit: 50, cursor: second.nextCursor }); + expect([...first.events, ...second.events, ...third.events].map(identity).sort()).toEqual( + source.map(identity).sort(), + ); + }); + + it('stops subsequent boundary requests when a filter change cancels the query', async () => { + const source = Array.from({ length: 125 }, (_, index) => event(index + 1)); + const read = legacyServer(source); + const first = await loadMemberActivityPage({ read, limit: 50 }); + const controller = new AbortController(); + const reliableRead = read.getMockImplementation()!; + read.mockImplementationOnce(async (params) => { + const response = await reliableRead(params); + controller.abort(); + return response; + }); + const beforeCalls = read.mock.calls.length; + + await expect( + loadMemberActivityPage({ + read, + limit: 50, + cursor: first.nextCursor, + signal: controller.signal, + }), + ).rejects.toMatchObject({ name: 'AbortError' }); + expect(read.mock.calls.length - beforeCalls).toBe(1); + }); + + it('fails visibly instead of ending pagination when a response cannot advance', async () => { + const read = legacyServer([event(1)]); + read.mockResolvedValueOnce({ + events: [], + meta: { pagination: { page: 1, pages: 2, limit: 50, total: 100, next: null, prev: null } }, + }); + await expect(loadMemberActivityPage({ read, limit: 50 })).rejects.toThrow( + 'did not make progress', + ); + }); +}); diff --git a/apps/admin/src/layout/app-sidebar/nav-content.tsx b/apps/admin/src/layout/app-sidebar/nav-content.tsx index e4446672206..492eb2a9d40 100644 --- a/apps/admin/src/layout/app-sidebar/nav-content.tsx +++ b/apps/admin/src/layout/app-sidebar/nav-content.tsx @@ -88,6 +88,7 @@ function NavContent({ ...props }: React.ComponentProps) { const routing = useEmberRouting(); const automationsEnabled = useFeatureFlag('automations'); const isMembersRouteActive = useIsActiveLink({ path: 'members', activeOnSubpath: true }); + const isMemberActivityActive = useIsActiveLink({ path: 'members-activity' }); const showTags = currentUser && canManageTags(currentUser); const showMembers = currentUser && canManageMembers(currentUser); @@ -101,7 +102,7 @@ function NavContent({ ...props }: React.ComponentProps) { const membersExpanded = savedMembersExpanded; const membersNavActive = isMembersRouteActive ? !hasActiveMemberView || !membersExpanded - : routing.isRouteActive(LEGACY_MEMBERS_ACTIVE_ROUTES); + : isMemberActivityActive || routing.isRouteActive(LEGACY_MEMBERS_ACTIVE_ROUTES); const postsRoute = postNavigation.mainUrl; const postsNavActive = postNavigation.isMainActive || (!postsExpanded && hasActivePostChild); return ( diff --git a/apps/admin/src/member-activity-gate.acceptance.test.tsx b/apps/admin/src/member-activity-gate.acceptance.test.tsx new file mode 100644 index 00000000000..d939a3e8be3 --- /dev/null +++ b/apps/admin/src/member-activity-gate.acceptance.test.tsx @@ -0,0 +1,29 @@ +import { describe, expect, it } from 'vitest'; +import { page } from 'vitest/browser'; +import { configResponse, fakeAdminEndpoint, renderAdminApp } from '@test-utils/acceptance'; + +describe('Member activity route ownership', () => { + it.each([false, undefined])('leaves the page with Ember when the flag is %s', async (enabled) => { + const config = configResponse(); + config.config.labs ??= {}; + if (enabled === undefined) { + delete config.config.labs.membersActivityReact; + } else { + config.config.labs.membersActivityReact = enabled; + } + const events = fakeAdminEndpoint('GET', /^\/members\/events\//, { events: [] }); + await renderAdminApp('/members-activity', { + boot: { browseConfig: { response: config } }, + }); + + // There is no Ember runtime in this tier; the shell exposes its host + // instead. The real off-flag UI journey is covered by browser E2E. + await expect + .poll(() => document.getElementById('ember-app')?.parentElement?.hidden) + .toBe(false); + await expect + .element(page.getByRole('heading', { name: 'Member activity' })) + .not.toBeInTheDocument(); + expect(events.requests).toHaveLength(0); + }); +}); diff --git a/apps/admin/src/member-activity-gate.tsx b/apps/admin/src/member-activity-gate.tsx new file mode 100644 index 00000000000..64a370e7b65 --- /dev/null +++ b/apps/admin/src/member-activity-gate.tsx @@ -0,0 +1,9 @@ +import { lazy } from 'react'; +import { FlagGatedRoute } from './flag-gated-route'; +import { lazyMemberActivityScreen } from './members/api'; + +const MemberActivityReact = lazy(lazyMemberActivityScreen); + +export function MemberActivityGate() { + return ; +} diff --git a/apps/admin/src/members/activity/activity-email-preview-data.ts b/apps/admin/src/members/activity/activity-email-preview-data.ts new file mode 100644 index 00000000000..0594ef11edc --- /dev/null +++ b/apps/admin/src/members/activity/activity-email-preview-data.ts @@ -0,0 +1,68 @@ +import type { Config } from '@tryghost/admin-x-framework/api/config'; + +function record(value: unknown): Record | undefined { + return value !== null && typeof value === 'object' && !Array.isArray(value) + ? (value as Record) + : undefined; +} + +function string(value: unknown): string | undefined { + return typeof value === 'string' ? value : undefined; +} + +/** Activity carries historical email objects, occasionally nested in a post. */ +export function activityEmailPreviewData(value: unknown) { + const source = record(value); + const nested = record(source?.email); + const stored = [source, nested].find( + (candidate) => string(candidate?.html) && typeof candidate?.subject === 'string', + ); + const newsletter = record(source?.newsletter) ?? record(nested?.newsletter); + // An email id is not a post id. Only actual post-shaped objects may fall + // back to their own id; welcome/automation ids must never hit this endpoint. + const postId = + string(source?.post_id) || + string(nested?.post_id) || + (typeof source?.title === 'string' ? string(source.id) : undefined); + + return { + stored: stored ? { html: string(stored.html)!, subject: string(stored.subject)! } : undefined, + postId, + newsletter: newsletter + ? { + sender_name: string(newsletter.sender_name), + sender_email: string(newsletter.sender_email), + } + : undefined, + }; +} + +/** Matches Ember's sender-email-address helper, including managed domains. */ +export function activitySenderAddress( + sender: string | undefined, + defaultAddress: string | undefined, + config: Pick | undefined, +): string { + const managed = config?.hostSettings?.managedEmail; + if ( + managed?.enabled && + (!managed.sendingDomain || sender?.split('@')[1] !== managed.sendingDomain) + ) { + return defaultAddress || ''; + } + return sender || defaultAddress || ''; +} + +/** The document is additionally sandboxed; links in a historical preview are inert. */ +export function activityPreviewDocument(html: string): string { + const document = new DOMParser().parseFromString(html, 'text/html'); + document.querySelectorAll('a, area').forEach((link) => { + link.removeAttribute('href'); + link.removeAttribute('target'); + }); + document.querySelectorAll('base, meta[http-equiv], form').forEach((element) => element.remove()); + const style = document.createElement('style'); + style.textContent = 'html { scrollbar-width: none; } html::-webkit-scrollbar { display: none; }'; + document.head.append(style); + return `${document.documentElement.outerHTML}`; +} diff --git a/apps/admin/src/members/activity/activity-email-preview.test.tsx b/apps/admin/src/members/activity/activity-email-preview.test.tsx new file mode 100644 index 00000000000..882319ac7c6 --- /dev/null +++ b/apps/admin/src/members/activity/activity-email-preview.test.tsx @@ -0,0 +1,175 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { useState } from 'react'; +import ActivityEmailPreview from './activity-email-preview'; +import { + activityEmailPreviewData, + activityPreviewDocument, + activitySenderAddress, +} from './activity-email-preview-data'; + +const queries = vi.hoisted(() => ({ + preview: vi.fn(), + newsletters: vi.fn(), + config: vi.fn(), + settings: vi.fn(), + retry: vi.fn(), +})); + +vi.mock('@tryghost/admin-x-framework/api/email-previews', () => ({ + useEmailPreview: queries.preview, +})); +vi.mock('@tryghost/admin-x-framework/api/newsletters', () => ({ + useBrowseNewsletters: queries.newsletters, +})); +vi.mock('@tryghost/admin-x-framework/api/config', () => ({ useBrowseConfig: queries.config })); +vi.mock('@tryghost/admin-x-framework/api/settings', async () => ({ + ...(await vi.importActual( + '@tryghost/admin-x-framework/api/settings', + )), + useBrowseSettings: queries.settings, +})); + +const savedEmail = { + html: '

Saved body

A link', + subject: 'Original subject', + post_id: 'post-1', +}; + +beforeEach(() => { + vi.clearAllMocks(); + queries.preview.mockReturnValue({ isLoading: false, isError: false, refetch: queries.retry }); + queries.newsletters.mockReturnValue({ + data: { newsletters: [{ sender_name: 'Newsletter', sender_email: 'news@example.com' }] }, + }); + queries.config.mockReturnValue({ data: { config: {} } }); + queries.settings.mockReturnValue({ + data: { + settings: [ + { key: 'title', value: 'Publication' }, + { key: 'default_email_address', value: 'default@example.com' }, + ], + }, + }); +}); + +describe('Activity email preview', () => { + it('uses stored HTML and subject without generating a new preview, and disables navigation', () => { + render(); + expect(screen.getByRole('dialog', { name: 'Email preview' })).toBeTruthy(); + expect(screen.getByText('Original subject', { exact: false })).toBeTruthy(); + expect(screen.getByText('Newsletter ', { exact: false })).toBeTruthy(); + expect(screen.getByText('Jamie Larson ', { exact: false })).toBeTruthy(); + const frame = screen.getByTitle('Email content'); + expect(frame.getAttribute('sandbox')).toBe(''); + expect(frame.getAttribute('srcdoc')).toContain('Saved body'); + expect(frame.getAttribute('srcdoc')).not.toContain('href='); + expect(queries.preview).toHaveBeenCalledWith('post-1', { enabled: false }); + fireEvent.click(screen.getByRole('radio', { name: 'Mobile' })); + expect( + screen.getByTitle('Email content').closest('[data-device]')?.getAttribute('data-device'), + ).toBe('mobile'); + }); + + it('supports keyboard dismissal and the close button', () => { + const onClose = vi.fn(); + render(); + fireEvent.keyDown(document, { key: 'Escape' }); + expect(onClose).toHaveBeenCalledOnce(); + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + expect(onClose).toHaveBeenCalledTimes(2); + }); + + it('returns focus to the activity link after closing', async () => { + function ActivityLink() { + const [open, setOpen] = useState(false); + return ( + <> + + {open && setOpen(false)} />} + + ); + } + render(); + const opener = screen.getByRole('button', { name: 'Preview saved email' }); + opener.focus(); + fireEvent.click(opener); + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + await waitFor(() => expect(window.document.activeElement).toBe(opener)); + }); + + it('loads a missing historical body using the post id and offers retry after a failed request', () => { + queries.preview.mockReturnValue({ isLoading: true }); + const { rerender } = render( + , + ); + expect(queries.preview).toHaveBeenCalledWith('post-1', { enabled: true }); + expect(screen.queryByTitle('Email content')).toBeNull(); + queries.preview.mockReturnValue({ isLoading: false, isError: true, refetch: queries.retry }); + rerender(); + fireEvent.click(screen.getByRole('button', { name: 'Retry' })); + expect(queries.retry).toHaveBeenCalledOnce(); + queries.preview.mockReturnValue({ + data: { email_previews: [{ html: '

Fallback

', subject: 'Rendered subject' }] }, + }); + rerender(); + expect(screen.getByTitle('Email content').getAttribute('srcdoc')).toContain('Fallback'); + expect(screen.getByText('Rendered subject', { exact: false })).toBeTruthy(); + }); + + it('does not interpret a deleted email or automated email id as a post id', () => { + render( + , + ); + expect(queries.preview).toHaveBeenCalledWith('', { enabled: false }); + expect(screen.getByText('The original email content is no longer available.')).toBeTruthy(); + expect(screen.queryByRole('button', { name: 'Retry' })).toBeNull(); + }); + + it('keeps the historical body visible and allows retry when sender details fail', () => { + queries.settings.mockReturnValue({ isError: true, refetch: queries.retry }); + queries.config.mockReturnValue({ refetch: queries.retry }); + queries.newsletters.mockReturnValue({ refetch: queries.retry }); + render(); + expect(screen.getByTitle('Email content').getAttribute('srcdoc')).toContain('Saved body'); + expect(screen.getByRole('alert').textContent).toBe('Couldn’t load sender details.'); + fireEvent.click(screen.getByRole('button', { name: 'Retry sender details' })); + expect(queries.retry).toHaveBeenCalledTimes(3); + }); +}); + +describe('Activity email data', () => { + it('accepts nested stored emails with an empty subject and does not require a style tag or doctype', () => { + expect( + activityEmailPreviewData({ email: { html: '

Original

', subject: '' } }).stored, + ).toEqual({ html: '

Original

', subject: '' }); + expect(activityEmailPreviewData({ id: 'post-2', title: 'A post' }).postId).toBe('post-2'); + expect(activityPreviewDocument('

Original

')).toContain('

Original

'); + }); + + it('neutralizes refresh redirects, bases, form submission and image-map links', () => { + const html = activityPreviewDocument( + '
', + ); + expect(html).not.toMatch(/ { + expect( + activitySenderAddress('other@example.com', 'default@managed.com', { + hostSettings: { managedEmail: { enabled: true } }, + }), + ).toBe('default@managed.com'); + expect( + activitySenderAddress('other@example.com', 'default@managed.com', { + hostSettings: { managedEmail: { enabled: true, sendingDomain: 'managed.com' } }, + }), + ).toBe('default@managed.com'); + expect( + activitySenderAddress('news@managed.com', 'default@managed.com', { + hostSettings: { managedEmail: { enabled: true, sendingDomain: 'managed.com' } }, + }), + ).toBe('news@managed.com'); + }); +}); diff --git a/apps/admin/src/members/activity/activity-email-preview.tsx b/apps/admin/src/members/activity/activity-email-preview.tsx new file mode 100644 index 00000000000..1e12833219c --- /dev/null +++ b/apps/admin/src/members/activity/activity-email-preview.tsx @@ -0,0 +1,214 @@ +import { useMemo, useState } from 'react'; +import { + Button, + Dialog, + DialogContent, + DialogTitle, + EmptyIndicator, + LoadingIndicator, + PreviewChrome, + ToggleGroup, + ToggleGroupItem, +} from '@tryghost/shade/components'; +import { Grid, Inline, Stack } from '@tryghost/shade/primitives'; +import { LucideIcon, cn } from '@tryghost/shade/utils'; +import { useBrowseConfig } from '@tryghost/admin-x-framework/api/config'; +import { useEmailPreview } from '@tryghost/admin-x-framework/api/email-previews'; +import { useBrowseNewsletters } from '@tryghost/admin-x-framework/api/newsletters'; +import { getSettingValues, useBrowseSettings } from '@tryghost/admin-x-framework/api/settings'; +import { + activityEmailPreviewData, + activityPreviewDocument, + activitySenderAddress, +} from './activity-email-preview-data'; + +interface ActivityEmailPreviewProps { + /** The event's email object, a nested email, or a post with its identity. */ + email: unknown; + onClose: () => void; +} + +export default function ActivityEmailPreview({ email, onClose }: ActivityEmailPreviewProps) { + const [device, setDevice] = useState<'desktop' | 'mobile'>('desktop'); + const [opener] = useState(() => window.document.activeElement); + const source = activityEmailPreviewData(email); + const fallbackEnabled = !source.stored && !!source.postId; + const previewQuery = useEmailPreview(source.postId ?? '', { enabled: fallbackEnabled }); + const newslettersQuery = useBrowseNewsletters({ + enabled: !source.newsletter, + searchParams: { filter: 'status:active', limit: '1' }, + }); + const settingsQuery = useBrowseSettings(); + const configQuery = useBrowseConfig(); + const [siteTitle, defaultEmailAddress] = getSettingValues( + settingsQuery.data?.settings ?? [], + ['title', 'default_email_address'], + ); + const newsletter = source.newsletter ?? newslettersQuery.data?.newsletters[0]; + const senderName = newsletter?.sender_name || siteTitle; + const senderAddress = activitySenderAddress( + newsletter?.sender_email ?? undefined, + defaultEmailAddress, + configQuery.data?.config, + ); + const preview = + source.stored ?? + (fallbackEnabled && !previewQuery.isError ? previewQuery.data?.email_previews[0] : undefined); + const document = useMemo( + () => (preview ? activityPreviewDocument(preview.html) : undefined), + [preview?.html], + ); + const metadataError = + settingsQuery.isError || + configQuery.isError || + (!source.newsletter && newslettersQuery.isError); + const metadataLoading = + settingsQuery.isLoading || + configQuery.isLoading || + (!source.newsletter && newslettersQuery.isLoading); + const loading = fallbackEnabled && previewQuery.isLoading; + + return ( + { + if (!open) { + onClose(); + } + }} + > + { + if (opener instanceof HTMLElement && opener.isConnected) { + event.preventDefault(); + opener.focus(); + } + }} + onInteractOutside={(event) => event.preventDefault()} + > + + Email preview + { + if (value === 'desktop' || value === 'mobile') { + setDevice(value); + } + }} + > + + + + + + + + + + + div]:rounded-2xl [&>div]:shadow-2xl' + : 'shadow-2xl', + )} + data-device={device} + device={device} + > + + +

{preview?.subject}

+

+ From: + {metadataLoading ? ( + 'Loading sender…' + ) : metadataError ? ( + 'Sender unavailable' + ) : ( + <> + {senderName} + {senderAddress && ` <${senderAddress}>`} + + )} +

+

+ To: Jamie Larson + <jamie@example.com> +

+ {metadataError && ( + +

Couldn’t load sender details.

+ + + )} + + {loading ? ( + + + + ) : preview ? ( +