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.

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

Remove unused date range params from partners csv export link #4996

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

coalest
Copy link
Collaborator

@coalest coalest commented Feb 7, 2025

Doesn't resolve any issue.

Description

While working on performance of the partners csv export, I noticed the request applies some default date range parameters to the export csv requests.

Let's remove those date params because we aren't actually doing anything will them (they aren't permitted params in the controller) so as not to confuse users in to thinking the export is only for the time range supplied.

Type of change

I don't know how to categorize this. It is user-facing in that on hover the link looks different, but it doesn't effect the CSV export in any way.

How Has This Been Tested?

Test suite

Screenshot

These are screenshots of me hovering over the export partner agencies button. Note the link in the lower lefthand corner.

Before:

Screenshot from 2025-02-07 14-39-46

After:

Screenshot from 2025-02-07 14-40-04

@dorner
Copy link
Collaborator

dorner commented Feb 7, 2025

@coalest there's a spec that's failing now - I thought it was flaky but I tried 3 times and it keeps failing.

@coalest
Copy link
Collaborator Author

coalest commented Feb 8, 2025

@dorner Whoops, glad we have that system spec. I had removed the status filter as well as the date range filter, when I just wanted to remove the date range filter.

Should be good now.

@coalest
Copy link
Collaborator Author

coalest commented Feb 8, 2025

Should we communicate somewhere to users (either on the button, or csv file name, or inside the csv file), that clicking export will only export partners with that status?

@cielf
Copy link
Collaborator

cielf commented Feb 8, 2025

All the exports work in a "take the filters into account" way -- at least they're supposed to -- so I don't think we need to call it out specifically in the UI here.

Changing the insides of the csv could mess with the users' external work that the csv feeds into, so I'd avoid that.

Changing how the exports are named according to the filter has potential, for ease of working with the files later -- if we did, we'd want to apply that idea across the board on all the exports for consistent UX. Let me ponder -- I think we could end up with some spectacularly long file names on distribution.

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.

3 participants