Skip to content

Commit b71a99b

Browse files
authored
Fixed stats endpoints inconsistently handling timezone (#24574)
ref https://linear.app/ghost/issue/PROD-2373/ - more declarative about passing timezone with date_from and date_to in stats endpoint options - centralized date calculations and date filter application to knex query objects; this allows us to cleanly handle timezone adjustments if someone is on the day before/after UTC's current date __The Problem__ We do not consistently handle date translations w/r to timezones. We pass the `date_from`, `date_to`, and `timezone` options to Tinybird, which does handle timezone translations, and passes the data back in a localized format. We __do not__ currently do that with Ghost data. __Solution__ For the moment, we'll centralize utils for translating `date_from` and `date_to` to UTC dates based on the provided `timezone`, and clean up the endpoints by using a centralized filter, ensuring we're applying the date filtering in the same way everywhere. Ideally, we'll set standards around endpoints always being in UTC, and letting the caller handle the conversion, which feels more appropriate (the client knows its timezone, why does the server need to know it?).
1 parent 07ad96e commit b71a99b

File tree

7 files changed

+175
-174
lines changed

7 files changed

+175
-174
lines changed

ghost/core/core/server/api/endpoints/stats.js

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -533,12 +533,7 @@ const controller = {
533533
};
534534
},
535535
async query(frame) {
536-
return await statsService.api.getTopSourcesWithRange(
537-
frame.options.date_from,
538-
frame.options.date_to,
539-
frame.options.order || 'free_members desc',
540-
frame.options.limit || 50
541-
);
536+
return await statsService.api.getTopSourcesWithRange(frame.options);
542537
}
543538
}
544539

ghost/core/core/server/services/stats/PostsStatsService.js

Lines changed: 48 additions & 105 deletions
Large diffs are not rendered by default.

ghost/core/core/server/services/stats/ReferrersStatsService.js

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
const moment = require('moment');
22

