Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
0d84540
fix: Update CoreState type to include userResource for k8s API integr…
Leo6Leo Sep 22, 2025
2c59766
feat: Add userResource management to core actions and reducer
Leo6Leo Sep 22, 2025
e81d4fc
feat: Introduce useUser hook for centralized user data management
Leo6Leo Sep 22, 2025
308f1d7
refactor: Replace direct user data fetching with centralized useUser …
Leo6Leo Sep 22, 2025
824bb98
test: auto generated the unit tests for useUser hook to validate user…
Leo6Leo Sep 22, 2025
2cbbed8
feat: Enhance useUser hook to provide robust display name handling wi…
Leo6Leo Sep 22, 2025
ccb1777
fix: apply the review feedback
Leo6Leo Sep 22, 2025
aedb5b8
Apply suggestions from code review
Leo6Leo Sep 22, 2025
535a460
feat: run i18n
Leo6Leo Sep 22, 2025
009e7de
feat: Add UserKind type import
Leo6Leo Sep 22, 2025
acf30a3
Apply suggestions from code review
Leo6Leo Sep 25, 2025
ad7479f
Update frontend/public/locales/en/public.json
Leo6Leo Sep 25, 2025
b549dcc
Update frontend/packages/console-dynamic-plugin-sdk/src/app/core/acti…
Leo6Leo Sep 29, 2025
b004559
feat: Import UserKind type in core actions
Leo6Leo Sep 29, 2025
a8e3d23
refactor: Update GetUserResource type to use UserKind for improved ty…
Leo6Leo Sep 29, 2025
808452d
Merge remote-tracking branch 'upstream' into leo/ocpbugs/56892/userem…
Leo6Leo Oct 23, 2025
6d67070
test: Update useUser.spec.ts to mock setUserResource and correct disp…
Leo6Leo Oct 23, 2025
285ec8e
Merge remote-tracking branch 'upstream' into leo/ocpbugs/56892/userem…
Leo6Leo Oct 28, 2025
f3a3029
test: fixing the failing CI issue
Leo6Leo Oct 28, 2025
cbecd50
Merge remote-tracking branch 'upstream' into leo/ocpbugs/56892/userem…
Leo6Leo Nov 12, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { action, ActionType as Action } from 'typesafe-actions';
import type { UserKind } from '@console/internal/module/k8s/types';
import { UserInfo } from '../../../extensions';
import { AdmissionWebhookWarning } from '../../redux-types';

