-
-
Notifications
You must be signed in to change notification settings - Fork 572
Remove unused date range params from partners csv export link #4996
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
base: main
Are you sure you want to change the base?
Remove unused date range params from partners csv export link #4996
Conversation
|
@coalest there's a spec that's failing now - I thought it was flaky but I tried 3 times and it keeps failing. |
|
@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. |
|
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? |
|
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. |
|
@dorner -- If I understand correctly, the question that is on my plate does not prevent this going forward. |
dorner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coalest some specs are failing now. Maybe these params aren't unused?
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:
After: