boulder-food-rescue / food-rescue-robot

A Rails App for Managing "Just in Time" Food Rescue, Developed by/for Boulder Food Rescue in Boulder, CO, USA
49 stars 56 forks source link

Replace Volunteer.all with proper filtering at the Database level #141

Closed rylanb closed 6 years ago

rylanb commented 7 years ago

Expected Behavior

Volunteer.all is called all over the application and then iterated on in memory. We currently have 2342 volunteers across 40 regions.

Actual Behavior

These calls can all be scoped down to the region being worked on and/or active volunteers

rylanb commented 7 years ago

Example:

            @volunteers = Volunteer.all.keep_if do |volunteer|
                ((volunteer.region_ids & current_volunteer.region_ids).length > 0) && volunteer.needs_training?
            end

could be scoped down using Volunteer.joins(:regions).where(regions: {id: admin_region_ids}) + needs_training? logic

mooreds commented 6 years ago

Here are the files that have Volunteer.all in them:

app/helpers/application_helper.rb
app/controllers/application_controller.rb
app/controllers/volunteers_controller.rb
app/views/absences/_form.html.erb
app/views/volunteers/super_admin.html.erb
mooreds commented 6 years ago

app/controllers/application_controller.rb looks ok as it is doing a join right now.

mooreds commented 6 years ago

application_helper.rb has a method that isn't being used, so I removed it.

mooreds commented 6 years ago

app/views/absences/_form.html.erb uses the same logic that is here: https://github.com/boulder-food-rescue/food-rescue-robot/blob/master/app/views/volunteers/region_admin.html.erb#L24 which I know you modified in https://github.com/boulder-food-rescue/food-rescue-robot/pull/140

Can definitely move the fragment of html to a partial, but what is the best way to provide the @my_admin_volunteers across different controllers? Normally I'd move it to a service, but this app doesn't appear to have a service layer. Should I push it out to a interactor? @rylanb

rylanb commented 6 years ago

You could try an interactor or just redefine it more than once? Its ok to just clean up and have some code duplication in here.

rylanb commented 6 years ago

Addressed here: https://github.com/boulder-food-rescue/food-rescue-robot/pull/157#pullrequestreview-115414188