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

Add: Added a dedicated view for compliance audit reports #3952

Draft
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

a-h-abdelsalam
Copy link
Contributor

@a-h-abdelsalam a-h-abdelsalam commented Jan 25, 2024

What

  • Added a dedicated view for compliance audit reports. Compliance reports are now listed under Resilience tab and do not appear anymore under Scans tab. A dedicated view for a compliance report shows compliance of results.
  • Delta compliance reports can highlight changes in compliance.
  • Report links in Audits page now link to audit reports view.

Why

Currently GSA doesn't distinguish between different types of reports (vulnerability, compliance audit, etc.). Therefore there is no dedicated view yet, causing all filters to be accessible in different reports.

References

GEA-397
requires greenbone/gvmd#2125
requires greenbone/gsad#164

  • Tests

Copy link

Conventional Commits Report

Type Number
Added 1

🚀 Conventional commits found.

Copy link

codecov bot commented Jan 25, 2024

Codecov Report

Attention: Patch coverage is 48.11321% with 605 lines in your changes are missing coverage. Please review.

Project coverage is 59.64%. Comparing base (f358c4c) to head (d7c0f15).
Report is 112 commits behind head on main.

❗ Current head d7c0f15 differs from pull request most recent head c039b3e. Consider uploading reports for the commit c039b3e to get more accurate results

Files Patch % Lines
src/web/pages/reports/auditdetailspage.js 5.40% 222 Missing and 23 partials ⚠️
src/web/pages/reports/auditdeltadetailspage.js 5.38% 193 Missing and 18 partials ⚠️
src/web/pages/reports/auditdetailscontent.js 62.71% 22 Missing ⚠️
src/web/pages/reports/details/resultstab.js 8.33% 17 Missing and 5 partials ⚠️
src/web/pages/reports/auditreportslistpage.js 54.54% 18 Missing and 2 partials ⚠️
...c/web/pages/reports/details/auditthresholdpanel.js 56.75% 13 Missing and 3 partials ⚠️
src/gmp/commands/auditreports.js 62.50% 12 Missing ⚠️
src/web/pages/reports/auditreportrow.js 72.41% 8 Missing ⚠️
src/gmp/models/report/auditreport.js 82.85% 5 Missing and 1 partial ⚠️
src/web/store/entities/report/actions.js 94.78% 6 Missing ⚠️
... and 17 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3952      +/-   ##
==========================================
- Coverage   59.97%   59.64%   -0.33%     
==========================================
  Files        1025     1043      +18     
  Lines       25238    26255    +1017     
  Branches     7468     7806     +338     
==========================================
+ Hits        15136    15660     +524     
- Misses       9158     9608     +450     
- Partials      944      987      +43     

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

@a-h-abdelsalam a-h-abdelsalam force-pushed the add-dedicated-view-for-compliance-reports branch from dcd49bf to 0fa51d0 Compare February 2, 2024 15:30
@a-h-abdelsalam a-h-abdelsalam force-pushed the add-dedicated-view-for-compliance-reports branch from eb4a4af to b717560 Compare February 13, 2024 13:28
@a-h-abdelsalam a-h-abdelsalam marked this pull request as ready for review February 14, 2024 15:45
@a-h-abdelsalam a-h-abdelsalam requested a review from a team as a code owner February 14, 2024 15:45
@a-h-abdelsalam a-h-abdelsalam force-pushed the add-dedicated-view-for-compliance-reports branch from f2a6c69 to 811e5a9 Compare February 20, 2024 10:52
Copy link
Member

@bjoernricks bjoernricks left a comment

Choose a reason for hiding this comment

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

First of all this PR looks good. My comments are mostly only style and convention related.

Some rules that should be considered:

  • In JS we agreed on camelCase usage for functions, classes, variables and properties finally. Because I have a Python background and wrote most of the initial code we still have a lot of snake_case naming which we never get rid of. All new additions should use camelCase when possible. Personally I am not sure if this fits here with the report still using snake_case and the strong coupling between the audit, tasks, scan configs, policies and the two different reports.
  • We agreed on using absolute imports without the .js suffix some while ago. This might not be consistent on all places.
  • New react components should be function components if possible. React class components are still supported and will be supported a long time but are considered legacy. And because you can't use hooks inside class components it is better to write only function components for new code.

src/gmp/commands/__tests__/auditreport.js Outdated Show resolved Hide resolved
src/gmp/models/report/auditreport.js Outdated Show resolved Hide resolved
src/gmp/models/report/auditreport.js Outdated Show resolved Hide resolved
src/gmp/models/report/auditreport.js Show resolved Hide resolved
src/gmp/models/report/auditreport.js Show resolved Hide resolved
src/web/pages/tasks/row.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditdetailscontent.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditdetailscontent.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditdetailscontent.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditdetailscontent.js Outdated Show resolved Hide resolved
@a-h-abdelsalam
Copy link
Contributor Author

Thank you for your review @bjoernricks. I applied the review suggestions where possible and also rewrote auditdeltadetailspage, auditdetailspage, auditreportlistpage, compliancelevelsgroup into functional components.

@a-h-abdelsalam a-h-abdelsalam force-pushed the add-dedicated-view-for-compliance-reports branch from 83a22c9 to 8a16fa8 Compare March 7, 2024 14:14
src/web/components/powerfilter/compliancelevelsgroup.js Outdated Show resolved Hide resolved
src/web/components/powerfilter/compliancelevelsgroup.js Outdated Show resolved Hide resolved
src/web/components/powerfilter/compliancelevelsgroup.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditreportslistpage.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditreportslistpage.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditreportslistpage.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditdeltadetailspage.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditdeltadetailspage.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditdeltadetailspage.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditdetailspage.js Outdated Show resolved Hide resolved
@a-h-abdelsalam a-h-abdelsalam marked this pull request as draft March 12, 2024 14:58
@a-h-abdelsalam a-h-abdelsalam force-pushed the add-dedicated-view-for-compliance-reports branch from a4a9c06 to b474908 Compare April 4, 2024 16:18
@a-h-abdelsalam a-h-abdelsalam marked this pull request as ready for review April 4, 2024 16:53
@a-h-abdelsalam a-h-abdelsalam marked this pull request as draft April 5, 2024 07:58
- Compliance reports are now listed under Resilience tab and do
  not appear anymore under Scans tab.
- A dedicated view for a compliance report now shows compliance of
  results.
- Delta compliance reports can now highlight changes in compliance.
- Audit task reports should now open in audit reports detail page.
- Differentiate between report actions and audit report actions.
- Differentiate between delta report and delta audit report reducers.
- Added compliance to default filters for audit reports and results.
@a-h-abdelsalam a-h-abdelsalam force-pushed the add-dedicated-view-for-compliance-reports branch from b474908 to 349acd0 Compare May 13, 2024 12:28
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.

None yet

3 participants