diff --git a/core/server/services/stats/lib/members-stats-service.js b/core/server/services/stats/lib/members-stats-service.js index 0f3c51ad74..747a565624 100644 --- a/core/server/services/stats/lib/members-stats-service.js +++ b/core/server/services/stats/lib/members-stats-service.js @@ -28,8 +28,8 @@ class MembersStatsService { } /** - * Get the member deltas by status for all days (from new to old) - * @returns {Promise} The deltas of paid, free and comped users per day, sorted from new to old + * Get the member deltas by status for all days, sorted ascending + * @returns {Promise} The deltas of paid, free and comped users per day, sorted ascending */ async fetchAllStatusDeltas() { const knex = this.db.knex; @@ -54,7 +54,7 @@ class MembersStatsService { ELSE 0 END ) as free_delta`)) .groupByRaw('DATE(created_at)') - .orderByRaw('DATE(created_at) DESC'); + .orderByRaw('DATE(created_at)'); return rows; } @@ -73,7 +73,11 @@ class MembersStatsService { const today = DateTime.local().toISODate(); const cumulativeResults = []; - for (const row of rows) { + + // Loop in reverse order (needed to have correct sorted result) + for (let i = rows.length - 1; i >= 0; i -= 1) { + const row = rows[i]; + // Convert JSDates to YYYY-MM-DD (in UTC) const date = DateTime.fromJSDate(row.date).toISODate(); if (date > today) { @@ -82,9 +86,9 @@ class MembersStatsService { } cumulativeResults.unshift({ date, - paid, - free, - comped, + paid: Math.max(0, paid), + free: Math.max(0, free), + comped: Math.max(0, comped), // Deltas paid_subscribed: row.paid_subscribed, @@ -92,36 +96,28 @@ class MembersStatsService { }); // Update current counts - paid = Math.max(0, paid - row.paid_subscribed + row.paid_canceled); - free = Math.max(0, free - row.free_delta); - comped = Math.max(0, comped - row.comped_delta); + paid -= row.paid_subscribed - row.paid_canceled; + free -= row.free_delta; + comped -= row.comped_delta; } - // Always make sure we have at least one result - if (cumulativeResults.length === 0) { - cumulativeResults.push({ - date: today, - paid, - free, - comped, + // Now also add the oldest day we have left over (this one will be zero, which is also needed as a data point for graphs) + const oldestDate = rows.length > 0 ? DateTime.fromJSDate(rows[0].date).plus({days: -1}).toISODate() : today; - // Deltas - paid_subscribed: 0, - paid_canceled: 0 - }); - } + cumulativeResults.unshift({ + date: oldestDate, + paid: Math.max(0, paid), + free: Math.max(0, free), + comped: Math.max(0, comped), + + // Deltas + paid_subscribed: 0, + paid_canceled: 0 + }); return { data: cumulativeResults, meta: { - pagination: { - page: 1, - limit: 'all', - pages: 1, - total: cumulativeResults.length, - next: null, - prev: null - }, totals } }; diff --git a/test/e2e-api/admin/__snapshots__/stats.test.js.snap b/test/e2e-api/admin/__snapshots__/stats.test.js.snap index 250888d5f7..4fb27175f6 100644 --- a/test/e2e-api/admin/__snapshots__/stats.test.js.snap +++ b/test/e2e-api/admin/__snapshots__/stats.test.js.snap @@ -3,14 +3,6 @@ exports[`Stats API Can fetch member count history 1: [body] 1`] = ` Object { "meta": Object { - "pagination": Object { - "limit": "all", - "next": null, - "page": 1, - "pages": 1, - "prev": null, - "total": 1, - }, "totals": Object { "comped": 0, "free": 3, @@ -34,7 +26,7 @@ exports[`Stats API Can fetch member count history 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": "231", + "content-length": "149", "content-type": "application/json; charset=utf-8", "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, "vary": "Origin, Accept-Encoding", diff --git a/test/unit/server/services/stats/members-stats-service.test.js b/test/unit/server/services/stats/members-stats-service.test.js index 55845fcca9..936fc43817 100644 --- a/test/unit/server/services/stats/members-stats-service.test.js +++ b/test/unit/server/services/stats/members-stats-service.test.js @@ -16,19 +16,28 @@ describe('MembersStatsService', function () { /** * @type {MembersStatsService.MemberStatusDelta[]} */ - const events = []; + let events = []; const today = '2000-01-10'; const tomorrow = '2000-01-11'; const yesterday = '2000-01-09'; + const dayBeforeYesterday = '2000-01-08'; + const twoDaysBeforeYesterday = '2000-01-07'; const todayDate = DateTime.fromISO(today).toJSDate(); const tomorrowDate = DateTime.fromISO(tomorrow).toJSDate(); const yesterdayDate = DateTime.fromISO(yesterday).toJSDate(); + const dayBeforeYesterdayDate = DateTime.fromISO(dayBeforeYesterday).toJSDate(); before(function () { sinon.useFakeTimers(todayDate.getTime()); membersStatsService = new MembersStatsService({db: null}); fakeTotal = sinon.stub(membersStatsService, 'getCount').resolves(currentCounts); - fakeStatuses = sinon.stub(membersStatsService, 'fetchAllStatusDeltas').resolves(events); + fakeStatuses = sinon.stub(membersStatsService, 'fetchAllStatusDeltas').callsFake(() => { + // Sort here ascending to mimic same ordering + events.sort((a, b) => { + return a.date < b.date ? -1 : 1; + }); + return Promise.resolve(events); + }); }); afterEach(function () { @@ -38,7 +47,7 @@ describe('MembersStatsService', function () { it('Always returns at least one value', async function () { // No status events - events.splice(0, events.length); + events = []; currentCounts.paid = 1; currentCounts.free = 2; currentCounts.comped = 3; @@ -61,108 +70,15 @@ describe('MembersStatsService', function () { it('Passes paid_subscribers and paid_canceled', async function () { // Update faked status events - events.splice(0, events.length, { - date: todayDate, - paid_subscribed: 4, - paid_canceled: 3, - free_delta: 2, - comped_delta: 3 - }); - - // Update current faked counts - currentCounts.paid = 1; - currentCounts.free = 2; - currentCounts.comped = 3; - - const {data: results, meta} = await membersStatsService.getCountHistory(); - results.length.should.eql(1); - results[0].should.eql({ - date: today, - paid: 1, - free: 2, - comped: 3, - paid_subscribed: 4, - paid_canceled: 3 - }); - meta.totals.should.eql(currentCounts); - - fakeStatuses.calledOnce.should.eql(true); - fakeTotal.calledOnce.should.eql(true); - }); - - it('Correctly resolves deltas', async function () { - // Update faked status events - events.splice(0, events.length, + events = [ { date: todayDate, paid_subscribed: 4, paid_canceled: 3, free_delta: 2, comped_delta: 3 - }, - { - date: yesterdayDate, - paid_subscribed: 2, - paid_canceled: 1, - free_delta: 0, - comped_delta: 0 } - ); - - // Update current faked counts - currentCounts.paid = 2; - currentCounts.free = 3; - currentCounts.comped = 4; - - const {data: results, meta} = await membersStatsService.getCountHistory(); - results.should.eql([ - { - date: yesterday, - paid: 1, - free: 1, - comped: 1, - paid_subscribed: 2, - paid_canceled: 1 - }, - { - date: today, - paid: 2, - free: 3, - comped: 4, - paid_subscribed: 4, - paid_canceled: 3 - } - ]); - meta.totals.should.eql(currentCounts); - fakeStatuses.calledOnce.should.eql(true); - fakeTotal.calledOnce.should.eql(true); - }); - - it('Ignores events in the future', async function () { - // Update faked status events - events.splice(0, events.length, - { - date: tomorrowDate, - paid_subscribed: 10, - paid_canceled: 5, - free_delta: 8, - comped_delta: 9 - }, - { - date: todayDate, - paid_subscribed: 4, - paid_canceled: 3, - free_delta: 2, - comped_delta: 3 - }, - { - date: yesterdayDate, - paid_subscribed: 0, - paid_canceled: 0, - free_delta: 0, - comped_delta: 0 - } - ); + ]; // Update current faked counts currentCounts.paid = 1; @@ -192,5 +108,197 @@ describe('MembersStatsService', function () { fakeStatuses.calledOnce.should.eql(true); fakeTotal.calledOnce.should.eql(true); }); + + it('Correctly resolves deltas', async function () { + // Update faked status events + events = [ + { + date: yesterdayDate, + paid_subscribed: 2, + paid_canceled: 1, + free_delta: 0, + comped_delta: 0 + }, + { + date: todayDate, + paid_subscribed: 4, + paid_canceled: 3, + free_delta: 2, + comped_delta: 3 + } + ]; + + // Update current faked counts + currentCounts.paid = 2; + currentCounts.free = 3; + currentCounts.comped = 4; + + const {data: results, meta} = await membersStatsService.getCountHistory(); + results.should.eql([ + { + date: dayBeforeYesterday, + paid: 0, + free: 1, + comped: 1, + paid_subscribed: 0, + paid_canceled: 0 + }, + { + date: yesterday, + paid: 1, + free: 1, + comped: 1, + paid_subscribed: 2, + paid_canceled: 1 + }, + { + date: today, + paid: 2, + free: 3, + comped: 4, + paid_subscribed: 4, + paid_canceled: 3 + } + ]); + meta.totals.should.eql(currentCounts); + fakeStatuses.calledOnce.should.eql(true); + fakeTotal.calledOnce.should.eql(true); + }); + + it('Correctly handles negative numbers', async function () { + // Update faked status events + events = [ + { + date: dayBeforeYesterdayDate, + paid_subscribed: 2, + paid_canceled: 1, + free_delta: 2, + comped_delta: 10 + }, + { + date: yesterdayDate, + paid_subscribed: 2, + paid_canceled: 1, + free_delta: -100, + comped_delta: 0 + }, + { + date: todayDate, + paid_subscribed: 4, + paid_canceled: 3, + free_delta: 100, + comped_delta: 3 + } + ]; + + // Update current faked counts + currentCounts.paid = 2; + currentCounts.free = 3; + currentCounts.comped = 4; + + const {data: results, meta} = await membersStatsService.getCountHistory(); + results.should.eql([ + { + date: twoDaysBeforeYesterday, + paid: 0, + free: 1, + comped: 0, + paid_subscribed: 0, + paid_canceled: 0 + }, + { + date: dayBeforeYesterday, + paid: 0, + // note that this shouldn't be 100 (which is also what we test here): + free: 3, + comped: 1, + paid_subscribed: 2, + paid_canceled: 1 + }, + { + date: yesterday, + paid: 1, + // never return negative numbers, this is in fact -997: + free: 0, + comped: 1, + paid_subscribed: 2, + paid_canceled: 1 + }, + { + date: today, + paid: 2, + free: 3, + comped: 4, + paid_subscribed: 4, + paid_canceled: 3 + } + ]); + meta.totals.should.eql(currentCounts); + fakeStatuses.calledOnce.should.eql(true); + fakeTotal.calledOnce.should.eql(true); + }); + + it('Ignores events in the future', async function () { + // Update faked status events + events = [ + { + date: yesterdayDate, + paid_subscribed: 1, + paid_canceled: 0, + free_delta: 1, + comped_delta: 0 + }, + { + date: todayDate, + paid_subscribed: 4, + paid_canceled: 3, + free_delta: 2, + comped_delta: 3 + }, + { + date: tomorrowDate, + paid_subscribed: 10, + paid_canceled: 5, + free_delta: 8, + comped_delta: 9 + } + ]; + + // Update current faked counts + currentCounts.paid = 1; + currentCounts.free = 2; + currentCounts.comped = 3; + + const {data: results, meta} = await membersStatsService.getCountHistory(); + results.should.eql([ + { + date: dayBeforeYesterday, + paid: 0, + free: 0, + comped: 0, + paid_subscribed: 0, + paid_canceled: 0 + }, + { + date: yesterday, + paid: 0, + free: 0, + comped: 0, + paid_subscribed: 1, + paid_canceled: 0 + }, + { + date: today, + paid: 1, + free: 2, + comped: 3, + paid_subscribed: 4, + paid_canceled: 3 + } + ]); + meta.totals.should.eql(currentCounts); + fakeStatuses.calledOnce.should.eql(true); + fakeTotal.calledOnce.should.eql(true); + }); }); });