Skip to content

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.

@cielf
Copy link
Collaborator

cielf commented Aug 24, 2025

@dorner -- If I understand correctly, the question that is on my plate does not prevent this going forward.

Copy link
Collaborator

@dorner dorner left a 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?

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.

4 participants