Refactored code to reduce number of database queries (#960)

app/controllers/admins_controller.rb

@@ -292,11 +292,11 @@ class AdminsController < ApplicationController
   private
 
   def find_user
-    @user = User.where(uid: params[:user_uid]).includes(:roles).first
+    @user = User.find_by(uid: params[:user_uid])
   end
 
   def find_deleted_user
-    @user = User.deleted.where(uid: params[:user_uid]).includes(:roles).first
+    @user = User.deleted.find_by(uid: params[:user_uid])
   end
 
   # Verifies that admin is an administrator of the user in the action

app/controllers/application_controller.rb

@@ -29,7 +29,7 @@ class ApplicationController < ActionController::Base
 
   # Retrieves the current user.
   def current_user
-    @current_user ||= User.where(id: session[:user_id]).includes(:roles).first
+    @current_user ||= User.includes(:roles, :main_room).find_by(id: session[:user_id])
 
     if Rails.configuration.loadbalanced_configuration
       if @current_user && !@current_user.has_role?(:super_admin) &&
@@ -67,7 +67,7 @@ class ApplicationController < ActionController::Base
 
   # Sets the settinfs variable
   def set_user_settings
-    @settings = Setting.find_or_create_by(provider: @user_domain)
+    @settings = Setting.includes(:features).find_or_create_by(provider: @user_domain)
   end
 
   # Redirects the user to a Maintenance page if turned on

app/controllers/concerns/populator.rb

@@ -25,11 +25,11 @@ module Populator
 
     initial_user = case @tab
       when "active"
-        User.without_role(:pending).without_role(:denied)
+        User.includes(:roles).without_role(:pending).without_role(:denied)
       when "deleted"
-        User.deleted
+        User.includes(:roles).deleted
       else
-        User
+        User.includes(:roles)
     end
 
     current_role = Role.find_by(name: @tab, provider: @user_domain) if @tab == "pending" || @tab == "denied"
@@ -57,7 +57,7 @@ module Populator
           .admins_search(@search)
           .admins_order(@order_column, @order_direction)
     else
-      Room.all.admins_search(@search).admins_order(@order_column, @order_direction)
+      Room.includes(:owner).all.admins_search(@search).admins_order(@order_column, @order_direction)
     end
   end
 

app/controllers/rooms_controller.rb

@@ -293,7 +293,7 @@ class RoomsController < ApplicationController
 
   # Find the room from the uid.
   def find_room
-    @room = Room.find_by!(uid: params[:room_uid])
+    @room = Room.includes(:owner).find_by!(uid: params[:room_uid])
   end
 
   # Ensure the user either owns the room or is an admin of the room owner or the room is shared with him

app/controllers/users_controller.rb

@@ -190,7 +190,7 @@ class UsersController < ApplicationController
   private
 
   def find_user
-    @user = User.where(uid: params[:user_uid]).includes(:roles).first
+    @user = User.find_by(uid: params[:user_uid])
   end
 
   # Verify that GreenLight is configured to allow user signup.

app/helpers/application_helper.rb

@@ -57,7 +57,6 @@ module ApplicationHelper
 
   # Returns the page that the logo redirects to when clicked on
   def home_page
-    return root_path unless current_user
     return admins_path if current_user.has_role? :super_admin
     current_user.main_room
   end

app/models/role.rb

@@ -20,7 +20,7 @@ class Role < ApplicationRecord
   has_and_belongs_to_many :users, join_table: :users_roles
   has_many :role_permissions
 
-  default_scope { order(:priority) }
+  default_scope { includes(:role_permissions) }
   scope :by_priority, -> { order(:priority) }
   scope :editable_roles, ->(provider) { where(provider: provider).where.not(name: %w[super_admin denied pending]) }
 
@@ -85,23 +85,36 @@ class Role < ApplicationRecord
 
   # Returns the value if enabled or the default if not enabled
   def get_permission(name, return_boolean = true)
-    permission = role_permissions.find_or_create_by!(name: name)
+    value = nil
 
-    value = if permission[:enabled]
+    role_permissions.each do |permission|
-        permission[:value]
+      next if permission.name != name
+
+      value = if permission.enabled
+        permission.value
       else
-      case name
+        default_value(name)
-      when "can_appear_in_share_list"
-        Rails.configuration.shared_access_default.to_s
-      else
-        "false"
       end
     end
 
+    # Create the role_permissions since it doesn't exist
+    role_permissions.create(name: name) if value.nil?
+
     if return_boolean
       value == "true"
     else
       value
     end
   end
+
+  private
+
+  def default_value(name)
+    case name
+    when "can_appear_in_share_list"
+      Rails.configuration.shared_access_default.to_s
+    else
+      "false"
+    end
+  end
 end

app/models/room.rb

@@ -42,27 +42,21 @@ class Room < ApplicationRecord
 
     search_param = "%#{string}%"
 
-    joins(:owner).where(search_query, search: search_param)
+    where(search_query, search: search_param)
   end
 
   def self.admins_order(column, direction)
     # Include the owner of the table
     table = joins(:owner)
 
-    return table.order(Arel.sql("#{column} #{direction}")) if table.column_names.include?(column) || column == "users.name"
+    return table.order(Arel.sql("rooms.#{column} #{direction}")) if table.column_names.include?(column) || column == "users.name"
 
     table
   end
 
   # Determines if a user owns a room.
   def owned_by?(user)
-    return false if user.nil?
+    user_id == user&.id
-    user.rooms.include?(self)
-  end
-
-  # Determines whether room is a home room
-  def home_room?
-    owner.main_room == self
   end
 
   def shared_users

app/models/setting.rb

@@ -28,10 +28,23 @@ class Setting < ApplicationRecord
 
   # Returns the value if enabled or the default if not enabled
   def get_value(name)
-    feature = features.find_or_create_by!(name: name)
+    # Return feature value if already exists
-    if feature[:enabled]
+    features.each do |feature|
-      feature[:value]
+      next if feature.name != name
-    else
+
+      return feature.value if feature.enabled
+      return default_value(name)
+    end
+
+    # Create the feature since it doesn't exist
+    features.create(name: name)
+    default_value(name)
+  end
+
+  private
+
+  def default_value(name)
+    # return default value
     case name
     when "Branding Image"
       Rails.configuration.branding_image_default
@@ -47,5 +60,4 @@ class Setting < ApplicationRecord
       Rails.configuration.shared_access_default
     end
   end
-  end
 end

app/models/user.rb

@@ -32,7 +32,7 @@ class User < ApplicationRecord
   has_many :shared_access
   belongs_to :main_room, class_name: 'Room', foreign_key: :room_id, required: false
 
-  has_and_belongs_to_many :roles, -> { includes :role_permissions }, join_table: :users_roles
+  has_and_belongs_to_many :roles, join_table: :users_roles
 
   validates :name, length: { maximum: 256 }, presence: true
   validates :provider, presence: true
@@ -183,7 +183,7 @@ class User < ApplicationRecord
 
   # role functions
   def highest_priority_role
-    roles.by_priority.first
+    roles.min_by(&:priority)
   end
 
   def add_role(role)
@@ -217,7 +217,11 @@ class User < ApplicationRecord
   # rubocop:disable Naming/PredicateName
   def has_role?(role)
     # rubocop:enable Naming/PredicateName
-    roles.exists?(name: role)
+    roles.each do |single_role|
+      return true if single_role.name.eql? role.to_s
+    end
+
+    false
   end
 
   def self.with_role(role)

app/views/rooms/components/_room_block.html.erb

@@ -55,7 +55,7 @@
                   <i class="dropdown-icon fas fa-users"></i> <%= t("room.share") %>
                 </a>
               <% end %>
-              <% unless room == room.owner.main_room %>
+              <% unless room == current_user.main_room %>
                 <a href="" data-toggle="modal" data-target="#deleteRoomModal" data-path="<%= room_path(room) %>" data-name="<%= room.name %>" class="delete-room dropdown-item">
                   <i class="dropdown-icon far fa-trash-alt"></i> <%= t("delete") %>
                 </a>

spec/controllers/admins_controller_spec.rb

@@ -67,6 +67,8 @@ describe AdminsController, type: :controller do
 
         post :ban_user, params: { user_uid: @user.uid }
 
+        @user.reload
+
         expect(@user.has_role?(:denied)).to eq(true)
         expect(flash[:success]).to be_present
         expect(response).to redirect_to(admins_path)
@@ -82,6 +84,8 @@ describe AdminsController, type: :controller do
 
         post :unban_user, params: { user_uid: @user.uid }
 
+        @user.reload
+
         expect(@user.has_role?(:denied)).to eq(false)
         expect(flash[:success]).to be_present
         expect(response).to redirect_to(admins_path)
@@ -153,6 +157,8 @@ describe AdminsController, type: :controller do
 
         post :approve, params: { user_uid: @user.uid }
 
+        @user.reload
+
         expect(@user.has_role?(:pending)).to eq(false)
         expect(flash[:success]).to be_present
         expect(response).to redirect_to(admins_path)

spec/controllers/rooms_controller_spec.rb

@@ -246,7 +246,6 @@ describe RoomsController, type: :controller do
 
     it "should use join name if user is not logged in and meeting running" do
       allow_any_instance_of(BigBlueButton::BigBlueButtonApi).to receive(:is_meeting_running?).and_return(true)
-
       post :join, params: { room_uid: @room, join_name: "Join Name" }
 
       expect(response).to redirect_to(join_path(@owner.main_room, "Join Name", {}))

spec/controllers/users_controller_spec.rb

@@ -362,6 +362,8 @@ describe UsersController, type: :controller do
         params = params.merge!(user_uid: user, user: { role_ids: "#{tmp_role1.id} #{tmp_role2.id}" })
 
         expect { patch :update, params: params }.to change { ActionMailer::Base.deliveries.count }.by(1)
+
+        user.reload
         expect(user.roles.count).to eq(2)
         expect(user.highest_priority_role.name).to eq("test1")
         expect(response).to redirect_to(admins_path)