Skip to content
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

Various WebApp report fixes and new features (part 2) #9868

Merged
merged 14 commits into from
Feb 6, 2025

Conversation

tsteenbe
Copy link
Member

See individual commits for details - expect a part 3 PR to some of the WebApp issues that are still open after this PR.

@tsteenbe tsteenbe changed the title Webapp fixes updates part 2 Various WebApp report fixes and updates (part 2) Jan 30, 2025
@tsteenbe tsteenbe changed the title Various WebApp report fixes and updates (part 2) Various WebApp report fixes and new features (part 2) Jan 30, 2025
Copy link

codecov bot commented Jan 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.43%. Comparing base (7681705) to head (618ab4c).
Report is 14 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #9868   +/-   ##
=========================================
  Coverage     68.43%   68.43%           
  Complexity     1308     1308           
=========================================
  Files           250      250           
  Lines          8880     8880           
  Branches        921      921           
=========================================
  Hits           6077     6077           
  Misses         2413     2413           
  Partials        390      390           
Flag Coverage Δ
funTest-docker 65.14% <ø> (ø)
funTest-non-docker 33.37% <ø> (ø)
test-ubuntu-24.04 36.13% <ø> (ø)
test-windows-2022 36.11% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sschuberth
Copy link
Member

@mmurto, @Etsija, @lamppu, feel free to chime in to the review!

this.#isResolved = !!(this.#resolutionIndexes.size > 0);

if (this.#isResolved) {
this.#severityIndex = this.#severityIndex + 10;
Copy link
Member

Choose a reason for hiding this comment

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

Where does the "magic number" 10 come from here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Could have been any other number as long as it bigger than 3 but 10 allows for addition of future states and made things easier for remember e.g. SeverityIndex < 10 = unresolved error, warning or hint policy rule violation / issue, anything > 10 is resolved error, warning or hint policy rule violation / issue. Why the "bigger than 3" well you have to take into account column sorting which operates on comparing SeverityIndex integer instead of strings (Error, Warning, Hint).

  • Error - SeverityIndex 1
  • Warning - SeverityIndex 2
  • Hint - SeverityIndex 3
  • Resolved Error - SeverityIndex 11
  • Resolved Warning - SeverityIndex 12
  • Resolved Hint - SeverityIndex 13

@tsteenbe tsteenbe force-pushed the webapp-fixes-updates-part-2 branch 3 times, most recently from 1e7d1a3 to 874b212 Compare January 30, 2025 16:43
@tsteenbe tsteenbe requested a review from sschuberth January 30, 2025 16:44
@sschuberth sschuberth requested a review from a team January 30, 2025 16:48
@tsteenbe tsteenbe force-pushed the webapp-fixes-updates-part-2 branch from 20abb21 to dbc833a Compare February 2, 2025 20:21
Copy link
Member

@MarcelBochtler MarcelBochtler left a comment

Choose a reason for hiding this comment

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

Most changes look good to me.

However, I would like to split off the commit feat(web-app-template): Adhere to severe thresholds into a separate PR, to discuss it independently.

@MarcelBochtler
Copy link
Member

MarcelBochtler commented Feb 3, 2025

I haven't found the actual problematic code, but changes from this PR cause the report to display HINT issues as Warnings:

image

From the ort-result.yml:

issues:
  - timestamp: "2025-01-30T22:34:24.894998Z"
    source: "FossId"
    message: "This scan has 990 file(s) pending identification in FossID."
    severity: "HINT"

Prevent unnecessary recomputation of `SeverityIndex` and `isResolved`
values in React, ensuring they are only recalculated when their actual
values change.

Signed-off-by: Thomas Steenbergen <[email protected]>
Replace policy rule violation severity icons with tags
so they are more distinguishable and quicker to understand.

Signed-off-by: Thomas Steenbergen <[email protected]>
Address minor linter issues by running `yarn lint --fix`
to enhance code quality and maintainability.

Signed-off-by: Thomas Steenbergen <[email protected]>
Improve the filtering of vulnerability severity levels
to provide a clearer and more effective user experience.

Signed-off-by: Thomas Steenbergen <[email protected]>
Enhance comment to clearly indicate the elements
to which the CSS classes apply, improving code readability.

Signed-off-by: Thomas Steenbergen <[email protected]>
Align the red color of error text with the color
of the critical known security vulnerability tag
in `VulnerabilityRatingTag` to ensure consistency
in visual representation.

Signed-off-by: Thomas Steenbergen <[email protected]>
…tions

Ensure the `id` property of vulnerability resolutions [1] is displayed
in `ResolutionTable`, addressing the previous limitation where only
issue and policy rule violation resolutions [2] were supported.

[1]: https://github.com/oss-review-toolkit/ort/blob/ef8b61c/website/docs/configuration/ort-yml.md#resolving-vulnerabilities
[2]: https://github.com/oss-review-toolkit/ort/blob/ef8b61c/website/docs/configuration/ort-yml.md#resolution-basics

Signed-off-by: Thomas Steenbergen <[email protected]>
Improve the filtering of issue severity levels to provide a clearer
and more effective user experience.

Signed-off-by: Thomas Steenbergen <[email protected]>
Re-apply parts 3106879 which was reverted for unclear
reasons.

Signed-off-by: Thomas Steenbergen <[email protected]>
…rence

Implement score and vector following their addition to
VulnerabilityReference.kt in 60ef7c9.

Signed-off-by: Thomas Steenbergen <[email protected]>
In preparation for an upcoming change to show all references for
each vulnerability.

Signed-off-by: Thomas Steenbergen <[email protected]>
Show all reference information to facilitate the validation
of vulnerabilities, enhancing user understanding.

Signed-off-by: Thomas Steenbergen <[email protected]>
Ensure the `pageSize` is passed to pagination, allowing
the table to adhere to the `pageSize` set in the initial state.

Signed-off-by: Thomas Steenbergen <[email protected]>
Evaluator allows policy rule violations without a package see [1],
however this case was not supported in the data mapping from
`WebAppRuleViolation` to Ant Design table row data.

Fixes #9880.

[1]: https://github.com/oss-review-toolkit/ort/blob/cc6f09d/model/src/main/kotlin/RuleViolation.kt#L33

Signed-off-by: Thomas Steenbergen <[email protected]>
@tsteenbe tsteenbe force-pushed the webapp-fixes-updates-part-2 branch from dbc833a to 618ab4c Compare February 6, 2025 00:42
@tsteenbe
Copy link
Member Author

tsteenbe commented Feb 6, 2025

@MarcelBochtler I dropped the severe threshold commits and fix the hint label issue - was a copy-paste mistake in fix(web-app-template): Enhance rule severity icons and fix(web-app-template): Enhance issue severity filtering which I now have updated.

Once this PR is merged I will fill another PR with the severe threshold commits e.g. feat(web-app-template): Implement severe thresholds in the model and feat(web-app-template): Adhere to severe thresholds .

Copy link
Member

@MarcelBochtler MarcelBochtler left a comment

Choose a reason for hiding this comment

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

LGTM.

@MarcelBochtler MarcelBochtler merged commit 1df67af into main Feb 6, 2025
26 checks passed
@MarcelBochtler MarcelBochtler deleted the webapp-fixes-updates-part-2 branch February 6, 2025 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants