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

FilterChooser and GridFilter should modify date values for certain filter operators by default #3338

Open
saba-mo opened this issue Apr 27, 2023 · 4 comments · May be fixed by #3362
Open

FilterChooser and GridFilter should modify date values for certain filter operators by default #3338

saba-mo opened this issue Apr 27, 2023 · 4 comments · May be fixed by #3362
Assignees

Comments

@saba-mo
Copy link
Contributor

saba-mo commented Apr 27, 2023

The current default behavior when filtering a field with fieldType of date is that the time is set to the beginning of the day. So if the user filters by greater than (after) a date, they still see that day's rows included. The default behavior for operators > and <= should be that the time is set to end of day.

We currently do this in the FilterChooserFieldSpec's valueParser in Admin/Activity's tracking and client error tabs. The tracking tab also has column filters enabled, so if the user were to filter by > a timestamp, for example, they would see different results depending on whether they used the column filter or filter chooser.

@TomTirapani
Copy link
Member

Adding the idea from the above pull request to this ticket:

A dev should be able to leave the field as Date in the Grid and Store, but filter is as a LocalDate by setting FilterChooserModel.fieldSpecs.fieldType to localDate and it should work. We wouldn't need the valueParser or valueRenderer stuff as LocalDate already handles the text input & > & <= operators properly.

Somewhere in the FilterChooser / GridFilter code it should be able to:

  • (a) detect that the Record has Dates but you’ve specified LocalDate in the FilterFieldSpec
  • (b) confirm that is a valid pair it is able to transform, and then
  • (c) convert to a LocalDate by stripping the time before building its filters.

Step (b) should likely throw if you attempt to convert any other field type to any other.

@saba-mo
Copy link
Contributor Author

saba-mo commented May 10, 2023

Thank you, Tom. I'm looking into it.

@saba-mo
Copy link
Contributor Author

saba-mo commented May 17, 2023

@lbwexler and @amcclain, I'm moving the conversation here to the original ticket. Tom and I discussed the approach suggested in #3339 and the issue we ran into is that FieldFilter takes its value directly off the record before filtering and assumes the fieldType matches that of the field in the store. FieldFilter doesn't currently have access to the filter's fieldType to detect a difference, and we thought implementing that might be a bigger change than we were looking to make.

So we are now exploring the idea of detecting if the developer has indicated localDate on the fieldSpec while the store's field's fieldType is date, and then providing the valueParser logic of setting the time to end of day for > and <=. Not sure yet how that would look for GridFilter, which does not currently take a valueParser.

@amcclain
Copy link
Member

Thanks for continuing to think through this - important to take the time to get it right.

I thought of another feature we have considered in the past that might be relevant - adding the ability to filter date-based columns with relative tokens such as "today" or "this month" or "YTD". I'm imagining that there could be custom options in both filtering UIs to support such tokens. We've been asked about support for this multiple times from clients, and you can see how it would be useful for a saved gridView or dashboard that wanted to leverage a saved filter to show an always updated list of "today's trades" or "recent exceptions", etc.

I mention this NOT because I think we should link this ticket to that and do both things right away, but because it might be a good future use case to keep in mind when we think about the overall filtering APIs and how to give them some more type-specific behaviors and options. Just wanted to get the idea out there while you're looking closely at this - ideally any changes we make for this ticket might get us another step closer to the relative date filtering concept.

@saba-mo saba-mo linked a pull request May 19, 2023 that will close this issue
6 tasks
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 a pull request may close this issue.

3 participants