Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,15 @@ jobs:
NX_BASE: ${{ needs.job_setup.outputs.nx_base }}
NX_HEAD: ${{ env.HEAD_COMMIT }}

- name: Check for unexpected file changes
run: |
if [ -n "$(git status --porcelain)" ]; then
echo "Tests generated unexpected file changes. Commit them before merging:"
git status
git diff
exit 1
fi

- uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7
if: matrix.node == env.NODE_VERSION
with:
Expand Down Expand Up @@ -469,6 +478,15 @@ jobs:
- name: Integration tests
run: pnpm nx run ghost:test:ci:integration

- name: Check for unexpected file changes
run: |
if [ -n "$(git status --porcelain)" ]; then
echo "Tests generated unexpected file changes. Commit them before merging:"
git status
git diff
exit 1
fi

- uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7
if: matrix.node == env.NODE_VERSION && contains(matrix.env.DB, 'mysql')
with:
Expand Down Expand Up @@ -554,6 +572,15 @@ jobs:
- name: Legacy tests
run: pnpm nx run ghost:test:ci:legacy

- name: Check for unexpected file changes
run: |
if [ -n "$(git status --porcelain)" ]; then
echo "Tests generated unexpected file changes. Commit them before merging:"
git status
git diff
exit 1
fi

- uses: tryghost/actions/actions/slack-build@0cbdcbeb9030f46b109d5e6e44c14933026d8ca5 # main
if: failure() && github.event_name == 'push' && github.ref == 'refs/heads/main'
with:
Expand Down
30 changes: 30 additions & 0 deletions ghost/core/core/server/api/endpoints/automated-emails.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,36 @@ const controller = {
};
}
},
preview: {
headers: {
cacheInvalidate: false
},
options: [
'id'
],
data: [
'subject',
'lexical'
],
validation: {
options: {
id: {
required: true
}
}
},
permissions: {
method: 'edit'
},
async query(frame) {
memberWelcomeEmailService.init();
return await memberWelcomeEmailService.api.previewEmail({
subject: frame.data.subject,
lexical: frame.data.lexical,
automatedEmailId: frame.options.id
});
}
},
sendTestEmail: {
statusCode: 204,
headers: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,34 @@ const validateOptionalStringField = (value, errorMessage) => {
}
};

const validatePreviewData = (frame) => {
const subject = frame.data.subject;
const lexical = frame.data.lexical;

if (typeof subject !== 'string' || !subject.trim()) {
throw new ValidationError({
message: tpl(messages.subjectRequired),
property: 'subject'
});
}

if (typeof lexical !== 'string' || !lexical.trim()) {
throw new ValidationError({
message: tpl(messages.lexicalRequired),
property: 'lexical'
});
}

try {
JSON.parse(lexical);
} catch (e) {
throw new ValidationError({
message: tpl(messages.invalidLexical),
property: 'lexical'
});
}
};

