-
Notifications
You must be signed in to change notification settings - Fork 314
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
feat(web-app-reporter): Always show issue tab #7171
feat(web-app-reporter): Always show issue tab #7171
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #7171 +/- ##
============================================
- Coverage 64.28% 61.35% -2.94%
- Complexity 1970 1984 +14
============================================
Files 333 333
Lines 16681 16663 -18
Branches 2386 2412 +26
============================================
- Hits 10724 10223 -501
- Misses 4918 5453 +535
+ Partials 1039 987 -52
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please hard-wrap commit message body lines to avoid commit-lint complaining. While at it, "resolves" should be capitalized.
plugins/reporters/web-app-template/src/components/SummaryView.js
Outdated
Show resolved
Hide resolved
This should be configurable option as we had users that don't want to see violations, issues and vulnerabilities. Propose we move this change in behavior to a reporter option (e.g. boolean to show tabs always or not). |
Personally, I don't think it's worth to put the burden of implementing a reporter-specific option / feature flag on the OP for this. We should have a consistent UI independent of the number of issues / violations / vulnerabilities, and say in the tabs that there are none, if so. |
A user would need to select this behaviour every time the report is loaded. IMHO, this does not sound that user friendly and I agree with @sschuberth. How can we come to an agreement here? |
@hanna-modica That now what I proposed - it would be a configuration option at time of generation of the report, not upon opening the web report. No user action required only config change by an ORT admin. |
I agree with @sschuberth here. In the last community meeting there was no one present who actually preferred hiding the tab if there are no issues, but several participants preferred to always show it. As this also fixes the issue that it is currently impossible to review resolved issues if all issues are resolved, I would prefer to merge this as is and introduce an option only if there is an actual demand for it which justifies the work and increased complexity. |
@sschuberth @mnonnenmacher I am against merging this as is - as I said in the community meeting I don't believe this feature work as @hanna-modica expects it to work - if a scan has only resolved issues to my recollection it won't show these in issues (cam from old HERE decision) - this needs to be tested first. We have ORT users that use SBOM and ORT web reports in the FOSS reporting to their customers as a replacement for Excel and HTML reports from their previous commercial vendor. WebApp is provided as convenience for customer legal counsel to quickly browse the result (happy to get them off Excel). Just because ORT users are not at the community meeting doesn't mean their use cases should be taken into account. |
In our reports, resolved issues are always shown in the issues tab (if it is there). That aside, I believe it to be very confusing and not good UX nonetheless, if the shown tabs are not consistent for every report. |
I have verified that it works as expected, if all issues are resolved they are visible in the issues tab.
I still don't understand how an empty issues tab could break anyone's use of the web app report. To the contrary, if you are resolving issues, rule violations, or vulnerabilities one by one, you can always see the effects in the web app reporter, until you resolve the last one, because then those tabs just disappear which is a confusing UX. |
4f3cd66
to
bf81fbc
Compare
3bdc3fa
to
6238864
Compare
Always show the rule violations, issues and vulnerabilities tabs. This allows on the one hand the view to be more consistent, by always showing the same tabs, and on the other hand allows to check resolutions, even if there are no open violations, issues or vulnerabilities left. Resolves oss-review-toolkit#6232 Signed-off-by: Hanna Modica <Hanna.Modica@bosch.io>
6238864
to
d5b6236
Compare
This allows on one hand the view to be more consistent, by always showing the same tab, and on the other hand allows to check resolved issues, even if there are no open issues left.
resolves #6232