From cdbbf93b7412c81e793d78bc2bd10b8bc47c6d45 Mon Sep 17 00:00:00 2001 From: joe-salomon Date: Tue, 18 Jul 2017 12:53:39 -0700 Subject: [PATCH] Weekly/Monthly Habit reset counters resetting early - fixes #8570 (#8749) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * For habit reset logic, changed day check calculation to use user’s timezone instead of server time. Added unit tests to check following cases: - Weekly habit reset: Server tz is Sunday, User tz is Monday - Weekly habit reset: Server tz is Monday, User tz is Sunday - Monthly habit reset: Server tz is 1st of month, User tz is 2nd of month - Monthly habit reset: Server tz is end of prev month, User tz is 1st of month * use moment().zone() instead of utcOffset() * typo * Fixed check for daysMissed, added logic for CDS Added test for CDS, fixed previous tests --- test/api/v3/unit/libs/cron.test.js | 161 +++++++++++++++++++++++++++++ website/server/libs/cron.js | 8 +- 2 files changed, 165 insertions(+), 4 deletions(-) diff --git a/test/api/v3/unit/libs/cron.test.js b/test/api/v3/unit/libs/cron.test.js index febeaedc8b..c09a7854a3 100644 --- a/test/api/v3/unit/libs/cron.test.js +++ b/test/api/v3/unit/libs/cron.test.js @@ -585,6 +585,114 @@ describe('cron', () => { expect(tasksByType.habits[0].counterDown).to.equal(0); }); + it('should reset a weekly habit counter with custom daily start', () => { + clock.restore(); + + // Server clock: Monday 12am UTC + let monday = new Date('May 22, 2017 00:00:00 GMT').getTime(); + clock = sinon.useFakeTimers(monday); + + // cron runs at 2am + user.preferences.dayStart = 2; + + tasksByType.habits[0].frequency = 'weekly'; + tasksByType.habits[0].counterUp = 1; + tasksByType.habits[0].counterDown = 1; + daysMissed = 1; + + // should not reset + cron({user, tasksByType, daysMissed, analytics}); + + expect(tasksByType.habits[0].counterUp).to.equal(1); + expect(tasksByType.habits[0].counterDown).to.equal(1); + + clock.restore(); + + // Server clock: Monday 3am UTC + monday = new Date('May 22, 2017 03:00:00 GMT').getTime(); + clock = sinon.useFakeTimers(monday); + + // should reset after user CDS + cron({user, tasksByType, daysMissed, analytics}); + + expect(tasksByType.habits[0].counterUp).to.equal(0); + expect(tasksByType.habits[0].counterDown).to.equal(0); + }); + + it('should not reset a weekly habit counter when server tz is Monday but user\'s tz is Tuesday', () => { + clock.restore(); + + // Server clock: Monday 11pm UTC + let monday = new Date('May 22, 2017 23:00:00 GMT').getTime(); + clock = sinon.useFakeTimers(monday); + + // User clock: Tuesday 1am UTC + 2 + user.preferences.timezoneOffset = -120; + + tasksByType.habits[0].frequency = 'weekly'; + tasksByType.habits[0].counterUp = 1; + tasksByType.habits[0].counterDown = 1; + daysMissed = 1; + + // should not reset + cron({user, tasksByType, daysMissed, analytics}); + + expect(tasksByType.habits[0].counterUp).to.equal(1); + expect(tasksByType.habits[0].counterDown).to.equal(1); + + // User missed one cron, which will subtract User clock back to Monday 1am UTC + 2 + // should reset + daysMissed = 2; + cron({user, tasksByType, daysMissed, analytics}); + + expect(tasksByType.habits[0].counterUp).to.equal(0); + expect(tasksByType.habits[0].counterDown).to.equal(0); + }); + + it('should reset a weekly habit counter when server tz is Sunday but user\'s tz is Monday', () => { + clock.restore(); + + // Server clock: Sunday 11pm UTC + let sunday = new Date('May 21, 2017 23:00:00 GMT').getTime(); + clock = sinon.useFakeTimers(sunday); + + // User clock: Monday 2am UTC + 3 + user.preferences.timezoneOffset = -180; + + tasksByType.habits[0].frequency = 'weekly'; + tasksByType.habits[0].counterUp = 1; + tasksByType.habits[0].counterDown = 1; + daysMissed = 1; + + // should reset + cron({user, tasksByType, daysMissed, analytics}); + + expect(tasksByType.habits[0].counterUp).to.equal(0); + expect(tasksByType.habits[0].counterDown).to.equal(0); + }); + + it('should not reset a weekly habit counter when server tz is Monday but user\'s tz is Sunday', () => { + clock.restore(); + + // Server clock: Monday 2am UTC + let monday = new Date('May 22, 2017 02:00:00 GMT').getTime(); + clock = sinon.useFakeTimers(monday); + + // User clock: Sunday 11pm UTC - 3 + user.preferences.timezoneOffset = 180; + + tasksByType.habits[0].frequency = 'weekly'; + tasksByType.habits[0].counterUp = 1; + tasksByType.habits[0].counterDown = 1; + daysMissed = 1; + + // should not reset + cron({user, tasksByType, daysMissed, analytics}); + + expect(tasksByType.habits[0].counterUp).to.equal(1); + expect(tasksByType.habits[0].counterDown).to.equal(1); + }); + it('should reset a monthly habit counter the first day of each month', () => { tasksByType.habits[0].frequency = 'monthly'; tasksByType.habits[0].counterUp = 1; @@ -603,6 +711,59 @@ describe('cron', () => { expect(tasksByType.habits[0].counterUp).to.equal(0); expect(tasksByType.habits[0].counterDown).to.equal(0); }); + + it('should reset a monthly habit counter when server tz is last day of month but user tz is first day of the month', () => { + clock.restore(); + daysMissed = 0; + + // Server clock: 4/30/17 11pm UTC + let monday = new Date('April 30, 2017 23:00:00 GMT').getTime(); + clock = sinon.useFakeTimers(monday); + + // User clock: 5/1/17 2am UTC + 3 + user.preferences.timezoneOffset = -180; + + tasksByType.habits[0].frequency = 'monthly'; + tasksByType.habits[0].counterUp = 1; + tasksByType.habits[0].counterDown = 1; + daysMissed = 1; + + // should reset + cron({user, tasksByType, daysMissed, analytics}); + + expect(tasksByType.habits[0].counterUp).to.equal(0); + expect(tasksByType.habits[0].counterDown).to.equal(0); + }); + + it('should not reset a monthly habit counter when server tz is first day of month but user tz is 2nd day of the month', () => { + clock.restore(); + + // Server clock: 5/1/17 11pm UTC + let monday = new Date('May 1, 2017 23:00:00 GMT').getTime(); + clock = sinon.useFakeTimers(monday); + + // User clock: 5/2/17 2am UTC + 3 + user.preferences.timezoneOffset = -180; + + tasksByType.habits[0].frequency = 'monthly'; + tasksByType.habits[0].counterUp = 1; + tasksByType.habits[0].counterDown = 1; + daysMissed = 1; + + // should not reset + cron({user, tasksByType, daysMissed, analytics}); + + expect(tasksByType.habits[0].counterUp).to.equal(1); + expect(tasksByType.habits[0].counterDown).to.equal(1); + + // User missed one day, which will subtract User clock back to 5/1/17 2am UTC + 3 + // should reset + daysMissed = 2; + cron({user, tasksByType, daysMissed, analytics}); + + expect(tasksByType.habits[0].counterUp).to.equal(0); + expect(tasksByType.habits[0].counterDown).to.equal(0); + }); }); }); diff --git a/website/server/libs/cron.js b/website/server/libs/cron.js index c002918fe6..8f88bdd8fd 100644 --- a/website/server/libs/cron.js +++ b/website/server/libs/cron.js @@ -346,15 +346,15 @@ export function cron (options = {}) { // check if we've passed a day on which we should reset the habit counters, including today let resetWeekly = false; let resetMonthly = false; - for (let i = 0; i <= daysMissed; i++) { + for (let i = 0; i < daysMissed; i++) { if (resetWeekly === true && resetMonthly === true) { break; } - let thatDay = moment(now).subtract({days: i}).toDate(); - if (thatDay.getDay() === 1) { + let thatDay = moment(now).zone(user.preferences.timezoneOffset + user.preferences.dayStart * 60).subtract({days: i}); + if (thatDay.day() === 1) { resetWeekly = true; } - if (thatDay.getDate() === 1) { + if (thatDay.date() === 1) { resetMonthly = true; } }