From 6e732a67c93b3cf54650e003874ba500717de6c9 Mon Sep 17 00:00:00 2001 From: Keith Holliday Date: Sun, 28 Feb 2016 14:44:18 -0600 Subject: [PATCH 1/3] Added initial challenge model tests --- test/api/v3/unit/models/challenge.test.js | 121 ++++++++++++++++++++++ website/src/models/challenge.js | 8 +- 2 files changed, 125 insertions(+), 4 deletions(-) create mode 100644 test/api/v3/unit/models/challenge.test.js diff --git a/test/api/v3/unit/models/challenge.test.js b/test/api/v3/unit/models/challenge.test.js new file mode 100644 index 0000000000..dcafbe033c --- /dev/null +++ b/test/api/v3/unit/models/challenge.test.js @@ -0,0 +1,121 @@ +import { model as Challenge } from '../../../../../website/src/models/challenge'; +import { model as Group } from '../../../../../website/src/models/group'; +import { model as User } from '../../../../../website/src/models/user'; +import * as Tasks from '../../../../../website/src/models/task'; +import { each } from 'lodash'; + +describe('Challenge Model', () => { + let guild, leader, challenge, task; + let tasksToTest = { + habit: { + text: 'test habit', + type: 'habit', + up: false, + down: true, + notes: 1976, + }, + todo: { + text: 'test todo', + type: 'todo', + notes: 1976, + }, + daily: { + text: 'test daily', + type: 'daily', + notes: 1976, + frequency: 'daily', + everyX: 5, + startDate: new Date(), + }, + reward: { + text: 'test reward', + type: 'reward', + notes: 1976, + }, + }; + + beforeEach(async () => { + guild = new Group({ + name: 'test party', + type: 'guild', + }); + + leader = new User({ + guilds: [guild._id], + }); + + guild.leader = leader._id; + + challenge = new Challenge({ + name: 'Test Challenge', + shortName: 'Test', + leader: leader._id, + group: guild._id, + }); + + leader.challenges = [challenge._id]; + + await Promise.all([ + guild.save(), + leader.save(), + challenge.save(), + ]); + }); + + each(tasksToTest, (taskValue, taskType) => { + context(`${taskType}`, () => { + before(async() => { + task = new Tasks[`${taskType}`](Tasks.Task.sanitizeCreate(taskValue)); + }); + + it('adds tasks to challenge and challenge members', async () => { + await challenge.addTasks([task]); + + let updatedLeader = await User.findOne({_id: leader._id}); + + expect(updatedLeader.tasksOrder[`${taskType}s`].length).to.be.above(0); + }); + + it('syncs a challenge to a user', async () => { + await challenge.addTasks([task]); + + let newMember = new User({ + guilds: [guild._id], + }); + await newMember.save(); + + await challenge.syncToUser(newMember); + + let updatedNewMember = await User.findById(newMember._id); + + expect(updatedNewMember.challenges).to.contain(challenge._id); + expect(updatedNewMember.tags[3]._id).to.equal(challenge._id); + expect(updatedNewMember.tags[3].name).to.equal(challenge.shortName); + expect(updatedNewMember.tasksOrder[`${taskType}s`].length).to.be.above(0); + }); + + it('updates tasks to challenge and challenge members', async () => { + let updatedTaskName = 'Updated Test Habit'; + await challenge.addTasks([task]); + + _.assign(task, _.merge(task.toObject(), Tasks.Task.sanitizeUpdate({ text: updatedTaskName }))); + await challenge.updateTask(task); + + let updatedLeader = await User.findOne({_id: leader._id}); + let updatedUserTask = await Tasks.Task.findById(updatedLeader.tasksOrder[`${taskType}s`][0]); + + expect(updatedUserTask.text).to.equal(updatedTaskName); + }); + + it('removes a tasks to challenge and challenge members', async () => { + await challenge.addTasks([task]); + await challenge.removeTask(task); + + let updatedLeader = await User.findOne({_id: leader._id}); + let updatedUserTask = await Tasks.Task.findOne({_id: updatedLeader.tasksOrder[`${taskType}s`][0]}).exec(); + + expect(updatedUserTask.challenge.broken).to.equal('TASK_DELETED'); + }); + }); + }); +}); diff --git a/website/src/models/challenge.js b/website/src/models/challenge.js index b67e8b7832..d8392ec1d4 100644 --- a/website/src/models/challenge.js +++ b/website/src/models/challenge.js @@ -97,7 +97,6 @@ schema.methods.syncToUser = async function syncChallengeToUser (user) { let [challengeTasks, userTasks] = await Q.all([ // Find original challenge tasks Tasks.Task.find({ - userId: {$exists: false}, 'challenge.id': challenge._id, }).exec(), // Find user's tasks linked to this challenge @@ -186,9 +185,10 @@ schema.methods.updateTask = async function challengeUpdateTask (task) { let updateCmd = {$set: {}}; - _syncableAttrs(task).forEach((value, key) => { - updateCmd.$set[key] = value; - }); + let syncableAttrs = _syncableAttrs(task); + for (let key in syncableAttrs) { + updateCmd.$set[key] = syncableAttrs[key]; + } // TODO reveiw // Updating instead of loading and saving for performances, risks becoming a problem if we introduce more complexity in tasks From 20621b940e8dbfbf35c1b78365e6be577ceea886 Mon Sep 17 00:00:00 2001 From: Keith Holliday Date: Tue, 1 Mar 2016 14:38:09 -0600 Subject: [PATCH 2/3] Added better tests to ensure tasks are synced --- test/api/v3/unit/models/challenge.test.js | 22 ++++++++++++++-------- website/src/models/challenge.js | 1 + 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/test/api/v3/unit/models/challenge.test.js b/test/api/v3/unit/models/challenge.test.js index dcafbe033c..9f6ca65eb7 100644 --- a/test/api/v3/unit/models/challenge.test.js +++ b/test/api/v3/unit/models/challenge.test.js @@ -2,7 +2,7 @@ import { model as Challenge } from '../../../../../website/src/models/challenge' import { model as Group } from '../../../../../website/src/models/group'; import { model as User } from '../../../../../website/src/models/user'; import * as Tasks from '../../../../../website/src/models/task'; -import { each } from 'lodash'; +import { each, find } from 'lodash'; describe('Challenge Model', () => { let guild, leader, challenge, task; @@ -12,17 +12,14 @@ describe('Challenge Model', () => { type: 'habit', up: false, down: true, - notes: 1976, }, todo: { text: 'test todo', type: 'todo', - notes: 1976, }, daily: { text: 'test daily', type: 'daily', - notes: 1976, frequency: 'daily', everyX: 5, startDate: new Date(), @@ -30,7 +27,6 @@ describe('Challenge Model', () => { reward: { text: 'test reward', type: 'reward', - notes: 1976, }, }; @@ -64,16 +60,22 @@ describe('Challenge Model', () => { each(tasksToTest, (taskValue, taskType) => { context(`${taskType}`, () => { - before(async() => { + beforeEach(async() => { task = new Tasks[`${taskType}`](Tasks.Task.sanitizeCreate(taskValue)); + task.challenge.id = challenge._id; + await task.save(); }); it('adds tasks to challenge and challenge members', async () => { await challenge.addTasks([task]); let updatedLeader = await User.findOne({_id: leader._id}); + let updatedLeadersTasks = await Tasks.Task.find({_id: { $in: updatedLeader.tasksOrder[`${taskType}s`]}}); + let syncedTask = find(updatedLeadersTasks, function findNewTask (updatedLeadersTask) { + return updatedLeadersTask.type === taskValue.type && updatedLeadersTask.text === taskValue.text; + }); - expect(updatedLeader.tasksOrder[`${taskType}s`].length).to.be.above(0); + expect(syncedTask).to.exist; }); it('syncs a challenge to a user', async () => { @@ -87,11 +89,15 @@ describe('Challenge Model', () => { await challenge.syncToUser(newMember); let updatedNewMember = await User.findById(newMember._id); + let updatedNewMemberTasks = await Tasks.Task.find({_id: { $in: updatedNewMember.tasksOrder[`${taskType}s`]}}); + let syncedTask = find(updatedNewMemberTasks, function findNewTask (updatedNewMemberTask) { + return updatedNewMemberTask.type === taskValue.type && updatedNewMemberTask.text === taskValue.text; + }); expect(updatedNewMember.challenges).to.contain(challenge._id); expect(updatedNewMember.tags[3]._id).to.equal(challenge._id); expect(updatedNewMember.tags[3].name).to.equal(challenge.shortName); - expect(updatedNewMember.tasksOrder[`${taskType}s`].length).to.be.above(0); + expect(syncedTask).to.exist; }); it('updates tasks to challenge and challenge members', async () => { diff --git a/website/src/models/challenge.js b/website/src/models/challenge.js index d8392ec1d4..0e4606748f 100644 --- a/website/src/models/challenge.js +++ b/website/src/models/challenge.js @@ -97,6 +97,7 @@ schema.methods.syncToUser = async function syncChallengeToUser (user) { let [challengeTasks, userTasks] = await Q.all([ // Find original challenge tasks Tasks.Task.find({ + userId: {$exists: false}, 'challenge.id': challenge._id, }).exec(), // Find user's tasks linked to this challenge From bcc4d568df8b19796882d43fc91c93e1e62e9aff Mon Sep 17 00:00:00 2001 From: Keith Holliday Date: Fri, 4 Mar 2016 11:45:21 -0600 Subject: [PATCH 3/3] Moved unlinkChallengeTasks to challenge model and added tests --- test/api/v3/unit/models/challenge.test.js | 27 ++++++++++++++++ website/src/controllers/api-v3/challenges.js | 2 +- website/src/models/challenge.js | 34 ++++++++++++++++++++ website/src/models/group.js | 2 +- website/src/models/user.js | 33 ------------------- 5 files changed, 63 insertions(+), 35 deletions(-) diff --git a/test/api/v3/unit/models/challenge.test.js b/test/api/v3/unit/models/challenge.test.js index 9f6ca65eb7..d028933e33 100644 --- a/test/api/v3/unit/models/challenge.test.js +++ b/test/api/v3/unit/models/challenge.test.js @@ -122,6 +122,33 @@ describe('Challenge Model', () => { expect(updatedUserTask.challenge.broken).to.equal('TASK_DELETED'); }); + + it('unlinks and deletes challenge tasks for a user when remove-all is specified', async () => { + await challenge.addTasks([task]); + await challenge.unlinkTasks(leader, 'remove-all'); + + let updatedLeader = await User.findOne({_id: leader._id}); + let updatedLeadersTasks = await Tasks.Task.find({_id: { $in: updatedLeader.tasksOrder[`${taskType}s`]}}); + let syncedTask = find(updatedLeadersTasks, function findNewTask (updatedLeadersTask) { + return updatedLeadersTask.type === taskValue.type && updatedLeadersTask.text === taskValue.text; + }); + + expect(syncedTask).to.not.exist; + }); + + it('unlinks and keeps challenge tasks for a user when keep-all is specified', async () => { + await challenge.addTasks([task]); + await challenge.unlinkTasks(leader, 'keep-all'); + + let updatedLeader = await User.findOne({_id: leader._id}); + let updatedLeadersTasks = await Tasks.Task.find({_id: { $in: updatedLeader.tasksOrder[`${taskType}s`]}}); + let syncedTask = find(updatedLeadersTasks, function findNewTask (updatedLeadersTask) { + return updatedLeadersTask.type === taskValue.type && updatedLeadersTask.text === taskValue.text; + }); + + expect(syncedTask).to.exist; + expect(syncedTask.challenge._id).to.be.empty; + }); }); }); }); diff --git a/website/src/controllers/api-v3/challenges.js b/website/src/controllers/api-v3/challenges.js index 5102da8975..f98ceae36c 100644 --- a/website/src/controllers/api-v3/challenges.js +++ b/website/src/controllers/api-v3/challenges.js @@ -191,7 +191,7 @@ api.leaveChallenge = { challenge.memberCount -= 1; // Unlink challenge's tasks from user's tasks and save the challenge - await Q.all([user.unlinkChallengeTasks(challenge._id, keep), challenge.save()]); + await Q.all([challenge.unlinkTasks(user, keep), challenge.save()]); res.respond(200, {}); }, }; diff --git a/website/src/models/challenge.js b/website/src/models/challenge.js index 0e4606748f..e3c2f3ee97 100644 --- a/website/src/models/challenge.js +++ b/website/src/models/challenge.js @@ -5,6 +5,7 @@ import baseModel from '../libs/api-v3/baseModel'; import _ from 'lodash'; import * as Tasks from './task'; import { model as User } from './user'; +import { removeFromArray } from '../libs/api-v3/collectionManipulators'; let Schema = mongoose.Schema; @@ -214,4 +215,37 @@ schema.methods.removeTask = async function challengeRemoveTask (task) { }, {multi: true}).exec(); }; +// Unlink challenges tasks (and the challenge itself) from user +schema.methods.unlinkTasks = async function challengeUnlinkTasks (user, keep) { + let challengeId = this._id; + let findQuery = { + userId: user._id, + 'challenge.id': challengeId, + }; + + removeFromArray(user.challenges, challengeId); + + if (keep === 'keep-all') { + await Tasks.Task.update(findQuery, { + $set: {challenge: {}}, // TODO what about updatedAt? + }, {multi: true}).exec(); + + await user.save(); + } else { // keep = 'remove-all' + let tasks = await Tasks.Task.find(findQuery).select('_id type completed').exec(); + let taskPromises = tasks.map(task => { + // Remove task from user.tasksOrder and delete them + if (task.type !== 'todo' || !task.completed) { + removeFromArray(user.tasksOrder[`${task.type}s`], task._id); + } + + return task.remove(); + }); + user.markModified('tasksOrder'); + taskPromises.push(user.save()); + return Q.all(taskPromises); + } +}; + + export let model = mongoose.model('Challenge', schema); diff --git a/website/src/models/group.js b/website/src/models/group.js index 1620ed020c..7a57490405 100644 --- a/website/src/models/group.js +++ b/website/src/models/group.js @@ -587,7 +587,7 @@ schema.methods.leave = async function leaveGroup (user, keep = 'keep-all') { }); let challengesToRemoveUserFrom = challenges.map(chal => { - return user.unlinkChallengeTasks(chal._id, keep); + return chal.unlinkTasks(user, keep); }); await Q.all(challengesToRemoveUserFrom); diff --git a/website/src/models/user.js b/website/src/models/user.js index 71138aebe6..e8ba70f857 100644 --- a/website/src/models/user.js +++ b/website/src/models/user.js @@ -6,7 +6,6 @@ import moment from 'moment'; import * as Tasks from './task'; import Q from 'q'; import { schema as TagSchema } from './tag'; -import { removeFromArray } from '../libs/api-v3/collectionManipulators'; import baseModel from '../libs/api-v3/baseModel'; // import {model as Challenge} from './challenge'; @@ -696,38 +695,6 @@ schema.methods.getGroups = function getUserGroups () { return userGroups; }; -// Unlink challenges tasks (and the challenge itself) from user -schema.methods.unlinkChallengeTasks = async function unlinkChallengeTasks (challengeId, keep) { - let user = this; - let findQuery = { - userId: user._id, - 'challenge.id': challengeId, - }; - - removeFromArray(user.challenges, challengeId); - - if (keep === 'keep-all') { - await Tasks.Task.update(findQuery, { - $set: {challenge: {}}, // TODO what about updatedAt? - }, {multi: true}).exec(); - - await user.save(); - } else { // keep = 'remove-all' - let tasks = await Tasks.Task.find(findQuery).select('_id type completed').exec(); - let taskPromises = tasks.map(task => { - // Remove task from user.tasksOrder and delete them - if (task.type !== 'todo' || !task.completed) { - removeFromArray(user.tasksOrder[`${task.type}s`], task._id); - } - - return task.remove(); - }); - - taskPromises.push(user.save()); - return Q.all(taskPromises); - } -}; - export let model = mongoose.model('User', schema); // Initially export an empty object so external requires will get