toki/mattermost-mobile
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

[MM-62666] Cpa sort order (#8618)

Browse source 
* refactor: Use fetchCustomAttributes for user profile custom attributes

feat: Add optional filter for empty custom attributes

simplify and fix types

add again the request time, take into account errors

test: Update fetchCustomAttributes tests for filterEmpty parameter

feat: Add sort_order to CustomAttribute in fetchCustomAttributes

feat: Add optional sort_order to CustomAttribute type

fix: Add type definition for sort_order in CustomProfileField attrs

feat: Add sorting function for custom profile attributes

docs: Add JSDoc comment explaining sortCustomProfileAttributes function

refactor: Improve sortCustomProfileAttributes with nullish coalescing and localeCompare

add order to custom profile attributes

* remove blank line

* add ordering to edit_profile screen

* keep types together

* address code review
This commit is contained in:
Guillermo Vayá 2025-02-25 10:54:38 +01:00 committed by GitHub
parent 5590f853d7
commit 63bba67eaf
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 348 additions and 74 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

55
app/actions/remote/user.test.ts
View file

@ -364,29 +364,68 @@ describe('get users', () => {
});
describe('Custom Profile Attributes', () => {
it('fetchCustomAttributes - base case', async () => {
it('fetchCustomAttributes - base case without filter', async () => {
mockClient.getCustomProfileAttributeFields = jest.fn().mockResolvedValue([
{id: 'field1', name: 'Field 1'},
{id: 'field2', name: 'Field 2'},
{id: 'field1', name: 'Field 1', attrs: {sort_order: 1}},
{id: 'field2', name: 'Field 2', attrs: {sort_order: 2}},
{id: 'field3', name: 'Field 3', attrs: {sort_order: 3}},
]);
mockClient.getCustomProfileAttributeValues = jest.fn().mockResolvedValue({
field1: 'value1',
field2: 'value2',
field2: '',
field3: 'value3',
});
const result = await fetchCustomAttributes(serverUrl, 'user1');
expect(result.error).toBeUndefined();
expect(result.attributes).toBeDefined();
expect(Object.keys(result.attributes!)).toHaveLength(3);
expect(result.attributes!.field1).toEqual({
id: 'field1',
name: 'Field 1',
value: 'value1',
sort_order: 1,
});
expect(result.attributes!.field2).toEqual({
id: 'field2',
name: 'Field 2',
value: '',
sort_order: 2,
});
expect(result.attributes!.field3).toEqual({
id: 'field3',
name: 'Field 3',
value: 'value3',
sort_order: 3,
});
});
it('fetchCustomAttributes - with filter empty values', async () => {
mockClient.getCustomProfileAttributeFields = jest.fn().mockResolvedValue([
{id: 'field1', name: 'Field 1'},
{id: 'field2', name: 'Field 2'},
{id: 'field3', name: 'Field 3'},
]);
mockClient.getCustomProfileAttributeValues = jest.fn().mockResolvedValue({
field1: 'value1',
field2: '',
field3: 'value3',
});
const result = await fetchCustomAttributes(serverUrl, 'user1', true);
expect(result.error).toBeUndefined();
expect(result.attributes).toBeDefined();
expect(Object.keys(result.attributes!)).toHaveLength(2);
expect(result.attributes!.field1).toEqual({
id: 'field1',
name: 'Field 1',
value: 'value1',
});
expect(result.attributes!.field2).toEqual({
id: 'field2',
name: 'Field 2',
value: 'value2',
expect(result.attributes!.field2).toBeUndefined();
expect(result.attributes!.field3).toEqual({
id: 'field3',
name: 'Field 3',
value: 'value3',
});
});

20
app/actions/remote/user.ts
View file

@ -25,8 +25,8 @@ import {fetchGroupsByNames} from './groups';
import {forceLogoutIfNecessary} from './session';
import type {Model} from '@nozbe/watermelondb';
import type {CustomAttribute, CustomProfileAttributeSimple, CustomProfileField, CustomAttributeSet} from '@typings/api/custom_profile_attributes';
import type UserModel from '@typings/database/models/servers/user';
import type {CustomAttribute, CustomAttributeSet} from '@typings/screens/edit_profile';
export type MyUserRequest = {
user?: UserProfile;
@ -880,7 +880,7 @@ export const getAllSupportedTimezones = async (serverUrl: string) => {
}
};
export const fetchCustomAttributes = async (serverUrl: string, userId: string): Promise<{attributes: CustomAttributeSet; error: unknown}> => {
export const fetchCustomAttributes = async (serverUrl: string, userId: string, filterEmpty = false): Promise<{attributes: CustomAttributeSet; error: unknown}> => {
try {
const client = NetworkManager.getClient(serverUrl);
const [fields, attrValues] = await Promise.all([
@ -890,12 +890,16 @@ export const fetchCustomAttributes = async (serverUrl: string, userId: string):
if (fields?.length > 0) {
const attributes: Record<string, CustomAttribute> = {};
fields.forEach((field) => {
attributes[field.id] = {
id: field.id,
name: field.name,
value: attrValues[field.id] || '',
};
fields.forEach((field: CustomProfileField) => {
const value = attrValues[field.id] || '';
if (!filterEmpty || value) {
attributes[field.id] = {
id: field.id,
name: field.name,
value,
sort_order: field.attrs?.sort_order,
};
}
});
return {attributes, error: undefined};
}

1
app/client/rest/custom_profile_attributes.ts
View file

@ -2,6 +2,7 @@
// See LICENSE.txt for license information.
import type ClientBase from './base';
import type {CustomProfileAttributeSimple, CustomProfileField} from '@typings/api/custom_profile_attributes';
export interface ClientCustomAttributesMix {
getCustomProfileAttributeFields: () => Promise<CustomProfileField[]>;

46
app/screens/edit_profile/components/form.test.tsx
View file

@ -8,6 +8,7 @@ import {renderWithIntl} from '@test/intl-test-helper';
import ProfileForm from './form';
import type {CustomAttributeSet} from '@typings/api/custom_profile_attributes';
import type UserModel from '@typings/database/models/servers/user';
import type {UserInfo} from '@typings/screens/edit_profile';
@ -126,4 +127,49 @@ describe('ProfileForm', () => {
expect(queryByTestId('edit_profile_form.customAttributes.field1')).toBeNull();
});
test('should maintain custom attributes sort order', () => {
const customAttributes: CustomAttributeSet = {
attr1: {
id: 'attr1',
name: 'Department',
value: 'Engineering',
sort_order: 1,
},
attr2: {
id: 'attr2',
name: 'Location',
value: 'Remote',
sort_order: 0,
},
attr3: {
id: 'attr3',
name: 'Start Date',
value: '2023',
sort_order: 2,
},
};
const props = {
...baseProps,
enableCustomAttributes: true,
userInfo: {
...baseProps.userInfo,
customAttributes,
},
};
const {getAllByTestId} = renderWithIntl(
<ProfileForm
{...props}
/>,
);
const attributeFields = getAllByTestId(/^edit_profile_form.customAttributes\.attr\d$/);
// Verify fields are rendered in sort order
expect(attributeFields[0].props.testID).toBe('edit_profile_form.customAttributes.attr2'); // sort_order: 0
expect(attributeFields[1].props.testID).toBe('edit_profile_form.customAttributes.attr1'); // sort_order: 1
expect(attributeFields[2].props.testID).toBe('edit_profile_form.customAttributes.attr3'); // sort_order: 2
});
});

8
app/screens/edit_profile/components/form.tsx
View file

@ -10,6 +10,7 @@ import useFieldRefs from '@hooks/field_refs';
import {t} from '@i18n';
import {getErrorMessage} from '@utils/errors';
import {logError} from '@utils/log';
import {sortCustomProfileAttributes} from '@utils/user';
import DisabledFields from './disabled_fields';
import EmailField from './email_field';
@ -100,7 +101,12 @@ const ProfileForm = ({
), [enableCustomAttributes, userInfo.customAttributes]);
const formKeys = useMemo(() => {
return total_custom_attrs === 0 ? profileKeys : [...profileKeys, ...(Object.keys(userInfo.customAttributes).map((k) => `${CUSTOM_ATTRS_PREFIX}.${k}`))];
const newKeys = Object.keys(userInfo.customAttributes).sort(
(a: string, b: string): number => {
return sortCustomProfileAttributes(userInfo.customAttributes[a], userInfo.customAttributes[b]);
}).map((k) => `${CUSTOM_ATTRS_PREFIX}.${k}`);
return total_custom_attrs === 0 ? profileKeys : [...profileKeys, ...newKeys];
}, [userInfo.customAttributes, total_custom_attrs]);
const userProfileFields: FieldSequence = useMemo(() => {

6
app/screens/user_profile/custom_attributes.tsx
View file

@ -8,14 +8,16 @@ import {FlatList} from 'react-native-gesture-handler';
import UserProfileLabel from './label';
import type {CustomAttribute} from '@typings/api/custom_profile_attributes';
type Props = {
nickname?: string;
position?: string;
localTime?: string;
customAttributes?: DisplayCustomAttribute[];
customAttributes?: CustomAttribute[];
}
const renderAttribute: ListRenderItem<DisplayCustomAttribute> = ({item}) => (
const renderAttribute: ListRenderItem<CustomAttribute> = ({item}) => (
<UserProfileLabel
title={item.name}
description={item.value}

188
app/screens/user_profile/user_info.test.tsx Normal file
View file

@ -0,0 +1,188 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import React from 'react';
import {fetchCustomAttributes} from '@actions/remote/user';
import {renderWithIntlAndTheme, screen, waitFor} from '@test/intl-test-helper';
import UserInfo from './user_info';
import type UserModel from '@database/models/server/user';
const localhost = 'http://localhost:8065';
jest.mock('@actions/remote/user', () => ({
fetchCustomAttributes: jest.fn().mockResolvedValue({}),
}));
jest.mock('@context/server', () => ({
useServerUrl: jest.fn().mockReturnValue(localhost),
}));
describe('screens/user_profile/UserInfo', () => {
beforeEach(() => {
// Reset mock before each test
(fetchCustomAttributes as jest.Mock).mockReset();
});
const baseProps = {
user: {
id: 'user1',
firstName: 'First',
lastName: 'Last',
username: 'username',
nickname: 'nick',
position: 'Developer',
} as UserModel,
isMyUser: false,
showCustomStatus: false,
showLocalTime: true,
showNickname: true,
showPosition: true,
enableCustomAttributes: true,
};
const baseCustomAttributes = {
attr1: {
id: 'attr1',
name: 'Department',
value: 'Engineering',
sort_order: 1,
},
attr2: {
id: 'attr2',
name: 'Location',
value: 'Remote',
sort_order: 0,
},
attr3: {
id: 'attr3',
name: 'Start Date',
value: '2023',
sort_order: 2,
},
};
test('should display custom attributes in sort order', async () => {
(fetchCustomAttributes as jest.Mock).mockResolvedValue({attributes: baseCustomAttributes});
renderWithIntlAndTheme(
<UserInfo {...baseProps}/>,
);
// Wait for the fetch to be called
await waitFor(() => {
expect(fetchCustomAttributes).toHaveBeenCalledWith(
localhost,
'user1',
true,
);
});
// Wait for all items to be rendered
await waitFor(() => {
// Standard attributes
expect(screen.getByText('Nickname')).toBeTruthy();
expect(screen.getByText('Position')).toBeTruthy();
expect(screen.getByText('nick')).toBeTruthy();
expect(screen.getByText('Developer')).toBeTruthy();
// Custom attributes (sorted by sort_order)
expect(screen.getByText('Location')).toBeTruthy(); // sort_order: 0
expect(screen.getByText('Remote')).toBeTruthy();
expect(screen.getByText('Department')).toBeTruthy(); // sort_order: 1
expect(screen.getByText('Engineering')).toBeTruthy();
expect(screen.getByText('Start Date')).toBeTruthy(); // sort_order: 2
expect(screen.getByText('2023')).toBeTruthy();
});
// Verify the order of elements
const titles = screen.getAllByTestId(/.*\.title$/);
expect(titles.map((el) => el.props.children)).toEqual([
'Nickname',
'Position',
'Location', // sort_order: 0
'Department', // sort_order: 1
'Start Date', // sort_order: 2
]);
const descriptions = screen.getAllByTestId(/.*\.description$/);
expect(descriptions.map((el) => el.props.children)).toEqual([
'nick',
'Developer',
'Remote', // sort_order: 0
'Engineering', // sort_order: 1
'2023', // sort_order: 2
]);
});
it('should display custom attributes in order when some have no order', async () => {
const withEmptyCustomAttributes = {
attr1: {
id: 'attr1',
name: 'Department',
value: 'Engineering',
},
attr2: {
id: 'attr2',
name: 'Location',
value: 'Remote',
sort_order: 0,
},
attr3: {
id: 'attr3',
name: 'Start Date',
value: '2023',
sort_order: 2,
},
};
(fetchCustomAttributes as jest.Mock).mockResolvedValue({attributes: withEmptyCustomAttributes});
renderWithIntlAndTheme(
<UserInfo {...baseProps}/>,
);
await waitFor(() => {
expect(fetchCustomAttributes).toHaveBeenCalledWith(
localhost,
'user1',
true,
);
});
await waitFor(() => {
// Standard attributes remain
expect(screen.getByText('Nickname')).toBeTruthy();
expect(screen.getByText('Position')).toBeTruthy();
expect(screen.getByText('nick')).toBeTruthy();
expect(screen.getByText('Developer')).toBeTruthy();
// Custom attributes (sorted by sort_order)
expect(screen.getByText('Location')).toBeTruthy(); // sort_order: 0
expect(screen.getByText('Remote')).toBeTruthy();
expect(screen.getByText('Start Date')).toBeTruthy(); // sort_order: 2
expect(screen.getByText('2023')).toBeTruthy();
expect(screen.queryByText('Department')).toBeTruthy(); // sort_order: undefined
expect(screen.queryByText('Engineering')).toBeTruthy();
});
// Verify the order of elements
const titles = screen.getAllByTestId(/.*\.title$/);
expect(titles.map((el) => el.props.children)).toEqual([
'Nickname',
'Position',
'Location', // sort_order: 0
'Start Date', // sort_order: 2
'Department', // sort_order: undefined
]);
const descriptions = screen.getAllByTestId(/.*\.description$/);
expect(descriptions.map((el) => el.props.children)).toEqual([
'nick',
'Developer',
'Remote', // sort_order: 0
'2023', // sort_order: 2
'Engineering', // sort_order: undefined
]);
});
});

39
app/screens/user_profile/user_info.tsx
View file

@ -1,16 +1,17 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import React, {useEffect, useRef, useState} from 'react';
import React, {useEffect, useState, useRef} from 'react';
import {fetchCustomAttributes} from '@actions/remote/user';
import {useServerUrl} from '@context/server';
import NetworkManager from '@managers/network_manager';
import {getUserCustomStatus} from '@utils/user';
import {getUserCustomStatus, sortCustomProfileAttributes} from '@utils/user';
import CustomAttributes from './custom_attributes';
import UserProfileCustomStatus from './custom_status';
import type {UserModel} from '@database/models/server';
import type {CustomAttribute} from '@typings/api/custom_profile_attributes';
type Props = {
localTime?: string;
@ -22,12 +23,13 @@ type Props = {
enableCustomAttributes?: boolean;
}
const emptyList: DisplayCustomAttribute[] = []; /** avoid re-renders **/
const emptyList: CustomAttribute[] = []; /** avoid re-renders **/
const UserInfo = ({localTime, showCustomStatus, showLocalTime, showNickname, showPosition, user, enableCustomAttributes}: Props) => {
const customStatus = getUserCustomStatus(user);
const serverUrl = useServerUrl();
const [customAttributes, setCustomAttributes] = useState<DisplayCustomAttribute[]>(emptyList);
const [customAttributes, setCustomAttributes] = useState<CustomAttribute[]>(emptyList);
const lastRequest = useRef(0);
useEffect(() => {
@ -35,28 +37,11 @@ const UserInfo = ({localTime, showCustomStatus, showLocalTime, showNickname, sho
const fetchData = async () => {
const reqTime = Date.now();
lastRequest.current = reqTime;
try {
const client = NetworkManager.getClient(serverUrl);
const [fields, attrValues] = await Promise.all([
client.getCustomProfileAttributeFields(),
client.getCustomProfileAttributeValues(user.id),
]);
// ignore results if there was a newer request
if (fields && fields.length > 0 && lastRequest.current === reqTime) {
const attributes = fields.map((field) => {
if (attrValues[field.id]) {
return ({
id: field.id,
name: field.name,
value: attrValues[field.id] || '',
} as DisplayCustomAttribute);
}
return {} as DisplayCustomAttribute; // this will be cleaned out in CustomAttributes along with the fixed attributes.
});
setCustomAttributes(attributes);
}
} catch {
const {attributes, error} = await fetchCustomAttributes(serverUrl, user.id, true);
if (!error && lastRequest.current === reqTime) {
const attributesList = Object.values(attributes).sort(sortCustomProfileAttributes);
setCustomAttributes(attributesList);
} else {
setCustomAttributes(emptyList);
}
};

14
app/utils/user/index.ts
View file

@ -10,6 +10,7 @@ import {CustomStatusDurationEnum} from '@constants/custom_status';
import {DEFAULT_LOCALE, getLocalizedMessage, t} from '@i18n';
import {toTitleCase} from '@utils/helpers';
import type {CustomAttribute} from '@typings/api/custom_profile_attributes';
import type UserModel from '@typings/database/models/servers/user';
import type {IntlShape} from 'react-intl';
@ -393,3 +394,16 @@ export const getLastPictureUpdate = (user: UserModel | UserProfile) => {
return user.is_bot ? user.bot_last_icon_update : user.last_picture_update || 0;
};
/**
* Sorts custom profile attributes by their sort_order property, falling back to name comparison.
* Attributes with undefined sort_order are placed last.
* @param a First CustomAttribute to compare
* @param b Second CustomAttribute to compare
* @returns Negative if a comes first, positive if b comes first, 0 if equal
*/
export const sortCustomProfileAttributes = (a: CustomAttribute, b: CustomAttribute): number => {
const orderA = a.sort_order ?? Number.MAX_SAFE_INTEGER;
const orderB = b.sort_order ?? Number.MAX_SAFE_INTEGER;
return orderA === orderB ? a.name.localeCompare(b.name) : orderA - orderB;
};

26
types/api/custom_profile_attributes.d.ts vendored
View file

@ -20,7 +20,10 @@ type CustomProfileField = {
type: string;
/** any extra properties of the field **/
attrs?: unknown;
attrs?: {
sort_order?: number;
[key: string]: unknown;
};
/** id of the target element if empty it is a system property **/
target_id: string;
@ -33,17 +36,20 @@ type CustomProfileField = {
};
/**
* DisplayCustomAttribute
* @description a simplified version of a field with its value for display purposes
* CustomProfileAttributeSimple
* @description simpler type to display a field id with its value.
**/
type DisplayCustomAttribute = {
type CustomProfileAttributeSimple = {
[field_id: string]: string;
}
/** field id **/
export type CustomAttribute = {
id: string;
/** field name **/
name: string;
/** value assigned to that field **/
value: string;
};
sort_order?: number;
}
export interface CustomAttributeSet {
[key: string]: CustomAttribute;
}

8
types/api/users.d.ts vendored
View file

@ -53,14 +53,6 @@ type CustomProfileAttribute = {
delete_at: number;
}
/**
* CustomProfileAttributeSimple
* @description simpler type to display a field id with its value.
**/
type CustomProfileAttributeSimple = {
[field_id: string]: string;
}
type UserProfile = {
id: string;
create_at: number;

11
types/screens/edit_profile.ts
View file

@ -3,12 +3,9 @@
import type {AvailableScreens} from './navigation';
import type {FieldProps} from '@screens/edit_profile/components/field';
import type {CustomAttributeSet} from '@typings/api/custom_profile_attributes';
import type UserModel from '@typings/database/models/servers/user';
export interface CustomAttributeSet {
[key: string]: CustomAttribute;
}
export interface UserInfo {
email: string;
firstName: string;
@ -19,12 +16,6 @@ export interface UserInfo {
customAttributes: CustomAttributeSet;
}
export type CustomAttribute = {
id: string;
name: string;
value: string;
}
export type EditProfileProps = {
componentId: AvailableScreens;
currentUser?: UserModel;