Skip to content

Commit bbd8669

Browse files
Leo6Leoclaude
andcommitted
OCPBUGS-81520: Console can only show user name instead of full name as the display name
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent dda5808 commit bbd8669

File tree

14 files changed

+350
-45
lines changed

14 files changed

+350
-45
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: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,24 @@ 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+
39+
const mockUserResource = {};
40+
41+
jest.mock('@console/internal/components/utils/k8s-get-hook', () => ({
42+
useK8sGet: () => [mockUserResource, true],
43+
}));
44+
2745
const mockUserSettings = useUserSettings as jest.Mock;
2846

2947
const useResolvedExtensionsMock = useResolvedExtensions as jest.Mock;
@@ -135,10 +153,13 @@ describe('useTelemetry', () => {
135153
const fireTelemetryEvent = result.current;
136154
fireTelemetryEvent('test 1');
137155
expect(listener).toHaveBeenCalledTimes(1);
138-
expect(listener).toBeCalledWith('test 1', {
139-
consoleVersion: undefined,
140-
clusterType: undefined,
141-
});
156+
expect(listener).toBeCalledWith(
157+
'test 1',
158+
expect.objectContaining({
159+
consoleVersion: undefined,
160+
clusterType: undefined,
161+
}),
162+
);
142163
});
143164

144165
it('calls the listener with console version and clusterType when they are configured (x.y.z and OSD)', () => {
@@ -152,10 +173,13 @@ describe('useTelemetry', () => {
152173
const fireTelemetryEvent = result.current;
153174
fireTelemetryEvent('test 2');
154175
expect(listener).toHaveBeenCalledTimes(1);
155-
expect(listener).toBeCalledWith('test 2', {
156-
consoleVersion: 'x.y.z',
157-
clusterType: 'OSD',
158-
});
176+
expect(listener).toBeCalledWith(
177+
'test 2',
178+
expect.objectContaining({
179+
consoleVersion: 'x.y.z',
180+
clusterType: 'OSD',
181+
}),
182+
);
159183
});
160184

161185
it('calls the listener with the optional properties', () => {
@@ -169,12 +193,15 @@ describe('useTelemetry', () => {
169193
const fireTelemetryEvent = result.current;
170194
fireTelemetryEvent('test 3', { 'a-string': 'works fine', 'a-boolean': true });
171195
expect(listener).toHaveBeenCalledTimes(1);
172-
expect(listener).toBeCalledWith('test 3', {
173-
consoleVersion: 'x.y.z',
174-
clusterType: 'OSD',
175-
'a-string': 'works fine',
176-
'a-boolean': true,
177-
});
196+
expect(listener).toBeCalledWith(
197+
'test 3',
198+
expect.objectContaining({
199+
consoleVersion: 'x.y.z',
200+
clusterType: 'OSD',
201+
'a-string': 'works fine',
202+
'a-boolean': true,
203+
}),
204+
);
178205
});
179206

180207
it('calls the listener with clusterType DEVSANDBOX when CLUSTER_TYPE is OSD and DEVSANDBOX is "true"', () => {
@@ -192,10 +219,13 @@ describe('useTelemetry', () => {
192219
const fireTelemetryEvent = result.current;
193220
fireTelemetryEvent('test 4');
194221
expect(listener).toHaveBeenCalledTimes(1);
195-
expect(listener).toBeCalledWith('test 4', {
196-
consoleVersion: 'x.y.z',
197-
clusterType: 'DEVSANDBOX',
198-
});
222+
expect(listener).toBeCalledWith(
223+
'test 4',
224+
expect.objectContaining({
225+
consoleVersion: 'x.y.z',
226+
clusterType: 'DEVSANDBOX',
227+
}),
228+
);
199229
});
200230

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

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

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

0 commit comments

Comments
 (0)