Skip to content

Commit 883425d

Browse files
Merge pull request #15743 from Leo6Leo/leo/ocpbugs/65793/show-full-user-name/4.20
[release-4.20] OCPBUGS-65793: OpenShift Console can only show user name instead of full name as the display name
2 parents a7d3608 + f9c45c9 commit 883425d

File tree

14 files changed

+269
-27
lines changed

14 files changed

+269
-27
lines changed

frontend/packages/console-app/src/components/user-preferences/__tests__/UserPreferenceCheckboxField.spec.tsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,19 @@ import * as React from 'react';
22
import { Checkbox, Skeleton } from '@patternfly/react-core';
33
import { shallow, ShallowWrapper } from 'enzyme';
44
import { UserPreferenceFieldType } from '@console/dynamic-plugin-sdk/src/extensions/user-preferences';
5-
import { useUserSettings } from '@console/shared';
5+
import { useUserSettings, useTelemetry } from '@console/shared';
66
import UserPreferenceCheckboxField from '../UserPreferenceCheckboxField';
77

88
jest.mock('@console/shared/src/hooks/useUserSettings', () => ({
99
useUserSettings: jest.fn(),
1010
}));
1111

12+
jest.mock('@console/shared/src/hooks/useTelemetry', () => ({
13+
useTelemetry: jest.fn(),
14+
}));
15+
1216
const mockUserSettings = useUserSettings as jest.Mock;
17+
const mockUseTelemetry = useTelemetry as jest.Mock;
1318

