Part 2: Change report flow to POST request for Setup Scan and Saving report #3182

Rieven · 2024-07-03T14:36:49Z

Changes

This is part 2 of changing report requests to POST
An extension of : Change report flow to POST requests #3174
handles post request from choosing report types to the configuration page
handles post request from the setup scan page to the generate report
Sometimes from report type selection is goes directly to the generate report when all required plugins are enabled

Issue link

#3279

Closes #3279

Code Checklist

All the commits in this PR are properly PGP-signed and verified.
This PR only contains functionality relevant to the issue.
I have written unit tests for the changes or fixes I made.
I have checked the documentation and made changes where necessary.
I have performed a self-review of my code and refactored it to the best of my abilities.

Tickets have been created for newly discovered issues.
For any non-trivial functionality, I have added integration and/or end-to-end tests.
I have informed others of any required .env changes files if required and changed the .env-dist accordingly.
I have included comments in the code to elaborate on what is not self-evident from the code itself, including references to issues and discussions online, or implicit behavior of an interface.

Checklist for code reviewers:

Copy-paste the checklist from the docs/source/templates folder into your comment.

Checklist for QA:

Copy-paste the checklist from the docs/source/templates folder into your comment.

Rieven · 2024-07-03T14:45:01Z

rocky/reports/views/generate_report.py

@@ -115,109 +108,11 @@ def get_context_data(self, **kwargs):
        return context


-class SaveGenerateReportMixin(ReportPluginView):


Moved to mixins.py

Rieven · 2024-07-03T14:45:30Z

rocky/reports/views/aggregate_report.py

@@ -134,72 +131,6 @@ def get_context_data(self, **kwargs):
        return context


-class SaveAggregateReportMixin(ReportPluginView):


Moved to mixins.py

This is aggregate specific code and not a general reporting mixin. What is the reason to moving it to a mixins file instead of keeping it in the aggregate specific file? It doesn't seem a logical place to me.

If you hadn't unnecessarily move this there also wouldn't have been merge conflicts here.

I have move this code to mixins to keep the flow of the report wizard in the code more readable. As it follows a path top down.

I can also put it at the bottom of the file.

…vws/nl-kat-coordination into feat/report-flow-post-request-part-2

…t/report-flow-post-request-part-2

…vws/nl-kat-coordination into feat/report-flow-post-request-part-2

rocky/reports/report_types/aggregate_organisation_report/report.py

underdarknl · 2024-07-24T13:26:36Z

rocky/reports/templates/partials/report_types_selection.html

-              class="inline layout-wide">
-            {% csrf_token %}
-            {% include "forms/report_form_fields.html" %}
+            <form novalidate


Is this novalidate needed?

This validation has the HTML styling, we use our own Django validations with messages.

I mean also Manon styling for showing form errors

If validation can already be done by the browser / before submitting the form in my opinion that it is better user experience then having it done after the form is submitted. If there are styling issues with the validation errors we should fix those syling issues instead of not doing the validation.

…vws/nl-kat-coordination into feat/report-flow-post-request-part-2

TwistMeister · 2024-08-02T11:06:54Z

Checklist for QA:

I have checked out this branch, and successfully ran a fresh make reset.
I confirmed that there are no unintended functional regressions in this branch:
- I have managed to pass the onboarding flow
- Objects and Findings are created properly
- Tasks are created and completed properly
I confirmed that the PR's advertised feature or hotfix works as intended.
I checked the logs for errors and/or warnings and made issues where necessary

What works:

I believe I was able to verify that the mentioned API calls were done as POST requests, for either "select oois", "select report types" and "setup scan".

What doesn't work:

n.a.

Bug or feature?:

n.a.

…vws/nl-kat-coordination into feat/report-flow-post-request-part-2

changes for post request for the rest of the report flow

2fd2f11

Rieven commented Jul 3, 2024

View reviewed changes

Merge part 1

774bf6b

Rieven mentioned this pull request Jul 4, 2024

Change report flow to POST requests #3174

Merged

9 tasks

Rieven added 5 commits July 8, 2024 15:04

Merge branch 'feat/report-flow-post-request-part-1' of github.com:min…

7d31635

…vws/nl-kat-coordination into feat/report-flow-post-request-part-2

Fix get to post

3604a1b

Merge branch 'feat/report-flow-post-request-part-1' of github.com:min…

489ad38

…vws/nl-kat-coordination into feat/report-flow-post-request-part-2

Merge branch 'feat/report-flow-post-request-part-1' of github.com:min…

d1b537e

…vws/nl-kat-coordination into feat/report-flow-post-request-part-2

