From 83ea085c218b627976ce8c5f51733dc080d7c91c Mon Sep 17 00:00:00 2001 From: Alys Date: Sun, 22 Nov 2015 11:28:51 +1000 Subject: [PATCH] refactor api.shouldDo and associated code; fix new bug in sanitizeOptions() --- common/script/cron.js | 42 ++++++++++++++++++++++++++++++++-- common/script/index.js | 52 ++++-------------------------------------- 2 files changed, 45 insertions(+), 49 deletions(-) diff --git a/common/script/cron.js b/common/script/cron.js index 017f7e1d4a..639bbf6b65 100644 --- a/common/script/cron.js +++ b/common/script/cron.js @@ -22,8 +22,8 @@ export const DAY_MAPPING = { {now} is also passed in for various purposes, one example being the test scripts scripts testing different "now" times. */ -export function sanitizeOptions (o) { - let ref = Number(o.dayStart); +function sanitizeOptions (o) { + let ref = Number(o.dayStart || 0); let dayStart = !_.isNaN(ref) && ref >= 0 && ref <= 24 ? ref : 0; let timezoneOffset = o.timezoneOffset ? Number(o.timezoneOffset) : Number(moment().zone()); // TODO: check and clean timezoneOffset as for dayStart (e.g., might not be a number) @@ -70,3 +70,41 @@ export function daysSince (yesterday, options = {}) { return startOfDay(_.defaults({ now: o.now }, o)).diff(startOfDay(_.defaults({ now: yesterday }, o)), 'days'); } +/* + Should the user do this task on this date, given the task's repeat options and user.preferences.dayStart? + */ + +export function shouldDo (day, dailyTask, options = {}) { + if (dailyTask.type !== 'daily') { + return false; + } + let o = sanitizeOptions(options); + let startOfDayWithCDSTime = startOfDay(_.defaults({ now: day }, o)); + + // The time portion of the Start Date is never visible to or modifiable by the user so we must ignore it. + // Therefore, we must also ignore the time portion of the user's day start (startOfDayWithCDSTime), otherwise the date comparison will be wrong for some times. + // NB: The user's day start date has already been converted to the PREVIOUS day's date if the time portion was before CDS. + let taskStartDate = moment(dailyTask.startDate).zone(o.timezoneOffset); + + taskStartDate = moment(taskStartDate).startOf('day'); + if (taskStartDate > startOfDayWithCDSTime.startOf('day')) { + return false; // Daily starts in the future + } + if (dailyTask.frequency === 'daily') { // "Every X Days" + if (!dailyTask.everyX) { + return false; // error condition + } + let daysSinceTaskStart = startOfDayWithCDSTime.startOf('day').diff(taskStartDate, 'days'); + + return daysSinceTaskStart % dailyTask.everyX === 0; + } else if (dailyTask.frequency === 'weekly') { // "On Certain Days of the Week" + if (!dailyTask.repeat) { + return false; // error condition + } + let dayOfWeekNum = startOfDayWithCDSTime.day(); // e.g., 0 for Sunday + + return dailyTask.repeat[DAY_MAPPING[dayOfWeekNum]]; + } else { + return false; // error condition - unexpected frequency string + } +} diff --git a/common/script/index.js b/common/script/index.js index 8d97b3fc58..8f21077f1a 100644 --- a/common/script/index.js +++ b/common/script/index.js @@ -1,8 +1,6 @@ import { - DAY_MAPPING, // temporary, pending further refactoring - sanitizeOptions, // temporary, pending further refactoring - startOfDay, // temporary, pending further refactoring daysSince, + shouldDo, } from '../../common/script/cron'; var $w, _, api, content, i18n, moment, preenHistory, sortOrder, @@ -19,6 +17,7 @@ i18n = require('./i18n'); api = module.exports = {}; api.i18n = i18n; +api.shouldDo = shouldDo; $w = api.$w = function(s) { return s.split(' '); @@ -75,47 +74,6 @@ api.planGemLimits = { ------------------------------------------------------ */ -/* - Should the user do this task on this date, given the task's repeat options and user.preferences.dayStart? - */ - -api.shouldDo = function(day, dailyTask, options) { - var dayOfWeekCheck, dayOfWeekNum, daysSinceTaskStart, everyXCheck, o, startOfDayWithCDSTime, taskStartDate; - if (options == null) { - options = {}; - } - if (dailyTask.type !== 'daily') { - return false; - } - o = sanitizeOptions(options); - startOfDayWithCDSTime = startOfDay(_.defaults({ - now: day - }, o)); - taskStartDate = moment(dailyTask.startDate).zone(o.timezoneOffset); - taskStartDate = moment(taskStartDate).startOf('day'); - if (taskStartDate > startOfDayWithCDSTime.startOf('day')) { - return false; - } - if (dailyTask.frequency === 'daily') { - if (!dailyTask.everyX) { - return false; - } - daysSinceTaskStart = startOfDayWithCDSTime.startOf('day').diff(taskStartDate, 'days'); - everyXCheck = daysSinceTaskStart % dailyTask.everyX === 0; - return everyXCheck; - } else if (dailyTask.frequency === 'weekly') { - if (!dailyTask.repeat) { - return false; - } - dayOfWeekNum = startOfDayWithCDSTime.day(); - dayOfWeekCheck = dailyTask.repeat[DAY_MAPPING[dayOfWeekNum]]; - return dayOfWeekCheck; - } else { - return false; - } -}; - - /* ------------------------------------------------------ Level cap @@ -468,7 +426,7 @@ api.taskClasses = function(task, filters, dayStart, lastCron, showCompleted, mai classes += " beingEdited"; } if (type === 'todo' || type === 'daily') { - if (completed || (type === 'daily' && !api.shouldDo(+(new Date), task, { + if (completed || (type === 'daily' && !shouldDo(+(new Date), task, { dayStart: dayStart }))) { classes += " completed"; @@ -2532,7 +2490,7 @@ api.wrap = function(user, main) { thatDay = moment(now).subtract({ days: 1 }); - if (api.shouldDo(thatDay.toDate(), daily, user.preferences) || completed) { + if (shouldDo(thatDay.toDate(), daily, user.preferences) || completed) { _.each(daily.checklist, (function(box) { box.completed = false; return true; @@ -2586,7 +2544,7 @@ api.wrap = function(user, main) { thatDay = moment(now).subtract({ days: n + 1 }); - if (api.shouldDo(thatDay.toDate(), task, user.preferences)) { + if (shouldDo(thatDay.toDate(), task, user.preferences)) { scheduleMisses++; if (user.stats.buffs.stealth) { user.stats.buffs.stealth--;