Skip to content

Commit 7891079

Browse files
WillyfrogWillyfrog
authored
[MM-64358] don't save unneeded custom profile attributes (#8971)
* send only needed changes, take saml into account * adhere to naming conventions * fix tests * address review comments
1 parent 6bdcf2b commit 7891079

File tree

6 files changed

+519
-26
lines changed

6 files changed

+519
-26
lines changed

‎app/screens/edit_profile/components/form.test.tsx‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,4 +436,197 @@ describe('ProfileForm', () => {
436436
// Should render as a text field since no field definition exists
437437
expect(getByTestId('edit_profile_form.customAttributes.unknownField')).toBeTruthy();
438438
});
439+
440+
describe('SAML Field Disabling', () => {
441+
it('should disable custom fields when SAML-linked', () => {
442+
const customFields = [
443+
TestHelper.fakeCustomProfileFieldModel({
444+
id: 'saml-field',
445+
name: 'SAML Field',
446+
type: 'text',
447+
attrs: {
448+
saml: 'Department', // SAML-linked
449+
},
450+
}),
451+
TestHelper.fakeCustomProfileFieldModel({
452+
id: 'normal-field',
453+
name: 'Normal Field',
454+
type: 'text',
455+
attrs: {
456+
saml: '', // Not SAML-linked
457+
},
458+
}),
459+
];
460+
461+
const props = {
462+
...baseProps,
463+
enableCustomAttributes: true,
464+
customFields,
465+
userInfo: {
466+
...baseProps.userInfo,
467+
customAttributes: {
468+
'saml-field': {
469+
id: 'saml-field',
470+
name: 'SAML Field',
471+
type: 'text',
472+
value: 'Engineering',
473+
},
474+
'normal-field': {
475+
id: 'normal-field',
476+
name: 'Normal Field',
477+
type: 'text',
478+
value: 'Mobile Team',
479+
},
480+
},
481+
},
482+
};
483+
484+
const {getByTestId} = renderWithIntl(
485+
<ProfileForm {...props}/>,
486+
);
487+
488+
const samlField = getByTestId('edit_profile_form.customAttributes.saml-field.input.disabled');
489+
const normalField = getByTestId('edit_profile_form.customAttributes.normal-field.input');
490+
491+
// SAML field should be disabled
492+
expect(samlField.props.editable).toBe(false);
493+
494+
// Normal field should be enabled
495+
expect(normalField.props.editable).toBe(true);
496+
});
497+
498+
it('should enable custom fields when SAML attribute is empty', () => {
499+
const customFields = [
500+
TestHelper.fakeCustomProfileFieldModel({
501+
id: 'normal-field',
502+
name: 'Normal Field',
503+
type: 'text',
504+
attrs: {
505+
saml: '', // Empty SAML attribute
506+
},
507+
}),
508+
];
509+
510+
const props = {
511+
...baseProps,
512+
enableCustomAttributes: true,
513+
customFields,
514+
userInfo: {
515+
...baseProps.userInfo,
516+
customAttributes: {
517+
'normal-field': {
518+
id: 'normal-field',
519+
name: 'Normal Field',
520+
type: 'text',
521+
value: 'Some value',
522+
},
523+
},
524+
},
525+
};
526+
527+
const {getByTestId} = renderWithIntl(
528+
<ProfileForm {...props}/>,
529+
);
530+
531+
const normalField = getByTestId('edit_profile_form.customAttributes.normal-field.input');
532+
expect(normalField.props.editable).toBe(true);
533+
});
534+
535+
it('should enable custom fields when SAML attribute is missing', () => {
536+
const customFields = [
537+
TestHelper.fakeCustomProfileFieldModel({
538+
id: 'normal-field',
539+
name: 'Normal Field',
540+
type: 'text',
541+
attrs: {
542+
543+
// No saml attribute at all
544+
},
545+
}),
546+
];
547+
548+
const props = {
549+
...baseProps,
550+
enableCustomAttributes: true,
551+
customFields,
552+
userInfo: {
553+
...baseProps.userInfo,
554+
customAttributes: {
555+
'normal-field': {
556+
id: 'normal-field',
557+
name: 'Normal Field',
558+
type: 'text',
559+
value: 'Some value',
560+
},
561+
},
562+
},
563+
};
564+
565+
const {getByTestId} = renderWithIntl(
566+
<ProfileForm {...props}/>,
567+
);
568+
569+
const normalField = getByTestId('edit_profile_form.customAttributes.normal-field.input');
570+
expect(normalField.props.editable).toBe(true);
571+
});
572+
573+
it('should handle custom fields without field definitions (defaults to enabled)', () => {
574+
const props = {
575+
...baseProps,
576+
enableCustomAttributes: true,
577+
customFields: [], // No field definitions
578+
userInfo: {
579+
...baseProps.userInfo,
580+
customAttributes: {
581+
'unknown-field': {
582+
id: 'unknown-field',
583+
name: 'Unknown Field',
584+
type: 'text',
585+
value: 'Some value',
586+
},
587+
},
588+
},
589+
};
590+
591+
const {getByTestId} = renderWithIntl(
592+
<ProfileForm {...props}/>,
593+
);
594+
595+
const unknownField = getByTestId('edit_profile_form.customAttributes.unknown-field.input');
596+
597+
// Should default to enabled when no field definition exists
598+
expect(unknownField.props.editable).toBe(true);
599+
});
600+
601+
it('should disable standard profile fields when SAML/LDAP locked', () => {
602+
const props = {
603+
...baseProps,
604+
currentUser: TestHelper.fakeUserModel({
605+
...baseProps.currentUser,
606+
authService: 'saml',
607+
}),
608+
lockedFirstName: true,
609+
lockedLastName: false,
610+
lockedNickname: true,
611+
lockedPosition: false,
612+
};
613+
614+
const {getByTestId} = renderWithIntl(
615+
<ProfileForm {...props}/>,
616+
);
617+
618+
const firstNameField = getByTestId('edit_profile_form.firstName.input.disabled');
619+
const lastNameField = getByTestId('edit_profile_form.lastName.input');
620+
const nicknameField = getByTestId('edit_profile_form.nickname.input.disabled');
621+
const positionField = getByTestId('edit_profile_form.position.input');
622+
623+
// Locked fields should be disabled
624+
expect(firstNameField.props.editable).toBe(false);
625+
expect(nicknameField.props.editable).toBe(false);
626+
627+
// Unlocked fields should be enabled
628+
expect(lastNameField.props.editable).toBe(true);
629+
expect(positionField.props.editable).toBe(true);
630+
});
631+
});
439632
});

