diff --git a/website/client-old/js/controllers/notificationCtrl.js b/website/client-old/js/controllers/notificationCtrl.js index 5a72fbaf10..d4f868e117 100644 --- a/website/client-old/js/controllers/notificationCtrl.js +++ b/website/client-old/js/controllers/notificationCtrl.js @@ -15,7 +15,7 @@ habitrpg.controller('NotificationCtrl', var userLastCron = moment(User.user.lastCron).local(); var userDayStart = moment().startOf('day').add({ hours: User.user.preferences.dayStart }); - if (userLastCron.date() == userDayStart.date()) return; + if (!User.user.needsCron) return; var dailys = User.user.dailys; if (!Boolean(dailys) || dailys.length === 0) return; diff --git a/website/common/script/cron.js b/website/common/script/cron.js index 1205e15472..e9d1cd25e6 100644 --- a/website/common/script/cron.js +++ b/website/common/script/cron.js @@ -85,8 +85,10 @@ export function startOfDay (options = {}) { export function daysSince (yesterday, options = {}) { let o = sanitizeOptions(options); + let startOfNow = startOfDay(defaults({ now: o.now }, o)); + let startOfYesterday = startOfDay(defaults({ now: yesterday }, o)); - return startOfDay(defaults({ now: o.now }, o)).diff(startOfDay(defaults({ now: yesterday }, o)), 'days'); + return startOfNow.diff(startOfYesterday, 'days'); } /* diff --git a/website/server/controllers/api-v3/user.js b/website/server/controllers/api-v3/user.js index 45a0d6b269..272120f5df 100644 --- a/website/server/controllers/api-v3/user.js +++ b/website/server/controllers/api-v3/user.js @@ -78,6 +78,10 @@ api.getUser = { // Remove apiToken from response TODO make it private at the user level? returned in signup/login delete userToJSON.apiToken; + let {daysMissed} = user.daysUserHasMissed(new Date(), req); + userToJSON.needsCron = false; + if (daysMissed > 0) userToJSON.needsCron = true; + user.addComputedStatsToJSONObj(userToJSON.stats); return res.respond(200, userToJSON); }, diff --git a/website/server/middlewares/cron.js b/website/server/middlewares/cron.js index 18bed327e7..addf27c1df 100644 --- a/website/server/middlewares/cron.js +++ b/website/server/middlewares/cron.js @@ -1,6 +1,4 @@ -import _ from 'lodash'; import moment from 'moment'; -import common from '../../common'; import * as Tasks from '../models/task'; import Bluebird from 'bluebird'; import { model as Group } from '../models/group'; @@ -8,8 +6,6 @@ import { model as User } from '../models/user'; import { recoverCron, cron } from '../libs/cron'; import { v4 as uuid } from 'uuid'; -const daysSince = common.daysSince; - async function cronAsync (req, res) { let user = res.locals.user; if (!user) return null; // User might not be available when authentication is not mandatory @@ -18,83 +14,7 @@ async function cronAsync (req, res) { let now = new Date(); try { - // If the user's timezone has changed (due to travel or daylight savings), - // cron can be triggered twice in one day, so we check for that and use - // both timezones to work out if cron should run. - // CDS = Custom Day Start time. - let timezoneOffsetFromUserPrefs = user.preferences.timezoneOffset; - let timezoneOffsetAtLastCron = _.isFinite(user.preferences.timezoneOffsetAtLastCron) ? user.preferences.timezoneOffsetAtLastCron : timezoneOffsetFromUserPrefs; - let timezoneOffsetFromBrowser = Number(req.header('x-user-timezoneoffset')); - timezoneOffsetFromBrowser = _.isFinite(timezoneOffsetFromBrowser) ? timezoneOffsetFromBrowser : timezoneOffsetFromUserPrefs; - // NB: All timezone offsets can be 0, so can't use `... || ...` to apply non-zero defaults - - if (timezoneOffsetFromBrowser !== timezoneOffsetFromUserPrefs) { - // The user's browser has just told Habitica that the user's timezone has - // changed so store and use the new zone. - user.preferences.timezoneOffset = timezoneOffsetFromBrowser; - timezoneOffsetFromUserPrefs = timezoneOffsetFromBrowser; - } - - // How many days have we missed using the user's current timezone: - let daysMissed = daysSince(user.lastCron, _.defaults({now}, user.preferences)); - - if (timezoneOffsetAtLastCron !== timezoneOffsetFromUserPrefs) { - // Since cron last ran, the user's timezone has changed. - // How many days have we missed using the old timezone: - let daysMissedNewZone = daysMissed; - let daysMissedOldZone = daysSince(user.lastCron, _.defaults({ - now, - timezoneOffsetOverride: timezoneOffsetAtLastCron, - }, user.preferences)); - - if (timezoneOffsetAtLastCron < timezoneOffsetFromUserPrefs) { - // The timezone change was in the unsafe direction. - // E.g., timezone changes from UTC+1 (offset -60) to UTC+0 (offset 0). - // or timezone changes from UTC-4 (offset 240) to UTC-5 (offset 300). - // Local time changed from, for example, 03:00 to 02:00. - - if (daysMissedOldZone > 0 && daysMissedNewZone > 0) { - // Both old and new timezones indicate that we SHOULD run cron, so - // it is safe to do so immediately. - daysMissed = Math.min(daysMissedOldZone, daysMissedNewZone); - // use minimum value to be nice to user - } else if (daysMissedOldZone > 0) { - // The old timezone says that cron should run; the new timezone does not. - // This should be impossible for this direction of timezone change, but - // just in case I'm wrong... - // TODO - // console.log("zone has changed - old zone says run cron, NEW zone says no - stop cron now only -- SHOULD NOT HAVE GOT TO HERE", timezoneOffsetAtLastCron, timezoneOffsetFromUserPrefs, now); // used in production for confirming this never happens - } else if (daysMissedNewZone > 0) { - // The old timezone says that cron should NOT run -- i.e., cron has - // already run today, from the old timezone's point of view. - // The new timezone says that cron SHOULD run, but this is almost - // certainly incorrect. - // This happens when cron occurred at a time soon after the CDS. When - // you reinterpret that time in the new timezone, it looks like it - // was before the CDS, because local time has stepped backwards. - // To fix this, rewrite the cron time to a time that the new - // timezone interprets as being in today. - - daysMissed = 0; // prevent cron running now - let timezoneOffsetDiff = timezoneOffsetAtLastCron - timezoneOffsetFromUserPrefs; - // e.g., for dangerous zone change: 240 - 300 = -60 or -660 - -600 = -60 - - user.lastCron = moment(user.lastCron).subtract(timezoneOffsetDiff, 'minutes'); - // NB: We don't change user.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: - user.preferences.timezoneOffsetAtLastCron = timezoneOffsetFromUserPrefs; - } else { - // Both old and new timezones indicate that cron should - // NOT run. - daysMissed = 0; // prevent cron running now - } - } else if (timezoneOffsetAtLastCron > timezoneOffsetFromUserPrefs) { - daysMissed = daysMissedNewZone; - // TODO: Either confirm that there is nothing that could possibly go wrong here and remove the need for this else branch, or fix stuff. - // There are probably situations where the Dailies do not reset early enough for a user who was expecting the zone change and wants to use all their Dailies immediately in the new zone; - // if so, we should provide an option for easy reset of Dailies (can't be automatic because there will be other situations where the user was not prepared). - } - } + let {daysMissed, timezoneOffsetFromUserPrefs} = user.daysUserHasMissed(now, req); if (daysMissed <= 0) { if (user.isModified()) await user.save(); diff --git a/website/server/models/user/methods.js b/website/server/models/user/methods.js index 34cd5608f0..35bbeeda16 100644 --- a/website/server/models/user/methods.js +++ b/website/server/models/user/methods.js @@ -13,6 +13,9 @@ import amazonPayments from '../../libs/amazonPayments'; import stripePayments from '../../libs/stripePayments'; import paypalPayments from '../../libs/paypalPayments'; +const daysSince = common.daysSince; + + schema.methods.isSubscribed = function isSubscribed () { let now = new Date(); let plan = this.purchased.plan; @@ -27,7 +30,7 @@ schema.methods.hasNotCancelled = function hasNotCancelled () { // Get an array of groups ids the user is member of schema.methods.getGroups = function getUserGroups () { - let userGroups = this.guilds.slice(0); // clone user.guilds so we don't modify the original + let userGroups = this.guilds.slice(0); // clone this.guilds so we don't modify the original if (this.party._id) userGroups.push(this.party._id); userGroups.push(TAVERN_ID); return userGroups; @@ -35,7 +38,7 @@ schema.methods.getGroups = function getUserGroups () { /** - * Sends a message to a user. Archives a copy in sender's inbox. + * Sends a message to a this. Archives a copy in sender's inbox. * * @param userToReceiveMessage The receiver * @param options @@ -63,7 +66,7 @@ schema.methods.sendMessage = async function sendMessage (userToReceiveMessage, o * Creates a notification based on the input parameters and adds it to the local user notifications array. * This does not save the notification to the database or interact with the database in any way. * - * @param type The type of notification to add to the user. Possible values are defined in the UserNotificaiton Schema + * @param type The type of notification to add to the this. Possible values are defined in the UserNotificaiton Schema * @param data The data to add to the notification */ schema.methods.addNotification = function addUserNotification (type, data = {}) { @@ -80,7 +83,7 @@ schema.methods.addNotification = function addUserNotification (type, data = {}) * the user document(s) opened. * * @param query A Mongoose query defining the users to add the notification to. - * @param type The type of notification to add to the user. Possible values are defined in the UserNotificaiton Schema + * @param type The type of notification to add to the this. Possible values are defined in the UserNotificaiton Schema * @param data The data to add to the notification */ schema.statics.pushNotification = async function pushNotification (query, type, data = {}) { @@ -95,7 +98,7 @@ schema.statics.pushNotification = async function pushNotification (query, type, // Add stats.toNextLevel, stats.maxMP and stats.maxHealth // to a JSONified User stats object schema.methods.addComputedStatsToJSONObj = function addComputedStatsToUserJSONObj (statsObject) { - // NOTE: if an item is manually added to user.stats then + // NOTE: if an item is manually added to this.stats then // common/fns/predictableRandom must be tweaked so the new item is not considered. // Otherwise the client will have it while the server won't and the results will be different. statsObject.toNextLevel = common.tnl(this.stats.lvl); @@ -123,3 +126,85 @@ schema.methods.cancelSubscription = async function cancelSubscription () { return await payments.cancelSubscription({user: this}); }; + +schema.methods.daysUserHasMissed = function daysUserHasMissed (now, req = {}) { + // If the user's timezone has changed (due to travel or daylight savings), + // cron can be triggered twice in one day, so we check for that and use + // both timezones to work out if cron should run. + // CDS = Custom Day Start time. + let timezoneOffsetFromUserPrefs = this.preferences.timezoneOffset; + let timezoneOffsetAtLastCron = isFinite(this.preferences.timezoneOffsetAtLastCron) ? this.preferences.timezoneOffsetAtLastCron : timezoneOffsetFromUserPrefs; + let timezoneOffsetFromBrowser = typeof req.header === 'function' && Number(req.header('x-user-timezoneoffset')); + timezoneOffsetFromBrowser = isFinite(timezoneOffsetFromBrowser) ? timezoneOffsetFromBrowser : timezoneOffsetFromUserPrefs; + // NB: All timezone offsets can be 0, so can't use `... || ...` to apply non-zero defaults + + if (timezoneOffsetFromBrowser !== timezoneOffsetFromUserPrefs) { + // The user's browser has just told Habitica that the user's timezone has + // changed so store and use the new zone. + this.preferences.timezoneOffset = timezoneOffsetFromBrowser; + timezoneOffsetFromUserPrefs = timezoneOffsetFromBrowser; + } + + // How many days have we missed using the user's current timezone: + let daysMissed = daysSince(this.lastCron, defaults({now}, this.preferences)); + + if (timezoneOffsetAtLastCron !== timezoneOffsetFromUserPrefs) { + // Since cron last ran, the user's timezone has changed. + // How many days have we missed using the old timezone: + let daysMissedNewZone = daysMissed; + let daysMissedOldZone = daysSince(this.lastCron, defaults({ + now, + timezoneOffsetOverride: timezoneOffsetAtLastCron, + }, this.preferences)); + + if (timezoneOffsetAtLastCron < timezoneOffsetFromUserPrefs) { + // The timezone change was in the unsafe direction. + // E.g., timezone changes from UTC+1 (offset -60) to UTC+0 (offset 0). + // or timezone changes from UTC-4 (offset 240) to UTC-5 (offset 300). + // Local time changed from, for example, 03:00 to 02:00. + + if (daysMissedOldZone > 0 && daysMissedNewZone > 0) { + // Both old and new timezones indicate that we SHOULD run cron, so + // it is safe to do so immediately. + daysMissed = Math.min(daysMissedOldZone, daysMissedNewZone); + // use minimum value to be nice to user + } else if (daysMissedOldZone > 0) { + // The old timezone says that cron should run; the new timezone does not. + // This should be impossible for this direction of timezone change, but + // just in case I'm wrong... + // TODO + // console.log("zone has changed - old zone says run cron, NEW zone says no - stop cron now only -- SHOULD NOT HAVE GOT TO HERE", timezoneOffsetAtLastCron, timezoneOffsetFromUserPrefs, now); // used in production for confirming this never happens + } else if (daysMissedNewZone > 0) { + // The old timezone says that cron should NOT run -- i.e., cron has + // already run today, from the old timezone's point of view. + // The new timezone says that cron SHOULD run, but this is almost + // certainly incorrect. + // This happens when cron occurred at a time soon after the CDS. When + // you reinterpret that time in the new timezone, it looks like it + // was before the CDS, because local time has stepped backwards. + // To fix this, rewrite the cron time to a time that the new + // timezone interprets as being in today. + + daysMissed = 0; // prevent cron running now + let timezoneOffsetDiff = timezoneOffsetAtLastCron - timezoneOffsetFromUserPrefs; + // e.g., for dangerous zone change: 240 - 300 = -60 or -660 - -600 = -60 + + this.lastCron = moment(this.lastCron).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: + this.preferences.timezoneOffsetAtLastCron = timezoneOffsetFromUserPrefs; + } else { + // Both old and new timezones indicate that cron should + // NOT run. + daysMissed = 0; // prevent cron running now + } + } else if (timezoneOffsetAtLastCron > timezoneOffsetFromUserPrefs) { + daysMissed = daysMissedNewZone; + // TODO: Either confirm that there is nothing that could possibly go wrong here and remove the need for this else branch, or fix stuff. + // There are probably situations where the Dailies do not reset early enough for a user who was expecting the zone change and wants to use all their Dailies immediately in the new zone; + // if so, we should provide an option for easy reset of Dailies (can't be automatic because there will be other situations where the user was not prepared). + } + } + + return {daysMissed, timezoneOffsetFromUserPrefs}; +};