Addressing review feedbacks

trento-project · May 30, 2024 · 3219bf9 · 3219bf9
1 parent ee1bb60
commit 3219bf9
Show file tree

Hide file tree

Showing 6 changed files with 28 additions and 20 deletions.
diff --git a/assets/js/lib/test-utils/factories/users.js b/assets/js/lib/test-utils/factories/users.js
@@ -28,9 +28,9 @@ export const profileFactory = Factory.define(() => ({
   email: faker.internet.email(),
   abilities: abilityFactory.buildList(2),
   password_change_requested: false,
+  totp_enabled: faker.datatype.boolean(),
   created_at: formatISO(faker.date.past()),
   updated_at: formatISO(faker.date.past()),
-  totp_enabled: faker.datatype.boolean(),
 }));
 
 export const adminUser = userFactory.params({

diff --git a/assets/js/pages/Profile/ProfileForm.jsx b/assets/js/pages/Profile/ProfileForm.jsx
@@ -29,7 +29,7 @@ function ProfileForm({
   onSave = noop,
   onResetTotp = noop,
   onVerifyTotp = noop,
-  onTotpEnable = noop,
+  onEnableTotp = noop,
 }) {
   const [fullNameState, setFullName] = useState(fullName);
   const [fullNameErrorState, setFullNameError] = useState(null);
@@ -67,7 +67,7 @@ function ProfileForm({
 
   const toggleTotp = () => {
     if (!totpEnabled) {
-      onTotpEnable();
+      onEnableTotp();
       return;
     }
     setTotpDisableModalOpen(true);
@@ -139,7 +139,7 @@ function ProfileForm({
             <>
               <Label
                 className="col-start-1 col-span-1"
-                info="Use a second factor besides your password to increase security
+                info="Setup a multi factor TOTP authentication besides your password to increase security
               for your account."
               >
                 Authenticator App

diff --git a/assets/js/pages/Profile/ProfileForm.test.jsx b/assets/js/pages/Profile/ProfileForm.test.jsx
@@ -155,13 +155,13 @@ describe('ProfileForm', () => {
     ).toBe('false');
   });
 
-  it('should call onTotpEnable when totpEnabled is false and the switch is clicked', async () => {
+  it('should call onEnableTotp when totpEnabled is false and the switch is clicked', async () => {
     const username = faker.internet.userName();
     const fullName = faker.person.fullName();
     const email = faker.internet.email();
     const abilities = abilityFactory.buildList(2);
 
-    const onTotpEnable = jest.fn();
+    const onEnableTotp = jest.fn();
     const user = userEvent.setup();
 
     render(
@@ -171,7 +171,7 @@ describe('ProfileForm', () => {
         username={username}
         abilities={abilities}
         totpEnabled={false}
-        onTotpEnable={onTotpEnable}
+        onEnableTotp={onEnableTotp}
       />
     );
 
@@ -181,7 +181,7 @@ describe('ProfileForm', () => {
       await user.click(screen.getByRole('switch'));
     });
 
-    expect(onTotpEnable).toHaveBeenCalled();
+    expect(onEnableTotp).toHaveBeenCalled();
   });
 
   it('should call onResetTotp when totpEnabled is true, the switch is clicked and the user confirms with the modal', async () => {

diff --git a/assets/js/pages/Profile/ProfilePage.jsx b/assets/js/pages/Profile/ProfilePage.jsx
@@ -48,7 +48,7 @@ function ProfilePage() {
     setErrors([]);
   };
 
-  const verifyTotpEnrolling = async (enrollmentTotp) => {
+  const verifyTotpEnrollment = async (enrollmentTotp) => {
     setSaving(true);
     setErrors([]);
 
@@ -79,7 +79,7 @@ function ProfilePage() {
       setSaving(false);
       toast.success('TOTP Disabled');
     } catch {
-      toast.error('Error disabling totp, please refresh profile.');
+      toast.error('Error disabling totp, please refresh your profile.');
     }
     await loadUserProfile();
     setSaving(false);
@@ -161,8 +161,8 @@ function ProfilePage() {
         loading={loading || saving}
         disableForm={isDefaultAdmin}
         onSave={updateProfile}
-        onTotpEnable={totpInitiateEnrolling}
-        onVerifyTotp={verifyTotpEnrolling}
+        onEnableTotp={totpInitiateEnrolling}
+        onVerifyTotp={verifyTotpEnrollment}
         onResetTotp={disableTotp}
       />
     </>

diff --git a/assets/js/pages/Profile/ProfilePage.test.jsx b/assets/js/pages/Profile/ProfilePage.test.jsx
@@ -183,6 +183,10 @@ describe('ProfilePage', () => {
       await testUser.click(screen.getByRole('switch'));
     });
 
+    await act(async () => {
+      await testUser.click(screen.getByText('Disable'));
+    });
+
     expect(toast.success).toHaveBeenCalledWith('TOTP Disabled');
   });
 
@@ -206,8 +210,12 @@ describe('ProfilePage', () => {
       await testUser.click(screen.getByRole('switch'));
     });
 
+    await act(async () => {
+      await testUser.click(screen.getByText('Disable'));
+    });
+
     expect(toast.error).toHaveBeenCalledWith(
-      'Error disabling totp, please refresh profile.'
+      'Error disabling totp, please refresh your profile.'
     );
   });
 

diff --git a/assets/js/pages/Profile/TotpEnrollmentBox.stories.jsx b/assets/js/pages/Profile/TotpEnrollmentBox.stories.jsx
@@ -17,8 +17,13 @@ export default {
         type: 'text',
       },
     },
-    verifyError: {
-      description: 'Totp error during the enrollment verify procedure',
+    errors: {
+      description: 'Totp errors during the enrollment verify procedure',
+      control: {
+        type: 'boolean',
+      },
+    },
+    loading: {
       control: {
         type: 'boolean',
       },
@@ -27,11 +32,6 @@ export default {
       action: 'Verify enrollment TOTP',
     },
   },
-  args: {
-    secret: faker.string.uuid(),
-    qrData:
-      'otpauth://totp/Example:alice@google.com?secret=JBSWY3DPEHPK3PXP&issuer=Example',
-  },
 };
 
 export const Default = {