1419
describe('UserPreferenceCheckboxField', () => {
1520
type UserPreferenceCheckboxFieldProps = React.ComponentProps<typeof UserPreferenceCheckboxField>;
@@ -23,6 +28,10 @@ describe('UserPreferenceCheckboxField', () => {
2328
};
2429
let wrapper: ShallowWrapper<UserPreferenceCheckboxFieldProps>;
2530

31+
beforeEach(() => {
32+
mockUseTelemetry.mockReturnValue(jest.fn());
33+
});
34+
2635
afterEach(() => {
2736
jest.resetAllMocks();
2837
});

frontend/packages/console-app/src/components/user-preferences/__tests__/UserPreferenceDropdownField.spec.tsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,19 @@ import * as React from 'react';
22
import { Select, SelectProps } from '@patternfly/react-core';
33
import { shallow, ShallowWrapper } from 'enzyme';
44
import { UserPreferenceFieldType } from '@console/dynamic-plugin-sdk/src/extensions/user-preferences';
5-
import { useUserSettings } from '@console/shared';
5+
import { useUserSettings, useTelemetry } from '@console/shared';
66
import UserPreferenceDropdownField from '../UserPreferenceDropdownField';
77

88
jest.mock('@console/shared/src/hooks/useUserSettings', () => ({
99
useUserSettings: jest.fn(),
1010
}));
1111

12+
jest.mock('@console/shared/src/hooks/useTelemetry', () => ({
13+
useTelemetry: jest.fn(),
14+
}));
15+
1216
const mockUserSettings = useUserSettings as jest.Mock;
17+
const mockUseTelemetry = useTelemetry as jest.Mock;
1318

1419
describe('UserPreferenceDropdownField', () => {
1520
type UserPreferenceDropdownFieldProps = React.ComponentProps<typeof UserPreferenceDropdownField>;
@@ -24,6 +29,10 @@ describe('UserPreferenceDropdownField', () => {
2429
};
2530
let wrapper: ShallowWrapper<UserPreferenceDropdownFieldProps>;
2631

32+
beforeEach(() => {
33+
mockUseTelemetry.mockReturnValue(jest.fn());
34+
});
35+
2736
afterEach(() => {
2837
jest.resetAllMocks();
2938
});

frontend/packages/console-dynamic-plugin-sdk/src/app/core/actions/core.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { action, ActionType as Action } from 'typesafe-actions';
2+
import type { UserKind } from '@console/internal/module/k8s/types';
23
import { UserInfo } from '../../../extensions';
34
import { AdmissionWebhookWarning } from '../../redux-types';
45

56
export enum ActionType {
67
SetUser = 'setUser',
8+
SetUserResource = 'setUserResource',
79
BeginImpersonate = 'beginImpersonate',
810
EndImpersonate = 'endImpersonate',
911
SetActiveCluster = 'setActiveCluster',
@@ -12,6 +14,8 @@ export enum ActionType {
1214
}
1315

1416
export const setUser = (userInfo: UserInfo) => action(ActionType.SetUser, { userInfo });
17+
export const setUserResource = (userResource: UserKind) =>
18+
action(ActionType.SetUserResource, { userResource });
1519
export const beginImpersonate = (kind: string, name: string, subprotocols: string[]) =>
1620
action(ActionType.BeginImpersonate, { kind, name, subprotocols });
1721
export const endImpersonate = () => action(ActionType.EndImpersonate);
@@ -21,6 +25,7 @@ export const removeAdmissionWebhookWarning = (id) =>
2125
action(ActionType.RemoveAdmissionWebhookWarning, { id });
2226
const coreActions = {
2327
setUser,
28+
setUserResource,
2429
beginImpersonate,
2530
endImpersonate,
2631
setAdmissionWebhookWarning,

frontend/packages/console-dynamic-plugin-sdk/src/app/core/reducers/core.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { ActionType, CoreAction } from '../actions/core';
1414
export const coreReducer = (
1515
state: CoreState = {
1616
user: {},
17+
userResource: null,
1718
admissionWebhookWarnings: ImmutableMap<string, AdmissionWebhookWarning>(),
1819
},
1920
action: CoreAction,
@@ -47,6 +48,12 @@ export const coreReducer = (
4748
user: action.payload.userInfo,
4849
};
4950

51+
case ActionType.SetUserResource:
52+
return {
53+
...state,
54+
userResource: action.payload.userResource,
55+
};
56+
5057
case ActionType.SetAdmissionWebhookWarning:
5158
return {
5259
...state,

frontend/packages/console-dynamic-plugin-sdk/src/app/core/reducers/coreSelectors.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { Map as ImmutableMap } from 'immutable';
2+
import type { UserKind } from '@console/internal/module/k8s/types';
23
import { UserInfo } from '../../../extensions';
34
import { ImpersonateKind, SDKStoreState, AdmissionWebhookWarning } from '../../redux-types';
45

56
type GetImpersonate = (state: SDKStoreState) => ImpersonateKind;
67
type GetUser = (state: SDKStoreState) => UserInfo;
8+
type GetUserResource = (state: SDKStoreState) => UserKind;
79
type GetAdmissionWebhookWarnings = (
810
state: SDKStoreState,
911
) => ImmutableMap<string, AdmissionWebhookWarning>;
@@ -31,6 +33,13 @@ export const impersonateStateToProps = (state: SDKStoreState) => {
3133
*/
3234
export const getUser: GetUser = (state) => state.sdkCore.user;
3335

36+
/**
37+
* It provides user resource details from the redux store.
38+
* @param state the root state
39+
* @returns The user resource state.
40+
*/
41+
export const getUserResource: GetUserResource = (state) => state.sdkCore.userResource;
42+
3443
/**
3544
* It provides admission webhook warning data from the redux store.
3645
* @param state the root state

frontend/packages/console-dynamic-plugin-sdk/src/app/redux-types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Map as ImmutableMap } from 'immutable';
2+
import type { UserKind } from '@console/internal/module/k8s/types';
23
import { UserInfo } from '../extensions/console-types';
34

45
export type K8sState = ImmutableMap<string, any>;
@@ -16,6 +17,7 @@ export type ImpersonateKind = {
1617

1718
export type CoreState = {
1819
user?: UserInfo;
20+
userResource?: UserKind;
1921
impersonate?: ImpersonateKind;
2022
admissionWebhookWarnings?: ImmutableMap<string, AdmissionWebhookWarning>;
2123
};

frontend/packages/console-shared/src/hooks/__tests__/useTelemetry.spec.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,18 @@ jest.mock('@console/shared/src/hooks/useUserSettings', () => ({
2424
useUserSettings: jest.fn(),
2525
}));
2626

27+
jest.mock('@console/shared/src/hooks/useUser', () => ({
28+
useUser: jest.fn(() => ({
29+
user: {},
30+
userResource: {},
31+
userResourceLoaded: true,
32+
userResourceError: null,
33+
username: 'testuser',
34+
fullName: 'Test User',
35+
displayName: 'Test User',
36+
})),
37+
}));
38+
2739
const mockUserResource = {};
2840

2941
const exampleReturnValue = {
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import { useSelector, useDispatch } from 'react-redux';
2+
import { useK8sGet } from '@console/internal/components/utils/k8s-get-hook';
3+
import { testHook } from '@console/shared/src/test-utils/hooks-utils';
4+
import { useUser } from '../useUser';
5+
6+
jest.mock('react-i18next', () => ({
7+
useTranslation: () => ({
8+
t: (key: string) => key,
9+
}),
10+
}));
11+
12+
jest.mock('react-redux', () => ({
13+
...jest.requireActual('react-redux'),
14+
useSelector: jest.fn(),
15+
useDispatch: jest.fn(),
16+
}));
17+
18+
jest.mock('@console/internal/components/utils/k8s-get-hook', () => ({
19+
useK8sGet: jest.fn(),
20+
}));
21+
22+
const mockSetUserResource = jest.fn((userResource) => ({
23+
type: 'setUserResource',
24+
payload: { userResource },
25+
}));
26+
27+
jest.mock('@console/dynamic-plugin-sdk', () => ({
28+
...jest.requireActual('@console/dynamic-plugin-sdk'),
29+
getUser: jest.fn(),
30+
getUserResource: jest.fn(),
31+
setUserResource: (...args) => mockSetUserResource(...args),
32+
}));
33+
34+
const mockDispatch = jest.fn();
35+
const mockUseSelector = useSelector as jest.Mock;
36+
const mockUseK8sGet = useK8sGet as jest.Mock;
37+
const mockUseDispatch = useDispatch as jest.Mock;
38+
39+
describe('useUser', () => {
40+
beforeEach(() => {
41+
jest.clearAllMocks();
42+
mockUseDispatch.mockReturnValue(mockDispatch);
43+
});
44+
45+
it('should return user data with displayName from fullName when available', () => {
46+
const mockUser = { username: '[email protected]', uid: '123' };
47+
const mockUserResource = { fullName: 'Test User', identities: ['testuser'] };
48+
49+
mockUseSelector
50+
.mockReturnValueOnce(mockUser) // for getUser
51+
.mockReturnValueOnce(mockUserResource); // for getUserResource
52+
53+
mockUseK8sGet.mockReturnValue([mockUserResource, true, null]);
54+
55+
const { result } = testHook(() => useUser());
56+
57+
expect(result.current.user).toEqual(mockUser);
58+
expect(result.current.userResource).toEqual(mockUserResource);
59+
expect(result.current.username).toBe('[email protected]');
60+
expect(result.current.fullName).toBe('Test User');
61+
expect(result.current.displayName).toBe('Test User'); // Should prefer fullName
62+
});
63+
64+
it('should fallback to username when fullName is not available', () => {
65+
const mockUser = { username: '[email protected]', uid: '123' };
66+
const mockUserResource = { identities: ['testuser'] }; // No fullName
67+
68+
mockUseSelector.mockReturnValueOnce(mockUser).mockReturnValueOnce(mockUserResource);
69+
70+
mockUseK8sGet.mockReturnValue([mockUserResource, true, null]);
71+
72+
const { result } = testHook(() => useUser());
73+
74+
expect(result.current.displayName).toBe('[email protected]'); // Should fallback to username
75+
expect(result.current.fullName).toBeUndefined();
76+
});
77+
78+
it('should dispatch setUserResource when user resource is loaded', () => {
79+
const mockUser = { username: '[email protected]' };
80+
const mockUserResource = { fullName: 'Test User' };
81+
82+
mockUseSelector.mockReturnValueOnce(mockUser).mockReturnValueOnce(null); // No userResource in Redux yet
83+
84+
mockUseK8sGet.mockReturnValue([mockUserResource, true, null]);
85+
86+
testHook(() => useUser());
87+
88+
expect(mockDispatch).toHaveBeenCalledWith({
89+
type: 'setUserResource',
90+
payload: { userResource: mockUserResource },
91+
});
92+
});
93+
94+
it('should handle edge cases with empty strings and fallback to "Unknown user"', () => {
95+
const mockUser = { username: '' }; // Empty username
96+
const mockUserResource = { fullName: ' ' }; // Whitespace-only fullName
97+
98+
mockUseSelector.mockReturnValueOnce(mockUser).mockReturnValueOnce(mockUserResource);
99+
100+
mockUseK8sGet.mockReturnValue([mockUserResource, true, null]);
101+
102+
const { result } = testHook(() => useUser());
103+
104+
expect(result.current.displayName).toBe('Unknown user'); // Should fallback to translated "Unknown user"
105+
});
106+
107+
it('should trim whitespace from fullName and username', () => {
108+
const mockUser = { username: ' [email protected] ' };
109+
const mockUserResource = { fullName: ' Test User ' };
110+
111+
mockUseSelector.mockReturnValueOnce(mockUser).mockReturnValueOnce(mockUserResource);
112+
113+
mockUseK8sGet.mockReturnValue([mockUserResource, true, null]);
114+
115+
const { result } = testHook(() => useUser());
116+
117+
expect(result.current.displayName).toBe('Test User'); // Should be trimmed
118+
});
119+
});

frontend/packages/console-shared/src/hooks/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,4 @@ export * from './usePrometheusGate';
3737
export * from './useCopyCodeModal';
3838
export * from './useCopyLoginCommands';
3939
export * from './useQuickStartContext';
40+
export * from './useUser';

frontend/packages/console-shared/src/hooks/useTelemetry.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,13 @@ import {
66
TelemetryEventListener,
77
UserInfo,
88
} from '@console/dynamic-plugin-sdk';
9-
import { useK8sGet } from '@console/internal/components/utils/k8s-get-hook';
10-
import { UserModel } from '@console/internal/models';
119
import type { UserKind } from '@console/internal/module/k8s/types';
1210
import {
1311
CLUSTER_TELEMETRY_ANALYTICS,
1412
PREFERRED_TELEMETRY_USER_SETTING_KEY,
1513
USER_TELEMETRY_ANALYTICS,
1614
} from '../constants';
15+
import { useUser } from './useUser';
1716
import { useUserSettings } from './useUserSettings';
1817

1918
export interface ClusterProperties {
@@ -81,7 +80,8 @@ export const useTelemetry = () => {
8180
true,
8281
);
8382

84-
const [userResource, userResourceIsLoaded] = useK8sGet<UserKind>(UserModel, '~');
83+
// Use centralized user data instead of fetching directly
84+
const { userResource, userResourceLoaded: userResourceIsLoaded } = useUser();
8585

8686
const [extensions] = useResolvedExtensions<TelemetryListener>(isTelemetryListener);
8787

0 commit comments

Comments
 (0)