From f16b605c379daaf5b82bf59b32e9f7066720fe6c Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Wed, 27 Jan 2016 12:22:24 +0100 Subject: [PATCH] add update challenge with tests --- common/locales/en/api-v3.json | 1 + .../PUT-challenges_challengeId.test.js | 76 +++++++++++++++++++ website/src/controllers/api-v3/challenges.js | 41 +++++++++- website/src/controllers/api-v3/tasks.js | 2 +- website/src/models/challenge.js | 16 ++-- website/src/models/user.js | 2 +- 6 files changed, 130 insertions(+), 8 deletions(-) create mode 100644 test/api/v3/integration/challenges/PUT-challenges_challengeId.test.js diff --git a/common/locales/en/api-v3.json b/common/locales/en/api-v3.json index e6dc9bee55..a435098a1d 100644 --- a/common/locales/en/api-v3.json +++ b/common/locales/en/api-v3.json @@ -50,6 +50,7 @@ "winnerIdRequired": "\"winnerId\" must be a valid UUID.", "challengeNotFound": "Challenge not found.", "onlyLeaderDeleteChal": "Only the challenge leader can delete it.", + "onlyLeaderUpdateChal": "Only the challenge leader can update it.", "winnerNotFound": "Winner with id \"<%= userId %>\" not found or not part of the challenge.", "noCompletedTodosChallenge": "\"includeComepletedTodos\" is not supported when fetching a challenge tasks.", "userTasksNoChallengeId": "When \"tasksOwner\" is \"user\" \"challengeId\" can't be passed.", diff --git a/test/api/v3/integration/challenges/PUT-challenges_challengeId.test.js b/test/api/v3/integration/challenges/PUT-challenges_challengeId.test.js new file mode 100644 index 0000000000..e916ab5b7a --- /dev/null +++ b/test/api/v3/integration/challenges/PUT-challenges_challengeId.test.js @@ -0,0 +1,76 @@ +import { + generateUser, + generateChallenge, + createAndPopulateGroup, + translate as t, +} from '../../../../helpers/api-v3-integration.helper'; + +describe('PUT /challenges/:challengeId', () => { + let privateGuild, user, nonMember, challenge, member; + + beforeEach(async () => { + let { group, groupLeader, members } = await createAndPopulateGroup({ + groupDetails: { + name: 'TestPrivateGuild', + type: 'guild', + privacy: 'private', + }, + members: 1, + }); + + privateGuild = group; + user = groupLeader; + + nonMember = await generateUser(); + member = members[0]; + + challenge = await generateChallenge(user, group); + await member.post(`/challenges/${challenge._id}/join`); + }); + + it('fails if the user can\'t view the challenge', async () => { + await expect(nonMember.put(`/challenges/${challenge._id}`)) + .to.eventually.be.rejected.and.eql({ + code: 404, + error: 'NotFound', + message: t('challengeNotFound'), + }); + }); + + it('should only allow the leader or an admin to update the challenge', async () => { + await expect(member.put(`/challenges/${challenge._id}`)) + .to.eventually.be.rejected.and.eql({ + code: 401, + error: 'NotAuthorized', + message: t('onlyLeaderUpdateChal'), + }); + }); + + it('only updates allowed fields', async () => { + let res = await user.put(`/challenges/${challenge._id}`, { + // ignored + prize: 33, + groupId: 'blabla', + memberCount: 33, + tasksOrder: 'new order', + official: true, + shortName: 'new short name', + + // applied + name: 'New Challenge Name', + description: 'New challenge description.', + leader: member._id, + }); + + expect(res.prize).to.equal(0); + expect(res.groupId).to.equal(privateGuild._id); + expect(res.memberCount).to.equal(2); + expect(res.tasksOrder).not.to.equal('new order'); + expect(res.official).to.equal(false); + expect(res.shortName).not.to.equal('new short name'); + + expect(res.leader).to.equal(member._id); + expect(res.name).to.equal('New Challenge Name'); + expect(res.description).to.equal('New challenge description.'); + }); +}); diff --git a/website/src/controllers/api-v3/challenges.js b/website/src/controllers/api-v3/challenges.js index bd7f694cf6..172ff5783b 100644 --- a/website/src/controllers/api-v3/challenges.js +++ b/website/src/controllers/api-v3/challenges.js @@ -3,7 +3,9 @@ import _ from 'lodash'; import cron from '../../middlewares/api-v3/cron'; import { model as Challenge } from '../../models/challenge'; import { model as Group } from '../../models/group'; -import { model as User } from '../../models/user'; +import { + model as User, +} from '../../models/user'; import { NotFound, NotAuthorized, @@ -199,6 +201,43 @@ api.getChallenge = { }, }; +/** + * @api {put} /challenges/:challengeId Update a challenge + * @apiVersion 3.0.0 + * @apiName UpdateChallenge + * @apiGroup Challenge + * + * @apiParam {UUID} challengeId The challenge _id + * + * @apiSuccess {object} challenge The updated challenge object + */ +api.updateChallenge = { + method: 'PUT', + url: '/challenges/:challengeId', + middlewares: [authWithHeaders(), cron], + async handler (req, res) { + req.checkParams('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID(); + + let validationErrors = req.validationErrors(); + if (validationErrors) throw validationErrors; + + let user = res.locals.user; + let challengeId = req.params.challengeId; + + let challenge = await Challenge.findById(challengeId).exec(); + if (!challenge) throw new NotFound(res.t('challengeNotFound')); + + let group = await Group.getGroup({user, groupId: challenge.groupId, fields: '_id name type privacy', optionalMembership: true}); + if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound')); + if (!challenge.canModify(user)) throw new NotAuthorized(res.t('onlyLeaderUpdateChal')); + + _.merge(challenge, Challenge.sanitizeUpdate(req.body)); + + let savedChal = await challenge.save(); + res.respond(200, savedChal); + }, +}; + // TODO everything here should be moved to a worker // actually even for a worker it's probably just to big and will kill mongo function _closeChal (challenge, broken = {}) { diff --git a/website/src/controllers/api-v3/tasks.js b/website/src/controllers/api-v3/tasks.js index 2e72953ad1..0197d38dbb 100644 --- a/website/src/controllers/api-v3/tasks.js +++ b/website/src/controllers/api-v3/tasks.js @@ -302,7 +302,7 @@ api.updateTask = { delete req.body.tags; } - // TODO we have to convert task to an object because otherwise thigns doesn't get merged correctly, very bad for performances + // TODO we have to convert task to an object because otherwise thigns doesn't get merged correctly, bad for performances? // TODO regarding comment above make sure other models with nested fields are using this trick too _.assign(task, _.merge(task.toObject(), Tasks.Task.sanitizeUpdate(req.body))); // TODO console.log(task.modifiedPaths(), task.toObject().repeat === tep) diff --git a/website/src/models/challenge.js b/website/src/models/challenge.js index ed9a88213e..c044caaba6 100644 --- a/website/src/models/challenge.js +++ b/website/src/models/challenge.js @@ -10,9 +10,9 @@ let Schema = mongoose.Schema; let schema = new Schema({ name: {type: String, required: true}, - shortName: {type: String, required: true}, // TODO what is it? + shortName: {type: String, required: true}, description: String, - official: {type: Boolean, default: false}, // TODO only settable by admin + official: {type: Boolean, default: false}, tasksOrder: { habits: [{type: String, ref: 'Task'}], dailys: [{type: String, ref: 'Task'}], @@ -20,16 +20,22 @@ let schema = new Schema({ rewards: [{type: String, ref: 'Task'}], }, leader: {type: String, ref: 'User', validate: [validator.isUUID, 'Invalid uuid.'], required: true}, - groupId: {type: String, ref: 'Group', validate: [validator.isUUID, 'Invalid uuid.'], required: true}, // TODO no update, no set? - timestamp: {type: Date, default: Date.now, required: true}, // TODO what is this? use timestamps from plugin? not settable? + groupId: {type: String, ref: 'Group', validate: [validator.isUUID, 'Invalid uuid.'], required: true}, memberCount: {type: Number, default: 1}, prize: {type: Number, default: 0, min: 0}, // TODO no update? }); schema.plugin(baseModel, { - noSet: ['_id', 'memberCount', 'challengeCount', 'tasksOrder'], + noSet: ['_id', 'memberCount', 'tasksOrder'], + timestamps: true, }); +// A list of additional fields that cannot be updated (but can be set on creation) +let noUpdate = ['groupId', 'official', 'shortName', 'prize']; +schema.statics.sanitizeUpdate = function sanitizeUpdate (updateObj) { + return this.sanitize(updateObj, noUpdate); +}; + // Returns true if user is a member of the challenge schema.methods.isMember = function isChallengeMember (user) { return user.challenges.indexOf(this._id) !== -1; diff --git a/website/src/models/user.js b/website/src/models/user.js index db466ce829..cc1ebccf54 100644 --- a/website/src/models/user.js +++ b/website/src/models/user.js @@ -654,7 +654,7 @@ schema.methods.isSubscribed = function isSubscribed () { return !!this.purchased.plan.customerId; // eslint-disable-line no-implicit-coercion }; -// Unlink challenges tasks from user +// Unlink challenges tasks (and the challenge itself) from user schema.methods.unlinkChallengeTasks = async function unlinkChallengeTasks (challengeId, keep) { let user = this; let findQuery = {