From 35be0705defd5cc9c0d2a09c6043e56226ee97bf Mon Sep 17 00:00:00 2001
From: Dennis Kigen <kigen.work@gmail.com>
Date: Wed, 20 Nov 2024 15:50:41 +0300
Subject: [PATCH] (fix) Fix change password modal

This PR fixes the change password modal as follows:

- Adds default values to the change password form schema - this is to prevent the form from being empty when the user clicks on the change password button.
- Adds min validation to the old password, new password and password confirmation fields to ensure that the user has entered a value.
- Adds the appropriate dependencies to the submit handler useCallback hook.
- Surfaces the error message returned from the change password API.
- Fixes a translation key/value pair.
---
 .../change-password.component.tsx             | 81 ++++++++++++-------
 .../change-password/change-password.modal.tsx | 79 +++++++++++-------
 .../change-password/change-password.test.tsx  | 31 +++----
 .../apps/esm-login-app/translations/en.json   |  1 +
 4 files changed, 116 insertions(+), 76 deletions(-)

diff --git a/packages/apps/esm-login-app/src/change-password/change-password.component.tsx b/packages/apps/esm-login-app/src/change-password/change-password.component.tsx
index 64f3864a1..c63376f0d 100644
--- a/packages/apps/esm-login-app/src/change-password/change-password.component.tsx
+++ b/packages/apps/esm-login-app/src/change-password/change-password.component.tsx
@@ -13,17 +13,29 @@ const ChangePassword: React.FC = () => {
   const { t } = useTranslation();
   const [isChangingPassword, setIsChangingPassword] = useState(false);
 
-  const oldPasswordValidation = z.string({
-    required_error: t('oldPasswordRequired', 'Old password is required'),
-  });
+  const oldPasswordValidation = z
+    .string({
+      required_error: t('oldPasswordRequired', 'Old password is required'),
+    })
+    .min(1, {
+      message: t('oldPasswordRequired', 'Old password is required'),
+    });
 
-  const newPasswordValidation = z.string({
-    required_error: t('newPasswordRequired', 'New password is required'),
-  });
+  const newPasswordValidation = z
+    .string({
+      required_error: t('newPasswordRequired', 'New password is required'),
+    })
+    .min(1, {
+      message: t('newPasswordRequired', 'New password is required'),
+    });
 
-  const passwordConfirmationValidation = z.string({
-    required_error: t('passwordConfirmationRequired', 'Password confirmation is required'),
-  });
+  const passwordConfirmationValidation = z
+    .string({
+      required_error: t('passwordConfirmationRequired', 'Password confirmation is required'),
+    })
+    .min(1, {
+      message: t('passwordConfirmationRequired', 'Password confirmation is required'),
+    });
 
   const changePasswordFormSchema = z
     .object({
@@ -42,31 +54,40 @@ const ChangePassword: React.FC = () => {
     formState: { errors },
   } = useForm({
     resolver: zodResolver(changePasswordFormSchema),
+    defaultValues: {
+      oldPassword: '',
+      newPassword: '',
+      passwordConfirmation: '',
+    },
   });
 
-  const onSubmit: SubmitHandler<z.infer<typeof changePasswordFormSchema>> = useCallback((data) => {
-    setIsChangingPassword(true);
+  const onSubmit: SubmitHandler<z.infer<typeof changePasswordFormSchema>> = useCallback(
+    (data) => {
+      setIsChangingPassword(true);
 
-    const { oldPassword, newPassword } = data;
+      const { oldPassword, newPassword } = data;
 
-    changeUserPassword(oldPassword, newPassword)
-      .then(() => {
-        showSnackbar({
-          title: t('passwordChangedSuccessfully', 'Password changed successfully'),
-          kind: 'success',
+      changeUserPassword(oldPassword, newPassword)
+        .then(() => {
+          showSnackbar({
+            title: t('passwordChangedSuccessfully', 'Password changed successfully'),
+            kind: 'success',
+          });
+        })
+        .catch((error) => {
+          const errorMessage = error?.responseBody?.message ?? error?.message;
+          showSnackbar({
+            kind: 'error',
+            subtitle: errorMessage,
+            title: t('errorChangingPassword', 'Error changing password'),
+          });
+        })
+        .finally(() => {
+          setIsChangingPassword(false);
         });
-      })
-      .catch((error) => {
-        showSnackbar({
-          kind: 'error',
-          subtitle: error?.message,
-          title: t('errorChangingPassword', 'Error changing password'),
-        });
-      })
-      .finally(() => {
-        setIsChangingPassword(false);
-      });
-  }, []);
+    },
+    [t],
+  );
 
   const onError = useCallback(() => setIsChangingPassword(false), []);
 
@@ -121,7 +142,7 @@ const ChangePassword: React.FC = () => {
           />
           <Button className={styles.submitButton} disabled={isChangingPassword} type="submit">
             {isChangingPassword ? (
-              <InlineLoading description={t('changingLanguage', 'Changing password') + '...'} />
+              <InlineLoading description={t('changingPassword', 'Changing password') + '...'} />
             ) : (
               <span>{t('change', 'Change Password')}</span>
             )}
diff --git a/packages/apps/esm-login-app/src/change-password/change-password.modal.tsx b/packages/apps/esm-login-app/src/change-password/change-password.modal.tsx
index bdfaee162..feb586837 100644
--- a/packages/apps/esm-login-app/src/change-password/change-password.modal.tsx
+++ b/packages/apps/esm-login-app/src/change-password/change-password.modal.tsx
@@ -16,17 +16,26 @@ const ChangePasswordModal: React.FC<ChangePasswordModalProps> = ({ close }) => {
   const { t } = useTranslation();
   const [isChangingPassword, setIsChangingPassword] = useState(false);
 
-  const oldPasswordValidation = z.string({
-    required_error: t('oldPasswordRequired', 'Old password is required'),
-  });
+  const oldPasswordValidation = z
+    .string({
+      required_error: t('oldPasswordRequired', 'Old password is required'),
+    })
+    .trim()
+    .min(1, t('oldPasswordRequired', 'Old password is required'));
 
-  const newPasswordValidation = z.string({
-    required_error: t('newPasswordRequired', 'New password is required'),
-  });
+  const newPasswordValidation = z
+    .string({
+      required_error: t('newPasswordRequired', 'New password is required'),
+    })
+    .trim()
+    .min(1, t('newPasswordRequired', 'New password is required'));
 
-  const passwordConfirmationValidation = z.string({
-    required_error: t('passwordConfirmationRequired', 'Password confirmation is required'),
-  });
+  const passwordConfirmationValidation = z
+    .string({
+      required_error: t('passwordConfirmationRequired', 'Password confirmation is required'),
+    })
+    .trim()
+    .min(1, t('passwordConfirmationRequired', 'Password confirmation is required'));
 
   const changePasswordFormSchema = z
     .object({
@@ -45,33 +54,41 @@ const ChangePasswordModal: React.FC<ChangePasswordModalProps> = ({ close }) => {
     formState: { errors },
   } = useForm({
     resolver: zodResolver(changePasswordFormSchema),
+    defaultValues: {
+      oldPassword: '',
+      newPassword: '',
+      passwordConfirmation: '',
+    },
   });
 
-  const onSubmit: SubmitHandler<z.infer<typeof changePasswordFormSchema>> = useCallback((data) => {
-    setIsChangingPassword(true);
-
-    const { oldPassword, newPassword } = data;
+  const onSubmit: SubmitHandler<z.infer<typeof changePasswordFormSchema>> = useCallback(
+    (data) => {
+      setIsChangingPassword(true);
+      const { oldPassword, newPassword } = data;
 
-    changeUserPassword(oldPassword, newPassword)
-      .then(() => {
-        close();
+      changeUserPassword(oldPassword, newPassword)
+        .then(() => {
+          close();
 
-        showSnackbar({
-          title: t('passwordChangedSuccessfully', 'Password changed successfully'),
-          kind: 'success',
+          showSnackbar({
+            title: t('passwordChangedSuccessfully', 'Password changed successfully'),
+            kind: 'success',
+          });
+        })
+        .catch((error) => {
+          const errorMessage = error?.responseBody?.message ?? error?.message;
+          showSnackbar({
+            kind: 'error',
+            subtitle: errorMessage,
+            title: t('errorChangingPassword', 'Error changing password'),
+          });
+        })
+        .finally(() => {
+          setIsChangingPassword(false);
         });
-      })
-      .catch((error) => {
-        showSnackbar({
-          kind: 'error',
-          subtitle: error?.message,
-          title: t('errorChangingPassword', 'Error changing password'),
-        });
-      })
-      .finally(() => {
-        setIsChangingPassword(false);
-      });
-  }, []);
+    },
+    [close, t],
+  );
 
   const onError = () => setIsChangingPassword(false);
 
diff --git a/packages/apps/esm-login-app/src/change-password/change-password.test.tsx b/packages/apps/esm-login-app/src/change-password/change-password.test.tsx
index 1314f0dc8..e427b98e8 100644
--- a/packages/apps/esm-login-app/src/change-password/change-password.test.tsx
+++ b/packages/apps/esm-login-app/src/change-password/change-password.test.tsx
@@ -1,50 +1,51 @@
 import React from 'react';
 import userEvent from '@testing-library/user-event';
 import { render, screen } from '@testing-library/react';
-import ChangePasswordModal from './change-password.modal';
 import { changeUserPassword } from './change-password.resource';
+import ChangePasswordModal from './change-password.modal';
+
+const mockClose = jest.fn();
+const mockChangeUserPassword = jest.mocked(changeUserPassword);
 
 jest.mock('./change-password.resource', () => ({
   changeUserPassword: jest.fn().mockResolvedValue({}),
 }));
 
-const mockClose = jest.fn();
-const mockChangeUserPassword = changeUserPassword as jest.Mock;
-
 describe('ChangePasswordModal', () => {
-  beforeEach(() => {
+  it('validates the form before submitting', async () => {
+    const user = userEvent.setup();
+
     render(<ChangePasswordModal close={mockClose} />);
-  });
 
-  it('validates the form before submitting', async () => {
     const submitButton = screen.getByRole('button', {
       name: /change/i,
     });
+
     const oldPasswordInput = screen.getByLabelText(/old password/i);
     const newPasswordInput = screen.getByLabelText(/^new password$/i);
     const confirmPasswordInput = screen.getByLabelText(/confirm new password/i);
 
-    expect(screen.getByRole('heading', { name: /Change password/i })).toBeInTheDocument();
+    expect(screen.getByRole('heading', { name: /change password/i })).toBeInTheDocument();
     expect(oldPasswordInput).toBeInTheDocument();
     expect(newPasswordInput).toBeInTheDocument();
     expect(confirmPasswordInput).toBeInTheDocument();
 
-    await userEvent.click(submitButton);
+    await user.click(submitButton);
 
     expect(screen.getByText(/old password is required/i)).toBeInTheDocument();
     expect(screen.getByText(/new password is required/i)).toBeInTheDocument();
     expect(screen.getByText(/password confirmation is required/i)).toBeInTheDocument();
 
-    await userEvent.type(oldPasswordInput, 'P@ssw0rd123!');
-    await userEvent.type(newPasswordInput, 'N3wP@ssw0rd456!');
-    await userEvent.type(confirmPasswordInput, 'N3wP@ssw0rd456');
+    await user.type(oldPasswordInput, 'P@ssw0rd123!');
+    await user.type(newPasswordInput, 'N3wP@ssw0rd456!');
+    await user.type(confirmPasswordInput, 'N3wP@ssw0rd456');
 
     expect(screen.getByText(/passwords do not match/i)).toBeInTheDocument();
 
-    await userEvent.clear(confirmPasswordInput);
-    await userEvent.type(confirmPasswordInput, 'N3wP@ssw0rd456!');
+    await user.clear(confirmPasswordInput);
+    await user.type(confirmPasswordInput, 'N3wP@ssw0rd456!');
 
-    await userEvent.click(submitButton);
+    await user.click(submitButton);
 
     expect(mockChangeUserPassword).toHaveBeenCalledTimes(1);
     expect(mockChangeUserPassword).toHaveBeenCalledWith('P@ssw0rd123!', 'N3wP@ssw0rd456!');
diff --git a/packages/apps/esm-login-app/translations/en.json b/packages/apps/esm-login-app/translations/en.json
index 50242fbd7..3266b260c 100644
--- a/packages/apps/esm-login-app/translations/en.json
+++ b/packages/apps/esm-login-app/translations/en.json
@@ -5,6 +5,7 @@
   "change": "Change",
   "changePassword": "Change password",
   "changingLanguage": "Changing password",
+  "changingPassword": "Changing password",
   "confirmPassword": "Confirm new password",
   "continue": "Continue",
   "errorChangingPassword": "Error changing password",