Skip to content

Download empty report Na 31-2 Bijlage 1 #1830

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

Merged
merged 10 commits into from
Jul 28, 2025
Merged

Conversation

marlonbaeten
Copy link
Contributor

@marlonbaeten marlonbaeten commented Jul 22, 2025

Fixes #1824

Changes

  • Adds a section to the election detail page "Lege modellen voor deze verkiezing"
  • Adds a download button for "Na 31-2 Bijlage 1" (one PDF per polling station in a ZIP file)
  • Refactor of the pdf_gen module to better handle multiple models and better async support
  • Added a zip module to reuse the boilerplate code for creating a ZIP file response
  • Created an data input struct for "Na 31-2 Bijlage 1"

Tests

  • A rust test for the api endpoint that validates a correct zip with non-empty pdf files is returned
  • A playwright test is added to test the download button / action
  • Note that most missing test coverage that is reported originates from code that was just moved by this PR (like download.ts) and that the code coverage does not include the playwright test.

Performance

  • During development downloading a ZIP with 100 PDF files takes around 4000 ms (using external typst)
  • In a development build with embed-typst the same ZIP takes about 1700 ms te generate
  • In a production build with embed-typst it takes about 1600 ms
  • Downloading a ZIP with 3 PDF's takes about 60 ms

How to test

  • Run the application
  • Login as a coordinator
  • Navigate to /elections/1
  • Click the link at the bottom of the page to download a ZIP
  • Validate that the zip contains valid PDF files with the correct contents for that polling station

Screenshot

image

@marlonbaeten marlonbaeten self-assigned this Jul 22, 2025
@marlonbaeten marlonbaeten added the backend Issues or pull requests that relate to the backend label Jul 22, 2025
@marlonbaeten marlonbaeten requested a review from a team as a code owner July 22, 2025 19:25
@marlonbaeten marlonbaeten added frontend Issues or pull requests that relate to the frontend rust Pull requests that update Rust code labels Jul 22, 2025
Copy link

codecov bot commented Jul 22, 2025

Codecov Report

❌ Patch coverage is 76.35468% with 48 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.57%. Comparing base (d5bf7a8) to head (89463a4).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...src/features/election_management/utils/download.ts 10.00% 27 Missing ⚠️
backend/src/pdf_gen/embedded/mod.rs 81.08% 7 Missing ⚠️
backend/src/document/api.rs 89.13% 4 Missing and 1 partial ⚠️
backend/src/pdf_gen/embedded/world.rs 50.00% 3 Missing and 1 partial ⚠️
backend/src/bin/gen-test-election.rs 0.00% 2 Missing ⚠️
...lection_management/components/ElectionHomePage.tsx 90.90% 2 Missing ⚠️
backend/src/pdf_gen/models/mod.rs 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1830      +/-   ##
==========================================
- Coverage   92.59%   92.57%   -0.03%     
==========================================
  Files         364      367       +3     
  Lines       29254    29363     +109     
  Branches     2215     2217       +2     
==========================================
+ Hits        27087    27182      +95     
- Misses       2018     2030      +12     
- Partials      149      151       +2     

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

@praseodym
Copy link
Contributor

I think we should be able to stream the ZIP file so that we don't have to keep all PDF files in memory, but that would require structuring the code differently so I'm not sure if it's worth it.

@marlonbaeten
Copy link
Contributor Author

I think we should be able to stream the ZIP file so that we don't have to keep all PDF files in memory, but that would require structuring the code differently so I'm not sure if it's worth it.

I'd be happy to implement this in a new PR 👍🏻

praseodym
praseodym previously approved these changes Jul 24, 2025
@praseodym
Copy link
Contributor

@marlonbaeten this branch has conflicts

@marlonbaeten marlonbaeten added this pull request to the merge queue Jul 28, 2025
Merged via the queue into main with commit ed40885 Jul 28, 2025
16 checks passed
@marlonbaeten marlonbaeten deleted the download-na-31-2-bijlage-1 branch July 28, 2025 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Issues or pull requests that relate to the backend frontend Issues or pull requests that relate to the frontend rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Empty report Na 31-2 Bijlage 1 (CSO)
3 participants