Skip to content

Pr05/migrate user controller oct proposal 2 #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: pr05/migrate_user_controller_oct
Choose a base branch
from
Open

Pr05/migrate user controller oct proposal 2 #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f10b226
add proposal for updated logic for updateSettings
clairep94 Oct 11, 2025
8132e42
update to enable user to save both email/username change with passwor…
clairep94 Oct 11, 2025
343d696
cleanup
clairep94 Oct 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 40 additions & 57 deletions server/controllers/user.controller/__tests__/authManagement/updateSettings.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ describe('user.controller > auth management > updateSettings (email, username, p
const OLD_PASSWORD = 'oldpassword';
const NEW_PASSWORD = 'newpassword';

// minimum valid request body
const minimumValidRequest: UpdateSettingsRequestBody = {
username: OLD_USERNAME,
email: OLD_EMAIL
Expand Down Expand Up @@ -90,15 +89,14 @@ describe('user.controller > auth management > updateSettings (email, username, p
beforeEach(async () => {
(User.findById as jest.Mock).mockResolvedValue(null);
request.user = { id: 'nonexistent-id' };
request.body = minimumValidRequest;

await updateSettings(request, response, next);
});

it('returns 404 and a user-not-found error', async () => {
it('returns 404 and a user-not-found error', () => {
expect(response.status).toHaveBeenCalledWith(404);
expect(response.json).toHaveBeenCalledWith({
error: 'User not found'
});
expect(response.json).toHaveBeenCalledWith({ error: 'User not found' });
});

it('does not save the user', () => {
Expand All @@ -114,29 +112,32 @@ describe('user.controller > auth management > updateSettings (email, username, p
request.setBody(requestBody);
await updateSettings(request, response, next);
});

it('saves the user with the correct details exactly once', () => {
expect(saveUser).toHaveBeenCalledWith(response, { ...startingUser });
expect(saveUser).toHaveBeenCalledTimes(1);
});

it('does not send a confirmation email to the user', () => {
expect(mailerService.send).not.toHaveBeenCalled();
});
});

// duplicate username check happens client-side before this request is made
describe('when given new username and old email', () => {
beforeEach(async () => {
requestBody = { ...minimumValidRequest, username: NEW_USERNAME };
request.setBody(requestBody);
await updateSettings(request, response, next);
});

it('saves the user with the correct details exactly once', () => {
expect(saveUser).toHaveBeenCalledWith(response, {
...startingUser,
username: NEW_USERNAME
});
expect(saveUser).toHaveBeenCalledTimes(1);
});

it('does not send a confirmation email to the user', () => {
expect(mailerService.send).not.toHaveBeenCalled();
});
Expand All @@ -148,6 +149,7 @@ describe('user.controller > auth management > updateSettings (email, username, p
request.setBody(requestBody);
await updateSettings(request, response, next);
});

it('saves the user with the correct details & verification token once', () => {
expect(saveUser).toHaveBeenCalledWith(response, {
...startingUser,
Expand All @@ -158,6 +160,7 @@ describe('user.controller > auth management > updateSettings (email, username, p
});
expect(saveUser).toHaveBeenCalledTimes(1);
});

it('sends a confirmation email to the user', () => {
expect(mailerService.send).toHaveBeenCalledWith(
expect.objectContaining({
Expand All @@ -173,6 +176,7 @@ describe('user.controller > auth management > updateSettings (email, username, p
request.setBody(requestBody);
await updateSettings(request, response, next);
});

it('saves the user with the correct details once', () => {
expect(saveUser).toHaveBeenCalledWith(response, {
...startingUser,
Expand All @@ -184,6 +188,7 @@ describe('user.controller > auth management > updateSettings (email, username, p
});
expect(saveUser).toHaveBeenCalledTimes(1);
});

it('sends a confirmation email to the user', () => {
expect(mailerService.send).toHaveBeenCalledWith(
expect.objectContaining({
Expand All @@ -203,13 +208,15 @@ describe('user.controller > auth management > updateSettings (email, username, p
request.setBody(requestBody);
await updateSettings(request, response, next);
});

it('saves the user with the correct details once', () => {
expect(saveUser).toHaveBeenCalledWith(response, {
...startingUser,
password: NEW_PASSWORD
});
expect(saveUser).toHaveBeenCalledTimes(1);
});

it('does not send a confirmation email to the user', () => {
expect(mailerService.send).not.toHaveBeenCalled();
});
Expand All @@ -226,6 +233,7 @@ describe('user.controller > auth management > updateSettings (email, username, p
request.setBody(requestBody);
await updateSettings(request, response, next);
});

it('saves the user with the correct details once', () => {
expect(saveUser).toHaveBeenCalledWith(response, {
...startingUser,
Expand All @@ -234,12 +242,13 @@ describe('user.controller > auth management > updateSettings (email, username, p
});
expect(saveUser).toHaveBeenCalledTimes(1);
});

it('does not send a confirmation email to the user', () => {
expect(mailerService.send).not.toHaveBeenCalled();
});
});

describe.skip('when given old username, new email, and new password with valid current password', () => {
describe('when given old username, new email, and new password with valid current password', () => {
beforeEach(async () => {
requestBody = {
...minimumValidRequest,
Expand All @@ -250,6 +259,7 @@ describe('user.controller > auth management > updateSettings (email, username, p
request.setBody(requestBody);
await updateSettings(request, response, next);
});

it('saves the user with the correct details once', () => {
expect(saveUser).toHaveBeenCalledWith(response, {
...startingUser,
Expand All @@ -261,6 +271,7 @@ describe('user.controller > auth management > updateSettings (email, username, p
});
expect(saveUser).toHaveBeenCalledTimes(1);
});

it('sends a confirmation email to the user', () => {
expect(mailerService.send).toHaveBeenCalledWith(
expect.objectContaining({
Expand All @@ -270,7 +281,7 @@ describe('user.controller > auth management > updateSettings (email, username, p
});
});

describe.skip('when given new username, new email, and new password with valid current password', () => {
describe('when given new username, new email, and new password with valid current password', () => {
beforeEach(async () => {
requestBody = {
username: NEW_USERNAME,
Expand All @@ -281,6 +292,7 @@ describe('user.controller > auth management > updateSettings (email, username, p
request.setBody(requestBody);
await updateSettings(request, response, next);
});

it('saves the user with the correct details once', () => {
expect(saveUser).toHaveBeenCalledWith(response, {
...startingUser,
Expand All @@ -293,6 +305,7 @@ describe('user.controller > auth management > updateSettings (email, username, p
});
expect(saveUser).toHaveBeenCalledTimes(1);
});

it('sends a confirmation email to the user', () => {
expect(mailerService.send).toHaveBeenCalledWith(
expect.objectContaining({
Expand All @@ -304,30 +317,29 @@ describe('user.controller > auth management > updateSettings (email, username, p
});

describe('unhappy paths', () => {
// Client-side checks to require username
describe.skip('when missing username', () => {
describe('when missing username', () => {
beforeEach(async () => {
request.setBody({ email: OLD_EMAIL });
await updateSettings(request, response, next);
});

it('returns 401 with an "Missing username" message', () => {
expect(response.status).toHaveBeenCalledWith(400);
expect(response.status).toHaveBeenCalledWith(401);
expect(response.json).toHaveBeenCalledWith({
error: 'Username is required.'
});
});

it('does not save the user with the new password', () => {
it('does not save the user', () => {
expect(saveUser).not.toHaveBeenCalled();
});

it('does not send a confirmation email to the user', () => {
expect(mailerService.send).not.toHaveBeenCalled();
});
});

// Client-side checks to require email
describe.skip('when missing email', () => {
describe('when missing email', () => {
beforeEach(async () => {
request.setBody({ username: OLD_USERNAME });
await updateSettings(request, response, next);
Expand All @@ -340,63 +352,32 @@ describe('user.controller > auth management > updateSettings (email, username, p
});
});

it('does not save the user with the new password', () => {
it('does not save the user', () => {
expect(saveUser).not.toHaveBeenCalled();
});
it('does not send a confirmation email to the user', () => {
expect(mailerService.send).not.toHaveBeenCalled();
});
});

// Client-side checks to require new password if current password is provided
describe.skip('when given old username, old email, and matching current password and no new password', () => {
beforeEach(async () => {
requestBody = {
...minimumValidRequest,
currentPassword: OLD_PASSWORD
};
request.setBody(requestBody);
await updateSettings(request, response, next);
});

it('returns 401 with an "New password is required" message', () => {
expect(response.status).toHaveBeenCalledWith(401);
expect(response.json).toHaveBeenCalledWith({
error: 'New password is required.'
});
});

it('does not save the user with the new password', () => {
expect(saveUser).not.toHaveBeenCalled();
});
it('does not send a confirmation email to the user', () => {
expect(mailerService.send).not.toHaveBeenCalled();
});
});

describe('when given old username, old email, and non-matching current password and no new password', () => {
describe('when given old username, old email, and matching current password and no new password', () => {
beforeEach(async () => {
testUser.comparePassword = jest.fn().mockResolvedValue(false);

requestBody = {
...minimumValidRequest,
currentPassword: 'not the same password',
newPassword: NEW_PASSWORD
currentPassword: OLD_PASSWORD
};
request.setBody(requestBody);
await updateSettings(request, response, next);
});

it('returns 401 with an "Current password is invalid" message', () => {
expect(response.status).toHaveBeenCalledWith(401);
expect(response.json).toHaveBeenCalledWith({
error: 'Current password is invalid.'
it('saves the user with the correct details once', () => {
expect(saveUser).toHaveBeenCalledWith(response, {
...startingUser
});
expect(saveUser).toHaveBeenCalledTimes(1);
});

it('does not save the user with the new password', () => {
expect(saveUser).not.toHaveBeenCalled();
});
it('does not send a confirmation email to the user', () => {
expect(mailerService.send).not.toHaveBeenCalled();
});
Expand All @@ -405,10 +386,9 @@ describe('user.controller > auth management > updateSettings (email, username, p
describe('when given old username, old email, and non-matching current password and a new password', () => {
beforeEach(async () => {
testUser.comparePassword = jest.fn().mockResolvedValue(false);

requestBody = {
...minimumValidRequest,
currentPassword: 'not the same password',
currentPassword: 'wrong',
newPassword: NEW_PASSWORD
};
request.setBody(requestBody);
Expand All @@ -422,9 +402,10 @@ describe('user.controller > auth management > updateSettings (email, username, p
});
});

it('does not save the user with the new password', () => {
it('does not save the user', () => {
expect(saveUser).not.toHaveBeenCalled();
});

it('does not send a confirmation email to the user', () => {
expect(mailerService.send).not.toHaveBeenCalled();
});
Expand All @@ -447,9 +428,10 @@ describe('user.controller > auth management > updateSettings (email, username, p
});
});

it('does not save the user with the new password', () => {
it('does not save the user', () => {
expect(saveUser).not.toHaveBeenCalled();
});

it('does not send a confirmation email to the user', () => {
expect(mailerService.send).not.toHaveBeenCalled();
});
Expand All @@ -459,11 +441,12 @@ describe('user.controller > auth management > updateSettings (email, username, p

describe('and when there is any other error', () => {
beforeEach(async () => {
User.findById = jest.fn().mockRejectedValue('db error');
(User.findById as jest.Mock).mockRejectedValue('db error');
requestBody = minimumValidRequest;
request.setBody(requestBody);
await updateSettings(request, response, next);
});

it('returns a 500 error', () => {
expect(response.status).toHaveBeenCalledWith(500);
expect(response.json).toHaveBeenCalledWith({ error: 'db error' });
Expand Down
Loading
Loading