diff --git a/migrations/migration-runner.js b/migrations/migration-runner.js index 60926b4264..36f8c10259 100644 --- a/migrations/migration-runner.js +++ b/migrations/migration-runner.js @@ -18,7 +18,7 @@ function setUpServer () { setUpServer(); // Replace this with your migration -const processUsers = () => {}; // require('').default; +const processUsers = require('./tasks/rewards-flip-negative-costs').default; processUsers() .then(() => { diff --git a/migrations/tasks/rewards-flip-negative-costs.js b/migrations/tasks/rewards-flip-negative-costs.js new file mode 100644 index 0000000000..f55a2ca79b --- /dev/null +++ b/migrations/tasks/rewards-flip-negative-costs.js @@ -0,0 +1,54 @@ +// @migrationName = 'RewardsMigrationFlipNegativeCostsValues'; +// @authorName = 'hamboomger'; +// @authorUuid = '80b61b73-2278-4947-b713-a10112cfe7f5'; + +/* + * For each reward with negative cost, make it positive + * by assigning it an absolute value of itself + */ + +import { Task } from '../../website/server/models/task'; + +async function flipNegativeCostsValues () { + const query = { + type: 'reward', + value: { $lt: 0 }, + }; + + const fields = { + _id: 1, + value: 1, + }; + + // eslint-disable-next-line no-constant-condition + while (true) { + // eslint-disable-next-line no-await-in-loop + const rewards = await Task + .find(query) + .limit(250) + .sort({ _id: 1 }) + .select(fields) + .lean() + .exec(); + + if (rewards.length === 0) { + break; + } + + const promises = rewards.map(reward => { + const positiveValue = Math.abs(reward.value); + return Task.update({ _id: reward._id }, { $set: { value: positiveValue } }).exec(); + }); + + // eslint-disable-next-line no-await-in-loop + await Promise.all(promises); + + query._id = { + $gt: rewards[rewards.length - 1]._id, + }; + } + + console.log('All rewards with negative values were updated, migration finished'); +} + +export default flipNegativeCostsValues; 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 af08c21961..9c3c49c2a9 100644 --- a/test/api/v3/integration/tasks/POST-tasks_user.test.js +++ b/test/api/v3/integration/tasks/POST-tasks_user.test.js @@ -55,6 +55,18 @@ describe('POST /tasks/user', () => { }); }); + it('returns an error if reward value is a negative number', async () => { + await expect(user.post('/tasks/user', { + type: 'reward', + text: 'reward with negative value', + value: -10, + })).to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: 'reward validation failed', + }); + }); + it('does not update user.tasksOrder.{taskType} when the task is not saved because invalid', async () => { const originalHabitsOrder = (await user.get('/user')).tasksOrder.habits; await expect(user.post('/tasks/user', { 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 1ad083e50e..ef50ad8db4 100644 --- a/test/api/v3/integration/tasks/PUT-tasks_id.test.js +++ b/test/api/v3/integration/tasks/PUT-tasks_id.test.js @@ -530,5 +530,15 @@ describe('PUT /tasks/:id', () => { expect(savedReward.value).to.eql(100); }); + + it('returns an error if reward value is a negative number', async () => { + await expect(user.put(`/tasks/${reward._id}`, { + value: -10, + })).to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: 'reward validation failed', + }); + }); }); }); diff --git a/website/common/script/ops/scoreTask.js b/website/common/script/ops/scoreTask.js index 32d6c4655b..a6be2bda23 100644 --- a/website/common/script/ops/scoreTask.js +++ b/website/common/script/ops/scoreTask.js @@ -189,6 +189,37 @@ function _updateCounter (task, direction, times) { } } +function _lastHistoryEntryWasToday (lastHistoryEntry, user) { + if (!lastHistoryEntry || !lastHistoryEntry.date) { + return false; + } + + const { timezoneOffset } = user.preferences; + const { dayStart } = user.preferences; + + // Adjust the last entry date according to the user's timezone and CDS + const dateWithTimeZone = moment(lastHistoryEntry.date).zone(timezoneOffset); + if (dateWithTimeZone.hour() < dayStart) dateWithTimeZone.subtract(1, 'day'); + + return moment().zone(timezoneOffset).isSame(dateWithTimeZone, 'day'); +} + +function _updateLastHistoryEntry (lastHistoryEntry, task, direction, times) { + lastHistoryEntry.value = task.value; + lastHistoryEntry.date = Number(new Date()); + + // @TODO remove this extra check after migration + // has run to set scoredUp and scoredDown in every task + lastHistoryEntry.scoredUp = lastHistoryEntry.scoredUp || 0; + lastHistoryEntry.scoredDown = lastHistoryEntry.scoredDown || 0; + + if (direction === 'up') { + lastHistoryEntry.scoredUp += times; + } else { + lastHistoryEntry.scoredDown += times; + } +} + export default function scoreTask (options = {}, req = {}, analytics) { const { user, task, direction, times = 1, cron = false, @@ -226,38 +257,14 @@ export default function scoreTask (options = {}, req = {}, analytics) { // Save history entry for habit task.history = task.history || []; - const { timezoneOffset } = user.preferences; - const { dayStart } = user.preferences; const historyLength = task.history.length; const lastHistoryEntry = task.history[historyLength - 1]; - // Adjust the last entry date according to the user's timezone and CDS - let lastHistoryEntryDate; - - if (lastHistoryEntry && lastHistoryEntry.date) { - lastHistoryEntryDate = moment(lastHistoryEntry.date).zone(timezoneOffset); - if (lastHistoryEntryDate.hour() < dayStart) lastHistoryEntryDate.subtract(1, 'day'); - } - - if ( - lastHistoryEntryDate - && moment().zone(timezoneOffset).isSame(lastHistoryEntryDate, 'day') - ) { - lastHistoryEntry.value = task.value; - lastHistoryEntry.date = Number(new Date()); - - // @TODO remove this extra check after migration - // has run to set scoredUp and scoredDown in every task - lastHistoryEntry.scoredUp = lastHistoryEntry.scoredUp || 0; - lastHistoryEntry.scoredDown = lastHistoryEntry.scoredDown || 0; - - if (direction === 'up') { - lastHistoryEntry.scoredUp += times; - } else { - lastHistoryEntry.scoredDown += times; + if (_lastHistoryEntryWasToday(lastHistoryEntry, user)) { + _updateLastHistoryEntry(lastHistoryEntry, task, direction, times); + if (task.markModified) { + task.markModified(`history.${historyLength - 1}`); } - - if (task.markModified) task.markModified(`history.${historyLength - 1}`); } else { task.history.push({ date: Number(new Date()), @@ -334,12 +341,7 @@ export default function scoreTask (options = {}, req = {}, analytics) { // Don't adjust values for rewards delta += _changeTaskValue(user, task, direction, times, cron); // purchase item - stats.gp -= Math.abs(task.value); - // hp - gp difference - if (stats.gp < 0) { - stats.hp += stats.gp; - stats.gp = 0; - } + stats.gp -= task.value; } req.yesterDailyScored = task.yesterDailyScored; diff --git a/website/server/controllers/api-v3/tasks.js b/website/server/controllers/api-v3/tasks.js index 8ec0aca71d..d63b4a1071 100644 --- a/website/server/controllers/api-v3/tasks.js +++ b/website/server/controllers/api-v3/tasks.js @@ -105,7 +105,8 @@ const requiredGroupFields = '_id leader tasksOrder name'; * for "Good habits"- * @apiParam (Body) {Boolean} [down=true] Only valid for type "habit" If true, enables * the "-" under "Directions/Action" for "Bad habits" - * @apiParam (Body) {Number} [value=0] Only valid for type "reward." The cost in gold of the reward + * @apiParam (Body) {Number} [value=0] Only valid for type "reward." The cost + * in gold of the reward. Should be greater then or equal to 0. * * @apiParamExample {json} Request-Example: * { @@ -174,6 +175,8 @@ const requiredGroupFields = '_id leader tasksOrder name'; * underscores and dashes. * @apiError (400) {BadRequest} Value-ValidationFailed `x` is not a valid enum value * for path `(body param)`. + * @apiError (400) {BadRequest} Value-ValidationFailed Reward cost should be a + * positive number or 0.`. * @apiError (401) {NotAuthorized} NoAccount There is no account that uses those credentials. * * @apiErrorExample {json} Error-Response: diff --git a/website/server/models/task.js b/website/server/models/task.js index 2fa0693ab2..1cee740503 100644 --- a/website/server/models/task.js +++ b/website/server/models/task.js @@ -96,7 +96,17 @@ export const TaskSchema = new Schema({ validate: [v => validator.isUUID(v), 'Invalid uuid for task tags.'], }], // redness or cost for rewards Required because it must be settable (for rewards) - value: { $type: Number, default: 0, required: true }, + value: { + $type: Number, + default: 0, + required: true, + validate: { + validator (value) { + return this.type === 'reward' ? value >= 0 : true; + }, + msg: 'Reward cost should be a positive number or 0.', + }, + }, priority: { $type: Number, default: 1,