From 6c0265d9b4f0132d6b6f2029e781a2ee0968ca5a Mon Sep 17 00:00:00 2001 From: Pavel Angelov Date: Tue, 18 Nov 2025 18:07:05 +0200 Subject: [PATCH 1/4] implement this --- .../cards/ConfigurationCard.spec.tsx | 101 ++++++++++++++++++ .../navigation/cards/ConfigurationCard.tsx | 49 ++++++++- 2 files changed, 149 insertions(+), 1 deletion(-) diff --git a/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/ConfigurationCard.spec.tsx b/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/ConfigurationCard.spec.tsx index 387964297f..6c8ba67121 100644 --- a/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/ConfigurationCard.spec.tsx +++ b/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/ConfigurationCard.spec.tsx @@ -7,7 +7,20 @@ import ConfigurationCard, { ConfigurationCardProps } from './ConfigurationCard' const mockedProps = mock() +const mockUseSelector = jest.fn() +jest.mock('react-redux', () => ({ + ...jest.requireActual('react-redux'), + useSelector: () => mockUseSelector(), +})) + describe('ConfigurationCard', () => { + beforeEach(() => { + mockUseSelector.mockReturnValue({ + changes: {}, + configValidationErrors: [], + }) + }) + it('should render with correct title', () => { render() @@ -49,4 +62,92 @@ describe('ConfigurationCard', () => { expect(card).toHaveAttribute('tabIndex', '0') expect(card).toHaveAttribute('role', 'button') }) + + describe('Changes indicator', () => { + it('should not show changes indicator when no changes', () => { + mockUseSelector.mockReturnValue({ + changes: {}, + configValidationErrors: [], + }) + + render() + + expect( + screen.queryByTestId('updated-configuration-highlight'), + ).not.toBeInTheDocument() + }) + + it('should show changes indicator when config has changes', () => { + mockUseSelector.mockReturnValue({ + changes: { config: 'modified' }, + configValidationErrors: [], + }) + + render() + + expect( + screen.getByTestId('updated-configuration-highlight'), + ).toBeInTheDocument() + }) + }) + + describe('Validation errors', () => { + it('should not show error icon when config is valid', () => { + mockUseSelector.mockReturnValue({ + changes: {}, + configValidationErrors: [], + }) + + render() + + expect( + screen.queryByTestId('rdi-pipeline-nav__error-configuration'), + ).not.toBeInTheDocument() + }) + + it('should show error icon when config has validation errors', () => { + mockUseSelector.mockReturnValue({ + changes: {}, + configValidationErrors: [ + 'Invalid configuration', + 'Missing required field', + ], + }) + + render() + + expect( + screen.getByTestId('rdi-pipeline-nav__error-configuration'), + ).toBeInTheDocument() + }) + + it('should handle single validation error', () => { + mockUseSelector.mockReturnValue({ + changes: {}, + configValidationErrors: ['Single error'], + }) + + render() + + expect( + screen.getByTestId('rdi-pipeline-nav__error-configuration'), + ).toBeInTheDocument() + }) + }) + + it('should show both changes indicator and error icon when config has changes and errors', () => { + mockUseSelector.mockReturnValue({ + changes: { config: 'modified' }, + configValidationErrors: ['Invalid configuration'], + }) + + render() + + expect( + screen.getByTestId('updated-configuration-highlight'), + ).toBeInTheDocument() + expect( + screen.getByTestId('rdi-pipeline-nav__error-configuration'), + ).toBeInTheDocument() + }) }) diff --git a/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/ConfigurationCard.tsx b/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/ConfigurationCard.tsx index 0e8d623117..aa898f2cfa 100644 --- a/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/ConfigurationCard.tsx +++ b/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/ConfigurationCard.tsx @@ -1,6 +1,16 @@ import React from 'react' import { RdiPipelineTabs } from 'uiSrc/slices/interfaces' +import { useSelector } from 'react-redux' + +import { rdiPipelineSelector } from 'uiSrc/slices/rdi/pipeline' +import { RiTooltip } from 'uiSrc/components' +import { Indicator } from 'uiSrc/components/base/text/text.styles' +import { Row } from 'uiSrc/components/base/layout/flex' +import { Text } from 'uiSrc/components/base/text' +import { Icon, ToastNotificationIcon } from 'uiSrc/components/base/icons' + import BaseCard, { BaseCardProps } from './BaseCard' +import ValidationErrorsList from '../../validation-errors-list/ValidationErrorsList' export type ConfigurationCardProps = Omit< BaseCardProps, @@ -13,10 +23,15 @@ const ConfigurationCard = ({ onSelect, isSelected, }: ConfigurationCardProps) => { + const { changes, configValidationErrors } = useSelector(rdiPipelineSelector) + const handleClick = () => { onSelect(RdiPipelineTabs.Config) } + const hasChanges = !!changes.config + const isValid = configValidationErrors.length === 0 + return ( - Configuration file + + {!hasChanges && } + + {hasChanges && ( + + + + )} + + Configuration file + + {!isValid && ( + + } + > + + + )} + ) } From 7a32f671e89826b4d134644c32e9d453fd84f990 Mon Sep 17 00:00:00 2001 From: Pavel Angelov Date: Tue, 18 Nov 2025 18:32:49 +0200 Subject: [PATCH 2/4] extract logic in hook --- .../cards/ConfigurationCard.spec.tsx | 42 +++--- .../navigation/cards/ConfigurationCard.tsx | 9 +- .../navigation/cards/hooks/index.ts | 1 + .../cards/hooks/useConfigurationState.spec.ts | 141 ++++++++++++++++++ .../cards/hooks/useConfigurationState.ts | 21 +++ 5 files changed, 190 insertions(+), 24 deletions(-) create mode 100644 redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/hooks/index.ts create mode 100644 redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/hooks/useConfigurationState.spec.ts create mode 100644 redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/hooks/useConfigurationState.ts diff --git a/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/ConfigurationCard.spec.tsx b/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/ConfigurationCard.spec.tsx index 6c8ba67121..2493cc8e89 100644 --- a/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/ConfigurationCard.spec.tsx +++ b/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/ConfigurationCard.spec.tsx @@ -7,16 +7,16 @@ import ConfigurationCard, { ConfigurationCardProps } from './ConfigurationCard' const mockedProps = mock() -const mockUseSelector = jest.fn() -jest.mock('react-redux', () => ({ - ...jest.requireActual('react-redux'), - useSelector: () => mockUseSelector(), +const mockUseConfigurationState = jest.fn() +jest.mock('./hooks/useConfigurationState', () => ({ + useConfigurationState: () => mockUseConfigurationState(), })) describe('ConfigurationCard', () => { beforeEach(() => { - mockUseSelector.mockReturnValue({ - changes: {}, + mockUseConfigurationState.mockReturnValue({ + hasChanges: false, + isValid: true, configValidationErrors: [], }) }) @@ -65,8 +65,9 @@ describe('ConfigurationCard', () => { describe('Changes indicator', () => { it('should not show changes indicator when no changes', () => { - mockUseSelector.mockReturnValue({ - changes: {}, + mockUseConfigurationState.mockReturnValue({ + hasChanges: false, + isValid: true, configValidationErrors: [], }) @@ -78,8 +79,9 @@ describe('ConfigurationCard', () => { }) it('should show changes indicator when config has changes', () => { - mockUseSelector.mockReturnValue({ - changes: { config: 'modified' }, + mockUseConfigurationState.mockReturnValue({ + hasChanges: true, + isValid: true, configValidationErrors: [], }) @@ -93,8 +95,9 @@ describe('ConfigurationCard', () => { describe('Validation errors', () => { it('should not show error icon when config is valid', () => { - mockUseSelector.mockReturnValue({ - changes: {}, + mockUseConfigurationState.mockReturnValue({ + hasChanges: false, + isValid: true, configValidationErrors: [], }) @@ -106,8 +109,9 @@ describe('ConfigurationCard', () => { }) it('should show error icon when config has validation errors', () => { - mockUseSelector.mockReturnValue({ - changes: {}, + mockUseConfigurationState.mockReturnValue({ + hasChanges: false, + isValid: false, configValidationErrors: [ 'Invalid configuration', 'Missing required field', @@ -122,8 +126,9 @@ describe('ConfigurationCard', () => { }) it('should handle single validation error', () => { - mockUseSelector.mockReturnValue({ - changes: {}, + mockUseConfigurationState.mockReturnValue({ + hasChanges: false, + isValid: false, configValidationErrors: ['Single error'], }) @@ -136,8 +141,9 @@ describe('ConfigurationCard', () => { }) it('should show both changes indicator and error icon when config has changes and errors', () => { - mockUseSelector.mockReturnValue({ - changes: { config: 'modified' }, + mockUseConfigurationState.mockReturnValue({ + hasChanges: true, + isValid: false, configValidationErrors: ['Invalid configuration'], }) diff --git a/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/ConfigurationCard.tsx b/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/ConfigurationCard.tsx index aa898f2cfa..633c86548c 100644 --- a/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/ConfigurationCard.tsx +++ b/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/ConfigurationCard.tsx @@ -1,13 +1,12 @@ import React from 'react' import { RdiPipelineTabs } from 'uiSrc/slices/interfaces' -import { useSelector } from 'react-redux' -import { rdiPipelineSelector } from 'uiSrc/slices/rdi/pipeline' import { RiTooltip } from 'uiSrc/components' import { Indicator } from 'uiSrc/components/base/text/text.styles' import { Row } from 'uiSrc/components/base/layout/flex' import { Text } from 'uiSrc/components/base/text' import { Icon, ToastNotificationIcon } from 'uiSrc/components/base/icons' +import { useConfigurationState } from './hooks' import BaseCard, { BaseCardProps } from './BaseCard' import ValidationErrorsList from '../../validation-errors-list/ValidationErrorsList' @@ -23,15 +22,13 @@ const ConfigurationCard = ({ onSelect, isSelected, }: ConfigurationCardProps) => { - const { changes, configValidationErrors } = useSelector(rdiPipelineSelector) + const { hasChanges, isValid, configValidationErrors } = + useConfigurationState() const handleClick = () => { onSelect(RdiPipelineTabs.Config) } - const hasChanges = !!changes.config - const isValid = configValidationErrors.length === 0 - return ( ({ + ...jest.requireActual('uiSrc/slices/rdi/pipeline'), + rdiPipelineSelector: jest.fn(), +})) + +const mockRdiPipelineSelector = rdiPipelineSelector as jest.MockedFunction< + typeof rdiPipelineSelector +> + +describe('useConfigurationState', () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + it('should return correct state when no changes and no validation errors', () => { + mockRdiPipelineSelector.mockReturnValue({ + changes: {}, + configValidationErrors: [], + } as any) + + const { result } = renderHook(() => useConfigurationState()) + + expect(result.current).toEqual({ + hasChanges: false, + isValid: true, + configValidationErrors: [], + }) + }) + + it('should return hasChanges as true when config has changes', () => { + mockRdiPipelineSelector.mockReturnValue({ + changes: { config: 'modified' }, + configValidationErrors: [], + } as any) + + const { result } = renderHook(() => useConfigurationState()) + + expect(result.current).toEqual({ + hasChanges: true, + isValid: true, + configValidationErrors: [], + }) + }) + + it('should return isValid as false when config has validation errors', () => { + mockRdiPipelineSelector.mockReturnValue({ + changes: {}, + configValidationErrors: [ + 'Invalid configuration', + 'Missing required field', + ], + } as any) + + const { result } = renderHook(() => useConfigurationState()) + + expect(result.current).toEqual({ + hasChanges: false, + isValid: false, + configValidationErrors: [ + 'Invalid configuration', + 'Missing required field', + ], + }) + }) + + it('should handle both changes and validation errors', () => { + mockRdiPipelineSelector.mockReturnValue({ + changes: { config: 'added' }, + configValidationErrors: ['Configuration error'], + } as any) + + const { result } = renderHook(() => useConfigurationState()) + + expect(result.current).toEqual({ + hasChanges: true, + isValid: false, + configValidationErrors: ['Configuration error'], + }) + }) + + it('should handle empty validation errors array', () => { + mockRdiPipelineSelector.mockReturnValue({ + changes: {}, + configValidationErrors: [], + } as any) + + const { result } = renderHook(() => useConfigurationState()) + + expect(result.current.isValid).toBe(true) + }) + + it('should handle single validation error', () => { + mockRdiPipelineSelector.mockReturnValue({ + changes: {}, + configValidationErrors: ['Single error'], + } as any) + + const { result } = renderHook(() => useConfigurationState()) + + expect(result.current).toEqual({ + hasChanges: false, + isValid: false, + configValidationErrors: ['Single error'], + }) + }) + + it('should handle multiple validation errors', () => { + const errors = ['Error 1', 'Error 2', 'Error 3'] + mockRdiPipelineSelector.mockReturnValue({ + changes: {}, + configValidationErrors: errors, + } as any) + + const { result } = renderHook(() => useConfigurationState()) + + expect(result.current).toEqual({ + hasChanges: false, + isValid: false, + configValidationErrors: errors, + }) + }) + + it('should handle changes in other files without affecting config state', () => { + mockRdiPipelineSelector.mockReturnValue({ + changes: { + job1: 'modified', + job2: 'added', + // no config changes + }, + configValidationErrors: [], + } as any) + + const { result } = renderHook(() => useConfigurationState()) + + expect(result.current.hasChanges).toBe(false) + }) +}) diff --git a/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/hooks/useConfigurationState.ts b/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/hooks/useConfigurationState.ts new file mode 100644 index 0000000000..eda3a3e541 --- /dev/null +++ b/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/hooks/useConfigurationState.ts @@ -0,0 +1,21 @@ +import { useSelector } from 'react-redux' +import { rdiPipelineSelector } from 'uiSrc/slices/rdi/pipeline' + +export interface ConfigurationState { + hasChanges: boolean + isValid: boolean + configValidationErrors: string[] +} + +export const useConfigurationState = (): ConfigurationState => { + const { changes, configValidationErrors } = useSelector(rdiPipelineSelector) + + const hasChanges = !!changes.config + const isValid = configValidationErrors.length === 0 + + return { + hasChanges, + isValid, + configValidationErrors, + } +} From 0d25da7028fbf1154986c5f24a2bada6dc739583 Mon Sep 17 00:00:00 2001 From: Pavel Angelov Date: Tue, 18 Nov 2025 20:14:07 +0200 Subject: [PATCH 3/4] improve typings --- .../cards/hooks/useConfigurationState.spec.ts | 104 +++++++++++------- 1 file changed, 65 insertions(+), 39 deletions(-) diff --git a/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/hooks/useConfigurationState.spec.ts b/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/hooks/useConfigurationState.spec.ts index 7b647f2fa3..b348a91690 100644 --- a/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/hooks/useConfigurationState.spec.ts +++ b/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/hooks/useConfigurationState.spec.ts @@ -1,5 +1,6 @@ import { renderHook } from 'uiSrc/utils/test-utils' import { rdiPipelineSelector } from 'uiSrc/slices/rdi/pipeline' +import { IStateRdiPipeline, FileChangeType } from 'uiSrc/slices/interfaces' import { useConfigurationState } from './useConfigurationState' jest.mock('uiSrc/slices/rdi/pipeline', () => ({ @@ -11,16 +12,27 @@ const mockRdiPipelineSelector = rdiPipelineSelector as jest.MockedFunction< typeof rdiPipelineSelector > +type MockRdiPipelineState = Pick< + IStateRdiPipeline, + 'changes' | 'configValidationErrors' +> + +// Helper function to create mock state with only the properties we need +const createMockState = (state: MockRdiPipelineState): IStateRdiPipeline => + state as IStateRdiPipeline + describe('useConfigurationState', () => { beforeEach(() => { jest.clearAllMocks() }) it('should return correct state when no changes and no validation errors', () => { - mockRdiPipelineSelector.mockReturnValue({ - changes: {}, - configValidationErrors: [], - } as any) + mockRdiPipelineSelector.mockReturnValue( + createMockState({ + changes: {}, + configValidationErrors: [], + }), + ) const { result } = renderHook(() => useConfigurationState()) @@ -32,10 +44,12 @@ describe('useConfigurationState', () => { }) it('should return hasChanges as true when config has changes', () => { - mockRdiPipelineSelector.mockReturnValue({ - changes: { config: 'modified' }, - configValidationErrors: [], - } as any) + mockRdiPipelineSelector.mockReturnValue( + createMockState({ + changes: { config: FileChangeType.Modified }, + configValidationErrors: [], + }), + ) const { result } = renderHook(() => useConfigurationState()) @@ -47,13 +61,15 @@ describe('useConfigurationState', () => { }) it('should return isValid as false when config has validation errors', () => { - mockRdiPipelineSelector.mockReturnValue({ - changes: {}, - configValidationErrors: [ - 'Invalid configuration', - 'Missing required field', - ], - } as any) + mockRdiPipelineSelector.mockReturnValue( + createMockState({ + changes: {}, + configValidationErrors: [ + 'Invalid configuration', + 'Missing required field', + ], + }), + ) const { result } = renderHook(() => useConfigurationState()) @@ -68,10 +84,12 @@ describe('useConfigurationState', () => { }) it('should handle both changes and validation errors', () => { - mockRdiPipelineSelector.mockReturnValue({ - changes: { config: 'added' }, - configValidationErrors: ['Configuration error'], - } as any) + mockRdiPipelineSelector.mockReturnValue( + createMockState({ + changes: { config: FileChangeType.Added }, + configValidationErrors: ['Configuration error'], + }), + ) const { result } = renderHook(() => useConfigurationState()) @@ -83,10 +101,12 @@ describe('useConfigurationState', () => { }) it('should handle empty validation errors array', () => { - mockRdiPipelineSelector.mockReturnValue({ - changes: {}, - configValidationErrors: [], - } as any) + mockRdiPipelineSelector.mockReturnValue( + createMockState({ + changes: {}, + configValidationErrors: [], + }), + ) const { result } = renderHook(() => useConfigurationState()) @@ -94,10 +114,12 @@ describe('useConfigurationState', () => { }) it('should handle single validation error', () => { - mockRdiPipelineSelector.mockReturnValue({ - changes: {}, - configValidationErrors: ['Single error'], - } as any) + mockRdiPipelineSelector.mockReturnValue( + createMockState({ + changes: {}, + configValidationErrors: ['Single error'], + }), + ) const { result } = renderHook(() => useConfigurationState()) @@ -110,10 +132,12 @@ describe('useConfigurationState', () => { it('should handle multiple validation errors', () => { const errors = ['Error 1', 'Error 2', 'Error 3'] - mockRdiPipelineSelector.mockReturnValue({ - changes: {}, - configValidationErrors: errors, - } as any) + mockRdiPipelineSelector.mockReturnValue( + createMockState({ + changes: {}, + configValidationErrors: errors, + }), + ) const { result } = renderHook(() => useConfigurationState()) @@ -125,14 +149,16 @@ describe('useConfigurationState', () => { }) it('should handle changes in other files without affecting config state', () => { - mockRdiPipelineSelector.mockReturnValue({ - changes: { - job1: 'modified', - job2: 'added', - // no config changes - }, - configValidationErrors: [], - } as any) + mockRdiPipelineSelector.mockReturnValue( + createMockState({ + changes: { + job1: FileChangeType.Modified, + job2: FileChangeType.Added, + // no config changes + }, + configValidationErrors: [], + }), + ) const { result } = renderHook(() => useConfigurationState()) From ced945ec06183277202b6de7f301d70cd9815122 Mon Sep 17 00:00:00 2001 From: Pavel Angelov Date: Tue, 18 Nov 2025 20:31:24 +0200 Subject: [PATCH 4/4] use the slice type --- .../cards/hooks/useConfigurationState.spec.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/hooks/useConfigurationState.spec.ts b/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/hooks/useConfigurationState.spec.ts index b348a91690..4e6ddafcbe 100644 --- a/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/hooks/useConfigurationState.spec.ts +++ b/redisinsight/ui/src/pages/rdi/pipeline-management/components/navigation/cards/hooks/useConfigurationState.spec.ts @@ -1,5 +1,5 @@ import { renderHook } from 'uiSrc/utils/test-utils' -import { rdiPipelineSelector } from 'uiSrc/slices/rdi/pipeline' +import { rdiPipelineSelector, initialState } from 'uiSrc/slices/rdi/pipeline' import { IStateRdiPipeline, FileChangeType } from 'uiSrc/slices/interfaces' import { useConfigurationState } from './useConfigurationState' @@ -12,14 +12,12 @@ const mockRdiPipelineSelector = rdiPipelineSelector as jest.MockedFunction< typeof rdiPipelineSelector > -type MockRdiPipelineState = Pick< - IStateRdiPipeline, - 'changes' | 'configValidationErrors' -> - -// Helper function to create mock state with only the properties we need -const createMockState = (state: MockRdiPipelineState): IStateRdiPipeline => - state as IStateRdiPipeline +const createMockState = ( + overrides: Partial = {}, +): IStateRdiPipeline => ({ + ...initialState, + ...overrides, +}) describe('useConfigurationState', () => { beforeEach(() => {