From db3d233ae519be4bcb881f6beecad095fcb28cb7 Mon Sep 17 00:00:00 2001 From: bakerty <43011371+bakerty@users.noreply.github.com> Date: Sun, 10 Jan 2021 07:02:40 -0800 Subject: [PATCH] Make start date and day of month aware of timezones (fixes #12555) (#12696) * WIP: #12555 -dayOfMonth takes timezone into account on task update for monthly dailys -startDate set to start of day in user's tz on task update for monthly dailys -tweaked a test so that it's actually testing something * WIP: 12555 -task.startDate gets set to start of day in user's tz on task creation and task update -set preferences.timezoneOffset to nonzero value in certian test user objects -removed date literals in test:api-v3:integration:tasks:PUT --- .../integration/tasks/POST-tasks_user.test.js | 9 ++++- .../v3/integration/tasks/PUT-tasks_id.test.js | 38 ++++++++++++++----- .../POST-tasks_challenge_id.test.js | 9 ++++- .../PUT-tasks_challenge_challengeId.test.js | 2 +- .../tasks/groups/POST-tasks_group_id.test.js | 10 ++++- website/server/controllers/api-v3/tasks.js | 33 ++++++++++------ website/server/libs/taskManager.js | 9 +++++ website/server/models/task.js | 2 +- 8 files changed, 83 insertions(+), 29 deletions(-) diff --git a/test/api/v3/integration/tasks/POST-tasks_user.test.js b/test/api/v3/integration/tasks/POST-tasks_user.test.js index 8ffbce920a..3089938816 100644 --- a/test/api/v3/integration/tasks/POST-tasks_user.test.js +++ b/test/api/v3/integration/tasks/POST-tasks_user.test.js @@ -8,9 +8,14 @@ import { describe('POST /tasks/user', () => { let user; + let tzoffset; + + before(async () => { + tzoffset = new Date().getTimezoneOffset(); + }); beforeEach(async () => { - user = await generateUser(); + user = await generateUser({ 'preferences.timezoneOffset': tzoffset }); }); context('validates params', async () => { @@ -544,7 +549,7 @@ describe('POST /tasks/user', () => { expect(task.everyX).to.eql(5); expect(task.daysOfMonth).to.eql([15]); expect(task.weeksOfMonth).to.eql([3]); - expect(new Date(task.startDate)).to.eql(now); + expect(new Date(task.startDate)).to.eql(new Date(now.setHours(0, 0, 0, 0))); expect(task.isDue).to.be.true; expect(task.nextDue.length).to.eql(6); }); diff --git a/test/api/v3/integration/tasks/PUT-tasks_id.test.js b/test/api/v3/integration/tasks/PUT-tasks_id.test.js index 86be383324..3279ffe8d0 100644 --- a/test/api/v3/integration/tasks/PUT-tasks_id.test.js +++ b/test/api/v3/integration/tasks/PUT-tasks_id.test.js @@ -10,9 +10,14 @@ import { describe('PUT /tasks/:id', () => { let user; + let tzoffset; + + before(async () => { + tzoffset = (new Date()).getTimezoneOffset(); + }); beforeEach(async () => { - user = await generateUser(); + user = await generateUser({ 'preferences.timezoneOffset': tzoffset }); }); context('validates params', () => { @@ -503,7 +508,8 @@ describe('PUT /tasks/:id', () => { let monthly; beforeEach(async () => { - const date1 = moment.utc('2020-07-01').toDate(); + // using date literals is discouraged here, daylight savings will break everything + const date1 = moment().toDate(); monthly = await user.post('/tasks/user', { text: 'test monthly', type: 'daily', @@ -514,7 +520,7 @@ describe('PUT /tasks/:id', () => { }); it('updates days of month when start date updated', async () => { - const date2 = moment.utc('2020-07-01').toDate(); + const date2 = moment().add(6, 'months').toDate(); const savedMonthly = await user.put(`/tasks/${monthly._id}`, { startDate: date2, }); @@ -523,18 +529,30 @@ describe('PUT /tasks/:id', () => { }); it('updates next due when start date updated', async () => { - const date2 = moment.utc('2022-07-01').toDate(); + const date2 = moment().add(6, 'months').toDate(); const savedMonthly = await user.put(`/tasks/${monthly._id}`, { startDate: date2, }); expect(savedMonthly.nextDue.length).to.eql(6); - expect(moment(savedMonthly.nextDue[0]).toDate()).to.eql(moment.utc('2022-08-01').toDate()); - expect(moment(savedMonthly.nextDue[1]).toDate()).to.eql(moment.utc('2022-09-01').toDate()); - expect(moment(savedMonthly.nextDue[2]).toDate()).to.eql(moment.utc('2022-10-01').toDate()); - expect(moment(savedMonthly.nextDue[3]).toDate()).to.eql(moment.utc('2022-11-01').toDate()); - expect(moment(savedMonthly.nextDue[4]).toDate()).to.eql(moment.utc('2022-12-01').toDate()); - expect(moment(savedMonthly.nextDue[5]).toDate()).to.eql(moment.utc('2023-01-01').toDate()); + expect(moment(savedMonthly.nextDue[0]).toDate()).to.eql( + moment(date2).add(1, 'months').startOf('day').toDate(), + ); + expect(moment(savedMonthly.nextDue[1]).toDate()).to.eql( + moment(date2).add(2, 'months').startOf('day').toDate(), + ); + expect(moment(savedMonthly.nextDue[2]).toDate()).to.eql( + moment(date2).add(3, 'months').startOf('day').toDate(), + ); + expect(moment(savedMonthly.nextDue[3]).toDate()).to.eql( + moment(date2).add(4, 'months').startOf('day').toDate(), + ); + expect(moment(savedMonthly.nextDue[4]).toDate()).to.eql( + moment(date2).add(5, 'months').startOf('day').toDate(), + ); + expect(moment(savedMonthly.nextDue[5]).toDate()).to.eql( + moment(date2).add(6, 'months').startOf('day').toDate(), + ); }); }); diff --git a/test/api/v3/integration/tasks/challenges/POST-tasks_challenge_id.test.js b/test/api/v3/integration/tasks/challenges/POST-tasks_challenge_id.test.js index f458d97e46..272dd858fd 100644 --- a/test/api/v3/integration/tasks/challenges/POST-tasks_challenge_id.test.js +++ b/test/api/v3/integration/tasks/challenges/POST-tasks_challenge_id.test.js @@ -11,13 +11,18 @@ describe('POST /tasks/challenge/:challengeId', () => { let user; let guild; let challenge; + let tzoffset; function findUserChallengeTask (memberTask) { return memberTask.challenge.id === challenge._id; } + before(async () => { + tzoffset = new Date().getTimezoneOffset(); + }); + beforeEach(async () => { - user = await generateUser({ balance: 1 }); + user = await generateUser({ balance: 1, 'preferences.timezoneOffset': tzoffset }); guild = await generateGroup(user); challenge = await generateChallenge(user, guild); await user.post(`/challenges/${challenge._id}/join`); @@ -165,7 +170,7 @@ describe('POST /tasks/challenge/:challengeId', () => { expect(task.type).to.eql('daily'); expect(task.frequency).to.eql('daily'); expect(task.everyX).to.eql(5); - expect(new Date(task.startDate)).to.eql(now); + expect(new Date(task.startDate)).to.eql(new Date(now.setHours(0, 0, 0, 0))); expect(userChallengeTask.notes).to.eql(task.notes); }); diff --git a/test/api/v3/integration/tasks/challenges/PUT-tasks_challenge_challengeId.test.js b/test/api/v3/integration/tasks/challenges/PUT-tasks_challenge_challengeId.test.js index d7ee94728c..c9ffec5984 100644 --- a/test/api/v3/integration/tasks/challenges/PUT-tasks_challenge_challengeId.test.js +++ b/test/api/v3/integration/tasks/challenges/PUT-tasks_challenge_challengeId.test.js @@ -12,7 +12,7 @@ describe('PUT /tasks/:id', () => { let challenge; before(async () => { - user = await generateUser(); + user = await generateUser({ 'preferences.timezoneOffset': new Date().getTimezoneOffset() }); guild = await generateGroup(user); challenge = await generateChallenge(user, guild); await user.post(`/challenges/${challenge._id}/join`); diff --git a/test/api/v3/integration/tasks/groups/POST-tasks_group_id.test.js b/test/api/v3/integration/tasks/groups/POST-tasks_group_id.test.js index 85d9dcc3f6..830852fcf6 100644 --- a/test/api/v3/integration/tasks/groups/POST-tasks_group_id.test.js +++ b/test/api/v3/integration/tasks/groups/POST-tasks_group_id.test.js @@ -8,12 +8,18 @@ import { describe('POST /tasks/group/:groupid', () => { let user; let guild; let manager; + let tzoffset; const groupName = 'Test Public Guild'; const groupType = 'guild'; + before(async () => { + tzoffset = new Date().getTimezoneOffset(); + }); + beforeEach(async () => { - user = await generateUser({ balance: 1 }); + // user = await generateUser({ balance: 1, 'preferences.timezoneOffset': tzoffset }); const { group, groupLeader, members } = await createAndPopulateGroup({ + leaderDetails: { balance: 10, 'preferences.timezoneOffset': tzoffset }, groupDetails: { name: groupName, type: groupType, @@ -128,7 +134,7 @@ describe('POST /tasks/group/:groupid', () => { expect(task.type).to.eql('daily'); expect(task.frequency).to.eql('daily'); expect(task.everyX).to.eql(5); - expect(new Date(task.startDate)).to.eql(now); + expect(new Date(task.startDate)).to.eql(new Date(now.setHours(0, 0, 0, 0))); }); it('allows a manager to add a group task', async () => { diff --git a/website/server/controllers/api-v3/tasks.js b/website/server/controllers/api-v3/tasks.js index 8e7d1fe663..578bb1d009 100644 --- a/website/server/controllers/api-v3/tasks.js +++ b/website/server/controllers/api-v3/tasks.js @@ -678,18 +678,29 @@ api.updateTask = { task.group.managerNotes = sanitizedObj.managerNotes; } - // If the daily task was set to repeat monthly on a day of the month, and the start date was - // updated, the task will then need to be updated to repeat on the same day of the month as the - // new start date. For example, if the start date is updated to 7/2/2020, the daily should - // repeat on the 2nd day of the month. It's possible that a task can repeat monthly on a - // week of the month, in which case we won't update the repetition at all. - if ( - task.type === 'daily' - && task.frequency === 'monthly' - && task.daysOfMonth.length - && task.startDate + // For daily tasks, update start date based on timezone to maintain consistency + if (task.type === 'daily' + && task.startDate ) { - task.daysOfMonth = [moment(task.startDate).date()]; + task.startDate = moment(task.startDate).utcOffset( + -user.preferences.timezoneOffset, + ).startOf('day').toDate(); + + // If the daily task was set to repeat monthly on a day of the month, and the start date was + // updated, the task will then need to be updated to repeat on the same day of the month as + // the new start date. For example, if the start date is updated to 7/2/2020, the daily + // should repeat on the 2nd day of the month. It's possible that a task can repeat monthly + // on a week of the month, in which case we won't update the repetition at all. + if ( + task.frequency === 'monthly' + && task.daysOfMonth.length + ) { + // We also need to aware of the user's timezone. Start date is represented as UTC, so the + // start date and day of month might be different + task.daysOfMonth = [moment(task.startDate).utcOffset( + -user.preferences.timezoneOffset, + ).date()]; + } } setNextDue(task, user); diff --git a/website/server/libs/taskManager.js b/website/server/libs/taskManager.js index a5d0f0ac88..39514aba15 100644 --- a/website/server/libs/taskManager.js +++ b/website/server/libs/taskManager.js @@ -124,6 +124,15 @@ export async function createTasks (req, res, options = {}) { } } + // set startDate to midnight in the user's timezone + if (taskType === 'daily') { + const awareStartDate = moment(newTask.startDate).utcOffset(-user.preferences.timezoneOffset); + if (awareStartDate.format('HMsS') !== '0000') { + awareStartDate.startOf('day'); + newTask.startDate = awareStartDate.toDate(); + } + } + setNextDue(newTask, user); // Validate that the task is valid and throw if it isn't diff --git a/website/server/models/task.js b/website/server/models/task.js index b948eaa441..84c45b9c2c 100644 --- a/website/server/models/task.js +++ b/website/server/models/task.js @@ -378,7 +378,7 @@ export const DailySchema = new Schema(_.defaults({ startDate: { $type: Date, default () { - return moment().startOf('day').toDate(); + return moment.utc().toDate(); }, required: true, },