Use role caching for Rolify to avoid N+1 queries

Resolves #2018

See https://github.com/RolifyCommunity/rolify#cached-roles-to-avoid-n1-issue

app/controllers/admin/base_controller.rb

@@ -13,10 +13,10 @@ module Admin
         redirect_to sign_in_path
         return false
       end
-      unless (current_user.has_role? :organizer, :any) || (current_user.has_role? :cfp, :any) ||
+      unless (current_user.has_cached_role? :organizer, :any) || (current_user.has_cached_role? :cfp, :any) ||
-             (current_user.has_role? :info_desk, :any) || (current_user.has_role? :organization_admin, :any) ||
+             (current_user.has_cached_role? :info_desk, :any) || (current_user.has_cached_role? :organization_admin, :any) ||
-             (current_user.has_role? :volunteers_coordinator, :any) ||
+             (current_user.has_cached_role? :volunteers_coordinator, :any) ||
-             (current_user.has_role? :track_organizer, :any) || current_user.is_admin
+             (current_user.has_cached_role? :track_organizer, :any) || current_user.is_admin
         raise CanCan::AccessDenied.new('You are not authorized to access this page.')
       end
     end

app/controllers/admin/organizations_controller.rb

@@ -45,7 +45,7 @@ module Admin
     end
 
     def assign_org_admins
-      if @user.has_role? 'organization_admin', @organization
+      if @user.has_cached_role? 'organization_admin', @organization
         flash[:error] = "User #{@user.email} already has the role organization admin"
       elsif @user.add_role 'organization_admin', @organization
         flash[:notice] = "Successfully added role organization admin to user #{@user.email}"

app/controllers/admin/roles_controller.rb

@@ -86,7 +86,7 @@ module Admin
         else
           flash[:error] = "Could not remove role #{@role.name} from user #{user.email}"
         end
-      elsif user.has_role? @role.name, role_resource
+      elsif user.has_cached_role? @role.name, role_resource
         flash[:error] = "User #{user.email} already has the role #{@role.name}"
         # Add user
       elsif user.add_role @role.name, role_resource

app/controllers/admin/versions_controller.rb

@@ -6,7 +6,7 @@ module Admin
     def index
       @conferences_with_role = current_user.is_admin? ? Conference.pluck(:short_title) : Conference.with_role([:organizer, :cfp, :info_desk], current_user).pluck(:short_title)
 
-      if current_user.has_role? :organization_admin, :any
+      if current_user.has_cached_role? :organization_admin, :any
         @conferences_with_role = Organization.with_role('organization_admin', current_user).map { |org| org.conferences.pluck :short_title }.flatten
       end
       @conferences_with_role.uniq!

app/helpers/admin/volunteers_helper.rb

@@ -1,7 +1,7 @@
 module Admin
   module VolunteersHelper
     def can_manage_volunteers?(conference)
-      current_user.has_role?(:organizer, conference) || current_user.has_role?(:volunteers_coordinator, conference)
+      current_user.has_cached_role?(:organizer, conference) || current_user.has_cached_role?(:volunteers_coordinator, conference)
     end
   end
 end

app/models/admin_ability.rb

@@ -5,8 +5,8 @@ class AdminAbility
     # Order Abilities
     # (Check https://github.com/CanCanCommunity/cancancan/wiki/Ability-Precedence)
     # Check roles of user, using rolify. Role name is *case sensitive*
-    # user.is_organizer? or user.has_role? :organizer
+    # user.is_organizer? or user.has_cached_role? :organizer
-    # user.is_cfp_of? Conference or user.has_role? :cfp, Conference
+    # user.is_cfp_of? Conference or user.has_cached_role? :cfp, Conference
     # user.is_info_desk_of? Conference
     # user.is_volunteers_coordinator_of? Conference
     # user.is_attendee_of? Conference
@@ -79,12 +79,12 @@ class AdminAbility
 
   # Abilities for signed in users with roles
   def signed_in_with_roles(user)
