Add OpenScanHub results link to dashboard #516
Conversation
|
Preview: https://packit-dashboard-pr-516.surge.sh (deployed at Tue 25 Nov 2025, 08:08 UTC) |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 28s |
There was a problem hiding this comment.
@majamassarini are we meant to continue returning .js format from the DB for the scan results URL? Or could we update the DB to return only .html as we only really use it here still? Unless there are more consumers of the Packit API that I'm not aware of. Currently we get this from the OSH API:
"scan_results_url": "http://openscanhub.fedoraproject.org/task/80669/log/python-specfile-0.37.1.post1.dev11+gd2bb6c5-1.20251107112836473283.pr496.11.gd2bb6c5/scan-results.js?format=raw",
Otherwise, implementation LGTM. Only thing I'd suggest is putting the href replacement outside of the return to make it clearer we're changing the value, but it's small enough that it is mostly fine.
Smaller screens is manageable too. Note for future changes: it's getting to the point where we need to consider a visual restructure to make it work on smaller screens
Unrelated to the PR but related for OSH: As for OSH results, what value does this provide to the user? Looking at this it seems confusing when there are no findings for the scan as it only shows RAW metadata
https://openscanhub.fedoraproject.org/task/80669/log/python-specfile-0.37.1.post1.dev11+gd2bb6c5-1.20251107112836473283.pr496.11.gd2bb6c5/scan-results.html
| scan: "OpenScanHub task", | ||
| newFindings: "New findings", | ||
| scan: "OpenScanHub Task", | ||
| scanResults: "OpenScanHub Results", |
There was a problem hiding this comment.
Shall it be replaced with "Full scan results"? This is the term we use among OpenScanHub contributors, but may be there could be a different way to tell an average user about the difference between new findings and full scan results.
There was a problem hiding this comment.
Do you mean replacing the "OpenScanHub Results" header with "Full Scan Results"? Though removing the OpenScanHub keyword from the header might be a little inconsistent with the "OpenScanHub Task" header. Maybe we could do:
"OpenScanHub Results" ----> "Full Scan Results"
"OpenScanHub Task" ----> "Task"
I'm not sure if there are better headings that we could use.
There was a problem hiding this comment.
To be consistent with the keywords we can use the words "All Findings" and "New Findings". You can probably leave "OpenScanHub Task" as it is. Thanks!
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 29s |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 26s |
| timeProcessed: "Time Processed", | ||
| scan: "OpenScanHub Task", | ||
| scanResults: "OpenScanHub Results", | ||
| scanResults: "All Findings", |
There was a problem hiding this comment.
Shall scanResults be replaced with allFindings? But it is hidden from the user and may need to be changed at other places too, if that is the case you do not have to change it.
There was a problem hiding this comment.
Good point. scanResults is just the name of the column and isn't used in any other file, so I think it makes sense to name it allFindings for consistency.
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 28s |
|
I am not familiar with the code, but it looks good from a high level. Thanks! |
|
Shall I squash all commits into one before merging? Also could someone please double check that the code added in this commit is ok? I'm new to Typescript, so I wouldn't want to merge something that could potentially have weird side effects. |
Venefilyn
left a comment
Venefilyn
left a comment
There was a problem hiding this comment.
I'd suggest we use a function to modify the URL instead of modifying the whole dataset that we get
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 29s |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 29s |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 32s |
|
Thank you all for your help with this. If you have any comments and concerns, let me know until this Wednesday (26th). I plan to squash this to a single commit and merge on Thursday otherwise. |
I don't know the backend here enough but code itself looks fine. As the self-designated front-end expert for Packit it's fine. @siteshwar if it's good to you then I'm for merging |
Please go ahead with merging. Thanks! |
A new column has been added to the OSH dashboard, which contains the link to OSH results. Columns have been resized appropriately and column headers have been capitalized for consistency.
betulependule
force-pushed
the
osh_scan_results
branch
from
ad29861 to
860d832
Compare
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 33s |
|
Build succeeded (gate pipeline). ✔️ pre-commit SUCCESS in 1m 30s |
|
Pull request merge failed: Merge commits are not allowed on this repository. |
4 participants
Fixes #2786
RELEASE NOTES BEGIN
Packit Service dashboard now includes a link to OpenScanHub results.
RELEASE NOTES END