Weekly/Monthly Habit reset counters resetting early - fixes #8570 (#8749)

* 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

@@ -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);
+      });
     });
   });
 

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();
+    let thatDay = moment(now).zone(user.preferences.timezoneOffset + user.preferences.dayStart * 60).subtract({days: i});
-    if (thatDay.getDay() === 1) {
+    if (thatDay.day() === 1) {
       resetWeekly = true;
     }
-    if (thatDay.getDate() === 1) {
+    if (thatDay.date() === 1) {
       resetMonthly = true;
     }
   }