diff --git a/test/api/v3/unit/libs/baseModel.test.js b/test/api/v3/unit/libs/baseModel.test.js index 17f3f7834a..ebde4ee3ea 100644 --- a/test/api/v3/unit/libs/baseModel.test.js +++ b/test/api/v3/unit/libs/baseModel.test.js @@ -1,11 +1,11 @@ import baseModel from '../../../../../website/src/libs/api-v3/baseModel'; -import { Schema } from 'mongoose'; +import mongoose from 'mongoose'; describe('Base model plugin', () => { let schema; beforeEach(() => { - schema = new Schema(); + schema = new mongoose.Schema(); sandbox.stub(schema, 'add'); }); diff --git a/test/api/v3/unit/libs/collectionManipulators.test.js b/test/api/v3/unit/libs/collectionManipulators.test.js new file mode 100644 index 0000000000..fff818b32e --- /dev/null +++ b/test/api/v3/unit/libs/collectionManipulators.test.js @@ -0,0 +1,88 @@ +import mongoose from 'mongoose'; +import { + removeFromArray, +} from '../../../../../website/src/libs/api-v3/collectionManipulators'; + +describe('Collection Manipulators', () => { + describe('removeFromArray', () => { + it('removes item from specified array on document', () => { + let array = ['a', 'b', 'c', 'd']; + + removeFromArray(array, 'c'); + + expect(array).to.not.include('c'); + }); + + it('removes object from array', () => { + let array = [ + { id: 'a', foo: 'bar' }, + { id: 'b', foo: 'bar' }, + { id: 'c', foo: 'bar' }, + { id: 'd', foo: 'bar' }, + { id: 'e', foo: 'bar' }, + ]; + + removeFromArray(array, { id: 'c' }); + + expect(array).to.not.include({ id: 'c', foo: 'bar' }); + }); + + it('does not change array if value is not found', () => { + let array = ['a', 'b', 'c', 'd']; + + removeFromArray(array, 'z'); + + expect(array).to.have.a.lengthOf(4); + expect(array[0]).to.eql('a'); + expect(array[1]).to.eql('b'); + expect(array[2]).to.eql('c'); + expect(array[3]).to.eql('d'); + }); + + it('returns the removed element', () => { + let array = ['a', 'b', 'c']; + + let result = removeFromArray(array, 'b'); + + expect(result).to.eql('b'); + }); + + it('returns the removed object element', () => { + let array = [ + { id: 'a', foo: 'bar' }, + { id: 'b', foo: 'bar' }, + { id: 'c', foo: 'bar' }, + { id: 'd', foo: 'bar' }, + { id: 'e', foo: 'bar' }, + ]; + + let result = removeFromArray(array, { id: 'c' }); + + expect(result).to.eql({ id: 'c', foo: 'bar' }); + }); + + it('returns false if item is not found', () => { + let array = ['a', 'b', 'c']; + + let result = removeFromArray(array, 'z'); + + expect(result).to.eql(false); + }); + + it('removal of element persists when mongoose document is saved', async () => { + let schema = new mongoose.Schema({ + array: Array, + }); + let Model = mongoose.model('ModelToTestRemoveFromArray', schema); + let model = await new Model({ + array: ['a', 'b', 'c'], + }).save(); // Initial creation + + removeFromArray(model.array, 'b'); + + let savedModel = await model.save(); + + expect(savedModel.array).to.not.include('b'); + }); + }); +}); diff --git a/test/helpers/api-unit.helper.js b/test/helpers/api-unit.helper.js index cefd1eeeeb..0a4a22df79 100644 --- a/test/helpers/api-unit.helper.js +++ b/test/helpers/api-unit.helper.js @@ -1,10 +1,27 @@ import '../../website/src/libs/api-v3/i18n'; +import mongoose from 'mongoose'; import { defaultsDeep as defaults } from 'lodash'; import { model as User } from '../../website/src/models/user'; import { model as Group } from '../../website/src/models/group'; -afterEach(() => { +mongoose.Promise = require('q').Promise; + +mongoose.connect('mongodb://localhost/habitica-unit-tests'); +let connection = mongoose.connection; + +before((done) => { + connection.on('open', () => { + connection.db.dropDatabase(done); + }); +}); + +after((done) => { + connection.close(done); +}); + +afterEach((done) => { sandbox.restore(); + connection.db.dropDatabase(done); }); export function generateUser (options = {}) { diff --git a/website/src/controllers/api-v3/chat.js b/website/src/controllers/api-v3/chat.js index 6c491c4e7c..5c8d4fd3ed 100644 --- a/website/src/controllers/api-v3/chat.js +++ b/website/src/controllers/api-v3/chat.js @@ -7,6 +7,7 @@ import { NotAuthorized, } from '../../libs/api-v3/errors'; import _ from 'lodash'; +import { removeFromArray } from '../../libs/api-v3/collectionManipulators'; import { sendTxn } from '../../libs/api-v3/email'; import nconf from 'nconf'; @@ -385,8 +386,7 @@ api.deleteChat = { if (chatUpdated) { group = group.toJSON(); - let i = _.findIndex(group.chat, {id: chatId}); - if (i !== -1) group.chat.splice(i, 1); + removeFromArray(group.chat, {id: chatId}); res.respond(200, group.chat); } else { res.respond(200, {}); diff --git a/website/src/controllers/api-v3/groups.js b/website/src/controllers/api-v3/groups.js index dd298adbf3..b9b633c111 100644 --- a/website/src/controllers/api-v3/groups.js +++ b/website/src/controllers/api-v3/groups.js @@ -13,6 +13,7 @@ import { BadRequest, NotAuthorized, } from '../../libs/api-v3/errors'; +import { removeFromArray } from '../../libs/api-v3/collectionManipulators'; import * as firebase from '../../libs/api-v3/firebase'; import { sendTxn as sendTxnEmail } from '../../libs/api-v3/email'; import { encrypt } from '../../libs/api-v3/encryption'; @@ -254,11 +255,10 @@ api.joinGroup = { isUserInvited = true; } else if (group.type === 'guild') { - let i = _.findIndex(user.invitations.guilds, {id: group._id}); + let hasInvitation = removeFromArray(user.invitations.guilds, { id: group._id }); - if (i !== -1) { + if (hasInvitation) { isUserInvited = true; - user.invitations.guilds.splice(i, 1); // Remove invitation } else { isUserInvited = group.privacy === 'private' ? false : true; } @@ -403,8 +403,7 @@ api.removeGroupMember = { } if (isInGroup === 'guild') { - let i = member.guilds.indexOf(group._id); - if (i !== -1) member.guilds.splice(i, 1); + removeFromArray(member.guilds, group._id); } if (isInGroup === 'party') member.party._id = undefined; // TODO remove quest information too? @@ -418,8 +417,7 @@ api.removeGroupMember = { } } else if (isInvited) { if (isInvited === 'guild') { - let i = _.findIndex(member.invitations.guilds, {id: group._id}); - if (i !== -1) member.invitations.guilds.splice(i, 1); + removeFromArray(member.invitations.guilds, { id: group._id }); } if (isInvited === 'party') user.invitations.party = {}; // TODO mark modified? } else { diff --git a/website/src/controllers/api-v3/tasks.js b/website/src/controllers/api-v3/tasks.js index 020ae5061b..2e72953ad1 100644 --- a/website/src/controllers/api-v3/tasks.js +++ b/website/src/controllers/api-v3/tasks.js @@ -1,6 +1,7 @@ import { authWithHeaders } from '../../middlewares/api-v3/auth'; import cron from '../../middlewares/api-v3/cron'; import { sendTaskWebhook } from '../../libs/api-v3/webhook'; +import { removeFromArray } from '../../libs/api-v3/collectionManipulators'; import * as Tasks from '../../models/task'; import { model as Challenge } from '../../models/challenge'; import { @@ -383,14 +384,12 @@ api.scoreTask = { // If a todo was completed or uncompleted move it in or out of the user.tasksOrder.todos list if (task.type === 'todo') { if (!wasCompleted && task.completed) { - let i = user.tasksOrder.todos.indexOf(task._id); - if (i !== -1) user.tasksOrder.todos.splice(i, 1); + removeFromArray(user.tasksOrder.todos, task._id); } else if (wasCompleted && !task.completed) { - let i = user.tasksOrder.todos.indexOf(task._id); - if (i === -1) { + let hasTask = removeFromArray(user.tasksOrder.todos, task._id); + if (!hasTask) { user.tasksOrder.todos.push(task._id); // TODO push at the top? } else { // If for some reason it hadn't been removed TODO ok? - user.tasksOrder.todos.splice(i, 1); user.tasksOrder.push(task._id); } } @@ -684,10 +683,8 @@ api.removeChecklistItem = { } if (task.type !== 'daily' && task.type !== 'todo') throw new BadRequest(res.t('checklistOnlyDailyTodo')); - let itemI = _.findIndex(task.checklist, {_id: req.params.itemId}); - if (itemI === -1) throw new NotFound(res.t('checklistItemNotFound')); - - task.checklist.splice(itemI, 1); + let hasItem = removeFromArray(task.checklist, { _id: req.params.itemId }); + if (!hasItem) throw new NotFound(res.t('checklistItemNotFound')); let savedTask = await task.save(); res.respond(200, {}); // TODO what to return @@ -769,24 +766,14 @@ api.removeTagFromTask = { if (!task) throw new NotFound(res.t('taskNotFound')); - let tagI = task.tags.indexOf(req.params.tagId); - if (tagI === -1) throw new NotFound(res.t('tagNotFound')); - - task.tags.splice(tagI, 1); + let hasTag = removeFromArray(task.tags, req.params.tagId); + if (!hasTag) throw new NotFound(res.t('tagNotFound')); await task.save(); res.respond(200, {}); // TODO what to return }, }; -// Remove a task from (user|challenge).tasksOrder -function _removeTaskTasksOrder (userOrChallenge, taskId, taskType) { - let list = userOrChallenge.tasksOrder[`${taskType}s`]; - let index = list.indexOf(taskId); - - if (index !== -1) list.splice(index, 1); -} - // TODO this method needs some limitation, like to check if the challenge is really broken? /** * @api {post} /tasks/unlink/:taskId Unlink a challenge task @@ -826,7 +813,7 @@ api.unlinkTask = { await task.save(); } else { // remove if (task.type !== 'todo' || !task.completed) { // eslint-disable-line no-lonely-if - _removeTaskTasksOrder(user, taskId, task.type); + removeFromArray(user.tasksOrder[`${task.type}s`], taskId); await Q.all([user.save(), task.remove()]); } else { await task.remove(); @@ -904,7 +891,7 @@ api.deleteTask = { } if (task.type !== 'todo' || !task.completed) { - _removeTaskTasksOrder(challenge || user, taskId, task.type); + removeFromArray((challenge || user).tasksOrder[`${task.type}s`], taskId); await Q.all([(challenge || user).save(), task.remove()]); } else { await task.remove(); diff --git a/website/src/libs/api-v3/collectionManipulators.js b/website/src/libs/api-v3/collectionManipulators.js new file mode 100644 index 0000000000..ce40085552 --- /dev/null +++ b/website/src/libs/api-v3/collectionManipulators.js @@ -0,0 +1,19 @@ +import { findIndex } from 'lodash'; + +export function removeFromArray (array, element) { + let elementIndex; + + if (typeof element === 'object') { + elementIndex = findIndex(array, element); + } else { + elementIndex = array.indexOf(element); + } + + if (elementIndex !== -1) { + let removedElement = array[elementIndex]; + array.splice(elementIndex, 1); + return removedElement; + } + + return false; +} diff --git a/website/src/models/group.js b/website/src/models/group.js index 8b11a57081..0acc03fe32 100644 --- a/website/src/models/group.js +++ b/website/src/models/group.js @@ -7,6 +7,7 @@ import shared from '../../../common'; import _ from 'lodash'; import { model as Challenge} from './challenge'; import validator from 'validator'; +import { removeFromArray } from '../libs/api-v3/collectionManipulators'; import * as firebase from '../libs/api-v2/firebase'; import baseModel from '../libs/api-v3/baseModel'; import Q from 'q'; @@ -147,8 +148,7 @@ schema.methods.removeGroupInvitations = async function removeGroupInvitations () if (group.type === 'party') { user.invitations.party = {}; // TODO mark modified } else { - let i = _.findIndex(user.invitations.guilds, {id: group._id}); - user.invitations.guilds.splice(i, 1); + removeFromArray(user.invitations.guilds, { id: group._id }); } return user.save(); }); diff --git a/website/src/models/user.js b/website/src/models/user.js index 6ea4ede281..db466ce829 100644 --- a/website/src/models/user.js +++ b/website/src/models/user.js @@ -6,6 +6,7 @@ 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'; @@ -661,8 +662,7 @@ schema.methods.unlinkChallengeTasks = async function unlinkChallengeTasks (chall 'challenge.id': challengeId, }; - let challengeIndex = user.challenges.indexOf(challengeId); - if (challengeIndex !== -1) user.challenges.splice(challengeIndex, 1); + removeFromArray(user.challenges, challengeId); if (keep === 'keep-all') { await Tasks.Task.update(findQuery, { @@ -675,9 +675,7 @@ schema.methods.unlinkChallengeTasks = async function unlinkChallengeTasks (chall let taskPromises = tasks.map(task => { // Remove task from user.tasksOrder and delete them if (task.type !== 'todo' || !task.completed) { - let list = user.tasksOrder[`${task.type}s`]; - let index = list.indexOf(task._id); - if (index !== -1) list.splice(index, 1); + removeFromArray(user.tasksOrder[`${task.type}s`], task._id); } return task.remove();