From 2103713552693cafd952185d79c649d985e12677 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 29 Apr 2024 13:45:56 -0500 Subject: [PATCH] Cleaned up the User model, controller, access periods and added a UserRoleService to help with this. --- app/controllers/users_controller.rb | 94 +++++------------------------ app/models/access_period.rb | 4 +- app/models/user.rb | 9 +++ app/services/user_role_service.rb | 24 ++++++++ 4 files changed, 49 insertions(+), 82 deletions(-) create mode 100644 app/services/user_role_service.rb diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index a2e76d0..45e8c75 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,78 +1,46 @@ class UsersController < ApplicationController before_action :authenticate_user! - before_action :set_user, only: [:edit, :update, :destroy] before_action :require_admin + before_action :set_user, only: [:show, :edit, :update, :destroy] load_and_authorize_resource - def index - @users = User.all - end - - def new - @user = User.new - @user.access_periods.build unless @user.access_periods.any? - end - - - - def edit - @user = User.find(params[:id]) - @user.access_periods.build if @user.access_periods.empty? - end - - def create @user = User.new(user_params) - # Ensure an access period is built if none exist and no start_date is provided - if @user.access_periods.empty? - @user.access_periods.build(start_date: Date.today) # Set start date automatically - end - if @user.save - update_user_roles(@user, params[:user][:roles] || ['user']) redirect_to users_path, notice: 'User was successfully created.' else render :new end end - + + def edit + # Since @user is set by set_user, there's no need to find the user again + end + def update - @user = User.find(params[:id]) # If @user is set in a before_action, this line can be removed. - - # Handling password change: if password fields are blank, they are removed from user_params to prevent updating the password to nil. + # Clean up password fields if they are blank cleaned_params = user_params if cleaned_params[:password].blank? cleaned_params.delete(:password) cleaned_params.delete(:password_confirmation) end - if @user.update(cleaned_params.except(:roles)) - # Update roles - update_user_roles(@user, params[:user][:roles] || []) - - # Check and update the access revoked status and end date - if params[:user][:access_revoked] == "1" - current_period = @user.access_periods.order(:created_at).last - current_period.update(end_date: Date.today) unless current_period.end_date.present? - end - + # Attempt to update the user with the cleaned parameters + if @user.update(cleaned_params) + UserRoleService.new(@user, params[:user][:roles] || []).update_roles # Handle roles separately redirect_to users_path, notice: 'User was successfully updated.' else - render :edit + render :edit # If there's an error, it will render the edit view where you can display error messages end end - - def show - @user = User.includes(:access_periods).find(params[:id]) - end - def destroy @user.destroy redirect_to users_path, notice: 'User was successfully deleted.' end + private def set_user @@ -84,46 +52,12 @@ class UsersController < ApplicationController :email, :password, :password_confirmation, :remember_me, :first_name, :last_name, :phone, :company, :access_revoked, :access_start_date, :access_end_date, - access_periods_attributes: [:id, :start_date, :end_date, :_destroy], - roles: [] + access_periods_attributes: [:id, :start_date, :end_date, :_destroy], ) end def require_admin - unless current_user.admin? - redirect_to root_path, alert: 'Only admins are allowed to access this section.' - end + redirect_to root_path, alert: 'Only admins are allowed to access this section.' unless current_user.admin? end - - def assign_roles(user) - user.roles = [] - params[:user][:roles].each do |role_name| - user.add_role(role_name) unless role_name.blank? - end if params[:user][:roles].present? - end - - - def update_user_roles(user, roles_names) - # Assume roles_names should only contain one role at most - new_role = roles_names.reject(&:blank?).uniq.first # Take the first valid role name - - # Clear all roles and set the new one - user.roles.delete_all - user.add_role(new_role) if new_role - end - - - - def handle_access_revocation - if params[:user][:access_revoked] == "1" - current_period = @user.access_periods.find_or_initialize_by(end_date: nil) - current_period.update(end_date: Date.today) unless current_period.end_date.present? - elsif params[:user][:access_revoked] == "0" && @user.access_periods.last&.end_date.present? - @user.access_periods.build(start_date: Date.today) - end - end - - - end diff --git a/app/models/access_period.rb b/app/models/access_period.rb index 0f25789..d801670 100644 --- a/app/models/access_period.rb +++ b/app/models/access_period.rb @@ -1,6 +1,7 @@ class AccessPeriod < ApplicationRecord belongs_to :user - before_validation :set_default_start_date, on: :create + + before_create :set_default_start_date validates :end_date, presence: true, if: -> { user&.access_revoked? } private @@ -9,4 +10,3 @@ class AccessPeriod < ApplicationRecord self.start_date ||= Date.today if self.new_record? && self.start_date.blank? end end - diff --git a/app/models/user.rb b/app/models/user.rb index 7d30f50..a47962c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -4,6 +4,7 @@ class User < ApplicationRecord has_many :access_periods, dependent: :destroy accepts_nested_attributes_for :access_periods, allow_destroy: true after_create :assign_default_role + before_update :handle_access_revocation validate :password_complexity @@ -33,6 +34,14 @@ class User < ApplicationRecord self.admin = has_role?(:admin) end + def handle_access_revocation + if access_revoked_changed? && access_revoked + access_periods.find_or_initialize_by(end_date: nil).update(end_date: Date.today) + elsif access_revoked_changed? && !access_revoked + access_periods.create(start_date: Date.today) + end + end + def end_date_after_start_date if access_start_date.present? errors.add(:access_end_date, 'must be provided when access is revoked') unless access_end_date.present? diff --git a/app/services/user_role_service.rb b/app/services/user_role_service.rb new file mode 100644 index 0000000..3556a3d --- /dev/null +++ b/app/services/user_role_service.rb @@ -0,0 +1,24 @@ +class UserRoleService + def initialize(user, role_names) + @user = user + @role_names = role_names.reject(&:blank?).uniq + end + + def update_roles + @user.roles = Role.where(name: @role_names) + end + + private + + def remove_unassigned_roles + current_roles = @user.roles.pluck(:name) + roles_to_remove = current_roles - @role_names + roles_to_remove.each { |role| @user.remove_role(role) } + end + + def add_new_roles + roles_to_add = @role_names - @user.roles.pluck(:name) + roles_to_add.each { |role| @user.add_role(role) } + end + end + \ No newline at end of file