From 65d3d5984fe967ffcbb10133aab1f07dfe48240d Mon Sep 17 00:00:00 2001 From: Devin Binnie <52460000+devinbinnie@users.noreply.github.com> Date: Wed, 11 Jun 2025 17:02:32 -0400 Subject: [PATCH] [MM-63041] Convert many inputs to the Input component, replace clientError with more correct client-side validation that conforms to the input (#31279) * [MM-63041] Convert many inputs to the Input component, replace clientError with more correct client-side validation that conforms to the input * Fix line length * PR feedback --------- Co-authored-by: Mattermost Build --- .../account_settings/account_settings_spec.ts | 10 +- .../account_settings/profile/email_spec.ts | 11 +- .../account_settings/profile/username_spec.ts | 20 +- .../security/password_spec.ts | 19 +- .../ui/components/channels/profile_modal.ts | 4 - .../user_settings.spec.ts | 12 +- .../setting_item_max.test.tsx.snap | 28 +-- .../src/components/setting_item_max.tsx | 34 +--- .../general/user_settings_general.test.tsx | 8 +- .../general/user_settings_general.tsx | 189 +++++++++++------- .../security/user_settings_security.tsx | 118 ++++++----- .../components/widgets/inputs/input/input.tsx | 9 + .../channels/src/sass/routes/_settings.scss | 13 ++ 13 files changed, 255 insertions(+), 220 deletions(-) diff --git a/e2e-tests/cypress/tests/integration/channels/account_settings/account_settings_spec.ts b/e2e-tests/cypress/tests/integration/channels/account_settings/account_settings_spec.ts index 8bcf9ce98a..cba613c892 100644 --- a/e2e-tests/cypress/tests/integration/channels/account_settings/account_settings_spec.ts +++ b/e2e-tests/cypress/tests/integration/channels/account_settings/account_settings_spec.ts @@ -36,7 +36,7 @@ describe('Account Settings', () => { cy.uiClose(); }); - it('MM-T2081 Password: Error on blank', () => { + it('MM-T2081 Password: Save should be disabled on blank', () => { // # Go to Profile > Security cy.uiOpenProfileModal('Security'); @@ -49,12 +49,8 @@ describe('Account Settings', () => { // # Click "Edit" to the right of "Password" cy.get('#passwordEdit').should('be.visible').click(); - // # Save the settings - cy.uiSave(); - - // * Check that there is an error - cy.get('#clientError').should('be.visible').should('contain', 'Please enter your current password.'); - cy.get('#serverError').should('not.exist'); + // # Check that save button is disabled + cy.get('button[type="submit"]').should('be.disabled'); cy.uiClose(); }); diff --git a/e2e-tests/cypress/tests/integration/channels/account_settings/profile/email_spec.ts b/e2e-tests/cypress/tests/integration/channels/account_settings/profile/email_spec.ts index 8a264d2579..4339ae4975 100644 --- a/e2e-tests/cypress/tests/integration/channels/account_settings/profile/email_spec.ts +++ b/e2e-tests/cypress/tests/integration/channels/account_settings/profile/email_spec.ts @@ -81,11 +81,11 @@ describe('Profile > Profile Settings > Email', () => { // # Click "Edit" to the right of "Email" cy.get('#emailEdit').should('be.visible').click(); - // # Save the settings - cy.uiSave().wait(TIMEOUTS.HALF_SEC); + // # Click on the input and blur it + cy.get('#primaryEmail').should('be.visible').click().blur(); // * Check that the correct error message is shown. - cy.get('#clientError').should('be.visible').should('have.text', 'Please enter a valid email address'); + cy.get('#error_primaryEmail').should('be.visible').should('have.text', 'Please enter a valid email address'); }); it('MM-T2067 email address already taken error', () => { @@ -113,11 +113,8 @@ describe('Profile > Profile Settings > Email', () => { cy.get('#confirmEmail').should('be.visible').clear(); cy.get('#currentPassword').should('be.visible').type('randompass'); - // # Save the settings - cy.uiSave().wait(TIMEOUTS.HALF_SEC); - // * Check that the correct error message is shown. - cy.get('#clientError').should('be.visible').should('have.text', 'The new emails you entered do not match.'); + cy.get('#error_confirmEmail').should('be.visible').should('have.text', 'The new emails you entered do not match.'); }); // This test is a combination of 4 sub-tests because they are sub-parts of the same test. diff --git a/e2e-tests/cypress/tests/integration/channels/account_settings/profile/username_spec.ts b/e2e-tests/cypress/tests/integration/channels/account_settings/profile/username_spec.ts index 6e2c5a7c6b..d42bfdb199 100644 --- a/e2e-tests/cypress/tests/integration/channels/account_settings/profile/username_spec.ts +++ b/e2e-tests/cypress/tests/integration/channels/account_settings/profile/username_spec.ts @@ -47,11 +47,10 @@ describe('Settings > Sidebar > General > Edit', () => { it('MM-T2050 Username cannot be blank', () => { // # Clear the username textfield contents cy.get('#usernameEdit').click(); - cy.get('#username').clear(); - cy.uiSave(); + cy.get('#username').click().clear().blur(); // * Check if element is present and contains expected text values - cy.get('#clientError').should('be.visible').should('contain', 'Username must begin with a letter, and contain between 3 to 22 lowercase characters made up of numbers, letters, and the symbols \'.\', \'-\', and \'_\'.'); + cy.get('#error_username').should('be.visible').should('contain', 'Username must begin with a letter, and contain between 3 to 22 lowercase characters made up of numbers, letters, and the symbols \'.\', \'-\', and \'_\'.'); // # Click "x" button to close Profile modal cy.uiClose(); @@ -62,11 +61,10 @@ describe('Settings > Sidebar > General > Edit', () => { cy.get('#usernameEdit').click(); // # Add the username to textfield contents - cy.get('#username').clear().type('te'); - cy.uiSave(); + cy.get('#username').clear().type('te').blur(); // * Check if element is present and contains expected text values - cy.get('#clientError').should('be.visible').should('contain', 'Username must begin with a letter, and contain between 3 to 22 lowercase characters made up of numbers, letters, and the symbols \'.\', \'-\', and \'_\'.'); + cy.get('#error_username').should('be.visible').should('contain', 'Username must begin with a letter, and contain between 3 to 22 lowercase characters made up of numbers, letters, and the symbols \'.\', \'-\', and \'_\'.'); // # Click "x" button to close Profile modal cy.uiClose(); @@ -151,11 +149,10 @@ describe('Settings > Sidebar > General > Edit', () => { for (const prefix of prefixes) { // # Add username to textfield contents - cy.get('#username').clear().type(prefix).type('{backspace}.').type(otherUser.username); - cy.uiSave(); + cy.get('#username').clear().type(prefix).type('{backspace}.').type(otherUser.username).blur(); // * Check if element is present and contains expected text values - cy.get('#clientError').should('be.visible').should('contain', 'Username must begin with a letter, and contain between 3 to 22 lowercase characters made up of numbers, letters, and the symbols \'.\', \'-\', and \'_\'.'); + cy.get('#error_username').should('be.visible').should('contain', 'Username must begin with a letter, and contain between 3 to 22 lowercase characters made up of numbers, letters, and the symbols \'.\', \'-\', and \'_\'.'); } // # Click "x" button to close Profile modal @@ -175,11 +172,10 @@ describe('Settings > Sidebar > General > Edit', () => { for (const username of usernames) { // # Add username to textfield contents - cy.get('#username').clear().type(username); - cy.uiSave(); + cy.get('#username').clear().type(username).blur(); // * Check if element is present and contains expected text values - cy.get('#clientError').should('be.visible').should('contain', 'This username is reserved, please choose a new one.'); + cy.get('#error_username').should('be.visible').should('contain', 'This username is reserved, please choose a new one.'); } // # Click "x" button to close Profile modal diff --git a/e2e-tests/cypress/tests/integration/channels/account_settings/security/password_spec.ts b/e2e-tests/cypress/tests/integration/channels/account_settings/security/password_spec.ts index e17e538f80..a48a75f01b 100644 --- a/e2e-tests/cypress/tests/integration/channels/account_settings/security/password_spec.ts +++ b/e2e-tests/cypress/tests/integration/channels/account_settings/security/password_spec.ts @@ -54,11 +54,15 @@ describe('Profile', () => { // # Enter valid values in password change fields enterPasswords(testUser.password, 'passwd', 'passwd'); + // * Check that there are no errors + cy.get('#error_currentPassword').should('not.exist'); + cy.get('#error_newPassword').should('not.exist'); + cy.get('#error_confirmPassword').should('not.exist'); + // # Save the settings cy.uiSave(); // * Check that there are no errors - cy.get('#clientError').should('not.exist'); cy.get('#serverError').should('not.exist'); }); @@ -66,22 +70,16 @@ describe('Profile', () => { // # Enter mismatching passwords for new password and confirm fields enterPasswords(testUser.password, 'newPW', 'NewPW'); - // # Save - cy.uiSave(); - // * Verify for error message: "The new passwords you entered do not match." - cy.get('#clientError').should('be.visible').should('have.text', 'The new passwords you entered do not match.'); + cy.get('#error_confirmPassword').should('be.visible').should('have.text', 'The new passwords you entered do not match.'); }); it('MM-T2083 Password: Too few characters in new password produces error', () => { // # Enter a New password two letters long enterPasswords(testUser.password, 'pw', 'pw'); - // # Save - cy.uiSave(); - // * Verify for error message: "Your password must be 5-72 characters long." - cy.get('#clientError').should('be.visible').should('have.text', 'Your password must be 5-72 characters long.'); + cy.get('#error_newPassword').should('be.visible').should('have.text', 'Your password must be 5-72 characters long.'); }); it('MM-T2084 Password: Cancel out of password changes causes no changes to be made', () => { @@ -151,4 +149,7 @@ function enterPasswords(currentPassword, newPassword, confirmPassword) { // # Retype New password incorrectly cy.get('#confirmPassword').should('be.visible').type(confirmPassword); + + // # Click on the input and blur it + cy.get('#currentPassword').should('be.visible').click().blur(); } diff --git a/e2e-tests/playwright/lib/src/ui/components/channels/profile_modal.ts b/e2e-tests/playwright/lib/src/ui/components/channels/profile_modal.ts index a10d0a9dd6..f1c944d01f 100644 --- a/e2e-tests/playwright/lib/src/ui/components/channels/profile_modal.ts +++ b/e2e-tests/playwright/lib/src/ui/components/channels/profile_modal.ts @@ -16,8 +16,6 @@ export default class ProfileModal { readonly saveButton; readonly cancelButton; - readonly errorText; - constructor(container: Locator) { this.container = container; @@ -30,8 +28,6 @@ export default class ProfileModal { this.closeButton = container.getByRole('button', {name: 'Close'}); this.saveButton = container.locator('button:has-text("Save")'); this.cancelButton = container.locator('button:has-text("Cancel")'); - - this.errorText = container.locator('#clientError'); } async toBeVisible() { diff --git a/e2e-tests/playwright/specs/functional/channels/custom_profile_attributes/user_settings.spec.ts b/e2e-tests/playwright/specs/functional/channels/custom_profile_attributes/user_settings.spec.ts index 9c3042a576..5d01eb93a4 100644 --- a/e2e-tests/playwright/specs/functional/channels/custom_profile_attributes/user_settings.spec.ts +++ b/e2e-tests/playwright/specs/functional/channels/custom_profile_attributes/user_settings.spec.ts @@ -346,13 +346,13 @@ test('MM-T5772 URL Validation in Custom Profile Attributes @custom_profile_attri await profileModal.container.locator(`#customAttribute_${fieldId}`).scrollIntoViewIfNeeded(); await profileModal.container.locator(`#customAttribute_${fieldId}`).clear(); await profileModal.container.locator(`#customAttribute_${fieldId}`).fill(TEST_INVALID_URL); - - // 4. Try to save the changes - await profileModal.saveButton.click(); + await profileModal.container.locator(`#customAttribute_${fieldId}`).blur(); // * Save button doesn't complete the operation with invalid URL - await expect(profileModal.errorText).toBeVisible(); - await expect(profileModal.errorText).toHaveText('Please enter a valid url.'); + await expect(profileModal.container.locator(`#error_customAttribute_${fieldId}`)).toBeVisible(); + await expect(profileModal.container.locator(`#error_customAttribute_${fieldId}`)).toHaveText( + 'Please enter a valid url.', + ); // 5. Edit Website field and enter a valid URL await profileModal.container.locator(`#customAttribute_${fieldId}`).clear(); @@ -362,6 +362,6 @@ test('MM-T5772 URL Validation in Custom Profile Attributes @custom_profile_attri await profileModal.saveButton.click(); // * Valid URL saves successfully with no error message - await expect(profileModal.errorText).not.toBeVisible(); + await expect(profileModal.container.locator(`#error_customAttribute_${fieldId}`)).not.toBeVisible(); await expect(profileModal.container).toContainText(TEST_VALID_URL); }); diff --git a/webapp/channels/src/components/__snapshots__/setting_item_max.test.tsx.snap b/webapp/channels/src/components/__snapshots__/setting_item_max.test.tsx.snap index 09266dc9f8..8703903636 100644 --- a/webapp/channels/src/components/__snapshots__/setting_item_max.test.tsx.snap +++ b/webapp/channels/src/components/__snapshots__/setting_item_max.test.tsx.snap @@ -78,33 +78,7 @@ exports[`components/SettingItemMax should match snapshot, on clientError 1`] = `
-
- -
-
+ /> { settingList: React.RefObject; @@ -120,31 +116,6 @@ export default class SettingItemMax extends React.PureComponent { }; render() { - let clientError = null; - if (this.props.clientError) { - clientError = ( -
- -
- ); - } - let serverError = null; if (this.props.serverError) { serverError = ( @@ -193,7 +164,7 @@ export default class SettingItemMax extends React.PureComponent { @@ -267,7 +238,6 @@ export default class SettingItemMax extends React.PureComponent { role='alert' > {serverError} - {clientError} {submit}