Skip to content

Commit

Permalink
address code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@Willyfrog
Willyfrog committed Feb 24, 2025
1 parent 0b14020 commit 396b484
Show file tree
Hide file tree
Showing 3 changed files with 237 additions and 5 deletions.
46 changes: 46 additions & 0 deletions app/screens/edit_profile/components/form.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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
});
});
188 changes: 188 additions & 0 deletions app/screens/user_profile/user_info.test.tsx
View file
Original file line number Diff line number Diff line change
@@ -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
]);
});
});
8 changes: 3 additions & 5 deletions app/screens/user_profile/user_info.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import React, {useEffect, useState, useRef, useCallback} from 'react';
import React, {useEffect, useState, useRef} from 'react';

import {fetchCustomAttributes} from '@actions/remote/user';
import {useServerUrl} from '@context/server';
Expand Down Expand Up @@ -32,16 +32,14 @@ const UserInfo = ({localTime, showCustomStatus, showLocalTime, showNickname, sho

const lastRequest = useRef(0);

const sortAttributes = useCallback(sortCustomProfileAttributes, []);

useEffect(() => {
if (enableCustomAttributes) {
const fetchData = async () => {
const reqTime = Date.now();
lastRequest.current = reqTime;
const {attributes, error} = await fetchCustomAttributes(serverUrl, user.id, true);
if (!error && lastRequest.current === reqTime) {
const attributesList = Object.values(attributes).sort(sortAttributes);
const attributesList = Object.values(attributes).sort(sortCustomProfileAttributes);
setCustomAttributes(attributesList);
} else {
setCustomAttributes(emptyList);
Expand All @@ -52,7 +50,7 @@ const UserInfo = ({localTime, showCustomStatus, showLocalTime, showNickname, sho
} else {
setCustomAttributes(emptyList);
}
}, [enableCustomAttributes, serverUrl, user.id, sortAttributes]);
}, [enableCustomAttributes, serverUrl, user.id]);

return (
<>
Expand Down

0 comments on commit 396b484

Please sign in to comment.