Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 9 additions & 1 deletion redisinsight/ui/src/components/base/icons/Icon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ type BaseIconProps = Omit<MonochromeIconProps, 'color' | 'size'> & {
| (string & {})
size?: IconSizeType | null
isSvg?: boolean
style?: React.CSSProperties
}

const sizesMap = {
Expand Down Expand Up @@ -43,6 +44,7 @@ export const Icon = ({
color = 'primary600',
size,
className,
style = {},
...rest
}: BaseIconProps) => {
let sizeValue: number | string | undefined
Expand Down Expand Up @@ -73,7 +75,13 @@ export const Icon = ({
? svgProps
: { color, customColor, size, customSize, ...rest }

return <IconComponent {...props} className={cx(className, 'RI-Icon')} />
return (
<IconComponent
{...props}
style={{ ...style, verticalAlign: 'middle' }}
className={cx(className, 'RI-Icon')}
/>
)
}

export type IconProps = Omit<BaseIconProps, 'icon'>
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
render,
screen,
} from 'uiSrc/utils/test-utils'
import { rdiPipelineSelector } from 'uiSrc/slices/rdi/pipeline'
import DeployPipelineButton, { Props } from './DeployPipelineButton'

const mockedProps: Props = {
Expand All @@ -25,6 +26,7 @@ jest.mock('uiSrc/slices/rdi/pipeline', () => ({
rdiPipelineSelector: jest.fn().mockReturnValue({
loading: false,
config: 'value',
isPipelineValid: true,
jobs: [
{ name: 'job1', value: '1' },
{ name: 'job2', value: '2' },
Expand Down Expand Up @@ -88,13 +90,43 @@ describe('DeployPipelineButton', () => {
})
})

it('should open confirmation popover', () => {
it('should open confirmation popover with default message', () => {
render(<DeployPipelineButton {...mockedProps} />)

expect(screen.queryByTestId('deploy-confirm-btn')).not.toBeInTheDocument()

fireEvent.click(screen.getByTestId('deploy-rdi-pipeline'))

expect(screen.queryByTestId('deploy-confirm-btn')).toBeInTheDocument()
expect(
screen.queryByText('Are you sure you want to deploy the pipeline?'),
).toBeInTheDocument()
expect(
screen.queryByText(
'Your RDI pipeline contains errors. Are you sure you want to continue?',
),
).not.toBeInTheDocument()
})

it('should open confirmation popover with warning message due to validation errors', () => {
;(rdiPipelineSelector as jest.Mock).mockImplementation(() => ({
isPipelineValid: false,
}))

render(<DeployPipelineButton {...mockedProps} />)

expect(screen.queryByTestId('deploy-confirm-btn')).not.toBeInTheDocument()

fireEvent.click(screen.getByTestId('deploy-rdi-pipeline'))

expect(screen.queryByTestId('deploy-confirm-btn')).toBeInTheDocument()
expect(
screen.queryByText('Are you sure you want to deploy the pipeline?'),
).not.toBeInTheDocument()
expect(
screen.queryByText(
'Your RDI pipeline contains errors. Are you sure you want to continue?',
),
).toBeInTheDocument()
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { sendEventTelemetry, TelemetryEvent } from 'uiSrc/telemetry'
import { createAxiosError, pipelineToJson } from 'uiSrc/utils'
import { addErrorNotification } from 'uiSrc/slices/app/notifications'
import { rdiErrorMessages } from 'uiSrc/pages/rdi/constants'
import { Text } from 'uiSrc/components/base/text'
import { ColorText, Text } from 'uiSrc/components/base/text'
import { FlexItem, Row } from 'uiSrc/components/base/layout/flex'
import { Spacer } from 'uiSrc/components/base/layout/spacer'
import { OutsideClickDetector } from 'uiSrc/components/base/utils'
Expand All @@ -36,7 +36,8 @@ const DeployPipelineButton = ({ loading, disabled, onReset }: Props) => {
const [isPopoverOpen, setIsPopoverOpen] = useState(false)
const [resetPipeline, setResetPipeline] = useState(false)

const { config, jobs, resetChecked } = useSelector(rdiPipelineSelector)
const { config, jobs, resetChecked, isPipelineValid } =
useSelector(rdiPipelineSelector)

const { rdiInstanceId } = useParams<{ rdiInstanceId: string }>()
const dispatch = useDispatch()
Expand Down Expand Up @@ -127,7 +128,19 @@ const DeployPipelineButton = ({ loading, disabled, onReset }: Props) => {
</PrimaryButton>
}
>
<Title size="XS">Are you sure you want to deploy the pipeline?</Title>
<Title size="XS">
{isPipelineValid ? (
<ColorText color="default">
Are you sure you want to deploy the pipeline?
</ColorText>
) : (
<ColorText color="warning">
<RiIcon type="ToastDangerIcon" size="L" color="attention500" />
Your RDI pipeline contains errors. Are you sure you want to
continue?
</ColorText>
)}
</Title>
<Spacer size="s" />
<Text size="s">
When deployed, this local configuration will overwrite any existing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ describe('PipelineActions', () => {
)
})

it('should dispatch validation errors if validation fails', () => {
it('should dispatch validation errors if validation fails but still deploy button should be enabled', () => {
;(validatePipeline as jest.Mock).mockReturnValue({
result: false,
configValidationErrors: ['Missing field'],
Expand Down Expand Up @@ -199,6 +199,8 @@ describe('PipelineActions', () => {
}),
]),
)

expect(screen.queryByTestId('deploy-rdi-pipeline')).not.toBeDisabled()
})

describe('TelemetryEvent', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
PipelineStatus,
} from 'uiSrc/slices/interfaces'

import { RiTooltip } from 'uiSrc/components'
import { FlexItem, Row } from 'uiSrc/components/base/layout/flex'
import DeployPipelineButton from '../buttons/deploy-pipeline-button'
import ResetPipelineButton from '../buttons/reset-pipeline-button'
Expand All @@ -38,7 +37,6 @@ export interface Props {
const PipelineActions = ({ collectorStatus, pipelineStatus }: Props) => {
const {
loading: deployLoading,
isPipelineValid,
schema,
config,
jobs,
Expand Down Expand Up @@ -141,7 +139,6 @@ const PipelineActions = ({ collectorStatus, pipelineStatus }: Props) => {
const isLoadingBtn = (actionBtn: PipelineAction) =>
action === actionBtn && actionLoading
const disabled = deployLoading || actionLoading
const isDeployButtonDisabled = disabled || !isPipelineValid

return (
<Row gap="m" justify="end" align="center">
Expand All @@ -168,21 +165,11 @@ const PipelineActions = ({ collectorStatus, pipelineStatus }: Props) => {
)}
</FlexItem>
<FlexItem>
<RiTooltip
content={
isPipelineValid
? ''
: 'Please fix the validation errors before deploying'
}
position="left"
anchorClassName="flex-row"
>
<DeployPipelineButton
loading={deployLoading}
disabled={isDeployButtonDisabled}
onReset={resetPipeline}
/>
</RiTooltip>
<DeployPipelineButton
loading={deployLoading}
disabled={disabled}
onReset={resetPipeline}
/>
</FlexItem>
<FlexItem style={{ margin: 0 }}>
<RdiConfigFileActionMenu />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,4 +289,153 @@ describe('JobsTree', () => {

expect(screen.getByTestId('apply-btn')).toBeDisabled()
})

it('should display ValidationErrorsList in tooltip when job has multiple validation errors', () => {
const validationErrors = [
'Missing required field: name',
'Invalid data type for age',
'Email format is incorrect'
]

;(rdiPipelineSelector as jest.Mock).mockImplementationOnce(() => ({
loading: false,
error: '',
jobs: [{ name: 'job1', value: 'value' }],
jobsValidationErrors: {
job1: validationErrors,
},
}))

render(<JobsTree {...instance(mockedProps)} />)

expect(screen.getByTestId('rdi-nav-job-job1')).toBeInTheDocument()
expect(screen.getByTestId('rdi-pipeline-nav__error')).toBeInTheDocument()

// The ValidationErrorsList is inside a tooltip, so we verify the error icon is present
const errorIcon = screen.getByTestId('rdi-pipeline-nav__error')
expect(errorIcon).toBeInTheDocument()
})

it('should display ValidationErrorsList in tooltip when job has single validation error', () => {
const validationErrors = ['Single validation error']

;(rdiPipelineSelector as jest.Mock).mockImplementationOnce(() => ({
loading: false,
error: '',
jobs: [{ name: 'job1', value: 'value' }],
jobsValidationErrors: {
job1: validationErrors,
},
}))

render(<JobsTree {...instance(mockedProps)} />)

expect(screen.getByTestId('rdi-nav-job-job1')).toBeInTheDocument()
expect(screen.getByTestId('rdi-pipeline-nav__error')).toBeInTheDocument()
})

it('should handle multiple jobs with different validation states', () => {
;(rdiPipelineSelector as jest.Mock).mockImplementationOnce(() => ({
loading: false,
error: '',
jobs: [
{ name: 'job1', value: 'value1' },
{ name: 'job2', value: 'value2' },
{ name: 'job3', value: 'value3' }
],
jobsValidationErrors: {
job1: ['Error in job1'],
job3: ['Error in job3', 'Another error in job3'],
},
}))

render(<JobsTree {...instance(mockedProps)} />)

// job1 should have error icon
const job1Element = screen.getByTestId('rdi-nav-job-job1')
expect(job1Element).toBeInTheDocument()
expect(job1Element).toHaveClass('invalid')

// job2 should not have error icon and should not have invalid class
const job2Element = screen.getByTestId('rdi-nav-job-job2')
expect(job2Element).toBeInTheDocument()
expect(job2Element).not.toHaveClass('invalid')

// job3 should have error icon
const job3Element = screen.getByTestId('rdi-nav-job-job3')
expect(job3Element).toBeInTheDocument()
expect(job3Element).toHaveClass('invalid')

// There should be exactly 2 error icons total (for job1 and job3)
const errorIcons = screen.getAllByTestId('rdi-pipeline-nav__error')
expect(errorIcons).toHaveLength(2)
})

it('should apply invalid class to job name when validation errors exist', () => {
;(rdiPipelineSelector as jest.Mock).mockImplementationOnce(() => ({
loading: false,
error: '',
jobs: [{ name: 'job1', value: 'value' }],
jobsValidationErrors: {
job1: ['Some validation error'],
},
}))

render(<JobsTree {...instance(mockedProps)} />)

expect(screen.getByTestId('rdi-nav-job-job1')).toHaveClass('invalid')
})

it('should not apply invalid class to job name when no validation errors exist', () => {
;(rdiPipelineSelector as jest.Mock).mockImplementationOnce(() => ({
loading: false,
error: '',
jobs: [{ name: 'job1', value: 'value' }],
jobsValidationErrors: {},
}))

render(<JobsTree {...instance(mockedProps)} />)

expect(screen.getByTestId('rdi-nav-job-job1')).not.toHaveClass('invalid')
})

it('should handle empty validation errors array', () => {
;(rdiPipelineSelector as jest.Mock).mockImplementationOnce(() => ({
loading: false,
error: '',
jobs: [{ name: 'job1', value: 'value' }],
jobsValidationErrors: {
job1: [],
},
}))

render(<JobsTree {...instance(mockedProps)} />)

expect(screen.getByTestId('rdi-nav-job-job1')).toBeInTheDocument()
expect(
screen.queryByTestId('rdi-pipeline-nav__error'),
).not.toBeInTheDocument()
})

it('should handle validation errors with special characters', () => {
const validationErrors = [
'Error with <script>alert("xss")</script>',
'Error with & special characters',
'Error with "quotes" and \'apostrophes\''
]

;(rdiPipelineSelector as jest.Mock).mockImplementationOnce(() => ({
loading: false,
error: '',
jobs: [{ name: 'job1', value: 'value' }],
jobsValidationErrors: {
job1: validationErrors,
},
}))

render(<JobsTree {...instance(mockedProps)} />)

expect(screen.getByTestId('rdi-nav-job-job1')).toBeInTheDocument()
expect(screen.getByTestId('rdi-pipeline-nav__error')).toBeInTheDocument()
})
})
Loading
Loading