From 713585f2a8e2482934f29829012b592b0aa22f9c Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 11 Aug 2026 12:43:50 -0500 Subject: [PATCH 1/4] Make email analytics queries helper a class We'll do some DI/testing soon. --- .../email-analytics-service-wrapper.js | 4 ++- .../server/services/email-analytics/index.ts | 6 +++- .../services/email-analytics/lib/queries.ts | 28 +++++++++---------- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.js b/ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.js index 0f0ecbdffd7..82207ee984b 100644 --- a/ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.js +++ b/ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.js @@ -2,6 +2,7 @@ const logging = require('@tryghost/logging'); const metrics = require('@tryghost/metrics'); const config = require('../../../shared/config'); const domainEvents = require('@tryghost/domain-events'); +/** @import {Queries} from './lib/queries' */ /** @import {PrometheusClient} from '@tryghost/prometheus-metrics' */ /** @import {BatchEventProcessor} from './batch-event-processor' */ /** @import {JobNames, CursorSeed, EmailAnalyticsFetchResult} from './email-analytics-service' */ @@ -28,6 +29,7 @@ class EmailAnalyticsServiceWrapper { /** * @param {object} options * @param {Parameters[0]} options.event + * @param {Queries} options.queries * @param {string[]} options.mailgunTags * @param {JobNames} options.jobNames * @param {CursorSeed} options.cursorSeed @@ -36,6 +38,7 @@ class EmailAnalyticsServiceWrapper { */ init({ event, + queries, mailgunTags, jobNames, cursorSeed, @@ -49,7 +52,6 @@ class EmailAnalyticsServiceWrapper { const {EmailAnalyticsService} = require('./email-analytics-service'); const {fetchMailgunEvents} = require('./fetch-mailgun-events'); const settings = require('../../../shared/settings-cache'); - const {queries} = require('./lib/queries'); this.service = new EmailAnalyticsService({ fetchEvents: (options) => fetchMailgunEvents({...options, config, settings, tags: mailgunTags}), diff --git a/ghost/core/core/server/services/email-analytics/index.ts b/ghost/core/core/server/services/email-analytics/index.ts index 34da53958d3..760e5c316aa 100644 --- a/ghost/core/core/server/services/email-analytics/index.ts +++ b/ghost/core/core/server/services/email-analytics/index.ts @@ -18,7 +18,7 @@ import {EmailRecipientFailure, EmailSpamComplaintEvent, Email} from '../../model import domainEvents from '@tryghost/domain-events'; // @ts-expect-error This module lacks type definitions. import prometheusClient from '../../../shared/prometheus-client'; -import {queries} from './lib/queries'; +import {Queries} from './lib/queries'; import {StartEmailAnalyticsJobEvent} from './events/start-email-analytics-job-event'; import {StartAutomationEmailAnalyticsJobEvent} from './events/start-automation-email-analytics-job-event'; import {AUTOMATION_EMAIL_TAG} from '../member-welcome-emails/constants'; @@ -34,6 +34,8 @@ export const automations = new EmailAnalyticsServiceWrapper({ }); export const init = () => { + const queries = new Queries(); + const newsletterEmailEventProcessor = new EmailEventProcessor({ domainEvents, db, @@ -58,6 +60,7 @@ export const init = () => { newsletters.init({ event: StartEmailAnalyticsJobEvent, + queries, mailgunTags: newsletterMailgunTags, jobNames: { latestNonOpened: 'email-analytics-latest-others', @@ -86,6 +89,7 @@ export const init = () => { automations.init({ event: StartAutomationEmailAnalyticsJobEvent, + queries, mailgunTags: [AUTOMATION_EMAIL_TAG], jobNames: { latestNonOpened: 'email-analytics-automation-latest-others', diff --git a/ghost/core/core/server/services/email-analytics/lib/queries.ts b/ghost/core/core/server/services/email-analytics/lib/queries.ts index 0df7080e8dd..e0ad65eb8f9 100644 --- a/ghost/core/core/server/services/email-analytics/lib/queries.ts +++ b/ghost/core/core/server/services/email-analytics/lib/queries.ts @@ -67,7 +67,7 @@ function parseJobMetadata(rawMetadata: unknown): JobMetadata { }; } -export const queries = { +export class Queries { /** * Retrieves the timestamp of the last seen event for the specified email analytics events. * @param jobName - The name of the job to update. @@ -118,7 +118,7 @@ export const queries = { debug(`getLastEventTimestamp: finished in ${Date.now() - startDate.getTime()}ms`); return lastSeenEventTimestamp; - }, + } /** * Retrieves the job data for the specified job name. @@ -126,7 +126,7 @@ export const queries = { * @returns The job data, or null if no job data is found. */ async getJobData(jobName: EmailAnalyticsJobName): Promise { - const row = await db.knex('jobs') + const row = await this.#knex('jobs') .select('finished_at', 'started_at', 'metadata') .where('name', jobName) .first(); @@ -135,7 +135,7 @@ export const queries = { started_at: row.started_at, metadata: parseJobMetadata(row.metadata) } : null; - }, + } /** * Retrieves the timestamp of the last job run for the specified job name. @@ -145,7 +145,7 @@ export const queries = { async getLastJobRunTimestamp(jobName: EmailAnalyticsJobName): Promise { const jobData = await this.getJobData(jobName); return jobData ? jobData.finished_at || jobData.started_at : null; - }, + } /** * Sets the timestamp of the last seen event for the specified email analytics events. @@ -176,7 +176,7 @@ export const queries = { const message = err instanceof Error ? err.message : String(err); debug(`Error setting ${field} timestamp for job ${jobName}: ${message}`); } - }, + } /** * Retrieves and parses the metadata JSON for the specified job. @@ -184,9 +184,9 @@ export const queries = { * @returns The parsed metadata object, or null. */ async getJobMetadata(jobName: EmailAnalyticsJobName): Promise { - const row = await db.knex('jobs').select('metadata').where('name', jobName).first(); + const row = await this.#knex('jobs').select('metadata').where('name', jobName).first(); return row ? parseJobMetadata(row.metadata) : null; - }, + } /** * Writes metadata JSON for the specified job. @@ -212,7 +212,7 @@ export const queries = { const message = err instanceof Error ? err.message : String(err); logging.error(`Error setting metadata for job ${jobName}: ${message}`); } - }, + } /** * Sets the status of the specified email analytics job. @@ -246,7 +246,7 @@ export const queries = { debug(`Error setting status for job ${jobName}: ${message}`); throw err; } - }, + } async aggregateEmailStats(emailId: string, updateOpenedCount: boolean): Promise { const [deliveredCount] = await db.knex('email_recipients').count('id as count').whereRaw('email_id = ? AND delivered_at IS NOT NULL', [emailId]); @@ -263,7 +263,7 @@ export const queries = { } await db.knex('emails').update(updateData).where('id', emailId); - }, + } async aggregateMemberStats(memberId: string): Promise { const {trackedEmailCount} = await db.knex('email_recipients') @@ -288,7 +288,7 @@ export const queries = { await db.knex('members') .update(updateQuery) .where('id', memberId); - }, + } async aggregateMemberStatsBatch(memberIds: string[]): Promise { if (!memberIds || memberIds.length === 0) { @@ -373,6 +373,4 @@ export const queries = { WHERE id IN (${memberIds.map(() => '?').join(',')}) `, bindings); } -}; - -export type Queries = typeof queries; +}; \ No newline at end of file From 54d4b08f7a503fd2851248c08c14c14e934d69d6 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 11 Aug 2026 12:55:27 -0500 Subject: [PATCH 2/4] `Queries` should take a database reference instead of importing it Dependency injection! --- .../server/services/email-analytics/index.ts | 2 +- .../services/email-analytics/lib/queries.ts | 76 +++++++++++-------- .../automation-email-analytics.test.js | 6 +- .../newsletter-email-event-storage.test.js | 4 +- .../mailgun-email-suppression-list.test.js | 4 +- ghost/core/test/utils/fixture-utils.js | 4 +- 6 files changed, 55 insertions(+), 41 deletions(-) diff --git a/ghost/core/core/server/services/email-analytics/index.ts b/ghost/core/core/server/services/email-analytics/index.ts index 760e5c316aa..f985e3b1c50 100644 --- a/ghost/core/core/server/services/email-analytics/index.ts +++ b/ghost/core/core/server/services/email-analytics/index.ts @@ -34,7 +34,7 @@ export const automations = new EmailAnalyticsServiceWrapper({ }); export const init = () => { - const queries = new Queries(); + const queries = new Queries(db.knex); const newsletterEmailEventProcessor = new EmailEventProcessor({ domainEvents, diff --git a/ghost/core/core/server/services/email-analytics/lib/queries.ts b/ghost/core/core/server/services/email-analytics/lib/queries.ts index e0ad65eb8f9..92a52233594 100644 --- a/ghost/core/core/server/services/email-analytics/lib/queries.ts +++ b/ghost/core/core/server/services/email-analytics/lib/queries.ts @@ -1,9 +1,8 @@ import _ from 'lodash'; import debugFactory from '@tryghost/debug'; -// @ts-expect-error This module lacks type definitions. -import db from '../../../data/db'; import logging from '@tryghost/logging'; import ObjectID from 'bson-objectid'; +import type {Knex} from 'knex'; import type {JsonObject} from 'type-fest'; const debug = debugFactory('services:email-analytics'); @@ -31,8 +30,8 @@ type JobData = { * Creates a job in the jobs table if it does not already exist. * @param jobName - The name of the job to create. */ -async function createJobIfNotExists(jobName: EmailAnalyticsJobName): Promise { - await db.knex('jobs').insert({ +async function createJobIfNotExists(knex: Knex, jobName: EmailAnalyticsJobName): Promise { + await knex('jobs').insert({ id: new ObjectID().toHexString(), name: jobName, started_at: new Date(), @@ -68,6 +67,12 @@ function parseJobMetadata(rawMetadata: unknown): JobMetadata { } export class Queries { + #knex: Knex; + + constructor(knex: Knex) { + this.#knex = knex; + } + /** * Retrieves the timestamp of the last seen event for the specified email analytics events. * @param jobName - The name of the job to update. @@ -80,6 +85,8 @@ export class Queries { events: EmailAnalyticsEvent[], cursorSeed: CursorSeed ): Promise { + const knex = this.#knex; + const startDate = new Date(); let timestamps: (Date | string | null)[] = []; @@ -97,13 +104,13 @@ export class Queries { if (!columnName) { continue; } - const row = await db.knex(cursorSeed.tableName) - .select(db.knex.raw('MAX(??) as maxTimestamp', [columnName])) + const row = await knex(cursorSeed.tableName) + .select(knex.raw('MAX(??) as maxTimestamp', [columnName])) .first(); timestamps.push(row.maxTimestamp); } - await createJobIfNotExists(jobName); + await createJobIfNotExists(knex, jobName); } // Convert string dates to Date objects for SQLite compatibility @@ -162,9 +169,9 @@ export class Queries { debug(`Setting ${field} timestamp for job ${jobName} to ${date}`); const updateField = field === 'finished' ? 'finished_at' : 'started_at'; const status = field === 'finished' ? 'finished' : 'started'; - const result = await db.knex('jobs').update({[updateField]: date, updated_at: new Date(), status: status}).where('name', jobName); + const result = await this.#knex('jobs').update({[updateField]: date, updated_at: new Date(), status: status}).where('name', jobName); if (result === 0) { - await db.knex('jobs').insert({ + await this.#knex('jobs').insert({ id: new ObjectID().toHexString(), name: jobName, [updateField]: date.toISOString(), // force to iso string for sqlite @@ -196,7 +203,7 @@ export class Queries { async setJobMetadata(jobName: EmailAnalyticsJobName, metadata: JobMetadata | null): Promise { try { const value = metadata ? JSON.stringify(metadata) : null; - await db.knex.transaction(async (trx: typeof db.knex) => { + await this.#knex.transaction(async (trx: Knex.Transaction) => { const result = await trx('jobs').update({metadata: value, updated_at: new Date()}).where('name', jobName); if (result === 0 && metadata) { await trx('jobs').insert({ @@ -225,7 +232,7 @@ export class Queries { async setJobStatus(jobName: EmailAnalyticsJobName, status: 'started' | 'finished' | 'failed'): Promise { debug(`Setting status for job ${jobName} to ${status}`); try { - const result = await db.knex('jobs') + const result = await this.#knex('jobs') .update({ status: status, updated_at: new Date() @@ -233,7 +240,7 @@ export class Queries { .where('name', jobName); if (result === 0) { - await db.knex('jobs').insert({ + await this.#knex('jobs').insert({ id: new ObjectID().toHexString(), name: jobName, status: status, @@ -249,8 +256,8 @@ export class Queries { } async aggregateEmailStats(emailId: string, updateOpenedCount: boolean): Promise { - const [deliveredCount] = await db.knex('email_recipients').count('id as count').whereRaw('email_id = ? AND delivered_at IS NOT NULL', [emailId]); - const [failedCount] = await db.knex('email_recipients').count('id as count').whereRaw('email_id = ? AND failed_at IS NOT NULL', [emailId]); + const [deliveredCount] = await this.#knex('email_recipients').count('id as count').whereRaw('email_id = ? AND delivered_at IS NOT NULL', [emailId]); + const [failedCount] = await this.#knex('email_recipients').count('id as count').whereRaw('email_id = ? AND failed_at IS NOT NULL', [emailId]); const updateData: Record = { delivered_count: deliveredCount.count, @@ -258,34 +265,41 @@ export class Queries { }; if (updateOpenedCount) { - const [openedCount] = await db.knex('email_recipients').count('id as count').whereRaw('email_id = ? AND opened_at IS NOT NULL', [emailId]); + const [openedCount] = await this.#knex('email_recipients').count('id as count').whereRaw('email_id = ? AND opened_at IS NOT NULL', [emailId]); updateData.opened_count = openedCount.count; } - await db.knex('emails').update(updateData).where('id', emailId); + await this.#knex('emails').update(updateData).where('id', emailId); } async aggregateMemberStats(memberId: string): Promise { - const {trackedEmailCount} = await db.knex('email_recipients') - .select(db.knex.raw('COUNT(email_recipients.id) as trackedEmailCount')) + const {trackedEmailCount} = await this.#knex('email_recipients') + .select(this.#knex.raw('COUNT(email_recipients.id) as trackedEmailCount')) .leftJoin('emails', 'email_recipients.email_id', 'emails.id') .where('email_recipients.member_id', memberId) .where('emails.track_opens', true) .first() || {}; - - const [emailCount] = await db.knex('email_recipients').count('id as count').whereRaw('member_id = ?', [memberId]); - const [emailOpenedCount] = await db.knex('email_recipients').count('id as count').whereRaw('member_id = ? AND opened_at IS NOT NULL', [memberId]); + const emailCountResult = await this.#knex('email_recipients') + .count('id as count') + .whereRaw('member_id = ?', [memberId]) + .first(); + const emailOpenedCountResult = await this.#knex('email_recipients') + .count('id as count') + .whereRaw('member_id = ? AND opened_at IS NOT NULL', [memberId]) + .first(); + const emailCount = Number(emailCountResult?.count || 0); + const emailOpenedCount = Number(emailOpenedCountResult?.count || 0); const updateQuery: Record = { - email_count: emailCount.count, - email_opened_count: emailOpenedCount.count + email_count: emailCount, + email_opened_count: emailOpenedCount }; if (trackedEmailCount >= MIN_EMAIL_COUNT_FOR_OPEN_RATE) { - updateQuery.email_open_rate = Math.round(emailOpenedCount.count / trackedEmailCount * 100); + updateQuery.email_open_rate = Math.round(emailOpenedCount / trackedEmailCount * 100); } - await db.knex('members') + await this.#knex('members') .update(updateQuery) .where('id', memberId); } @@ -296,13 +310,13 @@ export class Queries { } // Batch query to get stats for all members at once - const stats = await db.knex('email_recipients') + const stats = await this.#knex('email_recipients') .leftJoin('emails', 'emails.id', 'email_recipients.email_id') .select( 'email_recipients.member_id', - db.knex.raw('COUNT(email_recipients.id) as email_count'), - db.knex.raw('SUM(CASE WHEN email_recipients.opened_at IS NOT NULL THEN 1 ELSE 0 END) as email_opened_count'), - db.knex.raw('SUM(CASE WHEN emails.track_opens = 1 THEN 1 ELSE 0 END) as tracked_count') + this.#knex.raw('COUNT(email_recipients.id) as email_count'), + this.#knex.raw('SUM(CASE WHEN email_recipients.opened_at IS NOT NULL THEN 1 ELSE 0 END) as email_opened_count'), + this.#knex.raw('SUM(CASE WHEN emails.track_opens = 1 THEN 1 ELSE 0 END) as tracked_count') ) .whereIn('email_recipients.member_id', memberIds) .groupBy('email_recipients.member_id'); @@ -364,7 +378,7 @@ export class Queries { ]; // Execute batched update with CASE statements - await db.knex.raw(` + await this.#knex.raw(` UPDATE members SET email_count = CASE id ${emailCountCases.join(' ')} END, @@ -373,4 +387,4 @@ export class Queries { WHERE id IN (${memberIds.map(() => '?').join(',')}) `, bindings); } -}; \ No newline at end of file +}; diff --git a/ghost/core/test/integration/services/email-analytics/automation-email-analytics.test.js b/ghost/core/test/integration/services/email-analytics/automation-email-analytics.test.js index 582d78d9e5e..404911bec89 100644 --- a/ghost/core/test/integration/services/email-analytics/automation-email-analytics.test.js +++ b/ghost/core/test/integration/services/email-analytics/automation-email-analytics.test.js @@ -7,7 +7,7 @@ const {agentProvider} = require('../../../utils/e2e-framework'); const testUtils = require('../../../utils'); const MailgunClient = require('../../../../core/server/services/lib/mailgun-client'); const {AUTOMATION_EMAIL_TAG, DEFAULT_EMAIL_DESIGN_SETTING_SLUG} = require('../../../../core/server/services/member-welcome-emails/constants'); -const {queries} = require('../../../../core/server/services/email-analytics/lib/queries'); +const {Queries} = require('../../../../core/server/services/email-analytics/lib/queries'); const emailAnalytics = require('../../../../core/server/services/email-analytics'); const automationsApi = require('../../../../core/server/services/automations/automations-api'); @@ -28,9 +28,9 @@ describe('Automation email analytics', function () { let emailDesignSettingId; beforeAll(async function () { - sinon.stub(queries, 'getLastEventTimestamp').resolves(new Date(2000, 0, 1)); + sinon.stub(Queries.prototype, 'getLastEventTimestamp').resolves(new Date(2000, 0, 1)); // Same reason, for the cursor fetchMissing starts from. - sinon.stub(queries, 'getLastJobRunTimestamp').resolves(new Date(2000, 0, 1)); + sinon.stub(Queries.prototype, 'getLastJobRunTimestamp').resolves(new Date(2000, 0, 1)); await agentProvider.getAdminAPIAgent(); diff --git a/ghost/core/test/integration/services/email-service/newsletter-email-event-storage.test.js b/ghost/core/test/integration/services/email-service/newsletter-email-event-storage.test.js index e36491e7a59..c434be268e6 100644 --- a/ghost/core/test/integration/services/email-service/newsletter-email-event-storage.test.js +++ b/ghost/core/test/integration/services/email-service/newsletter-email-event-storage.test.js @@ -35,8 +35,8 @@ processingModes.forEach(({name, batchProcessing}) => { configUtils.set('emailAnalytics:batchProcessing', batchProcessing); // Stub queries before boot - const {queries} = require('../../../../core/server/services/email-analytics/lib/queries'); - sinon.stub(queries, 'getLastEventTimestamp').callsFake(async function () { + const {Queries} = require('../../../../core/server/services/email-analytics/lib/queries'); + sinon.stub(Queries.prototype, 'getLastEventTimestamp').callsFake(async function () { // This is required because otherwise the last event timestamp will be now, and that is too close to NOW to start fetching new events return new Date(2000, 0, 1); }); diff --git a/ghost/core/test/integration/services/mailgun-email-suppression-list.test.js b/ghost/core/test/integration/services/mailgun-email-suppression-list.test.js index 1d70a749f5e..f50ca9d3181 100644 --- a/ghost/core/test/integration/services/mailgun-email-suppression-list.test.js +++ b/ghost/core/test/integration/services/mailgun-email-suppression-list.test.js @@ -18,8 +18,8 @@ describe('MailgunEmailSuppressionList', function () { // fetched, so no suppression is created. The suite previously passed only // by free-riding on an earlier file's timestamp state, which breaks under // per-file isolation. Mirrors newsletter-email-event-storage.test.js. - const {queries} = require('../../../core/server/services/email-analytics/lib/queries'); - sinon.stub(queries, 'getLastEventTimestamp').callsFake(async function () { + const {Queries} = require('../../../core/server/services/email-analytics/lib/queries'); + sinon.stub(Queries.prototype, 'getLastEventTimestamp').callsFake(async function () { return new Date(2000, 0, 1); }); diff --git a/ghost/core/test/utils/fixture-utils.js b/ghost/core/test/utils/fixture-utils.js index ae0743b42e7..6a293038a5d 100644 --- a/ghost/core/test/utils/fixture-utils.js +++ b/ghost/core/test/utils/fixture-utils.js @@ -14,6 +14,7 @@ const {fixtureManager} = require('../../core/server/data/schema/fixtures'); const permissions = require('../../core/server/services/permissions'); const settingsService = require('../../core/server/services/settings/settings-service'); const labsService = require('../../core/shared/labs'); +const {Queries: EmailAnalyticsQueries} = require('../../core/server/services/email-analytics/lib/queries'); // Other Test Utilities const context = require('./fixtures/context'); @@ -715,8 +716,7 @@ const fixtures = { }, insertEmailsAndRecipients: async function insertEmailsAndRecipients(withFailed = false) { - // NOTE: This require touches the database, so it can't be done at the top of the file as test setup is being performed. - const {queries: emailAnalyticsQueries} = require('../../core/server/services/email-analytics/lib/queries'); + const emailAnalyticsQueries = new EmailAnalyticsQueries(models.Base.knex); for (const email of _.cloneDeep(DataGenerator.forKnex.emails)) { await models.Email.add(email, context.internal); From 5701bafdc5738eab09aab6dac5bb6c7c2e39e86a Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 11 Aug 2026 13:01:31 -0500 Subject: [PATCH 3/4] Test `Queries.prototype.getJobData` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was written entirely by GPT-5.6 Sol (Medium thinking) with the following prompt: > Write full unit tests for > `ghost/core/core/server/services/email-analytics/lib/queries.ts`. The > test should create a Knex database with in-memory SQLite—similar to > how > `ghost/core/test/unit/server/services/automations/automations-repository.test.ts` > does it—and then test everything against that Knex database. > > Before you go off and write everything, let's test a single method. I > want to make sure we're on the right track! I think `getJobData` is > probably the simplest, so test it. Make sure to test the case where > it's defined and not defined. > > `ghost/core/core/server/data/schema/schema.js:1098-1108` has the > schema for `jobs`, which you may wish to refer to. You may also wish > to refer to the jobs that are set up as part of email analytics. > > This should be a test-only change. --- .../services/email-analytics/queries.test.ts | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 ghost/core/test/unit/server/services/email-analytics/queries.test.ts diff --git a/ghost/core/test/unit/server/services/email-analytics/queries.test.ts b/ghost/core/test/unit/server/services/email-analytics/queries.test.ts new file mode 100644 index 00000000000..cb90484fc11 --- /dev/null +++ b/ghost/core/test/unit/server/services/email-analytics/queries.test.ts @@ -0,0 +1,80 @@ +import assert from 'node:assert/strict'; +import ObjectId from 'bson-objectid'; +import createKnex, {type Knex} from 'knex'; +import {Queries} from '../../../../../core/server/services/email-analytics/lib/queries'; + +const createDatabase = async (): Promise => { + const database = createKnex({ + client: 'better-sqlite3', + connection: { + filename: ':memory:' + }, + pool: { + min: 1, + max: 1 + }, + useNullAsDefault: true + }); + + await database.schema.createTable('jobs', (table) => { + table.text('id').primary(); + table.text('name').notNullable().unique(); + table.text('status').notNullable().defaultTo('queued'); + table.datetime('started_at'); + table.datetime('finished_at'); + table.datetime('created_at').notNullable(); + table.datetime('updated_at'); + table.text('metadata'); + table.integer('queue_entry').unsigned(); + }); + + return database; +}; + +describe('Email analytics queries', function () { + let knex: Knex; + let queries: Queries; + + beforeEach(async function () { + knex = await createDatabase(); + queries = new Queries(knex); + }); + + afterEach(async function () { + await knex.destroy(); + }); + + describe('getJobData', function () { + it('returns job data when job exists', async function () { + const startedAt = '2026-08-11T10:00:00.000Z'; + const finishedAt = '2026-08-11T10:05:00.000Z'; + const metadata = {begin: startedAt, end: finishedAt}; + + await knex('jobs').insert({ + id: ObjectId().toHexString(), + name: 'email-analytics-scheduled', + status: 'finished', + started_at: startedAt, + finished_at: finishedAt, + created_at: startedAt, + updated_at: finishedAt, + metadata: JSON.stringify(metadata), + queue_entry: 1 + }); + + const result = await queries.getJobData('email-analytics-scheduled'); + + assert.deepEqual(result, { + finished_at: finishedAt, + started_at: startedAt, + metadata + }); + }); + + it('returns null when job does not exist', async function () { + const result = await queries.getJobData('email-analytics-scheduled'); + + assert.equal(result, null); + }); + }); +}); From ce3793ef2060daa89cd83d6b852813f0f0020542 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 11 Aug 2026 13:07:57 -0500 Subject: [PATCH 4/4] Test remaining Queries methods I continued the conversation from the previous commit with the following: > Great work. I love what you've written. Now add unit tests for the > rest of the public methods. Make sure to test edge cases. I did a *very* cursory scan of the results, and asked it to "Verify that the test schema matches the real schema", which it did. --- .../services/email-analytics/queries.test.ts | 455 ++++++++++++++++++ 1 file changed, 455 insertions(+) diff --git a/ghost/core/test/unit/server/services/email-analytics/queries.test.ts b/ghost/core/test/unit/server/services/email-analytics/queries.test.ts index cb90484fc11..6c50e360a27 100644 --- a/ghost/core/test/unit/server/services/email-analytics/queries.test.ts +++ b/ghost/core/test/unit/server/services/email-analytics/queries.test.ts @@ -28,6 +28,30 @@ const createDatabase = async (): Promise => { table.integer('queue_entry').unsigned(); }); + await database.schema.createTable('emails', (table) => { + table.text('id').primary(); + table.boolean('track_opens').notNullable().defaultTo(false); + table.integer('delivered_count').notNullable().defaultTo(0); + table.integer('failed_count').notNullable().defaultTo(0); + table.integer('opened_count').notNullable().defaultTo(0); + }); + + await database.schema.createTable('members', (table) => { + table.text('id').primary(); + table.integer('email_count').notNullable().defaultTo(0); + table.integer('email_opened_count').notNullable().defaultTo(0); + table.integer('email_open_rate'); + }); + + await database.schema.createTable('email_recipients', (table) => { + table.text('id').primary(); + table.text('email_id').references('id').inTable('emails'); + table.text('member_id').references('id').inTable('members'); + table.datetime('delivered_at'); + table.datetime('opened_at'); + table.datetime('failed_at'); + }); + return database; }; @@ -35,6 +59,44 @@ describe('Email analytics queries', function () { let knex: Knex; let queries: Queries; + const insertJob = async (attributes: Record = {}) => { + const job = { + id: ObjectId().toHexString(), + name: 'email-analytics-scheduled', + status: 'queued', + started_at: null, + finished_at: null, + created_at: '2026-08-11T09:00:00.000Z', + updated_at: null, + metadata: null, + queue_entry: null, + ...attributes + }; + + await knex('jobs').insert(job); + return job; + }; + + const insertEmail = async (id: string, trackOpens: boolean) => { + await knex('emails').insert({id, track_opens: trackOpens}); + }; + + const insertMember = async (id: string, attributes: Record = {}) => { + await knex('members').insert({id, ...attributes}); + }; + + const insertRecipient = async (attributes: Record) => { + await knex('email_recipients').insert({ + id: ObjectId().toHexString(), + email_id: null, + member_id: null, + delivered_at: null, + opened_at: null, + failed_at: null, + ...attributes + }); + }; + beforeEach(async function () { knex = await createDatabase(); queries = new Queries(knex); @@ -44,6 +106,81 @@ describe('Email analytics queries', function () { await knex.destroy(); }); + describe('getLastEventTimestamp', function () { + const cursorSeed = { + tableName: 'email_recipients', + eventColumns: { + delivered: 'delivered_at', + opened: 'opened_at', + failed: 'failed_at' + } + } as const; + + it('returns stored finished timestamp without consulting recipient data', async function () { + const finishedAt = '2026-08-11T10:00:00.000Z'; + await insertJob({ + started_at: '2026-08-11T09:00:00.000Z', + finished_at: finishedAt + }); + await insertRecipient({delivered_at: '2026-08-11T12:00:00.000Z'}); + + const result = await queries.getLastEventTimestamp( + 'email-analytics-scheduled', + ['delivered'], + cursorSeed + ); + + assert.equal(result?.toISOString(), finishedAt); + }); + + it('uses latest configured recipient event and creates cursor job', async function () { + await insertRecipient({ + delivered_at: '2026-08-11T10:00:00.000Z', + opened_at: '2026-08-11T11:00:00.000Z', + failed_at: '2026-08-11T12:00:00.000Z' + }); + + const result = await queries.getLastEventTimestamp( + 'email-analytics-latest-others', + ['delivered', 'opened'], + cursorSeed + ); + + assert.equal(result?.toISOString(), '2026-08-11T11:00:00.000Z'); + const job = await knex('jobs').where('name', 'email-analytics-latest-others').first(); + assert.equal(job.status, 'started'); + }); + + it('skips events without a cursor column', async function () { + await insertRecipient({ + delivered_at: '2026-08-11T10:00:00.000Z', + failed_at: '2026-08-11T12:00:00.000Z' + }); + + const result = await queries.getLastEventTimestamp( + 'email-analytics-automation-latest-others', + ['delivered', 'failed'], + { + tableName: 'email_recipients', + eventColumns: {delivered: 'delivered_at'} + } + ); + + assert.equal(result?.toISOString(), '2026-08-11T10:00:00.000Z'); + }); + + it('returns null and creates cursor job when no events exist', async function () { + const result = await queries.getLastEventTimestamp( + 'email-analytics-missing', + [], + cursorSeed + ); + + assert.equal(result, null); + assert(await knex('jobs').where('name', 'email-analytics-missing').first()); + }); + }); + describe('getJobData', function () { it('returns job data when job exists', async function () { const startedAt = '2026-08-11T10:00:00.000Z'; @@ -77,4 +214,322 @@ describe('Email analytics queries', function () { assert.equal(result, null); }); }); + + describe('getLastJobRunTimestamp', function () { + it('prefers finished timestamp over started timestamp', async function () { + await insertJob({ + started_at: '2026-08-11T10:00:00.000Z', + finished_at: '2026-08-11T11:00:00.000Z' + }); + + assert.equal( + await queries.getLastJobRunTimestamp('email-analytics-scheduled'), + '2026-08-11T11:00:00.000Z' + ); + }); + + it('falls back to started timestamp', async function () { + await insertJob({started_at: '2026-08-11T10:00:00.000Z'}); + + assert.equal( + await queries.getLastJobRunTimestamp('email-analytics-scheduled'), + '2026-08-11T10:00:00.000Z' + ); + }); + + it('returns null when job or timestamps do not exist', async function () { + assert.equal(await queries.getLastJobRunTimestamp('email-analytics-missing'), null); + await insertJob(); + assert.equal(await queries.getLastJobRunTimestamp('email-analytics-scheduled'), null); + }); + }); + + describe('setJobTimestamp', function () { + it('sets started timestamp and status on existing job', async function () { + await insertJob(); + const date = new Date('2026-08-11T10:00:00.000Z'); + + await queries.setJobTimestamp('email-analytics-scheduled', 'started', date); + + const job = await knex('jobs').where('name', 'email-analytics-scheduled').first(); + assert.equal(new Date(job.started_at).toISOString(), date.toISOString()); + assert.equal(job.finished_at, null); + assert.equal(job.status, 'started'); + assert(job.updated_at); + }); + + it('sets finished timestamp and status on existing job', async function () { + await insertJob({started_at: '2026-08-11T10:00:00.000Z'}); + const date = new Date('2026-08-11T11:00:00.000Z'); + + await queries.setJobTimestamp('email-analytics-scheduled', 'finished', date); + + const job = await knex('jobs').where('name', 'email-analytics-scheduled').first(); + assert.equal(new Date(job.finished_at).toISOString(), date.toISOString()); + assert.equal(job.status, 'finished'); + }); + + it('swallows database errors', async function () { + await knex.schema.dropTable('jobs'); + + await assert.doesNotReject( + queries.setJobTimestamp('email-analytics-scheduled', 'started', new Date()) + ); + }); + }); + + describe('getJobMetadata', function () { + it('returns parsed metadata', async function () { + await insertJob({metadata: JSON.stringify({begin: 'start', end: 'finish'})}); + + assert.deepEqual(await queries.getJobMetadata('email-analytics-scheduled'), { + begin: 'start', + end: 'finish' + }); + }); + + it('returns null if the job does not exist', async function () { + assert.equal(await queries.getJobMetadata('email-analytics-missing'), null); + }); + + it('returns empty metadata for missing, null, or invalid metadata', async function () { + await insertJob(); + assert.deepEqual(await queries.getJobMetadata('email-analytics-scheduled'), {begin: null, end: null}); + + await knex('jobs').where('name', 'email-analytics-scheduled').update({metadata: 'invalid-json'}); + assert.deepEqual(await queries.getJobMetadata('email-analytics-scheduled'), {begin: null, end: null}); + }); + }); + + describe('setJobMetadata', function () { + it('updates metadata on existing job', async function () { + await insertJob(); + + await queries.setJobMetadata('email-analytics-scheduled', {begin: 'start', end: 'finish'}); + + const job = await knex('jobs').where('name', 'email-analytics-scheduled').first(); + assert.equal(job.metadata, JSON.stringify({begin: 'start', end: 'finish'})); + assert(job.updated_at); + }); + + it('creates queued job when metadata is set for missing job', async function () { + await queries.setJobMetadata('email-analytics-scheduled', {begin: 'start', end: 'finish'}); + + const job = await knex('jobs').where('name', 'email-analytics-scheduled').first(); + assert.equal(job.status, 'queued'); + assert.equal(job.metadata, JSON.stringify({begin: 'start', end: 'finish'})); + assert(job.created_at); + }); + + it('clears existing metadata but does not create missing job for null', async function () { + await insertJob({metadata: JSON.stringify({begin: 'start'})}); + + await queries.setJobMetadata('email-analytics-scheduled', null); + await queries.setJobMetadata('email-analytics-missing', null); + + assert.deepEqual(await queries.getJobMetadata('email-analytics-scheduled'), {begin: null, end: null}); + assert.equal(await knex('jobs').where('name', 'email-analytics-missing').first(), undefined); + }); + }); + + describe('setJobStatus', function () { + it('updates existing job', async function () { + await insertJob(); + + await queries.setJobStatus('email-analytics-scheduled', 'finished'); + + const job = await knex('jobs').where('name', 'email-analytics-scheduled').first(); + assert.equal(job.status, 'finished'); + assert(job.updated_at); + }); + + it('creates missing job', async function () { + await queries.setJobStatus('email-analytics-scheduled', 'failed'); + + const job = await knex('jobs').where('name', 'email-analytics-scheduled').first(); + assert.equal(job.status, 'failed'); + assert(job.created_at); + assert(job.updated_at); + }); + + it('rethrows database errors', async function () { + await knex.schema.dropTable('jobs'); + + await assert.rejects( + queries.setJobStatus('email-analytics-scheduled', 'started'), + /no such table: jobs/ + ); + }); + }); + + describe('aggregateEmailStats', function () { + it('updates delivered, failed, and opened counts', async function () { + await insertEmail('email-1', true); + await insertRecipient({email_id: 'email-1', delivered_at: '2026-08-11T10:00:00.000Z'}); + await insertRecipient({email_id: 'email-1', failed_at: '2026-08-11T10:00:00.000Z'}); + await insertRecipient({email_id: 'email-1', opened_at: '2026-08-11T10:00:00.000Z'}); + await insertRecipient({ + email_id: 'email-1', + delivered_at: '2026-08-11T10:00:00.000Z', + failed_at: '2026-08-11T10:00:00.000Z' + }); + await insertEmail('email-2', true); + await insertRecipient({email_id: 'email-2', delivered_at: '2026-08-11T10:00:00.000Z'}); + + await queries.aggregateEmailStats('email-1', true); + + const email = await knex('emails').where('id', 'email-1').first(); + assert.deepEqual({ + delivered_count: email.delivered_count, + failed_count: email.failed_count, + opened_count: email.opened_count + }, { + delivered_count: 2, + failed_count: 2, + opened_count: 1 + }); + }); + + it('preserves opened count when opened aggregation is disabled', async function () { + await insertEmail('email-1', true); + await knex('emails').where('id', 'email-1').update({opened_count: 7}); + await insertRecipient({email_id: 'email-1', opened_at: '2026-08-11T10:00:00.000Z'}); + + await queries.aggregateEmailStats('email-1', false); + + const email = await knex('emails').where('id', 'email-1').first(); + assert.equal(email.opened_count, 7); + assert.equal(email.delivered_count, 0); + assert.equal(email.failed_count, 0); + }); + + it('does nothing when email does not exist', async function () { + await assert.doesNotReject(queries.aggregateEmailStats('missing-email', true)); + }); + }); + + describe('aggregateMemberStats', function () { + it('updates counts and calculates open rate after five tracked emails', async function () { + await insertMember('member-1'); + await insertEmail('tracked-email', true); + for (let index = 0; index < 5; index += 1) { + await insertRecipient({ + email_id: 'tracked-email', + member_id: 'member-1', + opened_at: index < 2 ? '2026-08-11T10:00:00.000Z' : null + }); + } + + await queries.aggregateMemberStats('member-1'); + + const member = await knex('members').where('id', 'member-1').first(); + assert.equal(member.email_count, 5); + assert.equal(member.email_opened_count, 2); + assert.equal(member.email_open_rate, 40); + }); + + it('counts untracked emails but preserves open rate below threshold', async function () { + await insertMember('member-1', {email_open_rate: 75}); + await insertEmail('tracked-email', true); + await insertEmail('untracked-email', false); + await insertRecipient({ + email_id: 'tracked-email', + member_id: 'member-1', + opened_at: '2026-08-11T10:00:00.000Z' + }); + await insertRecipient({email_id: 'untracked-email', member_id: 'member-1'}); + + await queries.aggregateMemberStats('member-1'); + + const member = await knex('members').where('id', 'member-1').first(); + assert.equal(member.email_count, 2); + assert.equal(member.email_opened_count, 1); + assert.equal(member.email_open_rate, 75); + }); + + it('resets counts for member without recipients', async function () { + await insertMember('member-1', { + email_count: 10, + email_opened_count: 4, + email_open_rate: 40 + }); + + await queries.aggregateMemberStats('member-1'); + + const member = await knex('members').where('id', 'member-1').first(); + assert.equal(member.email_count, 0); + assert.equal(member.email_opened_count, 0); + assert.equal(member.email_open_rate, 40); + }); + + it('does nothing when member does not exist', async function () { + await assert.doesNotReject(queries.aggregateMemberStats('missing-member')); + }); + }); + + describe('aggregateMemberStatsBatch', function () { + it('updates multiple members, including below-threshold and empty members', async function () { + await insertMember('member-1'); + await insertMember('member-2', {email_open_rate: 80}); + await insertMember('member-3', { + email_count: 4, + email_opened_count: 3, + email_open_rate: 75 + }); + await insertMember('untouched-member', { + email_count: 9, + email_opened_count: 8, + email_open_rate: 89 + }); + await insertEmail('tracked-email', true); + + for (let index = 0; index < 5; index += 1) { + await insertRecipient({ + email_id: 'tracked-email', + member_id: 'member-1', + opened_at: index < 2 ? '2026-08-11T10:00:00.000Z' : null + }); + } + await insertRecipient({ + email_id: 'tracked-email', + member_id: 'member-2', + opened_at: '2026-08-11T10:00:00.000Z' + }); + + await queries.aggregateMemberStatsBatch(['member-1', 'member-2', 'member-3']); + + const members = await knex('members').orderBy('id'); + assert.deepEqual(members, [{ + id: 'member-1', + email_count: 5, + email_opened_count: 2, + email_open_rate: 40 + }, { + id: 'member-2', + email_count: 1, + email_opened_count: 1, + email_open_rate: null + }, { + id: 'member-3', + email_count: 0, + email_opened_count: 0, + email_open_rate: null + }, { + id: 'untouched-member', + email_count: 9, + email_opened_count: 8, + email_open_rate: 89 + }]); + }); + + it('returns without querying for empty member list', async function () { + await knex.schema.dropTable('email_recipients'); + + await assert.doesNotReject(queries.aggregateMemberStatsBatch([])); + }); + + it('ignores missing member IDs', async function () { + await assert.doesNotReject(queries.aggregateMemberStatsBatch(['missing-member'])); + }); + }); });