Perf: Refactor to check user role in memory not db

coalest · coalest · commit 2c508666a9d1 · 2024-12-11T17:27:33.000+01:00
diff --git a/app/controllers/admin/broadcast_announcements_controller.rb b/app/controllers/admin/broadcast_announcements_controller.rb
@@ -3,7 +3,7 @@ class Admin::BroadcastAnnouncementsController < AdminController
   before_action :require_admin
 
   def require_admin
-    verboten! unless current_user.has_role?(Role::SUPER_ADMIN)
+    verboten! unless current_user.has_cached_role?(Role::SUPER_ADMIN)
   end
 
   def index
diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb
@@ -3,7 +3,7 @@ class AdminController < ApplicationController
   before_action :require_admin
 
   def require_admin
-    verboten! unless current_user.has_role?(Role::SUPER_ADMIN)
+    verboten! unless current_user.has_cached_role?(Role::SUPER_ADMIN)
   end
 
   def dashboard
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
@@ -59,22 +59,21 @@ def dashboard_path_from_current_role
   def authorize_user
     return unless params[:controller] # part of omniauth controller flow
     verboten! unless params[:controller].include?("devise") ||
-      current_user.has_role?(Role::SUPER_ADMIN) ||
-      current_user.has_role?(Role::ORG_USER, current_organization) ||
-      current_user.has_role?(Role::ORG_ADMIN, current_organization) ||
-      current_user.has_role?(Role::PARTNER, current_partner)
+      current_user.has_cached_role?(Role::SUPER_ADMIN) ||
+      current_user.has_cached_role?(Role::ORG_USER, current_organization) ||
+      current_user.has_cached_role?(Role::ORG_ADMIN, current_organization) ||
+      current_user.has_cached_role?(Role::PARTNER, current_partner)
   end
 
   def authorize_admin
-    verboten! unless current_user.has_role?(Role::SUPER_ADMIN) ||
-      current_user.has_role?(Role::ORG_ADMIN, current_organization)
+    verboten! unless current_user.has_cached_role?(Role::SUPER_ADMIN) ||
+      current_user.has_cached_role?(Role::ORG_ADMIN, current_organization)
   end
 
   def log_active_user
     if current_user && should_update_last_request_at?
       # we don't want the user record to validate or run callbacks when we're tracking activity
       current_user.update_columns(last_request_at: Time.now.utc)
-
     end
   end
 
diff --git a/app/controllers/distributions_controller.rb b/app/controllers/distributions_controller.rb
@@ -171,7 +171,7 @@ def edit
     @distribution = Distribution.includes(:line_items).includes(:storage_location).find(params[:id])
     @distribution.initialize_request_items
     if (!@distribution.complete? && @distribution.future?) ||
-        current_user.has_role?(Role::ORG_ADMIN, current_organization)
+        current_user.has_cached_role?(Role::ORG_ADMIN, current_organization)
       @distribution.line_items.build if @distribution.line_items.size.zero?
       @items = current_organization.items.alphabetized
       @partner_list = current_organization.partners.alphabetized
diff --git a/app/controllers/organizations_controller.rb b/app/controllers/organizations_controller.rb
@@ -81,8 +81,8 @@ def remove_user
   private
 
   def authorize_user
-    verboten! unless current_user.has_role?(Role::SUPER_ADMIN) ||
-      current_user.has_role?(Role::ORG_USER, current_organization)
+    verboten! unless current_user.has_cached_role?(Role::SUPER_ADMIN) ||
+      current_user.has_cached_role?(Role::ORG_USER, current_organization)
   end
 
   def organization_params
@@ -121,7 +121,7 @@ def request_type_formatter(params)
   end
 
   def user_update_redirect_path
-    if current_user.has_role?(Role::SUPER_ADMIN)
+    if current_user.has_cached_role?(Role::SUPER_ADMIN)
       admin_organization_path(current_organization.id)
     else
       organization_path
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
@@ -23,7 +23,7 @@ def menu_open?(controller_action_names)
   end
 
   def can_administrate?
-    current_user.has_role?(Role::ORG_ADMIN, current_organization)
+    current_user.has_cached_role?(Role::ORG_ADMIN, current_organization)
   end
 
   def navigation_link_to(*args)
diff --git a/app/views/distributions/show.html.erb b/app/views/distributions/show.html.erb
@@ -83,7 +83,7 @@
           </div>
           <div class="card-footer">
             <%= update_button_to picked_up_distribution_path(@distribution), {text: "Distribution Complete", size: "md"} if @distribution.scheduled? %>
-            <% if @distribution.future? || current_user.has_role?(Role::ORG_ADMIN, current_organization) %>
+            <% if @distribution.future? || current_user.has_cached_role?(Role::ORG_ADMIN, current_organization) %>
               <%= edit_button_to edit_distribution_path(@distribution), {
                 text: "Make a Correction",
                 enabled: !@distribution.has_inactive_item?,
diff --git a/app/views/donations/show.html.erb b/app/views/donations/show.html.erb
@@ -75,7 +75,7 @@
               enabled: !@donation.has_inactive_item?,
               size: "md" } %>
             <%= new_button_to new_distribution_path(donation_id: @donation.id, storage_location_id: @donation.storage_location_id), { text: "Start a new Distribution" } %>
