Skip to content

Commit a3a28b5

Browse files
RI-7217 allow to deplot rdi pipeline with validation errors (#4796)
* RI-7217 allow to deplot rdi pipeline with validation errors * align text and icon * RI-7217 move errors list to a separate component and add tests * RI-7217 fix PR comments * RI-7217 fix test --------- Co-authored-by: pd-redis <[email protected]>
1 parent 5172c03 commit a3a28b5

File tree

13 files changed

+456
-40
lines changed

13 files changed

+456
-40
lines changed

redisinsight/ui/src/components/base/icons/Icon.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ type BaseIconProps = Omit<MonochromeIconProps, 'color' | 'size'> & {
1212
| (string & {})
1313
size?: IconSizeType | null
1414
isSvg?: boolean
15+
style?: React.CSSProperties
1516
}
1617

1718
const sizesMap = {
@@ -43,6 +44,7 @@ export const Icon = ({
4344
color = 'primary600',
4445
size,
4546
className,
47+
style = {},
4648
...rest
4749
}: BaseIconProps) => {
4850
let sizeValue: number | string | undefined
@@ -73,7 +75,13 @@ export const Icon = ({
7375
? svgProps
7476
: { color, customColor, size, customSize, ...rest }
7577

76-
return <IconComponent {...props} className={cx(className, 'RI-Icon')} />
78+
return (
79+
<IconComponent
80+
{...props}
81+
style={{ ...style, verticalAlign: 'middle' }}
82+
className={cx(className, 'RI-Icon')}
83+
/>
84+
)
7785
}
7886

7987
export type IconProps = Omit<BaseIconProps, 'icon'>

redisinsight/ui/src/pages/rdi/instance/components/header/components/buttons/deploy-pipeline-button/DeployPipelineButton.spec.tsx

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
render,
99
screen,
1010
} from 'uiSrc/utils/test-utils'
11+
import { rdiPipelineSelector } from 'uiSrc/slices/rdi/pipeline'
1112
import DeployPipelineButton, { Props } from './DeployPipelineButton'
1213

1314
const mockedProps: Props = {
@@ -25,6 +26,7 @@ jest.mock('uiSrc/slices/rdi/pipeline', () => ({
2526
rdiPipelineSelector: jest.fn().mockReturnValue({
2627
loading: false,
2728
config: 'value',
29+
isPipelineValid: true,
2830
jobs: [
2931
{ name: 'job1', value: '1' },
3032
{ name: 'job2', value: '2' },
@@ -88,13 +90,43 @@ describe('DeployPipelineButton', () => {
8890
})
8991
})
9092

91-
it('should open confirmation popover', () => {
93+
it('should open confirmation popover with default message', () => {
9294
render(<DeployPipelineButton {...mockedProps} />)
9395

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

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

98100
expect(screen.queryByTestId('deploy-confirm-btn')).toBeInTheDocument()
101+
expect(
102+
screen.queryByText('Are you sure you want to deploy the pipeline?'),
103+
).toBeInTheDocument()
104+
expect(
105+
screen.queryByText(
106+
'Your RDI pipeline contains errors. Are you sure you want to continue?',
107+
),
108+
).not.toBeInTheDocument()
109+
})
110+
111+
it('should open confirmation popover with warning message due to validation errors', () => {
112+
;(rdiPipelineSelector as jest.Mock).mockImplementation(() => ({
113+
isPipelineValid: false,
114+
}))
115+
116+
render(<DeployPipelineButton {...mockedProps} />)
117+
118+
expect(screen.queryByTestId('deploy-confirm-btn')).not.toBeInTheDocument()
119+
120+
fireEvent.click(screen.getByTestId('deploy-rdi-pipeline'))
121+
122+
expect(screen.queryByTestId('deploy-confirm-btn')).toBeInTheDocument()
123+
expect(
124+
screen.queryByText('Are you sure you want to deploy the pipeline?'),
125+
).not.toBeInTheDocument()
126+
expect(
127+
screen.queryByText(
128+
'Your RDI pipeline contains errors. Are you sure you want to continue?',
129+
),
130+
).toBeInTheDocument()
99131
})
100132
})

redisinsight/ui/src/pages/rdi/instance/components/header/components/buttons/deploy-pipeline-button/DeployPipelineButton.tsx

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { sendEventTelemetry, TelemetryEvent } from 'uiSrc/telemetry'
1414
import { createAxiosError, pipelineToJson } from 'uiSrc/utils'
1515
import { addErrorNotification } from 'uiSrc/slices/app/notifications'
1616
import { rdiErrorMessages } from 'uiSrc/pages/rdi/constants'
17-
import { Text } from 'uiSrc/components/base/text'
17+
import { ColorText, Text } from 'uiSrc/components/base/text'
1818
import { FlexItem, Row } from 'uiSrc/components/base/layout/flex'
1919
import { Spacer } from 'uiSrc/components/base/layout/spacer'
2020
import { OutsideClickDetector } from 'uiSrc/components/base/utils'
@@ -36,7 +36,8 @@ const DeployPipelineButton = ({ loading, disabled, onReset }: Props) => {
3636
const [isPopoverOpen, setIsPopoverOpen] = useState(false)
3737
const [resetPipeline, setResetPipeline] = useState(false)
3838

39-
const { config, jobs, resetChecked } = useSelector(rdiPipelineSelector)
39+
const { config, jobs, resetChecked, isPipelineValid } =
40+
useSelector(rdiPipelineSelector)
4041

4142
const { rdiInstanceId } = useParams<{ rdiInstanceId: string }>()
4243
const dispatch = useDispatch()
@@ -127,7 +128,19 @@ const DeployPipelineButton = ({ loading, disabled, onReset }: Props) => {
127128
</PrimaryButton>
128129
}
129130
>
130-
<Title size="XS">Are you sure you want to deploy the pipeline?</Title>
131+
<Title size="XS">
132+
{isPipelineValid ? (
133+
<ColorText color="default">
134+
Are you sure you want to deploy the pipeline?
135+
</ColorText>
136+
) : (
137+
<ColorText color="warning">
138+
<RiIcon type="ToastDangerIcon" size="L" color="attention500" />
139+
Your RDI pipeline contains errors. Are you sure you want to
140+
continue?
141+
</ColorText>
142+
)}
143+
</Title>
131144
<Spacer size="s" />
132145
<Text size="s">
133146
When deployed, this local configuration will overwrite any existing

redisinsight/ui/src/pages/rdi/instance/components/header/components/pipeline-actions/PipelineActions.spec.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ describe('PipelineActions', () => {
168168
)
169169
})
170170

171-
it('should dispatch validation errors if validation fails', () => {
171+
it('should dispatch validation errors if validation fails but still deploy button should be enabled', () => {
172172
;(validatePipeline as jest.Mock).mockReturnValue({
173173
result: false,
174174
configValidationErrors: ['Missing field'],
@@ -199,6 +199,8 @@ describe('PipelineActions', () => {
199199
}),
200200
]),
201201
)
202+
203+
expect(screen.queryByTestId('deploy-rdi-pipeline')).not.toBeDisabled()
202204
})
203205

204206
describe('TelemetryEvent', () => {

redisinsight/ui/src/pages/rdi/instance/components/header/components/pipeline-actions/PipelineActions.tsx

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import {
2222
PipelineStatus,
2323
} from 'uiSrc/slices/interfaces'
2424

25-
import { RiTooltip } from 'uiSrc/components'
2625
import { FlexItem, Row } from 'uiSrc/components/base/layout/flex'
2726
import DeployPipelineButton from '../buttons/deploy-pipeline-button'
2827
import ResetPipelineButton from '../buttons/reset-pipeline-button'
@@ -38,7 +37,6 @@ export interface Props {
3837
const PipelineActions = ({ collectorStatus, pipelineStatus }: Props) => {
3938
const {
4039
loading: deployLoading,
41-
isPipelineValid,
4240
schema,
4341
config,
4442
jobs,
@@ -141,7 +139,6 @@ const PipelineActions = ({ collectorStatus, pipelineStatus }: Props) => {
141139
const isLoadingBtn = (actionBtn: PipelineAction) =>
142140
action === actionBtn && actionLoading
143141
const disabled = deployLoading || actionLoading
144-
const isDeployButtonDisabled = disabled || !isPipelineValid
145142

146143
return (
147144
<Row gap="m" justify="end" align="center">
@@ -168,21 +165,11 @@ const PipelineActions = ({ collectorStatus, pipelineStatus }: Props) => {
168165
)}
169166
</FlexItem>
170167
<FlexItem>
171-
<RiTooltip
172-
content={
173-
isPipelineValid
174-
? ''
175-
: 'Please fix the validation errors before deploying'
176-
}
177-
position="left"
178-
anchorClassName="flex-row"
179-
>
180-
<DeployPipelineButton
181-
loading={deployLoading}
182-
disabled={isDeployButtonDisabled}
183-
onReset={resetPipeline}
184-
/>
185-
</RiTooltip>
168+
<DeployPipelineButton
169+
loading={deployLoading}
170+
disabled={disabled}
171+
onReset={resetPipeline}
172+
/>
186173
</FlexItem>
187174
<FlexItem style={{ margin: 0 }}>
188175
<RdiConfigFileActionMenu />

redisinsight/ui/src/pages/rdi/pipeline-management/components/jobs-tree/JobsTree.spec.tsx

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,4 +289,153 @@ describe('JobsTree', () => {
289289

290290
expect(screen.getByTestId('apply-btn')).toBeDisabled()
291291
})
292+
293+
it('should display ValidationErrorsList in tooltip when job has multiple validation errors', () => {
294+
const validationErrors = [
295+
'Missing required field: name',
296+
'Invalid data type for age',
297+
'Email format is incorrect'
298+
]
299+
300+
;(rdiPipelineSelector as jest.Mock).mockImplementationOnce(() => ({
301+
loading: false,
302+
error: '',
303+
jobs: [{ name: 'job1', value: 'value' }],
304+
jobsValidationErrors: {
305+
job1: validationErrors,
306+
},
307+
}))
308+
309+
render(<JobsTree {...instance(mockedProps)} />)
310+
311+
expect(screen.getByTestId('rdi-nav-job-job1')).toBeInTheDocument()
312+
expect(screen.getByTestId('rdi-pipeline-nav__error')).toBeInTheDocument()
313+
314+
// The ValidationErrorsList is inside a tooltip, so we verify the error icon is present
315+
const errorIcon = screen.getByTestId('rdi-pipeline-nav__error')
316+
expect(errorIcon).toBeInTheDocument()
317+
})
318+
319+
it('should display ValidationErrorsList in tooltip when job has single validation error', () => {
320+
const validationErrors = ['Single validation error']
321+
322+
;(rdiPipelineSelector as jest.Mock).mockImplementationOnce(() => ({
323+
loading: false,
324+
error: '',
325+
jobs: [{ name: 'job1', value: 'value' }],
326+
jobsValidationErrors: {
327+
job1: validationErrors,
328+
},
329+
}))
330+
331+
render(<JobsTree {...instance(mockedProps)} />)
332+
333+
expect(screen.getByTestId('rdi-nav-job-job1')).toBeInTheDocument()
334+
expect(screen.getByTestId('rdi-pipeline-nav__error')).toBeInTheDocument()
335+
})
336+
337+
it('should handle multiple jobs with different validation states', () => {
338+
;(rdiPipelineSelector as jest.Mock).mockImplementationOnce(() => ({
339+
loading: false,
340+
error: '',
341+
jobs: [
342+
{ name: 'job1', value: 'value1' },
343+
{ name: 'job2', value: 'value2' },
344+
{ name: 'job3', value: 'value3' }
345+
],
346+
jobsValidationErrors: {
347+
job1: ['Error in job1'],
348+
job3: ['Error in job3', 'Another error in job3'],
349+
},
350+
}))
351+
352+
render(<JobsTree {...instance(mockedProps)} />)
353+
354+
// job1 should have error icon
355+
const job1Element = screen.getByTestId('rdi-nav-job-job1')
356+
expect(job1Element).toBeInTheDocument()
357+
expect(job1Element).toHaveClass('invalid')
358+
359+
// job2 should not have error icon and should not have invalid class
360+
const job2Element = screen.getByTestId('rdi-nav-job-job2')
361+
expect(job2Element).toBeInTheDocument()
362+
expect(job2Element).not.toHaveClass('invalid')
363+
364+
// job3 should have error icon
365+
const job3Element = screen.getByTestId('rdi-nav-job-job3')
366+
expect(job3Element).toBeInTheDocument()
367+
expect(job3Element).toHaveClass('invalid')
368+
369+
// There should be exactly 2 error icons total (for job1 and job3)
370+
const errorIcons = screen.getAllByTestId('rdi-pipeline-nav__error')
371+
expect(errorIcons).toHaveLength(2)
372+
})
373+
374+
it('should apply invalid class to job name when validation errors exist', () => {
375+
;(rdiPipelineSelector as jest.Mock).mockImplementationOnce(() => ({
376+
loading: false,
377+
error: '',
378+
jobs: [{ name: 'job1', value: 'value' }],
379+
jobsValidationErrors: {
380+
job1: ['Some validation error'],
381+
},
382+
}))
383+
384+
render(<JobsTree {...instance(mockedProps)} />)
385+
386+
expect(screen.getByTestId('rdi-nav-job-job1')).toHaveClass('invalid')
387+
})
388+
389+
it('should not apply invalid class to job name when no validation errors exist', () => {
390+
;(rdiPipelineSelector as jest.Mock).mockImplementationOnce(() => ({
391+
loading: false,
392+
error: '',
393+
jobs: [{ name: 'job1', value: 'value' }],
394+
jobsValidationErrors: {},
395+
}))
396+
397+
render(<JobsTree {...instance(mockedProps)} />)
398+
399+
expect(screen.getByTestId('rdi-nav-job-job1')).not.toHaveClass('invalid')
400+
})
401+
402+
it('should handle empty validation errors array', () => {
403+
;(rdiPipelineSelector as jest.Mock).mockImplementationOnce(() => ({
404+
loading: false,
405+
error: '',
406+
jobs: [{ name: 'job1', value: 'value' }],
407+
jobsValidationErrors: {
408+
job1: [],
409+
},
410+
}))
411+
412+
render(<JobsTree {...instance(mockedProps)} />)
413+
414+
expect(screen.getByTestId('rdi-nav-job-job1')).toBeInTheDocument()
415+
expect(
416+
screen.queryByTestId('rdi-pipeline-nav__error'),
417+
).not.toBeInTheDocument()
418+
})
419+
420+
it('should handle validation errors with special characters', () => {
421+
const validationErrors = [
422+
'Error with <script>alert("xss")</script>',
423+
'Error with & special characters',
424+
'Error with "quotes" and \'apostrophes\''
425+
]
426+
427+
;(rdiPipelineSelector as jest.Mock).mockImplementationOnce(() => ({
428+
loading: false,
429+
error: '',
430+
jobs: [{ name: 'job1', value: 'value' }],
431+
jobsValidationErrors: {
432+
job1: validationErrors,
433+
},
434+
}))
435+
436+
render(<JobsTree {...instance(mockedProps)} />)
437+
438+
expect(screen.getByTestId('rdi-nav-job-job1')).toBeInTheDocument()
439+
expect(screen.getByTestId('rdi-pipeline-nav__error')).toBeInTheDocument()
440+
})
292441
})

0 commit comments

Comments
 (0)