Minor: Fix user details page bugs (#16358)

* Fix the empty string passed to the API when no displayName input is provided

* Fix no characters displaying in the avatar when displayName is an empty string

* Add tests for user page bug fixes

* Resolve comments

* Fix the failing unit tests
This commit is contained in:
Aniket Katkar 2024-06-27 15:47:45 +05:30 committed by GitHub
parent fe04b0a201
commit fd942a27a6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 142 additions and 83 deletions

View File

@ -14,6 +14,7 @@
import { ExclamationCircleFilled } from '@ant-design/icons'; import { ExclamationCircleFilled } from '@ant-design/icons';
import { Button, Divider, Input, Space, Tooltip, Typography } from 'antd'; import { Button, Divider, Input, Space, Tooltip, Typography } from 'antd';
import { AxiosError } from 'axios'; import { AxiosError } from 'axios';
import { isEmpty } from 'lodash';
import React, { useCallback, useMemo, useState } from 'react'; import React, { useCallback, useMemo, useState } from 'react';
import { useTranslation } from 'react-i18next'; import { useTranslation } from 'react-i18next';
import { ReactComponent as EditIcon } from '../../../../../assets/svg/edit-new.svg'; import { ReactComponent as EditIcon } from '../../../../../assets/svg/edit-new.svg';
@ -94,14 +95,16 @@ const UserProfileDetails = ({
[userData] [userData]
); );
const onDisplayNameChange = (e: React.ChangeEvent<HTMLInputElement>) => const onDisplayNameChange = useCallback(
setDisplayName(e.target.value); (e: React.ChangeEvent<HTMLInputElement>) => setDisplayName(e.target.value),
[]
);
const handleDisplayNameSave = useCallback(async () => { const handleDisplayNameSave = useCallback(async () => {
if (displayName !== userData.displayName) { if (displayName !== userData.displayName) {
setIsLoading(true); setIsLoading(true);
await updateUserDetails( await updateUserDetails(
{ displayName: displayName ?? '' }, { displayName: isEmpty(displayName) ? undefined : displayName },
'displayName' 'displayName'
); );
setIsLoading(false); setIsLoading(false);
@ -114,67 +117,70 @@ const UserProfileDetails = ({
setIsDisplayNameEdit(false); setIsDisplayNameEdit(false);
}, [userData.displayName]); }, [userData.displayName]);
const displayNameRenderComponent = useMemo( const displayNameRenderComponent = useMemo(() => {
() => const displayNamePlaceHolder = isEmpty(userData.displayName)
isDisplayNameEdit ? ( ? t('label.add-entity', { entity: t('label.display-name') })
<InlineEdit : userData.displayName;
isLoading={isLoading}
onCancel={handleCloseEditDisplayName} const displayNameText = hasEditPermission
onSave={handleDisplayNameSave}> ? displayNamePlaceHolder
<Input : getEntityName(userData);
className="w-full"
data-testid="displayName" return isDisplayNameEdit ? (
id="displayName" <InlineEdit
name="displayName" isLoading={isLoading}
placeholder={t('label.display-name')} onCancel={handleCloseEditDisplayName}
type="text" onSave={handleDisplayNameSave}>
value={displayName} <Input
onChange={onDisplayNameChange} className="w-full"
/> data-testid="displayName"
</InlineEdit> id="displayName"
) : ( name="displayName"
<Space align="center"> placeholder={t('label.display-name')}
<Typography.Text type="text"
className="font-medium text-md" value={displayName}
data-testid="user-name" onChange={onDisplayNameChange}
ellipsis={{ tooltip: true }} />
style={{ maxWidth: '400px' }}> </InlineEdit>
{hasEditPermission ) : (
? userData.displayName || <Space align="center">
t('label.add-entity', { entity: t('label.display-name') }) <Typography.Text
: getEntityName(userData)} className="font-medium text-md"
</Typography.Text> data-testid="user-name"
{hasEditPermission && ( ellipsis={{ tooltip: true }}
<Tooltip style={{ maxWidth: '400px' }}>
title={t('label.edit-entity', { {displayNameText}
entity: t('label.display-name'), </Typography.Text>
})}> {hasEditPermission && (
<EditIcon <Tooltip
className="cursor-pointer align-middle" title={t('label.edit-entity', {
color={DE_ACTIVE_COLOR} entity: t('label.display-name'),
data-testid="edit-displayName" })}>
{...ICON_DIMENSION} <EditIcon
onClick={(e) => { className="cursor-pointer align-middle"
// Used to stop click propagation event to parent User.component collapsible panel color={DE_ACTIVE_COLOR}
e.stopPropagation(); data-testid="edit-displayName"
setIsDisplayNameEdit(true); {...ICON_DIMENSION}
}} onClick={(e) => {
/> // Used to stop click propagation event to parent User.component collapsible panel
</Tooltip> e.stopPropagation();
)} setIsDisplayNameEdit(true);
</Space> }}
), />
[ </Tooltip>
userData, )}
displayName, </Space>
isDisplayNameEdit, );
hasEditPermission, }, [
getEntityName, userData,
onDisplayNameChange, displayName,
handleDisplayNameSave, isDisplayNameEdit,
handleCloseEditDisplayName, hasEditPermission,
] getEntityName,
); onDisplayNameChange,
handleDisplayNameSave,
handleCloseEditDisplayName,
]);
const changePasswordRenderComponent = useMemo( const changePasswordRenderComponent = useMemo(
() => () =>

View File

@ -11,6 +11,7 @@
* limitations under the License. * limitations under the License.
*/ */
import { act, fireEvent, render, screen } from '@testing-library/react'; import { act, fireEvent, render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import React from 'react'; import React from 'react';
import { MemoryRouter } from 'react-router-dom'; import { MemoryRouter } from 'react-router-dom';
import { AuthProvider } from '../../../../../generated/settings/settings'; import { AuthProvider } from '../../../../../generated/settings/settings';
@ -305,23 +306,25 @@ describe('Test User Profile Details Component', () => {
}); });
it('should call updateUserDetails on click of DisplayNameButton', async () => { it('should call updateUserDetails on click of DisplayNameButton', async () => {
render(<UserProfileDetails {...mockPropsData} />, { await act(async () => {
wrapper: MemoryRouter, render(<UserProfileDetails {...mockPropsData} />, {
wrapper: MemoryRouter,
});
}); });
act(() => { await act(async () => {
fireEvent.click(screen.getByTestId('edit-displayName')); fireEvent.click(screen.getByTestId('edit-displayName'));
}); });
expect(screen.getByText('InlineEdit')).toBeInTheDocument(); expect(screen.getByText('InlineEdit')).toBeInTheDocument();
act(() => { await act(async () => {
fireEvent.change(screen.getByTestId('displayName'), { fireEvent.change(screen.getByTestId('displayName'), {
target: { value: 'test' }, target: { value: 'test' },
}); });
}); });
act(() => { await act(async () => {
fireEvent.click(screen.getByTestId('display-name-save-button')); fireEvent.click(screen.getByTestId('display-name-save-button'));
}); });
@ -331,6 +334,43 @@ describe('Test User Profile Details Component', () => {
); );
}); });
it('should pass displayName undefined to the updateUserDetails in case of empty string', async () => {
await act(async () => {
render(
<UserProfileDetails
{...mockPropsData}
userData={{ ...mockPropsData.userData, displayName: 'Test' }}
/>,
{
wrapper: MemoryRouter,
}
);
});
await act(async () => {
userEvent.click(screen.getByTestId('edit-displayName'));
});
expect(screen.getByText('InlineEdit')).toBeInTheDocument();
await act(async () => {
fireEvent.change(screen.getByTestId('displayName'), {
target: {
value: '',
},
});
});
await act(async () => {
userEvent.click(screen.getByTestId('display-name-save-button'));
});
expect(mockPropsData.updateUserDetails).toHaveBeenCalledWith(
{ displayName: undefined },
'displayName'
);
});
it('should call updateUserDetails on click of PersonaSaveButton', async () => { it('should call updateUserDetails on click of PersonaSaveButton', async () => {
render(<UserProfileDetails {...mockPropsData} />, { render(<UserProfileDetails {...mockPropsData} />, {
wrapper: MemoryRouter, wrapper: MemoryRouter,

View File

@ -28,13 +28,11 @@ jest.mock('../../../../../utils/ProfilerUtils', () => ({
})); }));
jest.mock('../../../../common/ProfilePicture/ProfilePicture', () => { jest.mock('../../../../common/ProfilePicture/ProfilePicture', () => {
return jest.fn().mockReturnValue(<p>ProfilePicture</p>); return jest
.fn()
.mockImplementation(({ displayName }) => <p>{displayName}</p>);
}); });
jest.mock('../../../../../utils/EntityUtils', () => ({
getEntityName: jest.fn().mockReturnValue('getEntityName'),
}));
describe('Test User User Profile Image Component', () => { describe('Test User User Profile Image Component', () => {
it('should render user profile image component', async () => { it('should render user profile image component', async () => {
render(<UserProfileImage {...mockPropsData} />); render(<UserProfileImage {...mockPropsData} />);
@ -49,7 +47,20 @@ describe('Test User User Profile Image Component', () => {
expect(screen.getByTestId('profile-image-container')).toBeInTheDocument(); expect(screen.getByTestId('profile-image-container')).toBeInTheDocument();
expect(screen.getByText('ProfilePicture')).toBeInTheDocument(); expect(screen.queryByText(mockUserData.name)).toBeNull();
expect(screen.getByText(mockUserData.displayName)).toBeInTheDocument();
});
it('should fallback to name for the displayName prop of ProfilePicture', async () => {
await act(async () => {
render(
<UserProfileImage userData={{ ...mockUserData, displayName: '' }} />
);
});
expect(screen.getByTestId('profile-image-container')).toBeInTheDocument();
expect(screen.getByText(mockUserData.name)).toBeInTheDocument();
}); });
it('should render user profile picture component with image', async () => { it('should render user profile picture component with image', async () => {

View File

@ -14,7 +14,7 @@
import { Typography } from 'antd'; import { Typography } from 'antd';
import { AxiosError } from 'axios'; import { AxiosError } from 'axios';
import { compare } from 'fast-json-patch'; import { compare } from 'fast-json-patch';
import { isEmpty, isUndefined } from 'lodash'; import { isEmpty, isUndefined, omitBy } from 'lodash';
import Qs from 'qs'; import Qs from 'qs';
import { import {
default as React, default as React,
@ -88,8 +88,8 @@ const UserPage = () => {
}); });
}; };
const ErrorPlaceholder = () => { const errorPlaceholder = useMemo(
return ( () => (
<div <div
className="d-flex items-center justify-center h-full" className="d-flex items-center justify-center h-full"
data-testid="error"> data-testid="error">
@ -103,8 +103,9 @@ const UserPage = () => {
/> />
</Typography.Paragraph> </Typography.Paragraph>
</div> </div>
); ),
}; [username]
);
const updateUserDetails = useCallback( const updateUserDetails = useCallback(
async (data: Partial<User>, key: keyof User) => { async (data: Partial<User>, key: keyof User) => {
@ -128,7 +129,7 @@ const UserPage = () => {
...currentUser, ...currentUser,
...updatedKeyData, ...updatedKeyData,
}; };
const newUserData = { ...userData, ...updatedKeyData }; const newUserData: User = { ...userData, ...updatedKeyData };
if (key === 'defaultPersona') { if (key === 'defaultPersona') {
if (isUndefined(response.defaultPersona)) { if (isUndefined(response.defaultPersona)) {
@ -140,7 +141,8 @@ const UserPage = () => {
if (userData.id === currentUser?.id) { if (userData.id === currentUser?.id) {
updateCurrentUser(newCurrentUserData as User); updateCurrentUser(newCurrentUserData as User);
} }
setUserData(newUserData); // Omit the undefined values from the User object
setUserData(omitBy(newUserData, isUndefined) as User);
} else { } else {
throw t('message.unexpected-error'); throw t('message.unexpected-error');
} }
@ -173,7 +175,7 @@ const UserPage = () => {
} }
if (isError && isEmpty(userData)) { if (isError && isEmpty(userData)) {
return <ErrorPlaceholder />; return errorPlaceholder;
} }
return ( return (