From bc477455bb433d6cd7ff7d576d33a50704ac09a7 Mon Sep 17 00:00:00 2001
From: Keith Holliday <krh121791@yahoo.com>
Date: Thu, 13 Jul 2017 16:11:27 -0600
Subject: [PATCH] Tasks cron ryamodal fixes (#8871)

* Changed assumption of timezone location

* Added checks for RYA and moved cron check

* Fixed modal scope issue
---
 .../js/controllers/notificationCtrl.js        |  7 +--
 .../client-old/js/services/userServices.js    |  2 +-
 website/server/libs/taskManager.js            |  4 +-
 website/server/middlewares/cron.js            | 53 ++++++++++++-------
 4 files changed, 41 insertions(+), 25 deletions(-)

diff --git a/website/client-old/js/controllers/notificationCtrl.js b/website/client-old/js/controllers/notificationCtrl.js
index e66be981f3..dc55e8395c 100644
--- a/website/client-old/js/controllers/notificationCtrl.js
+++ b/website/client-old/js/controllers/notificationCtrl.js
@@ -1,8 +1,8 @@
 'use strict';
 
 habitrpg.controller('NotificationCtrl',
-  ['$scope', '$rootScope', 'Shared', 'Content', 'User', 'Guide', 'Notification', 'Analytics', 'Achievement', 'Social', 'Tasks',
-  function ($scope, $rootScope, Shared, Content, User, Guide, Notification, Analytics, Achievement, Social, Tasks) {
+  ['$scope', '$rootScope', 'Shared', 'Content', 'User', 'Guide', 'Notification', 'Analytics', 'Achievement', 'Social', 'Tasks', '$modal',
+  function ($scope, $rootScope, Shared, Content, User, Guide, Notification, Analytics, Achievement, Social, Tasks, $modal) {
     var isRunningYesterdailies = false;
 
     $rootScope.$watch('user', function (after, before) {
@@ -53,7 +53,8 @@ habitrpg.controller('NotificationCtrl',
       modalScope.processingYesterdailies = true;
 
       $scope.yesterDailiesModalOpen = true;
-      $rootScope.openModal('yesterDailies', {
+      $modal.open({
+        templateUrl: 'modals/yesterDailies.html',
         scope: modalScope,
         backdrop: 'static',
         controller: ['$scope', 'Tasks', 'User', '$rootScope', function ($scope, Tasks, User, $rootScope) {
diff --git a/website/client-old/js/services/userServices.js b/website/client-old/js/services/userServices.js
index 5ba7b008c4..9001450b86 100644
--- a/website/client-old/js/services/userServices.js
+++ b/website/client-old/js/services/userServices.js
@@ -423,7 +423,7 @@ angular.module('habitrpg')
             url: 'api/v3/cron',
           })
           .then(function (response) {
-            sync();
+            return sync();
           })
         },
 
diff --git a/website/server/libs/taskManager.js b/website/server/libs/taskManager.js
index 5aba20be57..fcf580e7e5 100644
--- a/website/server/libs/taskManager.js
+++ b/website/server/libs/taskManager.js
@@ -30,12 +30,14 @@ export function setNextDue (task, user, dueDateOption) {
   let dateTaskIsDue = Date.now();
   if (dueDateOption) {
     // @TODO Add required ISO format
-    dateTaskIsDue = moment(dueDateOption).add(user.preferences.timezoneOffset, 'minutes');
+    dateTaskIsDue = moment(dueDateOption);
 
     // If not time is supplied. Let's assume we want start of Custom Day Start day.
     if (dateTaskIsDue.hour() === 0 && dateTaskIsDue.minute() === 0 && dateTaskIsDue.second() === 0 && dateTaskIsDue.millisecond() === 0) {
+      dateTaskIsDue.add(user.preferences.timezoneOffset, 'minutes');
       dateTaskIsDue.add(user.preferences.dayStart, 'hours');
     }
+
     now = dateTaskIsDue;
   }
 
diff --git a/website/server/middlewares/cron.js b/website/server/middlewares/cron.js
index 55bff14066..409a5edefd 100644
--- a/website/server/middlewares/cron.js
+++ b/website/server/middlewares/cron.js
@@ -7,6 +7,36 @@ import { recoverCron, cron } from '../libs/cron';
 import { v4 as uuid } from 'uuid';
 import logger from '../libs/logger';
 
+async function checkForActiveCron (user, now) {
+  let _cronSignature = uuid();
+
+  // To avoid double cron we first set _cronSignature and then check that it's not changed while processing
+  let userUpdateResult = await User.update({
+    _id: user._id,
+    _cronSignature: 'NOT_RUNNING', // Check that in the meantime another cron has not started
+  }, {
+    $set: {
+      _cronSignature,
+      lastCron: now, // setting lastCron now so we don't risk re-running parts of cron if it fails
+      'auth.timestamps.loggedin': now,
+    },
+  }).exec();
+
+  // If the cron signature is already set, cron is running in another request
+  // throw an error and recover later,
+  if (userUpdateResult.nMatched === 0 || userUpdateResult.nModified === 0) {
+    throw new Error('CRON_ALREADY_RUNNING');
+  }
+}
+
+async function unlockUser (user) {
+  await User.update({
+    _id: user._id,
+  }, {
+    _cronSignature: 'NOT_RUNNING',
+  }).exec();
+}
+
 async function cronAsync (req, res) {
   let user = res.locals.user;
   if (!user) return null; // User might not be available when authentication is not mandatory
@@ -17,31 +47,14 @@ async function cronAsync (req, res) {
   try {
     let {daysMissed, timezoneOffsetFromUserPrefs} = user.daysUserHasMissed(now, req);
 
+    await checkForActiveCron(user, now);
+
     if (daysMissed <= 0) {
       if (user.isModified()) await user.save();
+      await unlockUser(user);
       return null;
     }
 
-    let _cronSignature = uuid();
-
-    // To avoid double cron we first set _cronSignature and then check that it's not changed while processing
-    let userUpdateResult = await User.update({
-      _id: user._id,
-      _cronSignature: 'NOT_RUNNING', // Check that in the meantime another cron has not started
-    }, {
-      $set: {
-        _cronSignature,
-        lastCron: now, // setting lastCron now so we don't risk re-running parts of cron if it fails
-        'auth.timestamps.loggedin': now,
-      },
-    }).exec();
-
-    // If the cron signature is already set, cron is running in another request
-    // throw an error and recover later,
-    if (userUpdateResult.nMatched === 0 || userUpdateResult.nModified === 0) {
-      throw new Error('CRON_ALREADY_RUNNING');
-    }
-
     let tasks = await Tasks.Task.find({
       userId: user._id,
       $or: [ // Exclude completed todos