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.

Sign up for GitHub

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

Jump to bottom

Quick fix for PDF table overflow #2562

Merged
merged 3 commits into from
Feb 27, 2024
Merged

Quick fix for PDF table overflow #2562

merged 3 commits into from
Feb 27, 2024

Conversation

madelondohmen
Copy link
Contributor

@madelondohmen madelondohmen commented Feb 26, 2024 •
edited
Loading

The problem

Tables in the vulnerability section of the new reporting module flow outside of the PDF-margin, making them unreadable. This happens in both the aggregate report and the normal generate report.

Another thing that is not correct is the font size of the text in the table. Some text appears larger than others.

Steps to reproduce

  1. Have vulnerabilities
  2. Create a report and select report type "Vulnerability Report".
  3. Press the "Download Report" button (or "Export" -> "Download PDF" for aggregated report flow)
  4. You will now see that the text in the table does not fit and that the text in the table has a different size.

Demo of the problem occuring

afbeelding

Possible solutions

  • Change the HTML of the table
  • Make the PDF page landscape
  • Decide what happens when the table overflows (for example, cutting of the sentence)
  • Make a few quick changes for the new release and fix the rest later

Solution

We've chosen for the last solution, which involves the following:

  • Making the font size the same for all the text in the table
  • Changing the finding.primary_key to finding.human_readable

It's not a complete solution, but it's better than before. We should concider the other solutions as well, because some parts of the table are still overflowing.

Demo of the solution

afbeelding

Issues

Closes

Closes none, but is a part of issue #2493

Checks

Code Checklist

  • All the commits in this PR are properly PGP-signed and verified.
  • This PR only contains functionality relevant to the issue; tickets have been created for newly discovered issues.
  • I have written unit tests for the changes or fixes I made.
  • For any non-trivial functionality, I have added integration and/or end-to-end tests.
  • I have performed a self-review of my code and refactored it to the best of my abilities.

Communication

  • I have informed others of any required .env changes files if required and changed the .env-dist accordingly.
  • I have made corresponding changes to the documentation, if necessary.
  • 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.

@madelondohmen madelondohmen self-assigned this Feb 26, 2024
@madelondohmen madelondohmen requested a review from a team as a code owner February 26, 2024 17:37
@madelondohmen madelondohmen added bug Something isn't working report labels Feb 26, 2024
underdarknl
underdarknl approved these changes Feb 27, 2024
View reviewed changes
Copy link
Contributor

@underdarknl underdarknl left a comment

Choose a reason for hiding this comment

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

looks good to me

@stephanie0x00
Copy link
Contributor

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.

What works:

Can confirm the improvement in the table overflow. The table overflow is still occurring but it is a bit better for now.

What doesn't work:

n/a

Bug or feature?:

n/a

@underdarknl underdarknl merged commit c3d7f69 into main Feb 27, 2024
10 checks passed
@underdarknl underdarknl deleted the fix/pdf-table-margin branch February 27, 2024 12:25
jpbruinsslot added a commit that referenced this pull request Mar 5, 2024
@jpbruinsslot
Merge branch 'main' into feature/mula/flexible-scheduling
85777ea 
* main: (85 commits)
  Fix wrong solving of merge conflict (#2585)
  Add metrics collection for scheduler using prometheus (#2468)
  Hotfix for where_in queries for abstract types (#2577)
  Update django (#2587)
  Fix octopoes typing (#2555)
  Create findings report (#2393)
  Raise exception if boefje input OOI has been deleted (#2573)
  Set a timeout on hanging test ssl container (#2560)
  Feature/efficient reporting (#2516)
  Updated findings database. Removed old findings, added Impact, Source… (#2569)
  add unit test for web report (#2528)
  Add pool size config and logs (#2541)
  Quick fix for PDF table overflow (#2562)
  Fix/2527 octopoes unicode (#2558)
  Add return typing to report test fixtures (#2557)
  Sort vulnerabilities in vulnerability report (#2378)
  Disable ruff split-on-trailing-comma and update ruff (#2544)
  Select all oois triggers toggle all (#2536)
  Remove unnecessary toplevel dependencies (#2554)
  Make valid time required parameter in the octopoes API (#2543)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@underdarknl underdarknl underdarknl approved these changes

Assignees

@madelondohmen madelondohmen

Labels
bug Something isn't working report
Projects
KAT
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@madelondohmen @stephanie0x00 @underdarknl