🐛 Fixed Analytics > Growth tab MRR chart lookback#24953
Conversation
WalkthroughThe change threads a date_from parameter from the frontend hook through the API endpoint to the stats services. The React hook now requests MRR history constrained by memberDataStartDate. The mrr API endpoint accepts date_from, includes it in cache key generation, and passes it to statsService.api.getMRRHistory. StatsService forwards options to MrrStatsService.getHistory. MrrStatsService methods now accept options/dateFrom to constrain fetched deltas, defaulting to 90 days otherwise. Unit tests are overhauled to be time-deterministic, add helpers, switch created_at to datetime, and cover multiple scenarios including custom dateFrom filtering. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
When testing this, the data generator works great. Just run |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ghost/core/core/server/services/stats/MrrStatsService.js (1)
76-82: Use UTC for “today” to avoid TZ drift in future-event filtering.Local time can shift “today” versus stored UTC dates.
Apply:
- const today = moment().format('YYYY-MM-DD'); + const today = moment.utc().format('YYYY-MM-DD');
🧹 Nitpick comments (5)
ghost/core/core/server/services/stats/StatsService.js (1)
26-28: Wiring options through to MRR is correct. Add JSDoc for clarity.Document the accepted options (dateFrom) like other methods to prevent misuse.
Apply:
} - async getMRRHistory(options = {}) { + /** + * @param {Object} [options] + * @param {string} [options.dateFrom] - Start date in YYYY-MM-DD (UTC) + */ + async getMRRHistory(options = {}) { return this.mrr.getHistory(options); }ghost/core/core/server/services/stats/MrrStatsService.js (2)
51-53: Filter future rows in SQL to reduce payload and work.No reason to fetch events beyond end-of-day UTC if we drop them later.
Apply:
- .where('created_at', '>=', startDate) + .whereBetween('created_at', [ + startDate, + moment.utc().endOf('day').format('YYYY-MM-DD HH:mm:ss') + ])
35-39: Fix JSDoc inconsistencies and typos.Doc says “old→new” and “new→old” simultaneously; also typo “respons”.
Apply:
- /** - * Get the MRR deltas for all days (from old to new), grouped by currency (ascending alphabetically) - * @param {string} [dateFrom] - Start date to fetch deltas from - * @returns {Promise<MrrDelta[]>} The deltas sorted from new to old - */ + /** + * Get MRR deltas since start date, grouped by currency. + * @param {string} [dateFrom] - Start date (UTC, YYYY-MM-DD) + * @returns {Promise<MrrDelta[]>} Unsorted raw deltas + */ ... - * The respons is in ascending date order, and currencies for the same date are always in ascending order. + * The response is in ascending date order, and currencies for the same date are in ascending order. - * @param {Object} [options] - * @param {string} [options.dateFrom] - Start date to fetch history from + * @param {Object} [options] + * @param {string} [options.dateFrom] - Start date (UTC, YYYY-MM-DD)Also applies to: 56-62
ghost/core/test/unit/server/services/stats/mrr.test.js (2)
111-121: Use numeric types for MRR fields in test schema.Avoids engine-dependent coercion in SUM().
Apply:
- await db.schema.createTable('members_paid_subscription_events', function (table) { + await db.schema.createTable('members_paid_subscription_events', function (table) { table.string('currency'); - table.string('mrr_delta'); + table.integer('mrr_delta'); table.datetime('created_at'); }); - await db.schema.createTable('members_stripe_customers_subscriptions', function (table) { + await db.schema.createTable('members_stripe_customers_subscriptions', function (table) { table.string('plan_currency'); - table.string('mrr'); + table.integer('mrr'); });
316-339: Great: explicit dateFrom includes earlier events. Add one test for invalid dateFrom.Cover invalid input to ensure service falls back to default window (after the proposed validation).
Proposed test:
+ it('Falls back when dateFrom is invalid', async function () { + await addMrrEvents([ + {date: yesterday, delta: 5}, + {date: today, delta: 5} + ]); + await addCurrentSubscriptions([{mrr: 10}]); + const history = await mrrStatsService.getHistory({dateFrom: 'not-a-date'}); + history.data[history.data.length - 1].should.eql({date: todayDate, mrr: 10, currency: 'usd'}); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/stats/src/hooks/useGrowthStats.ts(1 hunks)ghost/core/core/server/api/endpoints/stats.js(1 hunks)ghost/core/core/server/services/stats/MrrStatsService.js(1 hunks)ghost/core/core/server/services/stats/StatsService.js(1 hunks)ghost/core/test/unit/server/services/stats/mrr.test.js(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
ghost/core/core/server/services/stats/MrrStatsService.js (1)
ghost/core/core/server/services/stats/utils/date-utils.js (1)
dateFrom(13-13)
apps/stats/src/hooks/useGrowthStats.ts (1)
apps/admin-x-framework/src/api/stats.ts (1)
useMrrHistory(234-237)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Legacy tests (Node 22.13.1, sqlite3)
- GitHub Check: Acceptance tests (Node 22.13.1, sqlite3)
- GitHub Check: Legacy tests (Node 22.13.1, mysql8)
- GitHub Check: Acceptance tests (Node 22.13.1, mysql8)
- GitHub Check: Unit tests (Node 22.13.1)
- GitHub Check: Ghost-CLI tests
- GitHub Check: Lint
- GitHub Check: Cursor Bugbot
- GitHub Check: Build & Push
🔇 Additional comments (6)
apps/stats/src/hooks/useGrowthStats.ts (1)
207-211: LGTM: MRR history now respects the same lookback as members.Passing date_from keeps charts aligned.
ghost/core/core/server/api/endpoints/stats.js (3)
38-41: Expose date_from in MRR endpoint options.Good addition; matches memberCountHistory.
42-46: Include options in the cache key.Prevents collisions across different lookbacks.
48-51: Forward the mapped option as dateFrom.Correctly threads to service API.
ghost/core/test/unit/server/services/stats/mrr.test.js (2)
15-23: Deterministic “today” via fake timers is solid.Keeps tests stable across environments.
290-315: Good coverage for default 90‑day window behavior.Asserts the synthetic boundary and totals correctly.
| const startDate = dateFrom | ||
| ? moment.utc(dateFrom).startOf('day').utc().format('YYYY-MM-DD HH:mm:ss') | ||
| : moment.utc().subtract(90, 'days').startOf('day').utc().format('YYYY-MM-DD HH:mm:ss'); | ||
| const rows = await knex('members_paid_subscription_events') |
There was a problem hiding this comment.
Validate and normalize dateFrom; fall back if invalid.
An invalid dateFrom produces 'Invalid date' and an unexpected WHERE bound.
Apply:
- const startDate = dateFrom
- ? moment.utc(dateFrom).startOf('day').utc().format('YYYY-MM-DD HH:mm:ss')
- : moment.utc().subtract(90, 'days').startOf('day').utc().format('YYYY-MM-DD HH:mm:ss');
+ const parsed = dateFrom ? moment.utc(dateFrom, ['YYYY-MM-DD', moment.ISO_8601], true) : null;
+ const startDate = parsed?.isValid()
+ ? parsed.startOf('day').format('YYYY-MM-DD HH:mm:ss')
+ : moment.utc().subtract(90, 'days').startOf('day').format('YYYY-MM-DD HH:mm:ss');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const startDate = dateFrom | |
| ? moment.utc(dateFrom).startOf('day').utc().format('YYYY-MM-DD HH:mm:ss') | |
| : moment.utc().subtract(90, 'days').startOf('day').utc().format('YYYY-MM-DD HH:mm:ss'); | |
| const rows = await knex('members_paid_subscription_events') | |
| const parsed = dateFrom ? moment.utc(dateFrom, ['YYYY-MM-DD', moment.ISO_8601], true) : null; | |
| const startDate = parsed?.isValid() | |
| ? parsed.startOf('day').format('YYYY-MM-DD HH:mm:ss') | |
| : moment.utc().subtract(90, 'days').startOf('day').format('YYYY-MM-DD HH:mm:ss'); | |
| const rows = await knex('members_paid_subscription_events') |
🤖 Prompt for AI Agents
In ghost/core/core/server/services/stats/MrrStatsService.js around lines 42 to
45, the current ternary uses moment.utc(dateFrom) without validating it so an
invalid dateFrom yields "Invalid date" and bad query bindings; change this to
explicitly parse and validate dateFrom (e.g. const parsed =
moment.utc(dateFrom); if (dateFrom && parsed.isValid()) use
parsed.startOf('day').utc().format('YYYY-MM-DD HH:mm:ss') else fall back to
moment.utc().subtract(90, 'days').startOf('day').utc().format(...)); ensure you
handle non-string/undefined inputs, use the validated/normalized string in the
WHERE binding, and remove the original ternary.
ErisDS
added
core team
cmraible
left a comment
cmraible
left a comment
There was a problem hiding this comment.
Nice work! Works for me locally, looking good 👌
3 participants
ref https://linear.app/ghost/issue/ONC-1157/
/stats/mrrendpointWhen wiring this up, it appears we missed adding the lookback support to this endpoint. It was adapted from the previous Dashboard which always (and could only) look back 90d, so that's all the more we were returning. The endpoint now correctly looks back over the selected timeframe.