-    signed_in_with_organization_admin_role(user) if user.has_role? :organization_admin, :any
+    signed_in_with_organization_admin_role(user) if user.has_cached_role? :organization_admin, :any
-    signed_in_with_organizer_role(user) if user.has_role? :organizer, :any
+    signed_in_with_organizer_role(user) if user.has_cached_role? :organizer, :any
-    signed_in_with_cfp_role(user) if user.has_role? :cfp, :any
+    signed_in_with_cfp_role(user) if user.has_cached_role? :cfp, :any
-    signed_in_with_info_desk_role(user) if user.has_role? :info_desk, :any
+    signed_in_with_info_desk_role(user) if user.has_cached_role? :info_desk, :any
-    signed_in_with_volunteers_coordinator_role(user) if user.has_role? :volunteers_coordinator, :any
+    signed_in_with_volunteers_coordinator_role(user) if user.has_cached_role? :volunteers_coordinator, :any
-    signed_in_with_track_organizer_role(user) if user.has_role? :track_organizer, :any
+    signed_in_with_track_organizer_role(user) if user.has_cached_role? :track_organizer, :any
     common_abilities_for_roles(user)
   end
 

app/models/user.rb

@@ -6,6 +6,9 @@ end
 
 class User < ApplicationRecord
   rolify
+  # prevent N+1 queries with has_cached_role? by preloading roles *always*
+  default_scope { preload(:roles) }
+
   has_many :physical_tickets, through: :ticket_purchases do
     def by_conference(conference)
       where('ticket_purchases.conference_id = ?', conference)

app/views/tracks/index.html.haml

@@ -73,7 +73,7 @@
                       method: :patch, class: 'btn btn-mini btn-success', id: "resubmit_track_request_#{track.id}"
                   - if can? :edit, track
                     = link_to 'Edit', edit_conference_program_track_path(@conference.short_title, track), class: 'btn btn-default'
-                  - if current_user.has_role? :track_organizer, track
+                  - if current_user.has_cached_role? :track_organizer, track
                     = link_to 'Manage', admin_conference_program_track_path(@conference.short_title, track), class: 'btn btn-default'
 
   .row

app/views/tracks/show.html.haml

@@ -8,7 +8,7 @@
           .btn-group.pull-right
             - if can? :edit, @track
               = link_to 'Edit Track request', edit_conference_program_track_path(@conference.short_title, @track), class: 'btn btn-primary'
-            - if current_user.has_role? :track_organizer, @track
+            - if current_user.has_cached_role? :track_organizer, @track
               = link_to 'Manage', admin_conference_program_track_path(@conference.short_title, @track), class: 'btn btn-default'
   .row
     .col-md-8

spec/features/conference_spec.rb

@@ -30,7 +30,8 @@ feature Conference do
           .to eq('Conference was successfully created.')
       expect(Conference.count).to eq(expected_count)
       expect(Conference.last.organization).to eq(organization)
-      expect(user.has_role? :organizer, Conference.last).to eq(true)
+      user.reload
+      expect(user.has_cached_role? :organizer, Conference.last).to eq(true)
     end
 
     scenario 'update conference', feature: true, js: true do

spec/features/roles_spec.rb

@@ -58,7 +58,7 @@ feature Role do
       click_button 'Add'
       user_with_no_role.reload
 
-      expect(user_with_no_role.has_role?(role.name, conference)).to eq true
+      expect(user_with_no_role.has_cached_role?(role.name, conference)).to eq true
     end
 
     scenario "removes role #{role_name}", feature: true, js: true do
@@ -69,7 +69,8 @@ feature Role do
 
       expect(find('.alert').text).to eq "×Successfully removed role #{role_name} from user #{user_with_role.email}"
       expect(by_role_name).to eq(role_name) | eq('organizer')
-      expect(user_with_role.has_role?(role_name, conference)).to eq false
+      user_with_role.reload
+      expect(user_with_role.has_cached_role?(role_name, conference)).to eq false
     end
   end
 
@@ -118,14 +119,15 @@ feature Role do
         click_button 'Add'
         user_with_no_role.reload
 
