From c417ac1b5725fd5cf012efcb9819fac8b9fe9927 Mon Sep 17 00:00:00 2001 From: Scott Bishel Date: Wed, 2 Apr 2025 15:15:14 -0600 Subject: [PATCH] MM-62699 Add additional property types to user profile form (#30317) * fix for testing with server * revert feature flag * user settings to handle other property types * lint fixes * update css so container will grow with several multiselect values * change CPASelectOption to PropertyFieldOption * lint fix * lint and style fix * review fixes * fixes for change in UserPropertyFields * update url validation * update url validation * update unit test * Update webapp/channels/src/components/user_settings/general/user_settings_general.tsx Co-authored-by: Caleb Roseland * fix: Handle missing options in user settings attribute rendering * update handling of single/multi values * remove unused file * partially update properties * make attrs property required * revert change to user_properties_utils.ts * fix bad merge --------- Co-authored-by: Mattermost Build Co-authored-by: Caleb Roseland --- .../channel_bookmarks_create_modal.tsx | 19 +- .../general/user_settings_general.test.tsx | 203 ++++++++++++++++- .../general/user_settings_general.tsx | 208 +++++++++++++++--- webapp/channels/src/i18n/en.json | 2 + .../src/actions/users.test.ts | 2 +- .../mattermost-redux/src/actions/users.ts | 4 +- webapp/channels/src/utils/url.tsx | 17 ++ webapp/platform/client/src/client4.ts | 4 +- webapp/platform/types/src/properties.ts | 2 + webapp/platform/types/src/users.ts | 2 +- 10 files changed, 403 insertions(+), 60 deletions(-) diff --git a/webapp/channels/src/components/channel_bookmarks/channel_bookmarks_create_modal.tsx b/webapp/channels/src/components/channel_bookmarks/channel_bookmarks_create_modal.tsx index 559a51844b..a1ca9dd463 100644 --- a/webapp/channels/src/components/channel_bookmarks/channel_bookmarks_create_modal.tsx +++ b/webapp/channels/src/components/channel_bookmarks/channel_bookmarks_create_modal.tsx @@ -27,7 +27,7 @@ import LoadingSpinner from 'components/widgets/loading/loading_spinner'; import Constants from 'utils/constants'; import {isKeyPressed} from 'utils/keyboard'; -import {isValidUrl, parseLink, removeScheme} from 'utils/url'; +import {removeScheme, validHttpUrl} from 'utils/url'; import {generateId} from 'utils/utils'; import type {GlobalState} from 'types/store'; @@ -693,20 +693,3 @@ export const useBookmarkLinkValidation = (link: string, onValidated: (validatedL return [error, {loading: Boolean(loading), suppressed}] as const; }; - -export const validHttpUrl = (input: string) => { - const val = parseLink(input); - - if (!val || !isValidUrl(val)) { - return null; - } - - let url; - try { - url = new URL(val); - } catch { - return null; - } - - return url; -}; diff --git a/webapp/channels/src/components/user_settings/general/user_settings_general.test.tsx b/webapp/channels/src/components/user_settings/general/user_settings_general.test.tsx index 1d7e07fa65..74589bd9d2 100644 --- a/webapp/channels/src/components/user_settings/general/user_settings_general.test.tsx +++ b/webapp/channels/src/components/user_settings/general/user_settings_general.test.tsx @@ -263,7 +263,6 @@ describe('components/user_settings/general/UserSettingsGeneral', () => { expect(await screen.getByRole('button', {name: `${customProfileAttribute.name} Edit`})).toBeInTheDocument(); props.user = {...testUser, custom_profile_attributes: {field1: 'FieldOneValue'}}; - console.log(props.user); rerender(); expect(props.actions.getCustomProfileAttributeValues).toHaveBeenCalledTimes(1); expect(await screen.findByText('FieldOneValue')).toBeInTheDocument(); @@ -282,6 +281,62 @@ describe('components/user_settings/general/UserSettingsGeneral', () => { expect(await screen.getByRole('textbox', {name: `${customProfileAttribute.name}`})).toBeInTheDocument(); }); + test('should show select Custom Attribute Field with value', async () => { + const selectAttribute: UserPropertyField = { + ...customProfileAttribute, + type: 'select', + attrs: { + value_type: '', + visibility: 'when_set', + sort_order: 0, + options: [ + {id: 'opt1', name: 'Option 1', color: ''}, + {id: 'opt2', name: 'Option 2', color: ''}, + ], + }, + }; + + const testUser = {...user, custom_profile_attributes: {field1: 'opt1'}}; + const props = { + ...requiredProps, + enableCustomProfileAttributes: true, + customProfileAttributeFields: [selectAttribute], + user: testUser, + activeSection: 'customAttribute_field1', + }; + + renderWithContext(); + expect(await screen.getByText('Option 1')).toBeInTheDocument(); + }); + + test('should show multiselect Custom Attribute Field with value', async () => { + const multiselectAttribute: UserPropertyField = { + ...customProfileAttribute, + type: 'multiselect', + attrs: { + value_type: '', + visibility: 'when_set', + sort_order: 0, + options: [ + {id: 'opt1', name: 'Option 1', color: ''}, + {id: 'opt2', name: 'Option 2', color: ''}, + ], + }, + }; + + const testUser = {...user, custom_profile_attributes: {field1: 'opt2'}}; + const props = { + ...requiredProps, + enableCustomProfileAttributes: true, + customProfileAttributeFields: [multiselectAttribute], + user: testUser, + activeSection: 'customAttribute_field1', + }; + + renderWithContext(); + expect(await screen.getByText('Option 2')).toBeInTheDocument(); + }); + test('submitAttribute() should have called saveCustomProfileAttribute', async () => { const saveCustomProfileAttribute = jest.fn().mockResolvedValue({field1: 'Updated Value'}); const props = { @@ -304,4 +359,150 @@ describe('components/user_settings/general/UserSettingsGeneral', () => { expect(saveCustomProfileAttribute).toHaveBeenCalledTimes(1); expect(saveCustomProfileAttribute).toHaveBeenCalledWith('user_id', 'field1', 'Updated Value'); }); + + test('submitAttribute() should handle server error', async () => { + const saveCustomProfileAttribute = jest.fn().mockResolvedValue({error: {message: 'Server Error'}}); + const props = { + ...requiredProps, + enableCustomProfileAttributes: true, + actions: {...requiredProps.actions, saveCustomProfileAttribute}, + customProfileAttributeFields: [customProfileAttribute], + user: {...user}, + activeSection: 'customAttribute_field1', + }; + + renderWithContext(); + + userEvent.clear(screen.getByRole('textbox', {name: `${customProfileAttribute.name}`})); + userEvent.type(screen.getByRole('textbox', {name: `${customProfileAttribute.name}`}), 'Updated Value'); + userEvent.click(screen.getByRole('button', {name: 'Save'})); + + expect(await screen.findByText('Server Error')).toBeInTheDocument(); + }); + + test('updateSelectAttribute() should handle single select changes', 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: ''}, + {id: 'opt2', name: 'Option 2', color: ''}, + ], + }, + }; + + const props = { + ...requiredProps, + enableCustomProfileAttributes: true, + customProfileAttributeFields: [selectAttribute], + user: {...user}, + activeSection: 'customAttribute_field1', + actions: { + ...requiredProps.actions, + saveCustomProfileAttribute, + }, + }; + + renderWithContext(); + + const select = await screen.findByText('Select'); + userEvent.click(select); + userEvent.click(await screen.findByText('Option 2')); + + const saveButton = screen.getByRole('button', {name: 'Save'}); + userEvent.click(saveButton); + + expect(props.actions.saveCustomProfileAttribute).toHaveBeenCalledWith('user_id', 'field1', 'opt2'); + }); + + test('updateSelectAttribute() should handle multi-select changes', 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: 'opt2', name: 'Option 2', color: ''}, + ], + }, + }; + + const props = { + ...requiredProps, + enableCustomProfileAttributes: true, + customProfileAttributeFields: [multiselectAttribute], + user: {...user}, + activeSection: 'customAttribute_field1', + actions: { + ...requiredProps.actions, + saveCustomProfileAttribute, + }, + }; + + renderWithContext(); + + const select = await screen.findByText('Select'); + userEvent.click(select); + userEvent.click(await screen.findByText('Option 1')); + + userEvent.click(await screen.findByText('Option 1')); + userEvent.click(await screen.findByText('Option 2')); + + const saveButton = screen.getByRole('button', {name: 'Save'}); + userEvent.click(saveButton); + + expect(props.actions.saveCustomProfileAttribute).toHaveBeenCalledWith('user_id', 'field1', ['opt1', 'opt2']); + }); + + test('updateSelectAttribute() should handle clearing selections', 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: ''}, + {id: 'opt2', name: 'Option 2', color: ''}, + ], + }, + }; + + const testUser = {...user, custom_profile_attributes: {field1: 'opt1'}}; + const props = { + ...requiredProps, + enableCustomProfileAttributes: true, + customProfileAttributeFields: [selectAttribute], + user: testUser, + activeSection: 'customAttribute_field1', + actions: { + ...requiredProps.actions, + saveCustomProfileAttribute, + }, + }; + + const {container} = renderWithContext(); + + const clearIndicator = container.querySelector('.react-select__clear-indicator'); + expect(clearIndicator).toBeInTheDocument(); + + userEvent.click(clearIndicator!); + await screen.findByText('Select'); + + const saveButton = screen.getByRole('button', {name: 'Save'}); + userEvent.click(saveButton); + + expect(props.actions.saveCustomProfileAttribute).toHaveBeenCalledWith('user_id', 'field1', ''); + }); }); diff --git a/webapp/channels/src/components/user_settings/general/user_settings_general.tsx b/webapp/channels/src/components/user_settings/general/user_settings_general.tsx index f6d9cb3433..1a637bfdbe 100644 --- a/webapp/channels/src/components/user_settings/general/user_settings_general.tsx +++ b/webapp/channels/src/components/user_settings/general/user_settings_general.tsx @@ -4,10 +4,12 @@ /* eslint-disable max-lines */ import React, {PureComponent} from 'react'; -import {defineMessage, defineMessages, FormattedDate, FormattedMessage, injectIntl} from 'react-intl'; +import {defineMessage, defineMessages, FormattedDate, FormattedMessage, FormattedList, injectIntl} from 'react-intl'; import type {IntlShape} from 'react-intl'; +import ReactSelect from 'react-select'; +import type {OnChangeValue, ActionMeta, StylesConfig} from 'react-select'; -import type {UserPropertyField} from '@mattermost/types/properties'; +import type {UserPropertyField, PropertyFieldOption} from '@mattermost/types/properties'; import type {UserProfile} from '@mattermost/types/users'; import type {LogErrorOptions} from 'mattermost-redux/actions/errors'; @@ -23,6 +25,7 @@ import SettingPicture from 'components/setting_picture'; import LoadingWrapper from 'components/widgets/loading/loading_wrapper'; import {AnnouncementBarMessages, AnnouncementBarTypes, AcceptedProfileImageTypes, Constants, ValidationErrors} from 'utils/constants'; +import {validHttpUrl} from 'utils/url'; import * as Utils from 'utils/utils'; import SettingDesktopHeader from '../headers/setting_desktop_header'; @@ -45,6 +48,10 @@ const holders = defineMessages({ id: 'user.settings.general.validEmail', defaultMessage: 'Please enter a valid email address.', }, + validUrl: { + id: 'user.settings.general.validUrl', + defaultMessage: 'Please enter a valid url.', + }, emailMatch: { id: 'user.settings.general.emailMatch', defaultMessage: 'The new emails you entered do not match.', @@ -99,6 +106,34 @@ const holders = defineMessages({ }, }); +export type SelectOption = { + value: string; + label: string; +}; + +const selectStyles: StylesConfig = { + valueContainer: (baseStyles) => ({ + ...baseStyles, + height: 'auto', + minHeight: '38px', + flexWrap: 'wrap', + whiteSpace: 'normal', + }), + multiValue: (baseStyles) => ({ + ...baseStyles, + margin: '2px', + }), + control: (baseStyles) => ({ + ...baseStyles, + height: 'auto', + minHeight: '38px', + }), + multiValueLabel: (baseStyles) => ({ + ...baseStyles, + padding: '2px 6px', + }), +}; + export type Props = { intl: IntlShape; user: UserProfile; @@ -117,7 +152,7 @@ export type Props = { sendVerificationEmail: (email: string) => Promise; setDefaultProfileImage: (id: string) => void; uploadProfileImage: (id: string, file: File) => Promise; - saveCustomProfileAttribute: (userID: string, attributeID: string, attributeValue: string) => Promise>>; + saveCustomProfileAttribute: (userID: string, attributeID: string, attributeValue: string | string[]) => Promise>>; getCustomProfileAttributeValues: (userID: string) => Promise>>; }; requireEmailVerification?: boolean; @@ -151,7 +186,7 @@ type State = { clientError?: string | null; serverError?: string | {server_error_id: string; message: string}; emailError?: string; - customAttributeValues: Record; + customAttributeValues: Record; } export class UserSettingsGeneralTab extends PureComponent { @@ -407,12 +442,32 @@ export class UserSettingsGeneralTab extends PureComponent { }; submitAttribute = async (settings: string[]) => { + const {formatMessage} = this.props.intl; + const attributeID = settings[0]; - const attributeValue = this.state.customAttributeValues?.[attributeID]; - if (attributeValue == null) { + const attributeField = this.props.customProfileAttributeFields.find((field) => field.id === attributeID); + if (attributeField === undefined) { return; } + let attributeValue: string | string[] = this.state.customAttributeValues?.[attributeID]; + if (typeof attributeValue === 'string' && attributeField.attrs && attributeField.attrs.value_type) { + if (attributeField.attrs.value_type === 'email') { + if (attributeValue !== '' && !isEmail(attributeValue)) { + this.setState({clientError: formatMessage(holders.validEmail), emailError: '', serverError: ''}); + return; + } + } + if (attributeField.attrs.value_type === 'url') { + if (attributeValue !== '' && !validHttpUrl(attributeValue)) { + this.setState({clientError: formatMessage(holders.validUrl), emailError: '', serverError: ''}); + return; + } + } + } + if (attributeField.type === 'multiselect' && !attributeValue) { + attributeValue = []; + } trackEvent('settings', 'user_settings_update', {field: 'customAttributeValues-' + attributeID}); this.setState({sectionIsSaving: true}); @@ -423,7 +478,7 @@ export class UserSettingsGeneralTab extends PureComponent { this.updateSection(''); this.setState({customAttributeValues: {...this.state.customAttributeValues, ...data}}); } else if (err) { - const serverError = err; + const serverError = err.message; this.setState({serverError, emailError: '', clientError: '', sectionIsSaving: false}); } }); @@ -472,6 +527,27 @@ export class UserSettingsGeneralTab extends PureComponent { } }; + updateSelectAttribute = (selectedOption: OnChangeValue, action: ActionMeta, fieldID: string) => { + const attributeValues = {...this.state.customAttributeValues}; + + if (!selectedOption) { + attributeValues[fieldID] = ''; + } else if (Array.isArray(selectedOption)) { + // Handle multi-select + attributeValues[fieldID] = selectedOption. + filter((option): option is SelectOption => + Boolean(option && Object.hasOwn(option, 'value'))). + map((option) => option.value); + } else if ('value' in selectedOption) { + // Handle single select + attributeValues[fieldID] = selectedOption.value || ''; + } else { + attributeValues[fieldID] = ''; + } + + this.setState({customAttributeValues: attributeValues}); + }; + updateAttribute = (e: React.ChangeEvent) => { const attributeValues = Object.assign({}, this.state.customAttributeValues); const id = e.target.id.substring(e.target.id.indexOf('_') + 1); @@ -1320,7 +1396,8 @@ export class UserSettingsGeneralTab extends PureComponent { }; createCustomAttributeSection = () => { - if (!this.props.enableCustomProfileAttributes || this.props.customProfileAttributeFields == null) { + const {formatMessage} = this.props.intl; + if (this.props.customProfileAttributeFields == null) { return <>; } @@ -1329,6 +1406,31 @@ export class UserSettingsGeneralTab extends PureComponent { const active = this.props.activeSection === sectionName; let max = null; + const getDisplayValue = (attributeValue: string | string[]) => { + if (!attributeValue || (!Array.isArray(attributeValue) && !attributeValue.length)) { + return ''; + } + + if (attribute.type === 'select' || attribute.type === 'multiselect') { + const attribOptions = attribute.attrs.options; + if (!attribOptions) { + return ''; + } + if (Array.isArray(attributeValue)) { + return attributeValue.map((value) => { + const option = attribOptions.find((o) => o.id === value); + return {label: option?.name, value: option?.id}; + }); + } + + // Handle single select + const option = attribOptions.find((o) => o.id === attributeValue); + return {label: option?.name, value: option?.id}; + } + + return attributeValue as string; + }; + if (active) { const inputs = []; @@ -1339,29 +1441,58 @@ export class UserSettingsGeneralTab extends PureComponent { attributeLabel = ''; } - inputs.push( -
- -
- -
-
, - ); - + if (attribute.type === 'select' || attribute.type === 'multiselect') { + const attribOptions: PropertyFieldOption[] = attribute.attrs!.options as PropertyFieldOption[]; + const opts = attribOptions.map((o) => { + return {label: o.name, value: o.id} as SelectOption; + }); + inputs.push( + this.updateSelectAttribute(v, a, attribute.id)} + />, + ); + } else { + const inputType = attribute.type as string; + inputs.push( +
+ +
+ +
+
, + ); + } const extraInfo = ( { ); } let describe: JSX.Element|string = ''; - const attributeValue = this.props.user.custom_profile_attributes?.[attribute.id]; - if (attributeValue) { - describe = attributeValue; - } else { + if (this.props.user.custom_profile_attributes?.[attribute.id]) { + const attributeValue = getDisplayValue(this.props.user.custom_profile_attributes?.[attribute.id]); + if (typeof attributeValue === 'string') { + describe = attributeValue; + } else if (Array.isArray(attributeValue) && attributeValue.length > 0) { + describe = attrib.label)}/>; + } else if (!Array.isArray(attributeValue) && Object.hasOwn(attributeValue, 'label')) { + describe = attributeValue.label || ''; + } + } + if (!describe) { describe = ( { 123: 'NewValue', }); - const response = await store.dispatch(Actions.saveCustomProfileAttribute(currentUser.id, '123', ' NewValue ')); + const response = await store.dispatch(Actions.saveCustomProfileAttribute(currentUser.id, '123', 'NewValue')); const data = response.data!; expect(data).toEqual({123: 'NewValue'}); }); diff --git a/webapp/channels/src/packages/mattermost-redux/src/actions/users.ts b/webapp/channels/src/packages/mattermost-redux/src/actions/users.ts index 2534def587..ff8fe1a23f 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/actions/users.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/actions/users.ts @@ -988,10 +988,10 @@ export function updateMe(user: Partial): ActionFuncAsync> { +export function saveCustomProfileAttribute(userID: string, attributeID: string, attributeValue: string | string[]): ActionFuncAsync> { return async (dispatch) => { try { - const values = {[attributeID]: attributeValue.trim()}; + const values = {[attributeID]: attributeValue || ''}; const data = await Client4.updateCustomProfileAttributeValues(values); return {data}; } catch (error) { diff --git a/webapp/channels/src/utils/url.tsx b/webapp/channels/src/utils/url.tsx index c0be7b7617..f589fdd9bd 100644 --- a/webapp/channels/src/utils/url.tsx +++ b/webapp/channels/src/utils/url.tsx @@ -354,3 +354,20 @@ export function parseLink(href: string, defaultSecure = location.protocol === 'h return outHref; } + +export const validHttpUrl = (input: string) => { + const val = parseLink(input); + + if (!val || !isValidUrl(val)) { + return null; + } + + let url; + try { + url = new URL(val); + } catch { + return null; + } + + return url; +}; diff --git a/webapp/platform/client/src/client4.ts b/webapp/platform/client/src/client4.ts index 94e4a50791..7a803a4911 100644 --- a/webapp/platform/client/src/client4.ts +++ b/webapp/platform/client/src/client4.ts @@ -2106,8 +2106,8 @@ export default class Client4 { ); }; - updateCustomProfileAttributeValues = (attributeValues: Record) => { - return this.doFetch>( + updateCustomProfileAttributeValues = (attributeValues: Record) => { + return this.doFetch>( `${this.getCustomProfileAttributeValuesRoute()}`, {method: 'PATCH', body: JSON.stringify(attributeValues)}, ); diff --git a/webapp/platform/types/src/properties.ts b/webapp/platform/types/src/properties.ts index ebd1972e9d..8c9bbf2cab 100644 --- a/webapp/platform/types/src/properties.ts +++ b/webapp/platform/types/src/properties.ts @@ -34,7 +34,9 @@ export type PropertyValue = { delete_at: number; } +export type UserPropertyFieldType = 'text' | 'select' | 'multiselect'; export type UserPropertyFieldGroupID = 'custom_profile_attributes'; +export type UserPropertyValueType = 'phone' | 'url' | ''; export type FieldVisibility = 'always' | 'hidden' | 'when_set'; export type FieldValueType = diff --git a/webapp/platform/types/src/users.ts b/webapp/platform/types/src/users.ts index 4248ff2214..6f6a7ee8a3 100644 --- a/webapp/platform/types/src/users.ts +++ b/webapp/platform/types/src/users.ts @@ -61,7 +61,7 @@ export type UserProfile = { remote_id?: string; status?: string; failed_attempts?: number; - custom_profile_attributes?: Record; + custom_profile_attributes?: Record; }; export type UserProfileWithLastViewAt = UserProfile & {