module.exports = {
async add(apiConfig, frame) {
await validateAutomatedEmail(frame);
Expand All @@ -93,35 +121,18 @@ module.exports = {
});
}
},
preview(apiConfig, frame) {
validatePreviewData(frame);
},
sendTestEmail(apiConfig, frame) {
const email = frame.data.email;
const subject = frame.data.subject;
const lexical = frame.data.lexical;

if (typeof email !== 'string' || !validator.isEmail(email)) {
throw new ValidationError({
message: tpl(messages.invalidEmailReceived)
});
}

if (typeof subject !== 'string' || !subject.trim()) {
throw new ValidationError({
message: tpl(messages.subjectRequired)
});
}

if (typeof lexical !== 'string' || !lexical.trim()) {
throw new ValidationError({
message: tpl(messages.lexicalRequired)
});
}

try {
JSON.parse(lexical);
} catch (e) {
throw new ValidationError({
message: tpl(messages.invalidLexical)
});
}
validatePreviewData(frame);
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class EmailAnalyticsServiceWrapper {
* @param {number} totalDurationMs - Total duration in milliseconds
*/
_logJobCompletion(jobType, fetchResult, totalDurationMs) {
const {eventCount, apiPollingTimeMs, processingTimeMs, aggregationTimeMs, result} = fetchResult;
const {eventCount, apiPollingTimeMs, processingTimeMs, aggregationTimeMs, emailAggregationTimeMs, memberAggregationTimeMs, result} = fetchResult;

if (eventCount === 0) {
return;
Expand All @@ -91,7 +91,7 @@ class EmailAnalyticsServiceWrapper {
`[EmailAnalytics] Job complete: ${jobType}`,
`${eventCount} events in ${(totalDurationMs / 1000).toFixed(1)}s (${throughput.toFixed(2)} events/s)`,
`Mode: ${batchMode}`,
`Timings: API ${(apiPollingTimeMs / 1000).toFixed(1)}s (${apiPercent}%) / Processing ${(processingTimeMs / 1000).toFixed(1)}s (${processingPercent}%) / Aggregation ${(aggregationTimeMs / 1000).toFixed(1)}s (${aggregationPercent}%)`,
`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}`
].join(' | ');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ const errors = require('@tryghost/errors');
* @property {number} apiPollingTimeMs - Time spent polling the API in milliseconds
* @property {number} processingTimeMs - Time spent processing events in milliseconds
* @property {number} aggregationTimeMs - Time spent aggregating stats in milliseconds
* @property {number} emailAggregationTimeMs - Time spent aggregating email stats in milliseconds
* @property {number} memberAggregationTimeMs - Time spent aggregating member stats in milliseconds
* @property {EventProcessingResult} result - The processing result with event breakdown
*/

Expand All @@ -46,6 +48,8 @@ function createEmptyResult() {
apiPollingTimeMs: 0,
processingTimeMs: 0,
aggregationTimeMs: 0,
emailAggregationTimeMs: 0,
memberAggregationTimeMs: 0,
result: new EventProcessingResult()
};
}
Expand Down Expand Up @@ -323,6 +327,8 @@ module.exports = class EmailAnalyticsService {
const fetchStartMs = Date.now();
let processingTimeMs = 0;
let aggregationTimeMs = 0;
let emailAggregationTimeMs = 0;
let memberAggregationTimeMs = 0;

let lastAggregation = Date.now();
let eventCount = 0;
Expand Down Expand Up @@ -387,8 +393,10 @@ module.exports = class EmailAnalyticsService {
// We do this here because otherwise it could take a long time before the new events are visible in the stats
try {
const intermediateAggregationStart = Date.now();
await this.aggregateStats(processingResult, includeOpenedEvents);
const aggregationTimings = await this.aggregateStats(processingResult, includeOpenedEvents);
aggregationTimeMs += (Date.now() - intermediateAggregationStart);
emailAggregationTimeMs += aggregationTimings.emailAggregationTimeMs;
memberAggregationTimeMs += aggregationTimings.memberAggregationTimeMs;
lastAggregation = Date.now();
// Remove aggregated emailIds and memberIds from tracking sets to avoid re-aggregating at the end
processingResult.emailIds.forEach(id => allEmailIds.delete(id));
Expand Down Expand Up @@ -434,8 +442,10 @@ module.exports = class EmailAnalyticsService {
emailIds: finalEmailIds,
memberIds: finalMemberIds
};
await this.aggregateStats(finalAggregationResult, includeOpenedEvents);
const aggregationTimings = await this.aggregateStats(finalAggregationResult, includeOpenedEvents);
aggregationTimeMs += (Date.now() - aggregationStart);
emailAggregationTimeMs += aggregationTimings.emailAggregationTimeMs;
memberAggregationTimeMs += aggregationTimings.memberAggregationTimeMs;
} catch (err) {
logging.error('[EmailAnalytics] Error while aggregating stats');
logging.error(err);
Expand Down Expand Up @@ -476,6 +486,8 @@ module.exports = class EmailAnalyticsService {
apiPollingTimeMs,
processingTimeMs,
aggregationTimeMs,
emailAggregationTimeMs,
memberAggregationTimeMs,
result: cumulativeResult
};
}
Expand Down Expand Up @@ -625,17 +637,21 @@ module.exports = class EmailAnalyticsService {
/**
* @param {{emailIds?: string[], memberIds?: string[]}} stats
* @param {boolean} includeOpenedEvents
* @returns {Promise<{emailAggregationTimeMs: number, memberAggregationTimeMs: number}>}
*/
async aggregateStats({emailIds = [], memberIds = []}, includeOpenedEvents = true) {
const useBatchProcessing = this.config.get('emailAnalytics:batchProcessing');

const emailAggregationStart = Date.now();
for (const emailId of emailIds) {
await this.aggregateEmailStats(emailId, includeOpenedEvents);
}
const emailAggregationTimeMs = Date.now() - emailAggregationStart;

// @ts-expect-error
const memberMetric = this.prometheusClient?.getMetric('email_analytics_aggregate_member_stats_count');

const memberAggregationStart = Date.now();
if (useBatchProcessing) {
// Batched mode: process 100 members at a time
logging.info(`[EmailAnalytics] Aggregating stats for ${memberIds.length} members using BATCHED mode (batch size: 100)`);
Expand All @@ -653,6 +669,9 @@ module.exports = class EmailAnalyticsService {
memberMetric?.inc();
}
}
const memberAggregationTimeMs = Date.now() - memberAggregationStart;

return {emailAggregationTimeMs, memberAggregationTimeMs};
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@
<td class="{{classes.container}}">
<div class="content">
<table role="presentation" border="0" cellpadding="0" cellspacing="0" class="main" width="100%">
{{> @partial-block}}
{{~> @partial-block~}}
</table>
<!-- END CENTERED WHITE CONTAINER -->
</div>
Expand Down
38 changes: 33 additions & 5 deletions ghost/core/core/server/services/member-welcome-emails/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ class MemberWelcomeEmailService {
return Boolean(email && email.get('lexical') && row.get('status') === 'active');
}

async sendTestEmail({email, subject, lexical, automatedEmailId}) {
async #renderWelcomeEmailPreview({automatedEmailId, subject, lexical, memberEmail = 'jamie@example.com'}) {
// Still validate the automated email exists (for permission purposes)
const automation = await WelcomeEmailAutomation.findOne({id: automatedEmailId}, {
withRelated: this.#useDesignCustomization()
Expand All @@ -457,34 +457,62 @@ class MemberWelcomeEmailService {
});
}

if (!lexical) {
if (typeof lexical !== 'string' || !lexical.trim()) {
throw new errors.ValidationError({
message: MESSAGES.MISSING_EMAIL_CONTENT
});
}

if (!subject) {
if (typeof subject !== 'string' || !subject.trim()) {
throw new errors.ValidationError({
message: MESSAGES.MISSING_EMAIL_SUBJECT
});
}

const testMember = {
name: 'Jamie Larson',
email: email,
email: memberEmail,
uuid: '00000000-0000-4000-8000-000000000000'
};

const designSettings = this.#useDesignCustomization() ? automatedEmail.related('emailDesignSetting') : null;

const {html, text, subject: renderedSubject} = await this.#renderer.render({
const preview = await this.#renderer.render({
lexical,
subject,
designSettings: designSettings?.id ? designSettings.toJSON() : null,
member: testMember,
siteSettings: this.#getSiteSettings()
});

return {
...preview,
automatedEmail
};
}

async previewEmail({subject, lexical, automatedEmailId}) {
const {html, text, subject: renderedSubject} = await this.#renderWelcomeEmailPreview({
automatedEmailId,
subject,
lexical
});

return {
html,
plaintext: text,
subject: renderedSubject
};
}

async sendTestEmail({email, subject, lexical, automatedEmailId}) {
const {html, text, subject: renderedSubject, automatedEmail} = await this.#renderWelcomeEmailPreview({
automatedEmailId,
subject,
lexical,
memberEmail: email
});

// Test sends should always reflect latest newsletter fallback values.
this.#defaultNewsletterSenderOptions = await this.#getDefaultNewsletterSenderOptions();
const senderOptions = await this.#getEffectiveSenderOptions({
Expand Down
1 change: 1 addition & 0 deletions ghost/core/core/server/web/api/endpoints/admin/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ module.exports = function apiRoutes() {
router.post('/automated_emails', mw.authAdminApi, http(api.automatedEmails.add));
router.put('/automated_emails/design', mw.authAdminApi, http(api.automatedEmailDesign.edit));
router.put('/automated_emails/:id', mw.authAdminApi, http(api.automatedEmails.edit));
router.post('/automated_emails/:id/preview', mw.authAdminApi, http(api.automatedEmails.preview));
router.post('/automated_emails/:id/test', shared.middleware.brute.previewEmailLimiter, mw.authAdminApi, http(api.automatedEmails.sendTestEmail));

// ## Roles
Expand Down
Loading