Add the option to download a invoice

[lodewiges]

Fixes #843
fix authorisation issues and download button not working

Summary by CodeRabbit

• New Features

  • Added a Download column and per-invoice PDF download links in the invoices table; filenames are auto-suggested.
  • Supports token-based access for shared invoice PDF URLs.

• Bug Fixes / Security

  • Download actions are gated by the same permission checks as other invoice actions to ensure proper authorization.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds a per-invoice PDF download link in the invoices index view, implements token-aware PDF rendering in the InvoicesController, and introduces a new download? policy method to authorize downloads.

Changes

Cohort / File(s)	Summary
Invoice index view app/views/invoices/index.html.erb	Adds a "Download" column header and a per-row "Download" link to invoice_path(invoice, format: :pdf), shown only when authorized via download?. Filename uses the invoice human_id.
Controller: download & PDF access app/controllers/invoices_controller.rb	Refactors show to detect token-based (non-integer) IDs via a new integer_id?(id) helper, centralizes PDF rendering in render_invoice_pdf(token_based_access), and conditionally enforces download? authorization for integer IDs.
Authorization policy app/policies/invoice_policy.rb	Adds a public download? method that permits treasurer users to download invoices (mirrors existing send-invoice logic).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Areas needing attention:
  • invoices_controller.rb: token vs integer ID detection, authorization enforcement path, and PDF render options.
  • invoice_policy.rb: ensure download? semantics match access model and tests cover it.
  • index.html.erb: check view helper usage, filename escaping, and i18n/label consistency.

Poem

🐰
I nibble at code and hop to find,
A PDF trail for humankind,
A token, a click, a gentle thump,
The invoice arrives — a paper jump,
Hooray, I stash your file behind my mind!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check	❓ Inconclusive	The pull request description is minimal and lacks detail. It only references a GitHub issue and a brief note about authorization, but does not follow the provided template structure.	Expand the description to follow the template: include a Summary section describing UI changes with screenshots, and clarify which parts of issue #843 were addressed and which remain.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Add the option to download a invoice' accurately reflects the main change: adding invoice download functionality to the invoice list view.
Linked Issues check	✅ Passed	The PR implements the core requirement from issue #843: adding a download button to the invoice list that generates and downloads a PDF to the user's computer.
Out of Scope Changes check	✅ Passed	All changes are directly related to implementing the invoice download feature. The view addition, controller modifications for PDF rendering with token-based access, and policy authorization are all scoped to the requirement.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feature/add-invoice-download-button

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1bf837d and 1af980c.

📒 Files selected for processing (1)

• app/controllers/invoices_controller.rb (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• app/controllers/invoices_controller.rb

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Build

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.62%. Comparing base (7717565) to head (1af980c).
⚠️ Report is 17 commits behind head on staging.

Files with missing lines	Patch %	Lines
app/controllers/invoices_controller.rb	77.77%	2 Missing ⚠️
app/policies/invoice_policy.rb	50.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           staging    #1156      +/-   ##
===========================================
+ Coverage    77.22%   77.62%   +0.40%     
===========================================
  Files           54       54              
  Lines         1348     1350       +2     
===========================================
+ Hits          1041     1048       +7     
+ Misses         307      302       -5     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.