3+
// Import centralized date utilities
4+
const {getDateBoundaries, applyDateFilter} = require('./utils/date-utils');
5+
36
// Source normalization mapping - consolidated from frontend apps
47
const SOURCE_NORMALIZATION_MAP = new Map([
58
// Social Media Consolidation
@@ -252,45 +255,44 @@ class ReferrersStatsService {
252255

253256
/**
254257
* Fetch MRR sources with date range
255-
* @param {string} startDate
256-
* @param {string} endDate
258+
* @param {Object} options
257259
* @returns {Promise<MrrCountStatDate[]>}
258260
**/
259-
async fetchMrrSourcesWithRange(startDate, endDate) {
261+
async fetchMrrSourcesWithRange(options) {
260262
const knex = this.knex;
261-
const startDateTime = moment.utc(startDate).startOf('day').format('YYYY-MM-DD HH:mm:ss');
262-
const endDateTime = moment.utc(endDate).endOf('day').format('YYYY-MM-DD HH:mm:ss');
263+
const {dateFrom: startDateTime, dateTo: endDateTime} = getDateBoundaries(options);
263264

264265
// Join subscription created events with paid subscription events to get MRR changes
265-
const rows = await knex('members_subscription_created_events as msce')
266+
let query = knex('members_subscription_created_events as msce')
266267
.join('members_paid_subscription_events as mpse', function () {
267268
this.on('msce.member_id', '=', 'mpse.member_id')
268269
.andOn('msce.subscription_id', '=', 'mpse.subscription_id');
269270
})
270271
.select(knex.raw(`DATE(msce.created_at) as date`))
271272
.select(knex.raw(`SUM(mpse.mrr_delta) as mrr`))
272273
.select(knex.raw(`msce.referrer_source as source`))
273-
.where('msce.created_at', '>=', startDateTime)
274-
.where('msce.created_at', '<=', endDateTime)
275274
.where('mpse.mrr_delta', '>', 0) // Only positive MRR changes (new subscriptions)
276275
.whereNotNull('msce.referrer_source') // Only entries with attribution
277276
.groupBy('date', 'msce.referrer_source')
278277
.orderBy('date');
278+
279+
// Apply centralized date filtering
280+
applyDateFilter(query, startDateTime, endDateTime, 'msce.created_at');
281+
282+
const rows = await query;
279283

280284
return rows;
281285
}
282286

283287
/**
284288
* Fetch deduplicated member counts by source with date range
285289
* Returns both free signups (excluding those who converted) and paid conversions
286-
* @param {string} startDate
287-
* @param {string} endDate
290+
* @param {Object} options
288291
* @returns {Promise<{source: string, signups: number, paid_conversions: number}[]>}
289292
**/
290-
async fetchMemberCountsBySource(startDate, endDate) {
293+
async fetchMemberCountsBySource(options) {
291294
const knex = this.knex;
292-
const startDateTime = moment.utc(startDate).startOf('day').format('YYYY-MM-DD HH:mm:ss');
293-
const endDateTime = moment.utc(endDate).endOf('day').format('YYYY-MM-DD HH:mm:ss');
295+
const {dateFrom: startDateTime, dateTo: endDateTime} = getDateBoundaries(options);
294296

295297
// Query 1: Free members who haven't converted to paid within the same time window
296298
const freeSignupsQuery = knex('members_created_events as mce')
@@ -302,18 +304,20 @@ class ReferrersStatsService {
302304
.andOn('msce.created_at', '>=', knex.raw('?', [startDateTime]))
303305
.andOn('msce.created_at', '<=', knex.raw('?', [endDateTime]));
304306
})
305-
.where('mce.created_at', '>=', startDateTime)
306-
.where('mce.created_at', '<=', endDateTime)
307307
.whereNull('msce.id')
308308
.groupBy('mce.referrer_source');
309+
310+
// Apply date filtering to the main query
311+
applyDateFilter(freeSignupsQuery, startDateTime, endDateTime, 'mce.created_at');
309312

310313
// Query 2: Paid conversions
311314
const paidConversionsQuery = knex('members_subscription_created_events as msce')
312315
.select('msce.referrer_source as source')
313316
.select(knex.raw('COUNT(DISTINCT msce.member_id) as paid_conversions'))
314-
.where('msce.created_at', '>=', startDateTime)
315-
.where('msce.created_at', '<=', endDateTime)
316317
.groupBy('msce.referrer_source');
318+
319+
// Apply date filtering to the paid conversions query
320+
applyDateFilter(paidConversionsQuery, startDateTime, endDateTime, 'msce.created_at');
317321

318322
// Execute both queries in parallel
319323
const [freeResults, paidResults] = await Promise.all([
@@ -353,17 +357,21 @@ class ReferrersStatsService {
353357
/**
354358
* Return aggregated attribution sources for a date range, grouped by source only (not by date)
355359
* This is used for "Top Sources" tables that need server-side sorting
356-
* @param {string} startDate - Start date in YYYY-MM-DD format
357-
* @param {string} endDate - End date in YYYY-MM-DD format
358-
* @param {string} [orderBy='signups desc'] - Sort order: 'signups desc', 'paid_conversions desc', 'mrr desc', 'source desc'
359-
* @param {number} [limit=50] - Maximum number of sources to return
360+
* @param {Object} options
361+
* @param {string} [options.date_from] - Start date in YYYY-MM-DD format
362+
* @param {string} [options.date_to] - End date in YYYY-MM-DD format
363+
* @param {string} [options.timezone] - Timezone to use for date interpretation
364+
* @param {string} [options.orderBy='signups desc'] - Sort order: 'signups desc', 'paid_conversions desc', 'mrr desc', 'source desc'
365+
* @param {number} [options.limit=50] - Maximum number of sources to return
360366
* @returns {Promise<{data: AttributionCountStatWithMrr[], meta: {}}>}
361367
*/
362-
async getTopSourcesWithRange(startDate, endDate, orderBy = 'signups desc', limit = 50) {
368+
async getTopSourcesWithRange(options = {}) {
369+
const {orderBy = 'signups desc', limit = 50} = options;
370+
363371
// Get deduplicated member counts and MRR data in parallel
364372
const [memberCounts, mrrEntries] = await Promise.all([
365-
this.fetchMemberCountsBySource(startDate, endDate),
366-
this.fetchMrrSourcesWithRange(startDate, endDate)
373+
this.fetchMemberCountsBySource(options),
374+
this.fetchMrrSourcesWithRange(options)
367375
]);
368376

369377
// Aggregate by source (not by date + source)

ghost/core/core/server/services/stats/StatsService.js

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ class StatsService {
123123
* @param {number} [options.limit=20] - Max number of results to return
124124
* @param {string} [options.date_from] - Start date filter in YYYY-MM-DD format
125125
* @param {string} [options.date_to] - End date filter in YYYY-MM-DD format
126+
* @param {string} [options.timezone] - Timezone to use for date interpretation
126127
* @returns {Promise<{data: import('./PostsStatsService').NewsletterStatResult[]}>}
127128
*/
128129
async getNewsletterStats(options = {}) {
@@ -146,6 +147,7 @@ class StatsService {
146147
* @param {string} [options.newsletter_id] - ID of the specific newsletter to get stats for
147148
* @param {string} [options.date_from] - Start date filter in YYYY-MM-DD format
148149
* @param {string} [options.date_to] - End date filter in YYYY-MM-DD format
150+
* @param {string} [options.timezone] - Timezone to use for date interpretation
149151
* @returns {Promise<{data: import('./PostsStatsService').NewsletterSubscriberStats[]}>}
150152
*/
151153
async getNewsletterSubscriberStats(options = {}) {
@@ -190,6 +192,7 @@ class StatsService {
190192
* @param {string} [options.newsletter_id] - ID of the specific newsletter to get stats for
191193
* @param {string} [options.order='published_at desc'] - Order field and direction
192194
* @param {number} [options.limit=20] - Max number of results to return
195+
* @param {string} [options.timezone] - Timezone to use for date interpretation
193196
* @param {string} [options.date_from] - Start date filter in YYYY-MM-DD format
194197
* @param {string} [options.date_to] - End date filter in YYYY-MM-DD format
195198
* @returns {Promise<{data: import('./PostsStatsService').NewsletterStatResult[]}>}
@@ -229,8 +232,16 @@ class StatsService {
229232
return result;
230233
}
231234

232-
async getTopSourcesWithRange(startDate, endDate, orderBy, limit) {
233-
return this.referrers.getTopSourcesWithRange(startDate, endDate, orderBy, limit);
235+
/**
236+
* @param {Object} options
237+
* @param {string} [options.date_from] - Start date in YYYY-MM-DD format
238+
* @param {string} [options.date_to] - End date in YYYY-MM-DD format
239+
* @param {string} [options.timezone] - Timezone to use for date interpretation
240+
* @param {string} [options.orderBy='signups desc'] - Sort order: 'signups desc', 'paid_conversions desc', 'mrr desc', 'source desc'
241+
* @param {number} [options.limit=50] - Maximum number of sources to return
242+
*/
243+
async getTopSourcesWithRange(options = {}) {
244+
return this.referrers.getTopSourcesWithRange(options);
234245
}
235246

236247
/**
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
const moment = require('moment-timezone');
2+
3+
/**
4+
* Get processed date boundaries with timezone support
5+
* @param {Object} options - Options containing date and timezone info
6+
* @param {string} [options.date_from] - Start date in YYYY-MM-DD format
7+
* @param {string} [options.date_to] - End date in YYYY-MM-DD format
8+
* @param {string} [options.timezone='UTC'] - Timezone for date interpretation
9+
* @returns {{dateFrom: string|null, dateTo: string|null}} Processed dates in ISO format
10+
*/
11+
function getDateBoundaries(options) {
12+
const timezone = options.timezone || 'UTC';
13+
const dateFrom = options.date_from ? moment.tz(options.date_from, timezone).startOf('day').utc().toISOString() : null;
14+
const dateTo = options.date_to ? moment.tz(options.date_to, timezone).endOf('day').utc().toISOString() : null;
15+
return {dateFrom, dateTo};
16+
}
17+
18+
/**
19+
* Apply date filters to a query builder instance
20+
* @param {import('knex').Knex.QueryBuilder} query - The query builder to apply filters to
21+
* @param {string|null} dateFrom - Start date in ISO format
22+
* @param {string|null} dateTo - End date in ISO format
23+
* @param {string} dateColumn - The date column to filter on
24+
*/
25+
function applyDateFilter(query, dateFrom, dateTo, dateColumn) {
26+
if (dateFrom) {
27+
query.where(dateColumn, '>=', dateFrom);
28+
}
29+
if (dateTo) {
30+
query.where(dateColumn, '<=', dateTo);
31+
}
32+
}
33+
34+
module.exports = {
35+
getDateBoundaries,
36+
applyDateFilter
37+
};

ghost/core/test/unit/server/services/stats/posts.test.js

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
const knex = require('knex').default;
22
const assert = require('assert/strict');
3+
const moment = require('moment-timezone');
34
const PostsStatsService = require('../../../../../core/server/services/stats/PostsStatsService');
45

56
/**
@@ -81,7 +82,7 @@ describe('PostsStatsService', function () {
8182
attribution_url: `/${postId.replace('post', 'post-')}/`,
8283
referrer_source: referrerSource,
8384
referrer_url: referrerSource ? `https://${referrerSource}` : null,
84-
created_at: createdAt,
85+
created_at: moment(createdAt).utc().format('YYYY-MM-DD HH:mm:ss'),
8586
source: 'member'
8687
});
8788
}
@@ -100,15 +101,15 @@ describe('PostsStatsService', function () {
100101
attribution_url: `/${postId.replace('post', 'post-')}/`,
101102
referrer_source: referrerSource,
102103
referrer_url: referrerSource ? `https://${referrerSource}` : null,
103-
created_at: createdAt
104+
created_at: moment(createdAt).utc().format('YYYY-MM-DD HH:mm:ss')
104105
});
105106

106107
await db('members_paid_subscription_events').insert({
107108
id: paidEventId,
108109
member_id: memberId,
109110
subscription_id: subscriptionId,
110111
mrr_delta: mrr,
111-
created_at: createdAt
112+
created_at: moment(createdAt).utc().format('YYYY-MM-DD HH:mm:ss')
112113
});
113114
}
114115

@@ -155,7 +156,7 @@ describe('PostsStatsService', function () {
155156
id: clickEventId,
156157
member_id: memberId,
157158
redirect_id: redirectId,
158-
created_at: createdAt
159+
created_at: moment(createdAt).utc().format('YYYY-MM-DD HH:mm:ss')
159160
});
160161
}
161162

@@ -417,8 +418,9 @@ describe('PostsStatsService', function () {
417418
await _createFreeSignupEvent('post2', 'member_future', 'referrer_future', thirtyDaysAgo);
418419

419420
const lastFifteenDaysResult = await service.getTopPosts({
420-
date_from: fifteenDaysAgo,
421-
date_to: new Date()
421+
date_from: moment(fifteenDaysAgo).format('YYYY-MM-DD'),
422+
date_to: moment().format('YYYY-MM-DD'),
423+
timezone: 'UTC'
422424
});
423425

424426
// Only post1 should be returned since post2 has no events in the date range
@@ -428,8 +430,9 @@ describe('PostsStatsService', function () {
428430

429431
// Test filtering to include both dates
430432
const lastThirtyDaysResult = await service.getTopPosts({
431-
date_from: sixtyDaysAgo,
432-
date_to: new Date()
433+
date_from: moment(sixtyDaysAgo).format('YYYY-MM-DD'),
434+
date_to: moment().format('YYYY-MM-DD'),
435+
timezone: 'UTC'
433436
});
434437

435438
// Both posts should be returned
@@ -585,8 +588,9 @@ describe('PostsStatsService', function () {
585588
await _createFreeSignupEvent('post1', 'member_future', 'referrer_future', thirtyDaysAgo);
586589

587590
const lastFifteenDaysResult = await service.getReferrersForPost('post1', {
588-
date_from: fifteenDaysAgo,
589-
date_to: new Date()
591+
date_from: moment(fifteenDaysAgo).format('YYYY-MM-DD'),
592+
date_to: moment().format('YYYY-MM-DD'),
593+
timezone: 'UTC'
590594
});
591595

592596
// Make sure we have the result for referrer_past
@@ -596,8 +600,9 @@ describe('PostsStatsService', function () {
596600

597601
// Test filtering to include both dates
598602
const lastThirtyDaysResult = await service.getReferrersForPost('post1', {
599-
date_from: sixtyDaysAgo,
600-
date_to: new Date()
603+
date_from: moment(sixtyDaysAgo).format('YYYY-MM-DD'),
604+
date_to: moment().format('YYYY-MM-DD'),
605+
timezone: 'UTC'
601606
});
602607

603608
// Make sure we have results for both referrers

0 commit comments

Comments
 (0)