export enum ActionType {
SetUser = 'setUser',
SetUserResource = 'setUserResource',
BeginImpersonate = 'beginImpersonate',
EndImpersonate = 'endImpersonate',
SetActiveCluster = 'setActiveCluster',
Expand All @@ -12,6 +14,8 @@ export enum ActionType {
}

export const setUser = (userInfo: UserInfo) => action(ActionType.SetUser, { userInfo });
export const setUserResource = (userResource: UserKind) =>
action(ActionType.SetUserResource, { userResource });
export const beginImpersonate = (kind: string, name: string, subprotocols: string[]) =>
action(ActionType.BeginImpersonate, { kind, name, subprotocols });
export const endImpersonate = () => action(ActionType.EndImpersonate);
Expand All @@ -21,6 +25,7 @@ export const removeAdmissionWebhookWarning = (id) =>
action(ActionType.RemoveAdmissionWebhookWarning, { id });
const coreActions = {
setUser,
setUserResource,
beginImpersonate,
endImpersonate,
setAdmissionWebhookWarning,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { ActionType, CoreAction } from '../actions/core';
export const coreReducer = (
state: CoreState = {
user: {},
userResource: null,
admissionWebhookWarnings: ImmutableMap<string, AdmissionWebhookWarning>(),
},
action: CoreAction,
Expand Down Expand Up @@ -47,6 +48,12 @@ export const coreReducer = (
user: action.payload.userInfo,
};

case ActionType.SetUserResource:
return {
...state,
userResource: action.payload.userResource,
};

case ActionType.SetAdmissionWebhookWarning:
return {
...state,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { Map as ImmutableMap } from 'immutable';
import type { UserKind } from '@console/internal/module/k8s/types';
import { UserInfo } from '../../../extensions';
import { ImpersonateKind, SDKStoreState, AdmissionWebhookWarning } from '../../redux-types';

type GetImpersonate = (state: SDKStoreState) => ImpersonateKind;
type GetUser = (state: SDKStoreState) => UserInfo;
type GetUserResource = (state: SDKStoreState) => UserKind;
type GetAdmissionWebhookWarnings = (
state: SDKStoreState,
) => ImmutableMap<string, AdmissionWebhookWarning>;
Expand Down Expand Up @@ -31,6 +33,13 @@ export const impersonateStateToProps = (state: SDKStoreState) => {
*/
export const getUser: GetUser = (state) => state.sdkCore.user;

/**
* It provides user resource details from the redux store.
* @param state the root state
* @returns The user resource state.
*/
export const getUserResource: GetUserResource = (state) => state.sdkCore.userResource;

/**
* It provides admission webhook warning data from the redux store.
* @param state the root state
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Map as ImmutableMap } from 'immutable';
import type { UserKind } from '@console/internal/module/k8s/types';
import { UserInfo } from '../extensions/console-types';

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

export type CoreState = {
user?: UserInfo;
userResource?: UserKind;
impersonate?: ImpersonateKind;
admissionWebhookWarnings?: ImmutableMap<string, AdmissionWebhookWarning>;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,18 @@ jest.mock('@console/shared/src/hooks/useUserSettings', () => ({
useUserSettings: jest.fn(),
}));

jest.mock('@console/shared/src/hooks/useUser', () => ({
useUser: jest.fn(() => ({
user: {},
userResource: {},
userResourceLoaded: true,
userResourceError: null,
username: 'testuser',
fullName: 'Test User',
displayName: 'Test User',
})),
}));

const mockUserResource = {};

const exampleReturnValue = {
Expand Down
119 changes: 119 additions & 0 deletions frontend/packages/console-shared/src/hooks/__tests__/useUser.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
import { useSelector, useDispatch } from 'react-redux';
import { useK8sGet } from '@console/internal/components/utils/k8s-get-hook';
import { testHook } from '@console/shared/src/test-utils/hooks-utils';
import { useUser } from '../useUser';

jest.mock('react-i18next', () => ({
useTranslation: () => ({
t: (key: string) => key,
}),
}));

jest.mock('react-redux', () => ({
...jest.requireActual('react-redux'),
useSelector: jest.fn(),
useDispatch: jest.fn(),
}));

jest.mock('@console/internal/components/utils/k8s-get-hook', () => ({
useK8sGet: jest.fn(),
}));

const mockSetUserResource = jest.fn((userResource) => ({
type: 'setUserResource',
payload: { userResource },
}));

jest.mock('@console/dynamic-plugin-sdk', () => ({
...jest.requireActual('@console/dynamic-plugin-sdk'),
getUser: jest.fn(),
getUserResource: jest.fn(),
setUserResource: (...args) => mockSetUserResource(...args),
}));

const mockDispatch = jest.fn();
const mockUseSelector = useSelector as jest.Mock;
const mockUseK8sGet = useK8sGet as jest.Mock;
const mockUseDispatch = useDispatch as jest.Mock;

describe('useUser', () => {
beforeEach(() => {
jest.clearAllMocks();
mockUseDispatch.mockReturnValue(mockDispatch);
});

it('should return user data with displayName from fullName when available', () => {
const mockUser = { username: 'testuser@example.com', uid: '123' };
const mockUserResource = { fullName: 'Test User', identities: ['testuser'] };

mockUseSelector
.mockReturnValueOnce(mockUser) // for getUser
.mockReturnValueOnce(mockUserResource); // for getUserResource

mockUseK8sGet.mockReturnValue([mockUserResource, true, null]);

const { result } = testHook(() => useUser());

expect(result.current.user).toEqual(mockUser);
expect(result.current.userResource).toEqual(mockUserResource);
expect(result.current.username).toBe('testuser@example.com');
expect(result.current.fullName).toBe('Test User');
expect(result.current.displayName).toBe('Test User'); // Should prefer fullName
});

it('should fallback to username when fullName is not available', () => {
const mockUser = { username: 'testuser@example.com', uid: '123' };
const mockUserResource = { identities: ['testuser'] }; // No fullName

mockUseSelector.mockReturnValueOnce(mockUser).mockReturnValueOnce(mockUserResource);

mockUseK8sGet.mockReturnValue([mockUserResource, true, null]);

const { result } = testHook(() => useUser());

expect(result.current.displayName).toBe('testuser@example.com'); // Should fallback to username
expect(result.current.fullName).toBeUndefined();
});

it('should dispatch setUserResource when user resource is loaded', () => {
const mockUser = { username: 'testuser@example.com' };
const mockUserResource = { fullName: 'Test User' };

mockUseSelector.mockReturnValueOnce(mockUser).mockReturnValueOnce(null); // No userResource in Redux yet

mockUseK8sGet.mockReturnValue([mockUserResource, true, null]);

testHook(() => useUser());

expect(mockDispatch).toHaveBeenCalledWith({
type: 'setUserResource',
payload: { userResource: mockUserResource },
});
});

it('should handle edge cases with empty strings and fallback to "Unknown user"', () => {
const mockUser = { username: '' }; // Empty username
const mockUserResource = { fullName: ' ' }; // Whitespace-only fullName

mockUseSelector.mockReturnValueOnce(mockUser).mockReturnValueOnce(mockUserResource);

mockUseK8sGet.mockReturnValue([mockUserResource, true, null]);

const { result } = testHook(() => useUser());

expect(result.current.displayName).toBe('Unknown user'); // Should fallback to translated "Unknown user"
});

it('should trim whitespace from fullName and username', () => {
const mockUser = { username: ' testuser@example.com ' };
const mockUserResource = { fullName: ' Test User ' };

mockUseSelector.mockReturnValueOnce(mockUser).mockReturnValueOnce(mockUserResource);

mockUseK8sGet.mockReturnValue([mockUserResource, true, null]);

const { result } = testHook(() => useUser());

expect(result.current.displayName).toBe('Test User'); // Should be trimmed
});
});
1 change: 1 addition & 0 deletions frontend/packages/console-shared/src/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,4 @@ export * from './usePrometheusGate';
export * from './useCopyCodeModal';
export * from './useCopyLoginCommands';
export * from './useQuickStartContext';
export * from './useUser';
6 changes: 3 additions & 3 deletions frontend/packages/console-shared/src/hooks/useTelemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@ import {
TelemetryEventListener,
UserInfo,
} from '@console/dynamic-plugin-sdk';
import { useK8sGet } from '@console/internal/components/utils/k8s-get-hook';
import { UserModel } from '@console/internal/models';
import type { UserKind } from '@console/internal/module/k8s/types';
import {
CLUSTER_TELEMETRY_ANALYTICS,
PREFERRED_TELEMETRY_USER_SETTING_KEY,
USER_TELEMETRY_ANALYTICS,
} from '../constants';
import { useUser } from './useUser';
import { useUserSettings } from './useUserSettings';

export interface ClusterProperties {
Expand Down Expand Up @@ -81,7 +80,8 @@ export const useTelemetry = () => {
true,
);

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

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

Expand Down
67 changes: 67 additions & 0 deletions frontend/packages/console-shared/src/hooks/useUser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { useEffect } from 'react';
import { useTranslation } from 'react-i18next';
import { useDispatch, useSelector } from 'react-redux';
import { getUser, getUserResource, setUserResource } from '@console/dynamic-plugin-sdk';
import { useK8sGet } from '@console/internal/components/utils/k8s-get-hook';
import { UserModel } from '@console/internal/models';
import type { UserKind } from '@console/internal/module/k8s/types';

/**
* Custom hook that provides centralized user data fetching and management.
* This hook fetches both the UserInfo (from authentication) and UserKind (from k8s API)
* and stores them in Redux for use throughout the application.
*
* @returns Object containing user info, user resource, and loading states
*/
export const useUser = () => {
const { t } = useTranslation('public');
const dispatch = useDispatch();

// Get current user info from Redux (username, groups, etc.)
const user = useSelector(getUser);

// Get current user resource from Redux (fullName, identities, etc.)
const userResource = useSelector(getUserResource);

// Fetch user resource from k8s API
const [userResourceData, userResourceLoaded, userResourceError] = useK8sGet<UserKind>(
UserModel,
'~',
);

// Update Redux when user resource is loaded
useEffect(() => {
if (userResourceLoaded && userResourceData && !userResourceError) {
dispatch(setUserResource(userResourceData));
}
}, [dispatch, userResourceData, userResourceLoaded, userResourceError]);

const currentUserResource = userResource || userResourceData;
const currentUsername = user?.username;
const currentFullName = currentUserResource?.fullName;

// Create a robust display name that always has a fallback
const getDisplayName = () => {
// Prefer fullName if it exists and is not empty
if (currentFullName && currentFullName.trim()) {
return currentFullName.trim();
}
// Fallback to username if it exists and is not empty
if (currentUsername && currentUsername.trim()) {
return currentUsername.trim();
}
// Final fallback for edge cases
return t('Unknown user');
};

return {
user,
userResource: currentUserResource,
userResourceLoaded,
userResourceError,
// Computed properties for convenience
username: currentUsername,
fullName: currentFullName,
displayName: getDisplayName(),
};
};
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { screen, render, fireEvent } from '@testing-library/react';
import { screen, fireEvent } from '@testing-library/react';
import * as _ from 'lodash';
import { act } from 'react-dom/test-utils';
import * as k8sResourceModule from '@console/dynamic-plugin-sdk/src/utils/k8s/k8s-resource';
Expand Down Expand Up @@ -32,7 +32,7 @@ describe('ExportApplicationModal', () => {
});

it('should show cancel and ok buttons when export app resource is not found', async () => {
render(<ExportApplicationModal name="my-export" namespace="my-app" />);
renderWithProviders(<ExportApplicationModal name="my-export" namespace="my-app" />);
expect(screen.getByTestId('cancel-btn')).toBeInTheDocument();
expect(screen.getByTestId('close-btn')).toBeInTheDocument();
});
Expand All @@ -58,7 +58,7 @@ describe('ExportApplicationModal', () => {
});

it('should show cancel and ok buttons when export app resource is created', async () => {
render(
renderWithProviders(
<ExportApplicationModal
name="my-export"
namespace="my-app"
Expand All @@ -71,7 +71,9 @@ describe('ExportApplicationModal', () => {

it('should call k8sCreate with correct data on click of Ok button when the export resource is not created', async () => {
const spyk8sCreate = jest.spyOn(k8sResourceModule, 'k8sCreate');
render(<ExportApplicationModal namespace="my-app" name="my-export" cancel={jest.fn()} />);
renderWithProviders(
<ExportApplicationModal namespace="my-app" name="my-export" cancel={jest.fn()} />,
);
await act(async () => {
fireEvent.click(screen.getByTestId('close-btn'));
});
Expand All @@ -84,7 +86,7 @@ describe('ExportApplicationModal', () => {
const spyk8sKill = jest.spyOn(k8sResourceModule, 'k8sKill');
const spyk8sCreate = jest.spyOn(k8sResourceModule, 'k8sCreate');

render(
renderWithProviders(
<ExportApplicationModal
name="my-export"
namespace="my-app"
Expand Down
Loading