diff --git a/app/controllers/admins_controller.rb b/app/controllers/admins_controller.rb index 37de5067..f84c5eb3 100644 --- a/app/controllers/admins_controller.rb +++ b/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 diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 47ec8ea6..bc45b8eb 100644 --- a/app/controllers/application_controller.rb +++ b/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 diff --git a/app/controllers/concerns/populator.rb b/app/controllers/concerns/populator.rb index 214f8be7..771fa25e 100644 --- a/app/controllers/concerns/populator.rb +++ b/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 diff --git a/app/controllers/rooms_controller.rb b/app/controllers/rooms_controller.rb index d02fba87..2f4440d8 100644 --- a/app/controllers/rooms_controller.rb +++ b/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 diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 866ecb9a..c6c31af6 100644 --- a/app/controllers/users_controller.rb +++ b/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. diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 155335cf..eaf90a23 100644 --- a/app/helpers/application_helper.rb +++ b/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 diff --git a/app/models/role.rb b/app/models/role.rb index c6d6c598..61177a05 100644 --- a/app/models/role.rb +++ b/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] - permission[:value] - else - case name - when "can_appear_in_share_list" - Rails.configuration.shared_access_default.to_s + role_permissions.each do |permission| + next if permission.name != name + + value = if permission.enabled + permission.value else - "false" + default_value(name) 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 diff --git a/app/models/room.rb b/app/models/room.rb index b7354694..158fdc23 100644 --- a/app/models/room.rb +++ b/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.rooms.include?(self) - end - - # Determines whether room is a home room - def home_room? - owner.main_room == self + user_id == user&.id end def shared_users diff --git a/app/models/setting.rb b/app/models/setting.rb index cf2254a6..d657ad36 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -28,24 +28,36 @@ 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) - if feature[:enabled] - feature[:value] - else - case name - when "Branding Image" - Rails.configuration.branding_image_default - when "Primary Color" - Rails.configuration.primary_color_default - when "Registration Method" - Rails.configuration.registration_method_default - when "Room Authentication" - false - when "Room Limit" - Rails.configuration.number_of_rooms_default - when "Shared Access" - Rails.configuration.shared_access_default - end + # Return feature value if already exists + features.each do |feature| + next if feature.name != name + + 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 + when "Primary Color" + Rails.configuration.primary_color_default + when "Registration Method" + Rails.configuration.registration_method_default + when "Room Authentication" + false + when "Room Limit" + Rails.configuration.number_of_rooms_default + when "Shared Access" + Rails.configuration.shared_access_default end end end diff --git a/app/models/user.rb b/app/models/user.rb index bfd9f617..7d61bf64 100644 --- a/app/models/user.rb +++ b/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) diff --git a/app/views/rooms/components/_room_block.html.erb b/app/views/rooms/components/_room_block.html.erb index b2dfb822..26c1f5aa 100644 --- a/app/views/rooms/components/_room_block.html.erb +++ b/app/views/rooms/components/_room_block.html.erb @@ -55,7 +55,7 @@ <%= t("room.share") %> <% end %> - <% unless room == room.owner.main_room %> + <% unless room == current_user.main_room %> <%= t("delete") %> diff --git a/spec/controllers/admins_controller_spec.rb b/spec/controllers/admins_controller_spec.rb index d2e59de3..b86e0596 100644 --- a/spec/controllers/admins_controller_spec.rb +++ b/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) diff --git a/spec/controllers/rooms_controller_spec.rb b/spec/controllers/rooms_controller_spec.rb index 55fa0721..be8f384b 100644 --- a/spec/controllers/rooms_controller_spec.rb +++ b/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", {})) diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 453f981f..ee078f41 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/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)