-            <% if current_user.has_role?(Role::ORG_ADMIN, current_organization) %>
+            <% if current_user.has_cached_role?(Role::ORG_ADMIN, current_organization) %>
               <%= delete_button_to donation_path(@donation), {
                 size: "md",
                 enabled: !@donation.has_inactive_item?,
diff --git a/app/views/item_categories/show.html.erb b/app/views/item_categories/show.html.erb
@@ -77,7 +77,7 @@
                   <tr>
                     <td><%= item.name %> </td>
                     <td><%= view_button_to item_path(item) %>
-                      <% if current_user.has_role?(Role::ORG_ADMIN, current_organization) %>
+                      <% if current_user.has_cached_role?(Role::ORG_ADMIN, current_organization) %>
                         <%= delete_button_to(remove_category_item_path(item), method: :patch, text: "Remove from category") %>
                       <% end %>
                     </td>
diff --git a/app/views/items/_form.html.erb b/app/views/items/_form.html.erb
@@ -30,7 +30,7 @@
                 <%= f.input_field :distribution_quantity, class: "form-control" %>
               <% end %>
 
-              <% if current_user.has_role?(Role::ORG_ADMIN, current_organization) %>
+              <% if current_user.has_cached_role?(Role::ORG_ADMIN, current_organization) %>
                 <%= f.input :name, label: "On hand minimum quantity", wrapper: :input_group do %>
                   <%= f.input_field :on_hand_minimum_quantity, input_html: {value: 0}, class: "form-control" %>
                 <% end %>
diff --git a/app/views/layouts/_lte_navbar.html.erb b/app/views/layouts/_lte_navbar.html.erb
@@ -50,7 +50,7 @@
         <i class="fa fa-repeat text-aqua"></i><%= "Switch to: #{role.resource&.name || "Super Admin"}" %>
       <% end %>
     <% end %>
-    <% if current_user.has_role?(Role::ORG_ADMIN, current_organization) %>
+    <% if current_user.has_cached_role?(Role::ORG_ADMIN, current_organization) %>
         <div class="dropdown-divider"></div>
         <%= link_to users_path, class:"dropdown-item" do %>
           <i class="fas fa-users mr-2"></i> My Co-Workers
diff --git a/app/views/layouts/_lte_sidebar.html.erb b/app/views/layouts/_lte_sidebar.html.erb
@@ -113,7 +113,7 @@
           <i class="nav-icon fa fa-circle-o"></i> Inventory Adjustments
         <% end %>
       </li>
-      <% if current_user.has_role?(Role::ORG_ADMIN, current_organization) %>
+      <% if current_user.has_cached_role?(Role::ORG_ADMIN, current_organization) %>
         <li class="nav-item <%= active_class(['audits']) %>">
           <%= link_to(audits_path, class: "nav-link #{active_class(['audits'])}") do %>
             <i class="nav-icon fa fa-circle-o"></i> Inventory Audit
@@ -246,7 +246,7 @@
       </li>
     </ul>
   </li>
-  <% if current_user.has_role?(Role::ORG_ADMIN, current_organization) %>
+  <% if current_user.has_cached_role?(Role::ORG_ADMIN, current_organization) %>
     <li class="nav-item <%= 'active' if current_page?(organization_path) %>">
       <%= link_to(organization_path, class: "nav-link #{'active' if current_page?(organization_path)}") do %>
         <i class="nav-icon fas fa-home"></i>
diff --git a/app/views/partners/show.html.erb b/app/views/partners/show.html.erb
@@ -34,7 +34,7 @@
             <h2 class="card-title">Partner Actions</h2>
           </div>
           <div class="card-body p-3">
-            <% if current_user.has_role?(Role::ORG_ADMIN, current_organization) %>
+            <% if current_user.has_cached_role?(Role::ORG_ADMIN, current_organization) %>
               <%= link_to partner_users_path(@partner) do %>
                 <div class="btn btn-app bg-success">
                   <i class="fas fa-users"></i> Manage Users
diff --git a/app/views/product_drives/show.html.erb b/app/views/product_drives/show.html.erb
@@ -90,7 +90,7 @@
           </p>
           <div class="card-footer clearfix">
             <%= edit_button_to edit_product_drive_path(@product_drive), { text: "Make a correction", size: "md" } %>
-            <%= delete_button_to product_drive_path(@product_drive), { confirm: "Are you sure you want to permanently remove this product drive?", size: "md" } if current_user.has_role?(Role::ORG_ADMIN, current_organization) %>
+            <%= delete_button_to product_drive_path(@product_drive), { confirm: "Are you sure you want to permanently remove this product drive?", size: "md" } if current_user.has_cached_role?(Role::ORG_ADMIN, current_organization) %>
           </div>
           <!-- /.card-footer-->
         </div>
diff --git a/app/views/purchases/show.html.erb b/app/views/purchases/show.html.erb
@@ -81,7 +81,7 @@
             <%= edit_button_to edit_purchase_path(@purchase), { text: "Make a correction",
                                                                 enabled: !@purchase.has_inactive_item?,
                                                                 size: "md" } %>
-            <% if current_user.has_role?(Role::ORG_ADMIN, current_organization) %>
+            <% if current_user.has_cached_role?(Role::ORG_ADMIN, current_organization) %>
               <%= delete_button_to purchase_path(@purchase), {
                 size: "md",
                 enabled: !@purchase.has_inactive_item?,
diff --git a/app/views/users/_organization_user.html.erb b/app/views/users/_organization_user.html.erb
@@ -16,7 +16,7 @@
         </button>
         <ul class="dropdown-menu">
           <li>
-            <% if current_user.has_role?(Role::SUPER_ADMIN) %>
+            <% if current_user.has_current_role?(Role::SUPER_ADMIN) %>
               <%= edit_button_to(edit_admin_user_path(user), { text: 'Edit User' }) %>
             <% else %>
               <%= edit_button_to(
diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb
@@ -328,7 +328,9 @@
 end
 
 Warden::Manager.after_set_user do |user, auth, opts|
-  if user.roles.empty?
+  # Use blank instead of #empty? to load roles in memory for future
+  # current_user.has_cached_role? checks
+  if user.roles.blank?
     auth.logout
     throw(:warden)
   end
