✨ Bulk download analysis details #2142
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2142 +/- ##
==========================================
+ Coverage 39.20% 41.96% +2.76%
==========================================
Files 146 175 +29
Lines 4857 5635 +778
Branches 1164 1409 +245
==========================================
+ Hits 1904 2365 +461
- Misses 2939 3149 +210
- Partials 14 121 +107
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Shevijacobson
changed the title
Shevijacobson
force-pushed
the
bulk-download-reports
branch
from
c6b3ebf
to
8b0a431
Compare
| .post<Task[]>(`${TASKS}/multiple`, ids, { | ||
| headers: headers, | ||
| responseType: responseType, | ||
| }) |
There was a problem hiding this comment.
Is there a reasons this does not use the existing List endpoint with a filter?
Example:
GET /tasks?filter=id:(1|2|3)
Filter construction is done a lot in the dynamic analysis reporting parts of the ui.
There was a problem hiding this comment.
In light of @jortel feedback, I have updated the code to utilize the existing List endpoint with a filter, as suggested. Thank you for your input!
| const data = | ||
| selectedFormat === "yaml" | ||
| ? yaml.dump(tasks, { indent: 2 }) | ||
| : JSON.stringify(tasks, null, 2); |
There was a problem hiding this comment.
If the data is coming back in the requested "selectedFormat" why is it being formatted again?
There was a problem hiding this comment.
I realized that I was unnecessarily sending the selectedFormat parameter in the server request. Since the server function does not currently support returning data in that format, I’ve adjusted the implementation to handle JSON formatting on the UI side instead
|
|
||
| setIsDownloadModalOpen(false); | ||
| } catch (error) { | ||
| console.error("Error fetching tasks:", error); |
There was a problem hiding this comment.
The user should be alerted to an error with at least a pushNotification()
There was a problem hiding this comment.
Thank you for your valuable feedback. I have implemented the pushNotification() as you suggested to enhance user error alerts.
| <Modal | ||
| isOpen={isDownloadModalOpen} | ||
| variant="small" | ||
| title={t("actions.download", { what: "analysis details reports" })} | ||
| onClose={() => setIsDownloadModalOpen(false)} | ||
| > | ||
| <FormGroup label="Select Format" fieldId="format-select"> | ||
| <div> | ||
| <Button | ||
| variant={selectedFormat === "json" ? "primary" : "secondary"} | ||
| onClick={() => setSelectedFormat("json")} | ||
| > | ||
| {<CodeIcon />} JSON | ||
| </Button> | ||
| <Button | ||
| variant={selectedFormat === "yaml" ? "primary" : "secondary"} | ||
| onClick={() => setSelectedFormat("yaml")} | ||
| > | ||
| {<CodeIcon />} YAML | ||
| </Button> | ||
| </div> | ||
| <p>Selected Format: {selectedFormat}</p> | ||
| </FormGroup> | ||
| <Button variant="primary" onClick={handleDownload}> | ||
| {t("actions.download")} | ||
| </Button> | ||
| </Modal> |
There was a problem hiding this comment.
The look of the modal needs to be updated.
Have a look at the "Show dropdown modal": https://www.patternfly.org/components/modal#with-dropdown
Put the modal's action buttons in the actions parameter so they render in the correct place with all the good spacing.
Put the yaml vs json selection in a basic drop down: https://www.patternfly.org/components/forms/form-select#basic
For reference, this is what it looks like currently:
There was a problem hiding this comment.
Thank you for the feedback! I've updated the modal following your recommendations:
- The action buttons have been moved to the actions parameter to ensure proper alignment and spacing.
- A dropdown for selecting the format has been added, utilizing the basic dropdown from PatternFly as suggested.
I've attached a screenshot of the updated modal for your review.
Please let me know if there are any further adjustments you'd recommend. Thank you again for guiding me through this!
Signed-off-by: shevijacobson <[email protected]>
Signed-off-by: MiriSafra <[email protected]>
Signed-off-by: shevijacobson <[email protected]>
Signed-off-by: MiriSafra <[email protected]>
Signed-off-by: shevijacobson <[email protected]>
Signed-off-by: MiriSafra <[email protected]>
Signed-off-by: shevijacobson <[email protected]>
Signed-off-by: shevijacobson <[email protected]>
Signed-off-by: shevijacobson <[email protected]>
Shevijacobson
force-pushed
the
bulk-download-reports
branch
2 times, most recently
from
365ec09
to
b0a806a
Compare
Signed-off-by: MiriSafra <[email protected]>
✨ Update Download Modal
Signed-off-by: shevijacobson <[email protected]>
Signed-off-by: shevijacobson <[email protected]>
This PR introduces a new feature allowing users to download multiple analysis details at once.
Related to: #2143
Demo
Bulk Analysis Download Demo.