(fix) Fix change password modal (#1210)

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.
openmrs · Nov 20, 2024 · 4286482 · 4286482
1 parent 841f0d6
commit 4286482
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 70 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
@@ -44,29 +44,32 @@ const ChangePassword: React.FC = () => {
     resolver: zodResolver(changePasswordFormSchema),
   });
 
-  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) => {
+          showSnackbar({
+            kind: 'error',
+            subtitle: error?.message,
+            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 +124,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
@@ -3,7 +3,17 @@ import { useTranslation } from 'react-i18next';
 import { z } from 'zod';
 import { zodResolver } from '@hookform/resolvers/zod';
 import { Controller, useForm, type SubmitHandler } from 'react-hook-form';
-import { Button, Form, PasswordInput, InlineLoading, ModalBody, ModalFooter, ModalHeader, Stack } from '@carbon/react';
+import {
+  Button,
+  Form,
+  InlineLoading,
+  InlineNotification,
+  ModalBody,
+  ModalFooter,
+  ModalHeader,
+  PasswordInput,
+  Stack,
+} from '@carbon/react';
 import { showSnackbar } from '@openmrs/esm-framework';
 import { changeUserPassword } from './change-password.resource';
 import styles from './change-password-modal.scss';
@@ -15,18 +25,28 @@ interface ChangePasswordModalProps {
 const ChangePasswordModal: React.FC<ChangePasswordModalProps> = ({ close }) => {
   const { t } = useTranslation();
   const [isChangingPassword, setIsChangingPassword] = useState(false);
+  const [errorMessage, setErrorMessage] = useState(null);
 
-  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 +65,37 @@ 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',
-        });
-      })
-      .catch((error) => {
-        showSnackbar({
-          kind: 'error',
-          subtitle: error?.message,
-          title: t('errorChangingPassword', 'Error changing password'),
+          showSnackbar({
+            title: t('passwordChangedSuccessfully', 'Password changed successfully'),
+            kind: 'success',
+          });
+        })
+        .catch((error) => {
+          const errorMessage = error?.responseBody?.message ?? error?.message;
+          setErrorMessage(errorMessage);
+        })
+        .finally(() => {
+          setIsChangingPassword(false);
         });
-      })
-      .finally(() => {
-        setIsChangingPassword(false);
-      });
-  }, []);
+    },
+    [close, t],
+  );
 
   const onError = () => setIsChangingPassword(false);
 
@@ -122,6 +146,14 @@ const ChangePasswordModal: React.FC<ChangePasswordModalProps> = ({ close }) => {
               />
             )}
           />
+          {errorMessage && (
+            <InlineNotification
+              kind="error"
+              onClick={() => setErrorMessage('')}
+              subtitle={errorMessage}
+              title={t('errorChangingPassword', 'Error changing password')}
+            />
+          )}
         </Stack>
       </ModalBody>
       <ModalFooter>
@@ -130,7 +162,7 @@ const ChangePasswordModal: React.FC<ChangePasswordModalProps> = ({ close }) => {
         </Button>
         <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')}</span>
           )}

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
@@ -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
@@ -4,7 +4,7 @@
   "cancel": "Cancel",
   "change": "Change",
   "changePassword": "Change password",
-  "changingLanguage": "Changing password",
+  "changingPassword": "Changing password",
   "confirmPassword": "Confirm new password",
   "continue": "Continue",
   "errorChangingPassword": "Error changing password",