5
11
Fork
You've already forked tomato
1

Filter shift event occurrences #21

Manually merged
edwardloveall merged 3 commits from el-filter into main 2025年08月27日 15:45:40 +02:00

Pitch doc (Sorry Jason, I know you can't see this)

Organizations look at up to hundreds of shifts at once, and volunteer coordinators have to multitask. Information constantly comes at them from a myriad of places (email, help line, text, slack, etc). When they are looking up information to quickly answer a question and move on to the next thing, a long scrolling screen can feel overwhelming and distracting. Double so when they have to answer a volunteers time bound request and return to looking at the admin dashboard.

This adds a shift filter that lets users pick between many filters for those event occurrences. It's all implemented in ruby because there's no way to query event occurrences in the database (sigh maybe one
day).

The filter is also separated from the implicit filter of the "current week" a user sees in the list of shifts. This means it's possible to have exclusive filters like selecting a date outside of the visible week. The alternative would be to link all the filters together which
was more complex to implement and (I thought) confusing.

Right now, you can only filter for shifts with missing volunteers, but you can't filter for shifts that have volunteers. Not sure if we want that so I left it off.

Tech notes

The blank option of   exists instead of the default empty space because some Firefox based browsers show the empty option as very short for some reason. Adding the non-breaking space fixes it.

To clear filters, the form removes all params from the URL except for starts_at and then requests that page. This way, we can remove all the filters, but keep the date so the user doesn't have to get to a far off date again.

<video src="/attachments/0c77637e-e53b-4c34-85bc-53fcb90922aa" title="Waterfox" controls></video> [Pitch doc](https://docs.proton.me/doc?mode=open&volumeId=gt-aDv_z88sGWm34o9QiEnUphgY0Nc822eDjMLrSbemox-4O9oYyLlM5iDEbgLM0zwJ01771vvf5QaOgsiE7Bg%3D%3D&linkId=Gm-nPZjugd8DIfic7vHYinscGDMA2yqNUw6_RCWGln7xJ3S1VWNqpMpAeay-nd0I1aswNNFj52enljFp5u5ocg%3D%3D) (Sorry Jason, I know you can't see this) Organizations look at up to hundreds of shifts at once, and volunteer coordinators have to multitask. Information constantly comes at them from a myriad of places (email, help line, text, slack, etc). When they are looking up information to quickly answer a question and move on to the next thing, a long scrolling screen can feel overwhelming and distracting. Double so when they have to answer a volunteers time bound request and return to looking at the admin dashboard. This adds a shift filter that lets users pick between many filters for those event occurrences. It's all implemented in ruby because there's no way to query event occurrences in the database (_sigh_ maybe one day). The filter is also separated from the implicit filter of the "current week" a user sees in the list of shifts. This means it's possible to have exclusive filters like selecting a date outside of the visible week. The alternative would be to link all the filters together which was more complex to implement and (I thought) confusing. Right now, you can only filter for shifts with missing volunteers, but you can't filter for shifts that _have_ volunteers. Not sure if we want that so I left it off. ## Tech notes The blank option of `&nbsp;` exists instead of the default empty space because some Firefox based browsers show the empty option as very short for some reason. Adding the non-breaking space fixes it. To clear filters, the form removes all params from the URL except for `starts_at` and then requests that page. This way, we can remove all the filters, but keep the date so the user doesn't have to get to a far off date again.
It used to have more, but it just represents the users status now.
"attributes" it's a little confusing here
edwardloveall force-pushed el-filter from a54f636be0
Some checks failed
Setup Successful
Jest Successful
RSpec Failed
Static Analysis Successful
to 10afc71798
All checks were successful
Setup Successful
Jest Successful
Static Analysis Successful
RSpec Successful
2025年08月26日 22:22:17 +02:00
Compare
thetizzo left a comment
Copy link

Overall I think this looks great! The ShiftOccurenceFilter class is clean and understandable. Just left a couple comments and questions.

Overall I think this looks great! The `ShiftOccurenceFilter` class is clean and understandable. Just left a couple comments and questions.
@ -37,3 +37,3 @@
end
def missing_attributes?
def missing_users?
First-time contributor
Copy link

I like this rename, much more clear.

I like this rename, much more clear.
@ -0,0 +1,86 @@
class ShiftOccurrenceFilter
include ActiveModel::Model
First-time contributor
Copy link

I like the use of ActiveModel::Model here because it saves you a bunch of boilerplate that would otherwise be in the initializer to make sure all the filter params are set properly and if you wanted to add anything new, you would have to modify a lot more stuff. This seems a lot more flexible and extensible.

I like the use of `ActiveModel::Model` here because it saves you a bunch of boilerplate that would otherwise be in the initializer to make sure all the filter params are set properly and if you wanted to add anything new, you would have to modify a lot more stuff. This seems a lot more flexible and extensible.
@ -0,0 +37,4 @@
def filter_by_site_id(scope)
if site_id.present?
scope.select do
shift_event_tasks = _1.event.shift_event.shift_event_tasks
First-time contributor
Copy link

Does the EventsService preload all the associations for the event occurrences?

If not, I think I might be concerned about N+1s in a few of these filter methods for pages filtering large numbers of occurrences. Specifically filter_by_site_id, filter_by_unassigned, and filter_by_user_id all look like they have the potential to load one or more extra objects from the database per loop of the select.

Does the `EventsService` preload all the associations for the event occurrences? If not, I think I might be concerned about N+1s in a few of these filter methods for pages filtering large numbers of occurrences. Specifically `filter_by_site_id`, `filter_by_unassigned`, and `filter_by_user_id` all look like they have the potential to load one or more extra objects from the database per loop of the `select`.
Author
Owner
Copy link

I might be concerned about N+1s in a few of these filter methods for pages filtering large numbers of occurrences. Specifically filter_by_site_id, filter_by_unassigned, and filter_by_user_id all look like they have the potential to load one or more extra objects from the database per loop of the select.

You're 100% right that this creates lots of N+1s. We're a bit saved by the fact that no page (currently) shows anything more than a week worth of data, so at most we do like a couple hundred queries. It's still a lot, but that's the high end and performance is still good. I'm worried that if I add a bunch of includes to all the Event fetching in app/services/events_service/finder.rb I'll load a bunch of wasted data a lot of the time.

Ideally what I want to do is move the event occurrence stuff into the database via a view. That way it doesn't take up DB space, but I can still query it via ActiveRecord and then preload data per controller action. If performance gets bad though, I'll probably do something like pass a preload hash argument to the Finder methods.

If you think I should be way more concerned about this, please let me know! 😄 It didn't seem so urgent yet to me.

> I might be concerned about N+1s in a few of these filter methods for pages filtering large numbers of occurrences. Specifically filter_by_site_id, filter_by_unassigned, and filter_by_user_id all look like they have the potential to load one or more extra objects from the database per loop of the select. You're 100% right that this creates lots of N+1s. We're a bit saved by the fact that no page (currently) shows anything more than a week worth of data, so at most we do like a couple hundred queries. It's still a lot, but that's the high end and performance is still good. I'm worried that if I add a bunch of `includes` to all the `Event` fetching in `app/services/events_service/finder.rb` I'll load a bunch of wasted data a lot of the time. Ideally what I want to do is move the event occurrence stuff into the database via a view. That way it doesn't take up DB space, but I can still query it via ActiveRecord and then preload data per controller action. If performance gets bad though, I'll probably do something like pass a preload hash argument to the `Finder` methods. If you think I should be way more concerned about this, please let me know! 😄 It didn't seem so urgent yet to me.
First-time contributor
Copy link

This makes sense to me. I just wanted to call it out to make sure you were aware so we don't get surprised with some slow pages.

A middle ground in the meantime might be to load all the stuff you need once you're in the if for each method, but if performance is already good in the worst case at the moment, I wouldn't worry about this for now.

This makes sense to me. I just wanted to call it out to make sure you were aware so we don't get surprised with some slow pages. A middle ground in the meantime might be to load all the stuff you need once you're in the `if` for each method, but if performance is already good in the worst case at the moment, I wouldn't worry about this for now.
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
rootable/tomato!21
Reference in a new issue
rootable/tomato
No description provided.
Delete branch "el-filter"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?