fiux test

39d779f

Rieven self-assigned this Jul 9, 2024

Rieven added 7 commits July 18, 2024 10:18

Merge branch 'feat/report-flow-post-request-part-1' of github.com:min…

da70bea

…vws/nl-kat-coordination into feat/report-flow-post-request-part-2

Merge branch 'feat/report-flow-post-request-part-1' of github.com:min…

69a15e8

…vws/nl-kat-coordination into feat/report-flow-post-request-part-2

Add organization code

b264cfb

Merge branch 'main' of github.com:minvws/nl-kat-coordination into fea…

6f3dfd8

…t/report-flow-post-request-part-2

Merge branch 'feat/report-flow-post-request-part-1' of github.com:min…

8dae0d4

…vws/nl-kat-coordination into feat/report-flow-post-request-part-2

fix lang

654a311

Add test

d59c4da

Rieven commented Jul 23, 2024

View reviewed changes

rocky/reports/report_types/aggregate_organisation_report/report.py Show resolved Hide resolved

Rieven marked this pull request as ready for review July 23, 2024 10:23

Rieven requested a review from a team as a code owner July 23, 2024 10:23

fix tests

4bcb8af

underdarknl reviewed Jul 24, 2024

View reviewed changes

Merge branch 'feat/report-flow-post-request-part-1' of github.com:min…

54fbe65

…vws/nl-kat-coordination into feat/report-flow-post-request-part-2

Rieven added 4 commits August 5, 2024 11:28

Merge branch 'feat/report-flow-post-request-part-1' of github.com:min…

958119e

…vws/nl-kat-coordination into feat/report-flow-post-request-part-2

Fix post request for export page

888db7c

Change change selection button and back button to post requests

d253455

fix lang and tests

3719588

Rieven merged commit b3e1041 into feat/report-flow-post-request-part-1 Aug 5, 2024
10 checks passed

Rieven deleted the feat/report-flow-post-request-part-2 branch August 5, 2024 15:21

dekkers linked an issue Aug 6, 2024 that may be closed by this pull request

Change report type selection and generate report requests to POST requests #3279

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Part 2: Change report flow to POST request for Setup Scan and Saving report #3182

Part 2: Change report flow to POST request for Setup Scan and Saving report #3182

Rieven commented Jul 3, 2024 •

edited

Loading

Rieven Jul 3, 2024

Rieven Jul 3, 2024

dekkers Aug 5, 2024 •

edited

Loading

dekkers Aug 5, 2024

Rieven Aug 5, 2024

Rieven Aug 5, 2024 •

edited

Loading

underdarknl Jul 24, 2024

Rieven Jul 24, 2024 •

edited

Loading

Rieven Jul 30, 2024

dekkers Aug 5, 2024

TwistMeister commented Aug 2, 2024 •

edited

Loading

		@@ -115,109 +108,11 @@ def get_context_data(self, **kwargs):
		return context


		class SaveGenerateReportMixin(ReportPluginView):

		@@ -134,72 +131,6 @@ def get_context_data(self, **kwargs):
		return context


		class SaveAggregateReportMixin(ReportPluginView):

Part 2: Change report flow to POST request for Setup Scan and Saving report #3182

Part 2: Change report flow to POST request for Setup Scan and Saving report #3182

Conversation

Rieven commented Jul 3, 2024 • edited Loading

Changes

Issue link

Code Checklist

Checklist for code reviewers:

Checklist for QA:

Rieven Jul 3, 2024

Choose a reason for hiding this comment

Rieven Jul 3, 2024

Choose a reason for hiding this comment

dekkers Aug 5, 2024 • edited Loading

Choose a reason for hiding this comment

dekkers Aug 5, 2024

Choose a reason for hiding this comment

Rieven Aug 5, 2024

Choose a reason for hiding this comment

Rieven Aug 5, 2024 • edited Loading

Choose a reason for hiding this comment

underdarknl Jul 24, 2024

Choose a reason for hiding this comment

Rieven Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

Rieven Jul 30, 2024

Choose a reason for hiding this comment

dekkers Aug 5, 2024

Choose a reason for hiding this comment

TwistMeister commented Aug 2, 2024 • edited Loading

Checklist for QA:

What works:

What doesn't work:

Bug or feature?:

Rieven commented Jul 3, 2024 •

edited

Loading

dekkers Aug 5, 2024 •

edited

Loading

Rieven Aug 5, 2024 •

edited

Loading

Rieven Jul 24, 2024 •

edited

Loading

TwistMeister commented Aug 2, 2024 •

edited

Loading