Add coverage for Followups and Mileages reports#6856
Add coverage for Followups and Mileages reports#6856compwron merged 1 commit intorubyforgood:mainfrom
Conversation
Followup report handle CSV exports with authorization and the Mileage report handles Reimbursement CSV exports. Both have placeholder specs only. Let's add coverage to ensure they work as expected and no bugs are introduced. The services that generate the report data have extensive coverage already, so keep this unit test focused on the request, I only tested for the format and the headers.
stefannibrasil
requested review from
FireLemons,
compwron and
elasticspoon
as code owners
There was a problem hiding this comment.
Pull request overview
Adds request-level test coverage for the Followup and Mileage CSV report exports (including authorization behavior), replacing prior placeholder controller specs.
Changes:
- Add request specs asserting CSV download behavior (headers + filename) for Followup and Mileage reports
- Add request specs asserting unauthorized users are redirected with the expected flash notice
- Remove pending/placeholder controller specs and add
frozen_string_literal: trueheaders to both controllers
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/requests/mileage_reports_spec.rb | Adds request coverage for mileage CSV export response + authorization redirect |
| spec/requests/followup_reports_spec.rb | Adds request coverage for followup CSV export response + authorization redirect |
| spec/controllers/mileage_reports_controller_spec.rb | Removes placeholder pending controller spec |
| spec/controllers/followup_reports_controller_spec.rb | Removes placeholder pending controller spec |
| app/controllers/mileage_reports_controller.rb | Adds frozen string literal header (no functional change) |
| app/controllers/followup_reports_controller.rb | Adds frozen string literal header (no functional change) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| get mileage_reports_path(format: :csv) | ||
|
|
||
| expect(response).to have_http_status(:success) | ||
| expect(response.header["Content-Type"]).to eq("text/csv") |
There was a problem hiding this comment.
Content-Type for send_data CSV responses typically includes a charset (e.g., text/csv; charset=utf-8). Expecting exact equality to "text/csv" is brittle and likely to fail; align with other request specs by using include("text/csv") or match(/text\/csv/) instead.
| expect(response.header["Content-Type"]).to eq("text/csv") | |
| expect(response.header["Content-Type"]).to include("text/csv") |
| get followup_reports_path(format: :csv) | ||
|
|
||
| expect(response).to have_http_status(:success) | ||
| expect(response.header["Content-Type"]).to eq("text/csv") |
There was a problem hiding this comment.
The response Content-Type for CSV downloads usually includes a charset suffix, so eq("text/csv") is too strict. Use a partial match (e.g., include("text/csv") / match(/text\/csv/)) to avoid brittle failures and match existing report request specs.
| expect(response.header["Content-Type"]).to eq("text/csv") | |
| expect(response.header["Content-Type"]).to include("text/csv") |
What github issue is this PR for, if any?
Close #6842
What changed, and why?
Followup report handle CSV exports with authorization and the Mileage report handles
Reimbursement CSV exports. Both have placeholder specs only.
Let's add coverage to ensure they work as expected and no bugs are introduced.
The services that generate the report data have extensive coverage already, so keep this unit test focused on the request, I only tested for the format and the headers.