Fix cron not running if previous run failed (#13892)

* catch issue where cron wouldn’t run bc previous run failed

* add some more tests for cron middleware

* fix lint

test/api/unit/middlewares/cronMiddleware.js

@@ -128,6 +128,22 @@ describe('cron middleware', () => {
     });
   });
 
+  it('runs cron if previous cron was incomplete', async () => {
+    user.lastCron = moment(new Date()).subtract({ days: 1 });
+    user.auth.timestamps.loggedin = moment(new Date()).subtract({ days: 4 });
+    const now = new Date();
+    await user.save();
+
+    await new Promise((resolve, reject) => {
+      cronMiddleware(req, res, err => {
+        if (err) return reject(err);
+        expect(moment(now).isSame(user.lastCron, 'day'));
+        expect(moment(now).isSame(user.auth.timestamps.loggedin, 'day'));
+        return resolve();
+      });
+    });
+  });
+
   it('updates user.auth.timestamps.loggedin and lastCron', async () => {
     user.lastCron = moment(new Date()).subtract({ days: 2 });
     const now = new Date();
@@ -293,4 +309,33 @@ describe('cron middleware', () => {
       });
     });
   });
+
+  it('cron should not run more than once', async () => {
+    user.lastCron = moment(new Date()).subtract({ days: 2 });
+    await user.save();
+
+    sandbox.spy(cronLib, 'cron');
+
+    await Promise.all([new Promise((resolve, reject) => {
+      cronMiddleware(req, res, err => {
+        if (err) return reject(err);
+        return resolve();
+      });
+    }), new Promise((resolve, reject) => {
+      cronMiddleware(req, res, err => {
+        if (err) return reject(err);
+        return resolve();
+      });
+    }), new Promise((resolve, reject) => {
+      setTimeout(() => {
+        cronMiddleware(req, res, err => {
+          if (err) return reject(err);
+          return resolve();
+        });
+      }, 400);
+    }),
+    ]);
+
+    expect(cronLib.cron).to.be.calledOnce;
+  });
 });

test/api/unit/models/user.test.js

@@ -811,6 +811,16 @@ describe('User Model', () => {
       expect(daysMissed).to.eql(5);
     });
 
+    it('correctly handles a cron that did not complete', () => {
+      const now = moment();
+      user.lastCron = moment(now).subtract(2, 'days');
+      user.auth.timestamps.loggedIn = moment(now).subtract(5, 'days');
+
+      const { daysMissed } = user.daysUserHasMissed(now);
+
+      expect(daysMissed).to.eql(5);
+    });
+
     it('uses timezone from preferences to calculate days missed', () => {
       const now = moment('2017-07-08 01:00:00Z');
       user.lastCron = moment('2017-07-04 13:00:00Z');

website/server/models/user/methods.js

@@ -382,8 +382,12 @@ schema.methods.daysUserHasMissed = function daysUserHasMissed (now, req = {}) {
     timezoneUtcOffsetFromUserPrefs = timezoneUtcOffsetFromBrowser;
   }
 
+  let lastCronTime = this.lastCron;
+  if (this.auth.timestamps.loggedIn < lastCronTime) {
+    lastCronTime = this.auth.timestamps.loggedIn;
+  }
   // How many days have we missed using the user's current timezone:
-  let daysMissed = daysSince(this.lastCron, defaults({ now }, this.preferences));
+  let daysMissed = daysSince(lastCronTime, defaults({ now }, this.preferences));
 
   if (timezoneUtcOffsetAtLastCron !== timezoneUtcOffsetFromUserPrefs) {
     // Give the user extra time based on the difference in timezones
@@ -395,7 +399,7 @@ schema.methods.daysUserHasMissed = function daysUserHasMissed (now, req = {}) {
     // Since cron last ran, the user's timezone has changed.
     // How many days have we missed using the old timezone:
     const daysMissedNewZone = daysMissed;
-    const daysMissedOldZone = daysSince(this.lastCron, defaults({
+    const daysMissedOldZone = daysSince(lastCronTime, defaults({
       now,
       timezoneUtcOffsetOverride: timezoneUtcOffsetAtLastCron,
     }, this.preferences));
@@ -435,7 +439,7 @@ schema.methods.daysUserHasMissed = function daysUserHasMissed (now, req = {}) {
         const timezoneOffsetDiff = timezoneUtcOffsetFromUserPrefs - timezoneUtcOffsetAtLastCron;
         // e.g., for dangerous zone change: -300 - -240 = -60 or 600 - 660= -60
 
-        this.lastCron = moment(this.lastCron).subtract(timezoneOffsetDiff, 'minutes');
+        this.lastCron = moment(lastCronTime).subtract(timezoneOffsetDiff, 'minutes');
         // NB: We don't change this.auth.timestamps.loggedin so that will still record
         // the time that the previous cron actually ran.
         // From now on we can ignore the old timezone: