Skip to content

RI-7217 allow to deplot rdi pipeline with validation errors #4796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

ArtemHoruzhenko
Copy link
Collaborator

@ArtemHoruzhenko ArtemHoruzhenko commented Aug 4, 2025

Show validation errors in a tooltips and do not block deploy button in case of validation errors
Screenshot 2025-08-06 at 09 40 38
Screenshot 2025-08-06 at 09 41 01
Screenshot 2025-08-06 at 09 41 06

Copy link
Contributor

github-actions bot commented Aug 4, 2025

Code Coverage - Frontend unit tests

St.
Category Percentage Covered / Total
🟢 Statements 81.46% 18971/23289
🟡 Branches 66.79% 8281/12398
🟡 Functions 75.25% 4963/6595
🟢 Lines 81.87% 18570/22682

Test suite run success

4846 tests passing in 635 suites.

Report generated by 🧪jest coverage report action from c9ed88c

@ArtemHoruzhenko ArtemHoruzhenko marked this pull request as ready for review August 6, 2025 06:42
valkirilov
valkirilov previously approved these changes Aug 6, 2025
Comment on lines 14 to 18
<Text>
<ul>
{validationErrors.map((err, index) => (
// eslint-disable-next-line react/no-array-index-key
<li key={index}>{err}</li>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if should be used as top level wrapper of ul

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is pointless to wrap ul in p, it just

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KrumTy @pd-redis so the suggestion here is to remove <Text> tag right?

Comment on lines 10 to 22

return (
<>
{validationErrors?.length && (
<Text>
<ul>
{validationErrors.map((err, index) => (
// eslint-disable-next-line react/no-array-index-key
<li key={index}>{err}</li>
))}
</ul>
</Text>
)}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return (
<>
{validationErrors?.length && (
<Text>
<ul>
{validationErrors.map((err, index) => (
// eslint-disable-next-line react/no-array-index-key
<li key={index}>{err}</li>
))}
</ul>
</Text>
)}
if(!validationErrors?.length) {
return null
}
return (
<ul>
{validationErrors.map((err, index) => (
// eslint-disable-next-line react/no-array-index-key
<li key={index}>{err}</li>
))}
</ul>
)

@ArtemHoruzhenko ArtemHoruzhenko merged commit a3a28b5 into fe/feature/RI-7039-replace-eui Aug 7, 2025
20 checks passed
@ArtemHoruzhenko ArtemHoruzhenko deleted the fe/feature/RI-7217-allow-deplot-invalid-rdi-pipeline branch August 7, 2025 08:37
ArtemHoruzhenko added a commit that referenced this pull request Aug 7, 2025
* 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]>
(cherry picked from commit a3a28b5)
ArtemHoruzhenko added a commit that referenced this pull request Aug 7, 2025
* 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]>
(cherry picked from commit a3a28b5)

* RI-7186 validate job name separately (#4804)

* RI-7186 validate job name separately

* RI-7186 add tests

(cherry picked from commit 2e5d9ee)

---------

Co-authored-by: pd-redis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants