fix(e2e, playwright): Improve tests of the setting page (#52882)

Co-authored-by: Huyen Nguyen <25715018+huyenltnguyen@users.noreply.github.com>

client/src/components/settings/email.tsx

@@ -161,9 +161,7 @@ function EmailSettings({
   }
   return (
     <div className='email-settings'>
-      <SectionHeader dataPlaywrightTestLabel='email-settings-header'>
-        {t('settings.email.heading')}
-      </SectionHeader>
+      <SectionHeader>{t('settings.email.heading')}</SectionHeader>
       {isEmailVerified ? null : (
         <FullWidthRow>
           <HelpBlock>
@@ -194,9 +192,7 @@ function EmailSettings({
         >
           <FormGroup controlId='current-email'>
             <ControlLabel>{t('settings.email.current')}</ControlLabel>
-            <FormControl.Static data-playwright-test-label='current-email'>
-              {currentEmail}
-            </FormControl.Static>
+            <FormControl.Static>{currentEmail}</FormControl.Static>
           </FormGroup>
           <div role='group' aria-label={t('settings.email.heading')}>
             <FormGroup
@@ -208,7 +204,6 @@ function EmailSettings({
               </ControlLabel>
               <FormControl
                 data-cy='email-input'
-                data-playwright-test-label='new-email-input'
                 onChange={createHandleEmailFormChange('newEmail')}
                 type='email'
                 value={newEmail}
@@ -229,7 +224,6 @@ function EmailSettings({
               </ControlLabel>
               <FormControl
                 data-cy='confirm-email'
-                data-playwright-test-label='confirm-email-input'
                 onChange={createHandleEmailFormChange('confirmNewEmail')}
                 type='email'
                 value={confirmNewEmail}
@@ -243,7 +237,6 @@ function EmailSettings({
             </FormGroup>
           </div>
           <BlockSaveButton
-            data-playwright-test-label='save-email-button'
             aria-disabled={isDisabled}
             bgSize='lg'
             {...(isDisabled && { tabIndex: -1 })}
@@ -260,9 +253,7 @@ function EmailSettings({
           flag={sendQuincyEmail}
           flagName='sendQuincyEmail'
           offLabel={t('buttons.no-thanks')}
-          dataPlaywrightTestOffLabel='no-thanks-button'
           onLabel={t('buttons.yes-please')}
-          dataPlaywrightTestOnLabel='yes-please-button'
           toggleFlag={() => updateQuincyEmail(!sendQuincyEmail)}
         />
       </FullWidthRow>

client/src/components/settings/internet.tsx

@@ -197,9 +197,7 @@ class InternetSettings extends Component<InternetProps, InternetState> {
     const isDisabled = this.isFormPristine() || !this.isFormValid();
     return (
       <>
-        <SectionHeader dataPlaywrightTestLabel='your-internet-presence-header'>
-          {t('settings.headings.internet')}
-        </SectionHeader>
+        <SectionHeader>{t('settings.headings.internet')}</SectionHeader>
         <FullWidthRow>
           <form
             id='internet-presence'
@@ -237,7 +235,6 @@ class InternetSettings extends Component<InternetProps, InternetState> {
                   LinkedIn
                 </ControlLabel>
                 <FormControl
-                  data-playwright-test-label='internet-linkedin-input'
                   onChange={this.createHandleChange('linkedin')}
                   placeholder='https://www.linkedin.com/in/user-name'
                   type='url'
@@ -259,7 +256,6 @@ class InternetSettings extends Component<InternetProps, InternetState> {
                   Twitter
                 </ControlLabel>
                 <FormControl
-                  data-playwright-test-label='internet-twitter-input'
                   onChange={this.createHandleChange('twitter')}
                   placeholder='https://twitter.com/user-name'
                   type='url'
@@ -281,7 +277,6 @@ class InternetSettings extends Component<InternetProps, InternetState> {
                   {t('settings.labels.personal')}
                 </ControlLabel>
                 <FormControl
-                  data-playwright-test-label='internet-website-input'
                   onChange={this.createHandleChange('website')}
                   placeholder='https://example.com'
                   type='url'
@@ -297,7 +292,6 @@ class InternetSettings extends Component<InternetProps, InternetState> {
               </FormGroup>
             </div>
             <BlockSaveButton
-              data-playwright-test-label='internet-save-button'
               aria-disabled={isDisabled}
               bgSize='lg'
               {...(isDisabled && { tabIndex: -1 })}

e2e/email-settings.spec.ts

@@ -1,15 +1,9 @@
 import { test, expect } from '@playwright/test';
+import translations from '../client/i18n/locales/english/translations.json';
 
 const settingsPageElement = {
-  emailSettingsSectionHeader: 'email-settings-header',
   emailVerificationAlert: 'email-verification-alert',
   emailVerificationLink: 'email-verification-link',
-  currentEmailText: 'current-email',
-  newEmailInput: 'new-email-input',
-  confirmEmailInput: 'confirm-email-input',
-  saveButton: 'save-email-button',
-  emailSubscriptionYesPleaseButton: 'yes-please-button',
-  emailSubscriptionNoThanksButton: 'no-thanks-button',
   flashMessageAlert: 'flash-message'
 } as const;
 
@@ -20,18 +14,36 @@ test.beforeEach(async ({ page }) => {
 });
 
 test.describe('Email Settings', () => {
-  test('should display email settings section header on settings page', async ({
-    page
-  }) => {
+  test('should display the content correctly', async ({ page }) => {
     await expect(
-      page.getByTestId(settingsPageElement.emailSettingsSectionHeader)
-    ).toHaveText('Email Settings');
-  });
+      page.getByRole('heading', { name: translations.settings.email.heading })
+    ).toBeVisible();
+
+    await expect(page.getByText('foo@bar.com')).toBeVisible();
 
-  test('should display current email address', async ({ page }) => {
     await expect(
-      page.getByTestId(settingsPageElement.currentEmailText)
-    ).toHaveText('foo@bar.com');
+      page.getByRole('button', {
+        name: `${translations.buttons.save} ${translations.settings.email.heading}`
+      })
+    ).toBeDisabled();
+
+    await expect(
+      page
+        .getByRole('group', { name: translations.settings.email.weekly })
+        .locator('legend')
+    ).toBeVisible();
+
+    await expect(
+      page.getByRole('button', {
+        name: translations.buttons['yes-please']
+      })
+    ).toHaveAttribute('aria-pressed', 'false');
+
+    await expect(
+      page.getByRole('button', {
+        name: translations.buttons['no-thanks']
+      })
+    ).toHaveAttribute('aria-pressed', 'true');
   });
 
   test('should display email verification alert after email update', async ({
@@ -42,13 +54,21 @@ test.describe('Email Settings', () => {
 
     const newEmailAddress = 'foo-update@bar.com';
 
+    // Need exact match as there are "New email" and "Confirm new email" labels
     await page
-      .getByTestId(settingsPageElement.newEmailInput)
+      .getByLabel(translations.settings.email.new, { exact: true })
       .fill(newEmailAddress);
+
     await page
-      .getByTestId(settingsPageElement.confirmEmailInput)
+      .getByLabel(translations.settings.email.confirm)
       .fill(newEmailAddress);
-    await page.getByTestId(settingsPageElement.saveButton).click();
+
+    await page
+      .getByRole('button', {
+        name: `${translations.buttons.save} ${translations.settings.email.heading}`
+      })
+      .click();
+
     await expect(
       page.getByTestId(settingsPageElement.flashMessageAlert)
     ).toBeVisible();
@@ -67,6 +87,28 @@ test.describe('Email Settings', () => {
     );
   });
 
+  test('should toggle email subscription correctly', async ({
+    page,
+    browserName
+  }) => {
+    test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted');
+
+    const yesPleaseButton = page.getByRole('button', {
+      name: translations.buttons['yes-please']
+    });
+    const noThanksButton = page.getByRole('button', {
+      name: translations.buttons['no-thanks']
+    });
+
+    await yesPleaseButton.click();
+    await expect(yesPleaseButton).toHaveAttribute('aria-pressed', 'true');
+    await expect(noThanksButton).toHaveAttribute('aria-pressed', 'false');
+
+    await noThanksButton.click();
+    await expect(yesPleaseButton).toHaveAttribute('aria-pressed', 'false');
+    await expect(noThanksButton).toHaveAttribute('aria-pressed', 'true');
+  });
+
   test('should display flash message when email subscription is toggled', async ({
     page,
     browserName
@@ -74,7 +116,9 @@ test.describe('Email Settings', () => {
     test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted');
 
     await page
-      .getByTestId(settingsPageElement.emailSubscriptionYesPleaseButton)
+      .getByRole('button', {
+        name: translations.buttons['yes-please']
+      })
       .click();
 
     await expect(
@@ -89,7 +133,9 @@ test.describe('Email Settings', () => {
           response.status() === 200
       ),
       page
-        .getByTestId(settingsPageElement.emailSubscriptionNoThanksButton)
+        .getByRole('button', {
+          name: translations.buttons['no-thanks']
+        })
         .click()
     ]);
   });

e2e/internet-presence-settings.spec.ts

@@ -1,17 +1,14 @@
 import { test, expect } from '@playwright/test';
+import translations from '../client/i18n/locales/english/translations.json';
 
 const settingsPageElement = {
-  yourInternetPresenceSectionHeader: 'your-internet-presence-header',
   githubInput: 'internet-github-input',
   githubCheckmark: 'internet-github-check',
-  linkedinInput: 'internet-linkedin-input',
   linkedinCheckmark: 'internet-linkedin-check',
-  twitterInput: 'internet-twitter-input',
   twitterCheckmark: 'internet-twitter-check',
-  personalWebsiteInput: 'internet-website-input',
   personalWebsiteCheckmark: 'internet-website-check',
-  saveButton: 'internet-save-button',
-  flashMessageAlert: 'flash-message'
+  flashMessageAlert: 'flash-message',
+  internetPresenceForm: 'internet-presence'
 } as const;
 
 test.use({ storageState: 'playwright/.auth/certified-user.json' });
@@ -21,61 +18,86 @@ test.beforeEach(async ({ page }) => {
 });
 
 test.describe('Your Internet Presence', () => {
-  test('should display section header on settings page', async ({ page }) => {
+  test('should display the section with save button being disabled', async ({
+    page
+  }) => {
     await expect(
-      page.getByTestId(settingsPageElement.yourInternetPresenceSectionHeader)
-    ).toHaveText('Your Internet Presence');
+      page.getByRole('heading', {
+        level: 2,
+        name: translations.settings.headings.internet
+      })
+    ).toBeVisible();
+
+    await expect(
+      page
+        .getByTestId(settingsPageElement.internetPresenceForm)
+        .getByRole('button', { name: translations.buttons.save })
+    ).toBeVisible();
   });
 
   const socials = [
     {
       name: 'github',
       url: 'https://github.com/certified-user',
-      inputTestId: settingsPageElement.githubInput,
+      label: 'GitHub',
       checkTestId: settingsPageElement.githubCheckmark
     },
     {
       name: 'linkedin',
       url: 'https://www.linkedin.com/in/certified-user',
-      inputTestId: settingsPageElement.linkedinInput,
+      label: 'LinkedIn',
       checkTestId: settingsPageElement.linkedinCheckmark
     },
     {
       name: 'twitter',
       url: 'https://twitter.com/certified-user',
-      inputTestId: settingsPageElement.twitterInput,
+      label: 'Twitter',
       checkTestId: settingsPageElement.twitterCheckmark
     },
     {
       name: 'website',
       url: 'https://certified-user.com',
-      inputTestId: settingsPageElement.personalWebsiteInput,
+      label: translations.settings.labels.personal,
       checkTestId: settingsPageElement.personalWebsiteCheckmark
     }
   ];
 
   socials.forEach(social => {
+    test(`should hide ${social.name} checkmark by default`, async ({
+      page
+    }) => {
+      await expect(page.getByTestId(social.checkTestId)).toBeHidden();
+    });
+
     test(`should update ${social.name} URL`, async ({ browserName, page }) => {
       test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted');
 
-      await page.getByTestId(social.inputTestId).fill(social.url);
-      await expect(page.getByTestId(social.checkTestId)).toBeVisible();
+      const socialInput = page.getByLabel(social.label);
+      await socialInput.fill(social.url);
+      const socialCheckmark = page.getByTestId(social.checkTestId);
+      await expect(socialCheckmark).toBeVisible();
 
-      await page.getByTestId(settingsPageElement.saveButton).click();
+      const saveButton = page
+        .getByTestId(settingsPageElement.internetPresenceForm)
+        .getByRole('button', { name: translations.buttons.save });
+
+      await expect(saveButton).toBeVisible();
+      await saveButton.click();
       await expect(
         page.getByTestId(settingsPageElement.flashMessageAlert)
       ).toContainText('We have updated your social links');
 
       // clear value before next test
-      await page.getByTestId(social.inputTestId).clear();
+      await socialInput.clear();
       await Promise.all([
         page.waitForResponse(
           response =>
             response.url().includes('update-my-socials') &&
             response.status() === 200
         ),
-        page.getByTestId(settingsPageElement.saveButton).click()
+        saveButton.click()
       ]);
+      await expect(socialCheckmark).toBeHidden();
     });
   });
 });

e2e/settings.spec.ts

@@ -4,8 +4,6 @@ test.use({ storageState: 'playwright/.auth/certified-user.json' });
 
 const settingsTestIds = {
   settingsHeading: 'settings-heading',
-  newEmail: 'new-email-input',
-  confirmEmail: 'confirm-email-input',
   internetPresence: 'internet-presence',
   portfolioItems: 'portfolio-items',
   camperIdentity: 'camper-identity'
@@ -256,20 +254,42 @@ test.describe('Settings', () => {
     );
     await expect(addPortfolioButton).toBeVisible();
     await addPortfolioButton.click();
-    await expect(
-      page.getByTestId(settingsTestIds.portfolioItems)
-    ).toBeVisible();
+    await expect(addPortfolioButton).toBeDisabled(); // Add button should be disabled after clicking
+    const portfolioItems = page.getByTestId(settingsTestIds.portfolioItems);
+    await expect(portfolioItems).toBeVisible();
     const saveButton = page.getByRole('button', {
       name: translations.buttons['save-portfolio']
     });
     await expect(saveButton).toBeVisible();
+    await expect(saveButton).toBeDisabled();
     const removeButton = page.getByRole('button', {
       name: translations.buttons['remove-portfolio']
     });
     await expect(removeButton).toBeVisible();
+    await page
+      .getByLabel(translations.settings.labels.title)
+      .fill('My portfolio');
+    await page
+      .getByLabel(translations.settings.labels.url)
+      .fill('https://my-portfolio.com');
+    await page
+      .getByLabel(translations.settings.labels.image)
+      .fill('https://my-portfolio.com/image.png');
+    await page
+      .getByLabel(translations.settings.labels.description)
+      .fill('My description');
+    await saveButton.click();
+    await expect(
+      page.getByText(translations.flash['portfolio-item-updated'])
+    ).toBeVisible();
+    await removeButton.click();
+    await expect(addPortfolioButton).toBeEnabled();
+    await expect(portfolioItems).toBeHidden();
+    await expect(saveButton).toBeHidden();
+    await expect(removeButton).toBeHidden();
   });
 
-  test('Should validate Personal Portfolio Settings', async ({
+  test('Should validate Personal Information Settings', async ({
     page,
     browserName
   }) => {
@@ -286,7 +306,19 @@ test.describe('Settings', () => {
       name: translations.settings.headings['personal-info']
     });
     await expect(saveButton).toBeVisible();
-    await saveButton.press('Enter');
+    await expect(saveButton).toBeDisabled();
+    await expect(
+      page.getByLabel(translations.settings.labels.name, { exact: true })
+    ).toHaveValue('Full Stack User');
+    await expect(
+      page.getByLabel(translations.settings.labels.location)
+    ).toHaveValue('');
+    await expect(
+      page.getByLabel(translations.settings.labels.picture)
+    ).toHaveValue('');
+    await expect(
+      page.getByLabel(translations.settings.labels.about)
+    ).toHaveValue('');
     await expect(
       page
         .getByRole('group', {
@@ -310,15 +342,6 @@ test.describe('Settings', () => {
     await expect(
       page.getByText(translations.settings['scrollbar-width'])
     ).toBeVisible();
-    const addPortfolioButton = page.getByText(
-      translations.buttons['add-portfolio']
-    );
-    await expect(addPortfolioButton).toBeVisible();
-    await addPortfolioButton.click();
-    const removeButton = page.getByRole('button', {
-      name: translations.buttons['remove-portfolio']
-    });
-    await expect(removeButton).toBeVisible();
   });
 
   test('Should validate Academy Honesty Settings', async ({