-        expect(user_with_no_role.has_role?('organization_admin', organization)).to eq true
+        expect(user_with_no_role.has_cached_role?('organization_admin', organization)).to eq true
       end
 
       scenario 'successfully removes role organization_admin' do
         click_link('Admins', href: admins_admin_organization_path(organization.id))
 
         first('tr').find('.btn-danger').click
-        expect(organization_admin.has_role?('organization_admin', organization)).to eq false
+        organization_admin.reload
+        expect(organization_admin.has_cached_role?('organization_admin', organization)).to eq false
       end
     end
 

spec/models/track_spec.rb

@@ -347,17 +347,17 @@ describe Track do
     end
 
     it 'gives the role of the track organizer to the submitter of the track' do
-      expect(@submitter.has_role?(:track_organizer, self_organized_track)).to eq false
+      expect(@submitter.has_cached_role?(:track_organizer, self_organized_track)).to eq false
       self_organized_track.assign_role_to_submitter
-      expect(@submitter.has_role?(:track_organizer, self_organized_track)).to eq true
+      expect(@submitter.has_cached_role?(:track_organizer, self_organized_track)).to eq true
     end
 
     it 'is executed when the track is confirmed' do
       self_organized_track.state = 'accepted'
       self_organized_track.save!
-      expect(@submitter.has_role?(:track_organizer, self_organized_track)).to eq false
+      expect(@submitter.has_cached_role?(:track_organizer, self_organized_track)).to eq false
       self_organized_track.confirm
-      expect(@submitter.has_role?(:track_organizer, self_organized_track)).to eq true
+      expect(@submitter.has_cached_role?(:track_organizer, self_organized_track)).to eq true
     end
   end
 
@@ -374,9 +374,10 @@ describe Track do
     end
 
     it 'revokes the role of the track organizer' do
-      expect(@a_track_organizer.has_role?(:track_organizer, self_organized_track)).to eq true
+      expect(@a_track_organizer.has_cached_role?(:track_organizer, self_organized_track)).to eq true
       self_organized_track.revoke_role_and_cleanup
-      expect(@a_track_organizer.has_role?(:track_organizer, self_organized_track)).to eq false
+      @a_track_organizer.reload
+      expect(@a_track_organizer.has_cached_role?(:track_organizer, self_organized_track)).to eq false
     end
 
     it 'destroys the track\'s schedules' do
@@ -403,14 +404,16 @@ describe Track do
       self_organized_track.state = 'confirmed'
       self_organized_track.save!
       self_organized_track.cancel
-      expect(@a_track_organizer.has_role?(:track_organizer, self_organized_track)).to eq false
+      @a_track_organizer.reload
+      expect(@a_track_organizer.has_cached_role?(:track_organizer, self_organized_track)).to eq false
       @event_of_self_organized_track.reload
       expect(@event_of_self_organized_track.track).to eq nil
     end
 
     it 'is executed when the track is withdrawn' do
       self_organized_track.withdraw
-      expect(@a_track_organizer.has_role?(:track_organizer, self_organized_track)).to eq false
+      @a_track_organizer.reload
+      expect(@a_track_organizer.has_cached_role?(:track_organizer, self_organized_track)).to eq false
       @event_of_self_organized_track.reload
       expect(@event_of_self_organized_track.track).to eq nil
     end

spec/models/user_spec.rb

@@ -395,22 +395,22 @@ describe User do
       expect(another_user.roles[1]).to eq(cfp_role)
     end
 
-    describe '#has_role?' do
+    describe '#has_cached_role?' do
       describe 'when user has a role' do
         it 'returns true when the user has the role' do
           user = create(:user, role_ids: organizer_role.id)
-          expect(user.has_role?('organizer', conference)).to be true
+          expect(user.has_cached_role?('organizer', conference)).to be true
         end
 
         it 'returns false when the user does not have the role' do
           user = create(:user, role_ids: cfp_role.id)
-          expect(user.has_role?('organizer', conference)).to be false
+          expect(user.has_cached_role?('organizer', conference)).to be false
         end
       end
 
       it 'returns false when the user does not have a role' do
         user = create(:user, role_ids: [])
-        expect(user.has_role?('organizer', conference)).to be false
+        expect(user.has_cached_role?('organizer', conference)).to be false
       end
     end
   end