From b5bcbc4fc871005bf1f5a4a7ad0a8884c0803863 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 2 Nov 2017 17:54:56 -0600 Subject: [PATCH] added username edit, tests --- graph/mutators/user.js | 19 +----- services/users.js | 77 ++++++++++++++++++++++- test/server/services/users.js | 115 ++++++++++++++++++---------------- 3 files changed, 138 insertions(+), 73 deletions(-) diff --git a/graph/mutators/user.js b/graph/mutators/user.js index 44947a390..4d4350739 100644 --- a/graph/mutators/user.js +++ b/graph/mutators/user.js @@ -8,24 +8,7 @@ const { } = require('../../perms/constants'); const setUserUsernameStatus = async (ctx, id, status) => { - const user = await UserModel.findOneAndUpdate({id}, { - $set: { - 'status.username.status': status - }, - $push: { - 'status.username.history': { - status, - assigned_by: ctx.user.id, - created_at: Date.now() - } - } - }, { - new: true - }); - if (user === null) { - throw errors.ErrNotFound; - } - + const user = await UsersService.setUsernameStatus(id, status, ctx.user.id); if (status === 'REJECTED') { ctx.pubsub.publish('usernameRejected', user); } diff --git a/services/users.js b/services/users.js index 0ecf35582..ca09c8f7a 100644 --- a/services/users.js +++ b/services/users.js @@ -60,7 +60,7 @@ module.exports = class UsersService { } /** - * This records an unsucesfull login attempt for the given email address. If + * This records an unsuccessful login attempt for the given email address. If * the maximum has been reached, the promise will be rejected with: * * errors.ErrLoginAttemptMaximumExceeded @@ -80,8 +80,81 @@ module.exports = class UsersService { } } + static async setUsernameStatus(id, status, assignedBy = null) { + const user = await UserModel.findOneAndUpdate({id}, { + $set: { + 'status.username.status': status + }, + $push: { + 'status.username.history': { + status, + assigned_by: assignedBy, + created_at: Date.now() + } + } + }, { + new: true + }); + if (user === null) { + throw errors.ErrNotFound; + } + + return user; + } + + static async changeUsername(id, username) { + try { + let user = await UserModel.findOneAndUpdate({ + id, + username: {$ne: username}, + 'status.username.status': { + $in: ['UNSET', 'REJECTED'] + } + }, { + $set: { + username, + lowercaseUsername: username.toLowerCase(), + 'status.username.status': 'CHANGED', + }, + $push: { + 'status.username.history': { + status: 'CHANGED', + assigned_by: id, + created_at: Date.now() + } + } + }, { + new: true + }); + if (!user) { + user = await UsersService.findById(id); + if (user === null) { + throw errors.ErrNotFound; + } + + if (!['UNSET', 'REJECTED'].includes(user.status.username.status)) { + throw errors.ErrPermissionUpdateUsername; + } + + if (user.username === username) { + throw errors.ErrSameUsernameProvided; + } + + throw new Error('edit username failed for an unexpected reason'); + } + + return user; + } catch (err) { + if (err.code === 11000) { + throw errors.ErrUsernameTaken; + } + + throw err; + } + } + /** - * This checks to see if the current login attempts against a user exeeds the + * This checks to see if the current login attempts against a user exceeds the * maximum value allowed, if so, it rejects with: * * errors.ErrLoginAttemptMaximumExceeded diff --git a/test/server/services/users.js b/test/server/services/users.js index 72914e4c1..7f50c337c 100644 --- a/test/server/services/users.js +++ b/test/server/services/users.js @@ -223,63 +223,72 @@ describe('services.UsersService', () => { }); }); - // describe('#editName', () => { - // it('should let the user edit their username if the proper toggle is set', () => { - // return UsersService - // .toggleNameEdit(mockUsers[0].id, true) - // .then(() => UsersService.editName(mockUsers[0].id, 'Jojo')) - // .then(() => UsersService.findById(mockUsers[0].id)) - // .then((user) => { - // expect(user).to.have.property('username', 'Jojo'); - // expect(user).to.have.property('canEditName', false); - // }); - // }); + describe('#changeUsername', () => { + [ + {status: 'UNSET'}, + {status: 'REJECTED'}, + {error: 'EDIT_USERNAME_NOT_AUTHORIZED', status: 'SET'}, + {error: 'EDIT_USERNAME_NOT_AUTHORIZED', status: 'APPROVED'}, + {error: 'EDIT_USERNAME_NOT_AUTHORIZED', status: 'CHANGED'}, + ].forEach(({status, error}) => { + it(`${error ? 'should not' : 'should'} let them change the username if they have the status of ${status}`, async () => { + const user = mockUsers[0]; - // it('should let the user submit the same username if user is not banned (create username)', () => { - // return UsersService - // .toggleNameEdit(mockUsers[0].id, true) - // .then(() => UsersService.editName(mockUsers[0].id, mockUsers[0].username)) - // .then(() => UsersService.findById(mockUsers[0].id)) - // .then((user) => { - // expect(user).to.have.property('username', mockUsers[0].username); - // expect(user).to.have.property('canEditName', false); - // }); - // }); + // Set the user to the desired status. + await UsersService.setUsernameStatus(user.id, status); - // it('should return error when a banned user submits the same username (rejected username)', () => { - // return UsersService - // .toggleNameEdit(mockUsers[0].id, true) - // .then(() => UsersService.setStatus(mockUsers[0].id, 'BANNED')) - // .then(() => UsersService.editName(mockUsers[0].id, mockUsers[0].username)) - // .then(() => UsersService.findById(mockUsers[0].id)) - // .then(() => { - // throw new Error('Error expected'); - // }) - // .catch((err) => { - // expect(err.status).to.equal(400); - // expect(err.translation_key).to.equal('SAME_USERNAME_PROVIDED'); - // }); - // }); + try { + await UsersService.changeUsername(user.id, 'spock'); + } catch (err) { + if (error) { + expect(err).have.property('translation_key', error); + } else { + throw err; + } + } + }); + }); - // it('should return an error if canEditName is false', async () => { - // return expect(UsersService.editName(mockUsers[0].id, 'Jojo')).to.eventually.be.rejected; - // }); + it('should refuse changing the username to the same username', async () => { + const user = mockUsers[0]; - // it('should return an error if the username is already taken', async () => { - // await UsersService.toggleNameEdit(mockUsers[0].id, true); - // return expect(UsersService.editName(mockUsers[0].id, 'Marvel')).to.eventually.be.rejected; - // }); + // Set the user to the desired status. + await UsersService.setUsernameStatus(user.id, 'UNSET'); - // it('should not allow non-alphanumeric characters in usernames', () => { - // return UsersService - // .isValidUsername('hi🖕') - // .then(() => { - // expect(false).to.be.true; - // }) - // .catch((err) => { - // expect(err).to.be.ok; - // }); - // }); - // }); + try { + await UsersService.changeUsername(user.id, user.username); + throw new Error('edit was processed successfully'); + } catch (err) { + expect(err).have.property('translation_key', 'SAME_USERNAME_PROVIDED'); + } + }); + it('should refuse changing the username to one already taken', async () => { + const user = mockUsers[0]; + const otherUser = mockUsers[1]; + + // Set the user to the desired status. + await UsersService.setUsernameStatus(user.id, 'UNSET'); + + try { + await UsersService.changeUsername(user.id, otherUser.username); + throw new Error('edit was processed successfully'); + } catch (err) { + expect(err).have.property('translation_key', 'USERNAME_IN_USE'); + } + }); + }); + + describe('#isValidUsername', () => { + it('should not allow non-alphanumeric characters in usernames', () => { + return UsersService + .isValidUsername('hi🖕') + .then(() => { + expect(false).to.be.true; + }) + .catch((err) => { + expect(err).to.be.ok; + }); + }); + }); });