Mm 63903 handle undefined options (#30887)

* fixes for deleted/changed select/multiselect attributes

* add testing for prepending scheme to url

* test: add unit tests for select and multiselect with removed options

* trim trailing '/'

* update location property

* lint fixes

---------

Co-authored-by: Caleb Roseland <caleb@calebroseland.com>

webapp/channels/src/components/profile_popover/profile_popover_custom_attributes.test.tsx

@@ -105,6 +105,9 @@ describe('components/ProfilePopoverCustomAttributes', () => {
                     url_attribute_id: urlAttribute,
                     select_attribute_id: selectAttribute,
                 },
+                license: {
+                    Cloud: 'false',
+                },
             },
             users: {
                 profiles: {
@@ -137,6 +140,12 @@ describe('components/ProfilePopoverCustomAttributes', () => {
         expect(screen.getByText('text value')).toBeInTheDocument();
         expect(screen.getByText('+1 (555) 123-4567')).toBeInTheDocument();
         expect(screen.getByText('https://example.com')).toBeInTheDocument();
+
+        // URL attribute should be rendered as a link
+        const urlLink = screen.getByRole('link', {name: 'https://example.com'});
+        expect(urlLink).toBeInTheDocument();
+        expect(urlLink).toHaveAttribute('href', 'https://example.com');
+
         expect(screen.getByText('Option 1')).toBeInTheDocument();
     });
 

webapp/channels/src/components/profile_popover/profile_popover_custom_attributes.tsx

@@ -47,6 +47,25 @@ const ProfilePopoverCustomAttributes = ({
 
             if (!hasValue && visibility === 'when_set') {
                 return null;
+            } else if (visibility === 'when_set' && (attribute.type === 'multiselect' || attribute.type === 'select')) {
+                const attributeValue = userProfile.custom_profile_attributes[attribute.id];
+
+                // make sure attribute contains legitimate values
+                if (Array.isArray(attributeValue)) {
+                    // Handle multiselect
+                    const options = attributeValue.map((value) => {
+                        return attribute.attrs.options?.find((o) => o.id === value);
+                    }).filter((o) => o != null);
+                    if (options.length === 0) {
+                        return null;
+                    }
+                } else {
+                    // Handle single select
+                    const option = attribute.attrs.options?.find((o) => o.id === attributeValue);
+                    if (option === undefined) {
+                        return null;
+                    }
+                }
             }
 
             const valueType = (attribute.attrs?.value_type as UserPropertyValueType) || '';

webapp/channels/src/components/profile_popover/profile_popover_url.test.tsx

@@ -91,4 +91,14 @@ describe('components/ProfilePopoverUrl', () => {
         expect(container).toBeInTheDocument();
         expect(screen.getByRole('link')).toHaveTextContent(url);
     });
+
+    test('should render url with ExternalLink component', () => {
+        renderWithContext(<ProfilePopoverUrl {...baseProps}/>);
+
+        const url = 'https://example.com';
+        const link = screen.getByRole('link');
+        expect(link).toHaveAttribute('href', url);
+        expect(link).toHaveAttribute('target', '_blank');
+        expect(link).toHaveAttribute('rel', 'noopener noreferrer');
+    });
 });

webapp/channels/src/components/profile_popover/profile_popover_url.tsx

@@ -6,6 +6,8 @@ import React from 'react';
 import type {UserPropertyField} from '@mattermost/types/properties';
 import type {UserProfile} from '@mattermost/types/users';
 
+import ExternalLink from 'components/external_link';
+
 type Props = {
     attribute: UserPropertyField;
     userProfile: UserProfile;
@@ -28,11 +30,12 @@ const ProfilePopoverUrl = ({attribute, userProfile}: Props) => {
                 aria-hidden='true'
                 data-testid='url-icon'
             />
-            <a
+            <ExternalLink
+                location='profile_popover_url'
                 href={url}
             >
                 {url}
-            </a>
+            </ExternalLink>
         </div>
     );
 };

webapp/channels/src/components/user_settings/general/user_settings_general.test.tsx

@@ -506,6 +506,174 @@ describe('components/user_settings/general/UserSettingsGeneral', () => {
         expect(props.actions.saveCustomProfileAttribute).toHaveBeenCalledWith('user_id', 'field1', '');
     });
 
+    test('should handle select with removed options', async () => {
+        const saveCustomProfileAttribute = jest.fn().mockResolvedValue({});
+        const selectAttribute: UserPropertyField = {
+            ...customProfileAttribute,
+            type: 'select',
+            attrs: {
+                value_type: '',
+                visibility: 'when_set',
+                sort_order: 0,
+                options: [
+                    {id: 'opt1', name: 'Option 1', color: ''},
+
+                    // opt2 has been removed from options
+                ],
+            },
+        };
+
+        // User has a value for an option that no longer exists
+        const testUser = {...user, custom_profile_attributes: {field1: 'opt2'}};
+        const props = {
+            ...requiredProps,
+            enableCustomProfileAttributes: true,
+            customProfileAttributeFields: [selectAttribute],
+            user: testUser,
+            activeSection: '',
+            actions: {
+                ...requiredProps.actions,
+                saveCustomProfileAttribute,
+            },
+        };
+
+        renderWithContext(<UserSettingsGeneral {...props}/>);
+
+        // Should not display any value since the option no longer exists
+        expect(screen.queryByText('Option 2')).not.toBeInTheDocument();
+        expect(await screen.findByText('Click \'Edit\' to add your custom attribute')).toBeInTheDocument();
+    });
+
+    test('should handle multiselect with removed options', async () => {
+        const saveCustomProfileAttribute = jest.fn().mockResolvedValue({});
+        const multiselectAttribute: UserPropertyField = {
+            ...customProfileAttribute,
+            type: 'multiselect',
+            attrs: {
+                value_type: '',
+                visibility: 'when_set',
+                sort_order: 0,
+                options: [
+                    {id: 'opt1', name: 'Option 1', color: ''},
+
+                    // opt2 and opt3 have been removed from options
+                ],
+            },
+        };
+
+        // User has values for options that no longer exist
+        const testUser = {...user, custom_profile_attributes: {field1: ['opt1', 'opt2', 'opt3']}};
+        const props = {
+            ...requiredProps,
+            enableCustomProfileAttributes: true,
+            customProfileAttributeFields: [multiselectAttribute],
+            user: testUser,
+            activeSection: '',
+            actions: {
+                ...requiredProps.actions,
+                saveCustomProfileAttribute,
+            },
+        };
+
+        renderWithContext(<UserSettingsGeneral {...props}/>);
+
+        // Should only display the option that still exists
+        expect(await screen.findByText('Option 1')).toBeInTheDocument();
+        expect(screen.queryByText('Option 2')).not.toBeInTheDocument();
+        expect(screen.queryByText('Option 3')).not.toBeInTheDocument();
+    });
+
+    test('should handle editing select with removed options', async () => {
+        const saveCustomProfileAttribute = jest.fn().mockResolvedValue({});
+        const selectAttribute: UserPropertyField = {
+            ...customProfileAttribute,
+            type: 'select',
+            attrs: {
+                value_type: '',
+                visibility: 'when_set',
+                sort_order: 0,
+                options: [
+                    {id: 'opt1', name: 'Option 1', color: ''},
+
+                    // opt2 has been removed from options
+                ],
+            },
+        };
+
+        // User has a value for an option that no longer exists
+        const testUser = {...user, custom_profile_attributes: {field1: 'opt2'}};
+        const props = {
+            ...requiredProps,
+            enableCustomProfileAttributes: true,
+            customProfileAttributeFields: [selectAttribute],
+            user: testUser,
+            activeSection: 'customAttribute_field1',
+            actions: {
+                ...requiredProps.actions,
+                saveCustomProfileAttribute,
+            },
+        };
+
+        renderWithContext(<UserSettingsGeneral {...props}/>);
+
+        // Should show empty select since the option no longer exists
+        expect(await screen.findByText('Select')).toBeInTheDocument();
+
+        // Select a valid option and save
+        userEvent.click(screen.getByText('Select'));
+        userEvent.click(await screen.findByText('Option 1'));
+        userEvent.click(screen.getByRole('button', {name: 'Save'}));
+
+        expect(saveCustomProfileAttribute).toHaveBeenCalledWith('user_id', 'field1', 'opt1');
+    });
+
+    test('should handle editing multiselect with removed options', async () => {
+        const saveCustomProfileAttribute = jest.fn().mockResolvedValue({});
+        const multiselectAttribute: UserPropertyField = {
+            ...customProfileAttribute,
+            type: 'multiselect',
+            attrs: {
+                value_type: '',
+                visibility: 'when_set',
+                sort_order: 0,
+                options: [
+                    {id: 'opt1', name: 'Option 1', color: ''},
+                    {id: 'opt3', name: 'Option 3', color: ''},
+
+                    // opt2 has been removed from options
+                ],
+            },
+        };
+
+        // User has values including one for an option that no longer exists
+        const testUser = {...user, custom_profile_attributes: {field1: ['opt1', 'opt2']}};
+        const props = {
+            ...requiredProps,
+            enableCustomProfileAttributes: true,
+            customProfileAttributeFields: [multiselectAttribute],
+            user: testUser,
+            activeSection: 'customAttribute_field1',
+            actions: {
+                ...requiredProps.actions,
+                saveCustomProfileAttribute,
+            },
+        };
+
+        renderWithContext(<UserSettingsGeneral {...props}/>);
+
+        // Should only show the valid option
+        expect(await screen.findByText('Option 1')).toBeInTheDocument();
+        expect(screen.queryByText('Option 2')).not.toBeInTheDocument();
+
+        // Add another valid option and save
+        userEvent.click(await screen.findByText('Option 1'));
+        userEvent.click(await screen.findByText('Option 3'));
+        userEvent.click(screen.getByRole('button', {name: 'Save'}));
+
+        // Should save with only the valid options
+        expect(saveCustomProfileAttribute).toHaveBeenCalledWith('user_id', 'field1', ['opt1', 'opt3']);
+    });
+
     test('should not show custom attribute input field when LDAP attribute is set', async () => {
         const props = {
             ...requiredProps,
@@ -573,4 +741,78 @@ describe('components/user_settings/general/UserSettingsGeneral', () => {
         expect(await screen.getByRole('button', {name: 'Save'})).toBeInTheDocument();
         expect(screen.queryByRole('textbox', {name: customProfileAttribute.name})).toBeInTheDocument();
     });
+
+    test('should validate URL custom attribute field value', async () => {
+        const urlAttribute: UserPropertyField = {
+            ...customProfileAttribute,
+            attrs: {
+                ...customProfileAttribute.attrs,
+                value_type: 'url',
+            },
+        };
+
+        const saveCustomProfileAttribute = jest.fn().mockResolvedValue({});
+        const props = {
+            ...requiredProps,
+            enableCustomProfileAttributes: true,
+            customProfileAttributeFields: [urlAttribute],
+            user: {...user},
+            activeSection: 'customAttribute_field1',
+            actions: {
+                ...requiredProps.actions,
+                saveCustomProfileAttribute,
+            },
+        };
+
+        renderWithContext(<UserSettingsGeneral {...props}/>);
+
+        userEvent.type(screen.getByRole('textbox', {name: urlAttribute.name}), 'ftp://invalid-scheme');
+        userEvent.click(screen.getByRole('button', {name: 'Save'}));
+
+        expect(await screen.findByText('Please enter a valid url.')).toBeInTheDocument();
+        expect(saveCustomProfileAttribute).not.toHaveBeenCalled();
+
+        userEvent.clear(screen.getByRole('textbox', {name: urlAttribute.name}));
+        userEvent.type(screen.getByRole('textbox', {name: urlAttribute.name}), 'example.com');
+        userEvent.click(screen.getByRole('button', {name: 'Save'}));
+
+        expect(saveCustomProfileAttribute).toHaveBeenCalledWith('user_id', 'field1', 'http://example.com');
+    });
+
+    test('should validate email custom attribute field value', async () => {
+        const emailAttribute: UserPropertyField = {
+            ...customProfileAttribute,
+            attrs: {
+                ...customProfileAttribute.attrs,
+                value_type: 'email',
+            },
+        };
+
+        const saveCustomProfileAttribute = jest.fn().mockResolvedValue({});
+        const props = {
+            ...requiredProps,
+            enableCustomProfileAttributes: true,
+            customProfileAttributeFields: [emailAttribute],
+            user: {...user},
+            activeSection: 'customAttribute_field1',
+            actions: {
+                ...requiredProps.actions,
+                saveCustomProfileAttribute,
+            },
+        };
+
+        renderWithContext(<UserSettingsGeneral {...props}/>);
+
+        userEvent.type(screen.getByRole('textbox', {name: emailAttribute.name}), 'invalid-email');
+        userEvent.click(screen.getByRole('button', {name: 'Save'}));
+
+        expect(await screen.findByText('Please enter a valid email address.')).toBeInTheDocument();
+        expect(saveCustomProfileAttribute).not.toHaveBeenCalled();
+
+        userEvent.clear(screen.getByRole('textbox', {name: emailAttribute.name}));
+        userEvent.type(screen.getByRole('textbox', {name: emailAttribute.name}), 'test@example.com');
+        userEvent.click(screen.getByRole('button', {name: 'Save'}));
+
+        expect(saveCustomProfileAttribute).toHaveBeenCalledWith('user_id', 'field1', 'test@example.com');
+    });
 });

webapp/channels/src/components/user_settings/general/user_settings_general.tsx

@@ -459,10 +459,18 @@ export class UserSettingsGeneralTab extends PureComponent<Props, State> {
                 }
             }
             if (attributeField.attrs.value_type === 'url') {
-                if (attributeValue !== '' && !validHttpUrl(attributeValue)) {
+                if (attributeValue !== '') {
+                    const validURL = validHttpUrl(attributeValue);
+                    if (!validURL) {
                         this.setState({clientError: formatMessage(holders.validUrl), emailError: '', serverError: ''});
                         return;
                     }
+                    let validLink = validURL.toString();
+                    if (validLink.endsWith('/')) {
+                        validLink = validLink.slice(0, -1);
+                    }
+                    attributeValue = validLink;
+                }
             }
         }
         if (attributeField.type === 'multiselect' && !attributeValue) {
@@ -1419,14 +1427,20 @@ export class UserSettingsGeneralTab extends PureComponent<Props, State> {
                     if (Array.isArray(attributeValue)) {
                         return attributeValue.map((value) => {
                             const option = attribOptions.find((o) => o.id === value);
+                            if (option) {
                                 return {label: option?.name, value: option?.id};
-                        });
+                            }
+                            return null;
+                        }).filter((value) => value != null);
                     }
 
                     // Handle single select
                     const option = attribOptions.find((o) => o.id === attributeValue);
+                    if (option) {
                         return {label: option?.name, value: option?.id};
                     }
+                    return '';
+                }
 
                 return attributeValue as string;
             };
@@ -1535,14 +1549,16 @@ export class UserSettingsGeneralTab extends PureComponent<Props, State> {
             let describe: JSX.Element|string = '';
             if (this.props.user.custom_profile_attributes?.[attribute.id]) {
                 const attributeValue = getDisplayValue(this.props.user.custom_profile_attributes?.[attribute.id]);
+                if (attributeValue) {
                     if (typeof attributeValue === 'string') {
                         describe = attributeValue;
                     } else if (Array.isArray(attributeValue) && attributeValue.length > 0) {
-                    describe = <FormattedList value={attributeValue.map((attrib) => attrib.label)}/>;
+                        describe = <FormattedList value={attributeValue.map((attrib) => attrib?.label || null)}/>;
                     } else if (!Array.isArray(attributeValue) && Object.hasOwn(attributeValue, 'label')) {
                         describe = attributeValue.label || '';
                     }
                 }
+            }
             if (!describe) {
                 describe = (
                     <FormattedMessage