Skip to content

Conversation

bellitabellota
Copy link
Contributor

Resolves #5330

Description

I updated the csv export to match the view as described in the linked issue.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

The donation_sites_requests_spec.rb already contained a test that checks if the csv includes active as well as inactive donation sites if the donation_sites_path is called with the required arguments:

image image

The csv export did not match the filtered view because include_inactive_donation_sites was not passed in as argument. Thus this was not captured by the request tests.

Thus I used a system test to test the updated behavior as I could not think of another way to capture the corrected functionality.

Screenshots

wait_for_download
@csv_content = File.read(download)

expect(@csv_content).to have_content(active_donation_site.name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd prefer hardcoding these names for my own peace of mind. :)

expect(page.find(:xpath, "//table/tbody/tr[3]/td[1]")).to have_content(@third.name)
end

context "and exports a csv with include inactive donation sites selected" do
Copy link
Collaborator

Choose a reason for hiding this comment

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

Contexts usually start with when, not and, and describe a condition, not an action.

@bellitabellota
Copy link
Contributor Author

@dorner thank you for your feedback.

I updated my test in order to implement the suggested changes.

Please let me know if this is now as it should be.

@dorner
Copy link
Collaborator

dorner commented Sep 19, 2025

Passes technical review. Over to @awwaiid for functional.

@dorner dorner requested a review from awwaiid September 19, 2025 19:38
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.

donation site filtering should work on export as well
2 participants