resolve updateSetting tests with current logic

clairep94 · clairep94 · commit 899c10c1ec90 · 2025-10-10T07:35:49.000-04:00
diff --git a/server/controllers/user.controller/__tests__/authManagement/updateSettings.test.ts b/server/controllers/user.controller/__tests__/authManagement/updateSettings.test.ts
@@ -7,18 +7,19 @@ import { saveUser, generateToken, userResponse } from '../../helpers';
 import { createMockUser } from '../../__testUtils__';
 
 import { mailerService } from '../../../../utils/mail';
-import { UserDocument } from '../../../../types';
+import { UpdateSettingsRequestBody, UserDocument } from '../../../../types';
 
 jest.mock('../../../../models/user');
 jest.mock('../../../../utils/mail');
 jest.mock('../../helpers', () => ({
-  ...jest.requireActual('../../helpers'),
+  ...jest.requireActual('../../helpers'), // use actual userResponse
   saveUser: jest.fn(),
   generateToken: jest.fn()
 }));
 
-describe('user.controller > auth management', () => {
+describe('user.controller > auth management > updateSettings (email, username, password)', () => {
   let request: any;
+  let requestBody: UpdateSettingsRequestBody;
   let response: any;
   let next: MockNext;
   const fixedTime = 100000000;
@@ -69,13 +70,31 @@ describe('user.controller > auth management', () => {
 
     // the below tests match the current logic, but logic can be improved
     describe('if the user is found', () => {
+      const OLD_USERNAME = 'oldusername';
+      const NEW_USERNAME = 'newusername';
+
+      const OLD_EMAIL = 'old@email.com';
+      const NEW_EMAIL = 'new@email.com';
+
+      const OLD_PASSWORD = 'oldpassword';
+      const NEW_PASSWORD = 'newpassword';
+
       const startingUser = createMockUser({
-        username: 'oldusername',
-        email: 'old@email.com',
-        id: 'valid-id',
+        username: OLD_USERNAME,
+        email: OLD_EMAIL,
+        password: OLD_PASSWORD,
+        id: '123459',
         comparePassword: jest.fn().mockResolvedValue(true)
       });
 
+      // minimum valid request body to manipulate per test
+      // from manual testing on the account form:
+      // both username and email are required & there is client-side validation for valid email & username-taken prior to submit
+      const minimumValidRequest: UpdateSettingsRequestBody = {
+        username: OLD_USERNAME,
+        email: OLD_EMAIL
+      };
+
       beforeEach(() => {
         User.findById = jest.fn().mockResolvedValue(startingUser);
 
@@ -85,40 +104,145 @@ describe('user.controller > auth management', () => {
         (generateToken as jest.Mock).mockResolvedValue('token12343');
       });
 
-      describe('and when there is a username in the request', () => {
-        beforeEach(async () => {
-          request.setBody({
-            username: 'newusername'
-          });
-          await updateSettings(request, response, next);
+      // Q: should we add check & logic that if no username or email are on the request,
+      // we fallback to the username and/or email on the found user for safety?
+      // not sure if anyone is hitting this api directly, so the client-side checks may not be enough
+
+      // duplicate username check happens client-side before this request is made
+      it('saves the user with any username in the request', async () => {
+        // saves with old username
+        requestBody = { ...minimumValidRequest, username: OLD_USERNAME };
+        request.setBody(requestBody);
+
+        await updateSettings(request, response, next);
+
+        expect(saveUser).toHaveBeenCalledWith(response, {
+          ...startingUser
         });
-        it('calls saveUser', () => {
-          expect(saveUser).toHaveBeenCalledWith(response, {
-            ...startingUser,
-            username: 'newusername'
-          });
+
+        // saves with new username
+        requestBody = { ...minimumValidRequest, username: NEW_USERNAME };
+        request.setBody(requestBody);
+
+        await updateSettings(request, response, next);
+
+        expect(saveUser).toHaveBeenCalledWith(response, {
+          ...startingUser,
+          username: NEW_USERNAME
         });
       });
 
-      // currently frontend doesn't seem to call the below
-      describe('and when there is a newPassword in the request', () => {
-        beforeEach(async () => {});
+      // currently frontend doesn't seem to call password-change related things the below
+      // not sure if we should update the logic to be cleaner?
+      describe('and when there is a new password in the request', () => {
+        beforeEach(() => {
+          requestBody = { ...minimumValidRequest, newPassword: NEW_PASSWORD };
+        });
         describe('and the current password is not provided', () => {
-          it('returns 401 with a "current password not provided" message', () => {});
-          it('does not save the user with the new password', () => {});
+          beforeEach(async () => {
+            request.setBody(requestBody);
+            await updateSettings(request, response, next);
+          });
+          it('returns 401 with a "current password not provided" message', () => {
+            expect(response.status).toHaveBeenCalledWith(401);
+            expect(response.json).toHaveBeenCalledWith({
+              error: 'Current password is not provided.'
+            });
+          });
+          it('does not save the user with the new password', () => {
+            expect(saveUser).not.toHaveBeenCalled();
+          });
         });
       });
+
+      // this should be nested in the previous block but currently here to match existing logic as-is
+      // NOTE: will make a PR into this branch to propose the change
       describe('and when there is a currentPassword in the request', () => {
         describe('and the current password does not match', () => {
-          beforeEach(async () => {});
-          it('returns 401 with a "current password invalid" message', () => {});
-          it('does not save the user with the new password', () => {});
+          beforeEach(async () => {
+            requestBody = {
+              ...minimumValidRequest,
+              newPassword: NEW_PASSWORD,
+              currentPassword: 'SOMETHING ELSE'
+            };
+
+            // simulate check for password match failing
+            startingUser.comparePassword = jest.fn().mockResolvedValue(false);
+
+            request.setBody(requestBody);
+            await updateSettings(request, response, next);
+          });
+
+          it('returns 401 with a "current password invalid" message', () => {
+            expect(response.status).toHaveBeenCalledWith(401);
+            expect(response.json).toHaveBeenCalledWith({
+              error: 'Current password is invalid.'
+            });
+          });
+          it('does not save the user with the new password', () => {
+            expect(saveUser).not.toHaveBeenCalled();
+          });
         });
         describe('and when the current password does match', () => {
-          beforeEach(async () => {});
-          it('calls saveUser with the new password', () => {});
+          beforeEach(async () => {
+            requestBody = {
+              ...minimumValidRequest,
+              newPassword: NEW_PASSWORD,
+              currentPassword: OLD_PASSWORD
+            };
+
+            // simulate check for password match passing
+            startingUser.comparePassword = jest.fn().mockResolvedValue(true);
+
+            request.setBody(requestBody);
+            await updateSettings(request, response, next);
+          });
+          it('calls saveUser with the new password', () => {
+            expect(saveUser).toHaveBeenCalledWith(response, {
+              ...startingUser,
+              password: NEW_PASSWORD
+            });
+          });
+        });
+
+        // NOTE: This should not pass, but it currently does!!
+        describe('and when there is no new password on the request', () => {
+          beforeEach(async () => {
+            requestBody = {
+              ...minimumValidRequest,
+              newPassword: undefined,
+              currentPassword: OLD_PASSWORD
+            };
+
+            // simulate check for password match passing
+            startingUser.comparePassword = jest.fn().mockResolvedValue(true);
+
+            request.setBody(requestBody);
+            await updateSettings(request, response, next);
+          });
+          it('calls saveUser with the new empty password', () => {
+            expect(saveUser).toHaveBeenCalledWith(response, {
+              ...startingUser,
+              password: undefined
+            });
+          });
         });
       });
+
+      describe('and when there is an email in the request', () => {
+        describe('and the email is the same as the old email', () => {
+          it('saves the user with the old email', () => {});
+          it('does not send a verification email', () => {});
+        });
+        describe('and the email is not the same as the old email', () => {
+          it('saves the user with the new email', () => {});
+          it('sends a verification email', () => {});
+        });
+      });
+
+      describe('and when there is any other error', () => {
+        it('returns a 500 error', () => {});
+      });
     });
   });
 });
