ROX-29513: Replace CSV report preview with Non-optional columns#15467
Merged
pedrottimark merged 1 commit intomasterfrom May 28, 2025
Merged
ROX-29513: Replace CSV report preview with Non-optional columns#15467pedrottimark merged 1 commit intomasterfrom
pedrottimark merged 1 commit intomasterfrom
Conversation
Contributor
There was a problem hiding this comment.
Hey @pedrottimark - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
Images are ready for the commit at c115b3c. To use with deploy scripts, first |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #15467 +/- ##
=======================================
Coverage 49.25% 49.25%
=======================================
Files 2581 2581
Lines 189326 189326
=======================================
+ Hits 93251 93256 +5
+ Misses 88735 88732 -3
+ Partials 7340 7338 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pedrottimark
commented
May 28, 2025
.../src/Containers/Vulnerabilities/VulnerablityReporting/components/ReportParametersDetails.tsx
Show resolved
Hide resolved
bradr5
approved these changes
May 28, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Problem
Good eyes, Ross!
How did I overlook during 3 release cycles when we have added:
Thank you, Surabhi for examples of CSV files and answer to question about body of e-mail message when I was confused which preview has the problem.
Which leads to problems, other than inconsistency:
Because there are at least 10 RFE for additional columns, the simpler to add, the better.
Redundant with Optional columns in Report parameters.
Increasingly difficult for combinations of data not to become impossible.
Width of realistic Advisory Link data implies even more horizonatal scrolling.
Analysis
Non-optional columns rarely change and they precede Optional columns for which we have a pattern to update.
However, they have changed since the original design of the preview:
Solution
Less is more:
Source of truth for column headings:
https://github.com/stackrox/stackrox/blob/master/central/reports/scheduler/v2/reportgenerator/csv_gen.go
https://github.com/stackrox/stackrox/pull/15091/files#diff-cd4158d1cbd02c87b40f320498782084ad850e1d00a25305871ddcd265fab64aR89-R98
User-facing documentation
Testing and quality
Automated testing
How I validated my change
npm run tscin ui/apps/platform folder.npm run lintin ui/apps/platform folder.Manual testing
Visit /main/vulnerabilities/reports/configuration?action=create enter name, select one or more checkboxes under Optional columns, select collection, and advance to Review and create step.
Before changes, see presence of CVE report format but absence of any selected columns.
After changes, see absence of CVE report format but presence of Non-optional columns preceding Optional columns.