‎app/screens/edit_profile/components/form.tsx‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import useFieldRefs from '@hooks/field_refs';
1010
import {t} from '@i18n';
1111
import {getErrorMessage} from '@utils/errors';
1212
import {logError} from '@utils/log';
13-
import {sortCustomProfileAttributes, formatOptionsForSelector} from '@utils/user';
13+
import {sortCustomProfileAttributes, formatOptionsForSelector, isCustomFieldSamlLinked} from '@utils/user';
1414

1515
import DisabledFields from './disabled_fields';
1616
import EmailField from './email_field';
@@ -87,6 +87,8 @@ const POSITION_FIELD = 'position';
8787

8888
const profileKeys = [FIRST_NAME_FIELD, LAST_NAME_FIELD, USERNAME_FIELD, EMAIL_FIELD, NICKNAME_FIELD, POSITION_FIELD];
8989

90+
export const getFieldKey = (key: string) => `${CUSTOM_ATTRS_PREFIX}.${key}`;
91+
9092
const ProfileForm = ({
9193
canSave, currentUser, isTablet,
9294
lockedFirstName, lockedLastName, lockedNickname, lockedPosition,
@@ -99,18 +101,18 @@ const ProfileForm = ({
99101
const {formatMessage} = intl;
100102
const errorMessage = error == null ? undefined : getErrorMessage(error, intl) as string;
101103

102-
const total_custom_attrs = useMemo(() => (
104+
const totalCustomAttrs = useMemo(() => (
103105
enableCustomAttributes ? Object.keys(userInfo.customAttributes).length : 0
104106
), [enableCustomAttributes, userInfo.customAttributes]);
105107

106108
const formKeys = useMemo(() => {
107109
const newKeys = Object.keys(userInfo.customAttributes).sort(
108110
(a: string, b: string): number => {
109111
return sortCustomProfileAttributes(userInfo.customAttributes[a], userInfo.customAttributes[b]);
110-
}).map((k) => `${CUSTOM_ATTRS_PREFIX}.${k}`);
112+
}).map((k) => getFieldKey(k));
111113

112-
return total_custom_attrs === 0 ? profileKeys : [...profileKeys, ...newKeys];
113-
}, [userInfo.customAttributes, total_custom_attrs]);
114+
return totalCustomAttrs === 0 ? profileKeys : [...profileKeys, ...newKeys];
115+
}, [userInfo.customAttributes, totalCustomAttrs]);
114116

115117
// Create a map of field definitions for quick lookup
116118
const customFieldsMap = useMemo(() => {
@@ -167,8 +169,18 @@ const ProfileForm = ({
167169
};
168170
}
169171
});
172+
173+
// Handle custom attributes - check if SAML linked
174+
Object.keys(userInfo.customAttributes).forEach((key) => {
175+
const customField = customFieldsMap.get(key);
176+
const fieldKey = getFieldKey(key);
177+
if (customField && fields[fieldKey]) {
178+
fields[fieldKey].isDisabled = isCustomFieldSamlLinked(customField);
179+
}
180+
});
181+
170182
return fields;
171-
}, [lockedFirstName, lockedLastName, lockedNickname, lockedPosition, currentUser.authService, formKeys, errorMessage]);
183+
}, [lockedFirstName, lockedLastName, lockedNickname, lockedPosition, currentUser.authService, formKeys, errorMessage, customFieldsMap, userInfo.customAttributes]);
172184

173185
const onFocusNextField = useCallback(((fieldKey: string) => {
174186
const findNextField = () => {

0 commit comments

Comments
 (0)