From 61f4b17ebeb08efb2aabf6b3335a25a059001956 Mon Sep 17 00:00:00 2001 From: "A. Wilcox" Date: Thu, 26 Jun 2025 09:49:22 -0500 Subject: [PATCH 1/3] Overhaul Stack Pass Administrator pages * Move user management to its own controller and views. * Allow editing of user names. --- .../stack_pass_admin_controller.rb | 31 ---------- .../stack_pass_users_controller.rb | 60 +++++++++++++++++++ app/views/stack_pass_admin/admin.html.erb | 15 ++--- app/views/stack_pass_admin/users.html.erb | 58 ------------------ app/views/stack_pass_users/_form.html.erb | 15 +++++ app/views/stack_pass_users/edit.html.erb | 6 ++ app/views/stack_pass_users/index.html.erb | 36 +++++++++++ app/views/stack_pass_users/new.html.erb | 6 ++ config/locales/en.yml | 17 +++++- config/routes.rb | 4 +- 10 files changed, 147 insertions(+), 101 deletions(-) create mode 100644 app/controllers/stack_pass_users_controller.rb delete mode 100644 app/views/stack_pass_admin/users.html.erb create mode 100644 app/views/stack_pass_users/_form.html.erb create mode 100644 app/views/stack_pass_users/edit.html.erb create mode 100644 app/views/stack_pass_users/index.html.erb create mode 100644 app/views/stack_pass_users/new.html.erb diff --git a/app/controllers/stack_pass_admin_controller.rb b/app/controllers/stack_pass_admin_controller.rb index 35287322..71c65203 100644 --- a/app/controllers/stack_pass_admin_controller.rb +++ b/app/controllers/stack_pass_admin_controller.rb @@ -14,37 +14,6 @@ def refcards @requests = ReferenceCardForm.where('pass_date_end > ?', start_calendar_year).order("#{sort_column} #{sort_direction}") end - def users - @users = Role.stackpass_admin.framework_users.order(:name) - end - - def add_user - # First check if the user exists in framework user - or create the user - framework_user = FrameworkUsers.find_or_create_by(lcasid: params['lcasid']) do |new_user| - new_user.name = params['name'] - new_user.role = 'Admin' - end - - # Now that we have the user, create the assignment: - Assignment.create(framework_users: framework_user, role: Role.stackpass_admin) - - # And redirect back to the admin users page: - flash[:success] = "Added #{framework_user.name} as an administrator" - redirect_to forms_stack_pass_admin_users_path - end - - def destroy_user - # First grab the user (so we can grab the name!) - admin = FrameworkUsers.find(params[:id]) - admin_name = admin.name - - # Now lets find the assignment - user_assignment = Assignment.where(framework_users: admin, role: Role.stackpass_admin).first - user_assignment.destroy - flash[:success] = "Removed #{admin_name} from administrator list" - redirect_to forms_stack_pass_admin_users_path - end - private def init_form!; end diff --git a/app/controllers/stack_pass_users_controller.rb b/app/controllers/stack_pass_users_controller.rb new file mode 100644 index 00000000..58f2df52 --- /dev/null +++ b/app/controllers/stack_pass_users_controller.rb @@ -0,0 +1,60 @@ +# Manipulate stack pass / reference card admins. +class StackPassUsersController < ApplicationController + before_action :auth_as_admin! + before_action :set_user, only: %i[edit update destroy] + + def index + @users = Role.stackpass_admin.framework_users.order(:name) + end + + def new + @user = FrameworkUsers.new + end + + def create + # First check if the user exists in framework user - or create the user + @user = FrameworkUsers.find_or_create_by!(lcasid: user_params[:lcasid]) do |new_user| + new_user.name = user_params[:name] + new_user.role = 'Admin' + end + + Assignment.create!(framework_users: @user, role: Role.stackpass_admin) + redirect_to stack_pass_users_path, flash: { success: "Added #{@user.name} as an administrator" } + rescue + @user ||= FrameworkUsers.new + render :new, status: :unprocessable_entity + end + + def edit; end + + def update + @user.name = user_params[:name] + if @user.save + redirect_to stack_pass_users_path, flash: { success: "Saved new name for #{@user.name}" } + else + render :edit, status: :unprocessable_entity + end + end + + def destroy + user_assignment = Assignment.where(framework_users: @user, role: Role.stackpass_admin).first + user_assignment.destroy + redirect_to stack_pass_users_path, flash: { success: "Removed #{@user.name} from administrator list" } + end + + private + # Ensure the current user is authenticated and an admin. + def auth_as_admin! + authenticate! + @user_is_admin = current_user.role?(Role.stackpass_admin) + redirect_to stack_pass_forms_path unless @user_is_admin + end + + def set_user + @user = FrameworkUsers.find_by(lcasid: params[:id]) + end + + def user_params + params.require(:framework_users).permit(:lcasid, :name) + end +end \ No newline at end of file diff --git a/app/views/stack_pass_admin/admin.html.erb b/app/views/stack_pass_admin/admin.html.erb index d790951d..b3e22b55 100644 --- a/app/views/stack_pass_admin/admin.html.erb +++ b/app/views/stack_pass_admin/admin.html.erb @@ -1,8 +1,9 @@

Stack Pass/Reference Card - Administration

-
-<%= link_to 'View all Stack Pass requests', forms_stack_pass_admin_stack_passes_path %> -
-<%= link_to 'View all Reference Card requests', forms_stack_pass_admin_reference_cards_path %> -
-<%= link_to 'Edit/Add Administrators', forms_stack_pass_admin_users_path %> -
+ + + diff --git a/app/views/stack_pass_admin/users.html.erb b/app/views/stack_pass_admin/users.html.erb deleted file mode 100644 index d9b2d8e6..00000000 --- a/app/views/stack_pass_admin/users.html.erb +++ /dev/null @@ -1,58 +0,0 @@ - -

Manage Stack Pass/Reference Card Administrators

- <%= link_to 'Admin Home', forms_stack_pass_admin_path, class: "title-link text-nowrap" %> - -<%= form_with url: "/forms/stack-pass-admin/add_user", local:true do %> -
-

Add Admin:

-
- <%= label_tag(:lcasid, "User ID:", class: "col-2") %> - <%= text_field_tag(:lcasid, nil, size: 40, placeholder: "UID", class: "col-4", required: true, autofocus: true) %> -
-
- <%= label_tag(:name, "Full Name:", class: "col-2") %> - <%= text_field_tag(:name, nil, size: 40, placeholder: "John Doe", class: "col-4", required: true) %> -
-
- -
-
-<% end %> -
-

Edit Admin:

-
-
-
- Administrators -
-
- User ID -
-
- Action -
-
-
-
- <% @users.each do |u| %> -
- - <%= hidden_field( :FrameworkUsers, :id, :value => u.id ) %> - -
- <%= u.name %> -
- -
- <%= u.lcasid %> -
- -
- <%= link_to 'Remove', forms_stack_pass_delete_user_path(u), - method: :delete, - data: {confirm: "Are you sure you want to delete #{u.name}?"}, - class: 'btn btn-sm btn-danger btn-white-text' %> -
-
- <% end %> -
diff --git a/app/views/stack_pass_users/_form.html.erb b/app/views/stack_pass_users/_form.html.erb new file mode 100644 index 00000000..eef06770 --- /dev/null +++ b/app/views/stack_pass_users/_form.html.erb @@ -0,0 +1,15 @@ +<%= form_for(@user, url: action_url) do |f| %> + <%= render 'application/form_error', model: @user if @user.errors.any? %> + <% if @user.persisted? %> +
+ +

<%= @user.lcasid %>

+
+ <% else %> + <%= field_for f, :lcasid, required: true %> + <% end %> + <%= field_for f, :name, required: true %> +
+ <%= f.submit 'Save Administrator', class: 'btn btn-primary' %> +
+<% end %> diff --git a/app/views/stack_pass_users/edit.html.erb b/app/views/stack_pass_users/edit.html.erb new file mode 100644 index 00000000..96e4972b --- /dev/null +++ b/app/views/stack_pass_users/edit.html.erb @@ -0,0 +1,6 @@ + +

<%= t('.page_title') %>

+ <%= link_to 'Admin Home', forms_stack_pass_admin_path, class: "title-link text-nowrap" %> + + +<%= render 'form', action_url: stack_pass_user_url(@user.lcasid) %> diff --git a/app/views/stack_pass_users/index.html.erb b/app/views/stack_pass_users/index.html.erb new file mode 100644 index 00000000..2e9bd880 --- /dev/null +++ b/app/views/stack_pass_users/index.html.erb @@ -0,0 +1,36 @@ + +

<%= t('.page_title') %>

+ <%= link_to 'Admin Home', forms_stack_pass_admin_path, class: "title-link text-nowrap" %> + + +<%= link_to 'Add New', new_stack_pass_user_path, class: 'btn btn-sm btn-success btn-white-text m-3', title: 'Add New Administrator' %> + + + + + + + + + + + <% @users.each do |u| %> + + + + + + + <% end %> + +
Administrator NameUser IDAction
+ <%= u.name %> + + <%= u.lcasid %> + + <%= link_to 'Edit', edit_stack_pass_user_path(u.lcasid), class: 'btn btn-sm btn-warning', title: 'Edit Administrator' %> + <%= link_to 'Remove', stack_pass_user_path(u.lcasid), + method: :delete, + data: {confirm: "Are you sure you want to remove #{u.name} as an administrator?"}, + class: 'btn btn-sm btn-danger btn-white-text', title: 'Remove Administrator' %> +
diff --git a/app/views/stack_pass_users/new.html.erb b/app/views/stack_pass_users/new.html.erb new file mode 100644 index 00000000..a6727447 --- /dev/null +++ b/app/views/stack_pass_users/new.html.erb @@ -0,0 +1,6 @@ + +

<%= t('.page_title') %>

+ <%= link_to 'Admin Home', forms_stack_pass_admin_path, class: "title-link text-nowrap" %> + + +<%= render 'form', action_url: stack_pass_users_url %> diff --git a/config/locales/en.yml b/config/locales/en.yml index a3ad25cf..05a692e0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -59,6 +59,9 @@ en: research_middle: Middle name or initial of (research assistant) proxy dsp_rep: Name of DSP Representative date_term: The term of the Proxy Card + framework_users: + lcasid: User ID + name: Full Name location_request: email: Email address filename: Input file @@ -299,11 +302,21 @@ en: stack_pass_admin: admin: page_title: Stack Pass Administration - users: - page_title: Add/Edit Administrators requests: page_title: Stack Pass Requests + stack_pass_users: + index: + page_title: Manage Stack Pass/Reference Card Administrators + new: + page_title: Add Stack Pass/Reference Card Administrator + create: + page_title: Add Stack Pass/Reference Card Administrator + edit: + page_title: Edit Stack Pass/Reference Card Administrator + update: + page_title: Edit Stack Pass/Reference Card Administrator + fees: index: page_title: Library Fees List diff --git a/config/routes.rb b/config/routes.rb index e8f7bbdd..4e0f83b1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -60,9 +60,7 @@ get '/forms/stack-pass-admin/', to: 'stack_pass_admin#admin' get '/forms/stack-pass-admin/stack-passes', to: 'stack_pass_admin#stackpasses' get '/forms/stack-pass-admin/reference-cards', to: 'stack_pass_admin#refcards' - get '/forms/stack-pass-admin/users', to: 'stack_pass_admin#users' - post '/forms/stack-pass-admin/add_user', to: 'stack_pass_admin#add_user' - delete '/forms/stack-pass-admin/delete_user/:id(.:format)', to: 'stack_pass_admin#destroy_user', as: :forms_stack_pass_delete_user + resources :stack_pass_users, path: '/forms/stack-pass-admin/users', except: [:show] # Proxy Borrower Form (DSP and Faculty) Routes: get '/forms/proxy-borrower/dsp', to: 'proxy_borrower_forms#dsp_form' From b76b5bb2c4112d41f63f7962ac74fa7cc4e8c7e9 Mon Sep 17 00:00:00 2001 From: "A. Wilcox" Date: Thu, 26 Jun 2025 10:39:49 -0500 Subject: [PATCH 2/3] Fixes for Stack Pass Admin overhaul * Ensure errors are displayed correctly when form submission fails. * Validate that the lcasid is always a number (don't allow invalid). * Appease Rubocop. --- app/controllers/stack_pass_users_controller.rb | 13 ++++++++----- app/models/framework_users.rb | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/controllers/stack_pass_users_controller.rb b/app/controllers/stack_pass_users_controller.rb index 58f2df52..6d2309e9 100644 --- a/app/controllers/stack_pass_users_controller.rb +++ b/app/controllers/stack_pass_users_controller.rb @@ -13,16 +13,18 @@ def new def create # First check if the user exists in framework user - or create the user - @user = FrameworkUsers.find_or_create_by!(lcasid: user_params[:lcasid]) do |new_user| + @user = FrameworkUsers.find_or_create_by(lcasid: user_params[:lcasid]) do |new_user| new_user.name = user_params[:name] new_user.role = 'Admin' end + unless @user.persisted? + render :new, status: :unprocessable_entity + return + end + Assignment.create!(framework_users: @user, role: Role.stackpass_admin) redirect_to stack_pass_users_path, flash: { success: "Added #{@user.name} as an administrator" } - rescue - @user ||= FrameworkUsers.new - render :new, status: :unprocessable_entity end def edit; end @@ -43,6 +45,7 @@ def destroy end private + # Ensure the current user is authenticated and an admin. def auth_as_admin! authenticate! @@ -57,4 +60,4 @@ def set_user def user_params params.require(:framework_users).permit(:lcasid, :name) end -end \ No newline at end of file +end diff --git a/app/models/framework_users.rb b/app/models/framework_users.rb index c67818d5..5497d7bc 100644 --- a/app/models/framework_users.rb +++ b/app/models/framework_users.rb @@ -15,7 +15,7 @@ class FrameworkUsers < ActiveRecord::Base has_many :roles, through: :assignments validates :lcasid, - presence: true + presence: true, numericality: true validates :name, presence: true validates :role, From 471762396bf0026b73b11a247d8c3d9dddee78b9 Mon Sep 17 00:00:00 2001 From: "A. Wilcox" Date: Mon, 20 Oct 2025 12:49:07 -0500 Subject: [PATCH 3/3] Rework Stack Pass administrator name handling Instead of having the name edited by site admins, we update it on the user's log in action from CAS. This ensures it is always up to date. This also adds a method for the human-friendly status of the request. This helps DRY the views. This additionally adds a column to the database for the FrameworkUser ID of who processed the request, so that the updates cascade. --- app/controllers/sessions_controller.rb | 6 ++++++ .../stack_pass_forms_controller.rb | 2 +- .../stack_pass_users_controller.rb | 13 +----------- app/models/framework_users.rb | 4 ++++ app/models/stack_request.rb | 21 +++++++++++++++++++ app/views/reference_card_forms/show.html.erb | 3 +-- app/views/stack_pass_forms/show.html.erb | 2 +- app/views/stack_pass_users/edit.html.erb | 6 ------ app/views/stack_pass_users/index.html.erb | 1 - config/routes.rb | 2 +- ...7_add_processed_by_id_to_stack_requests.rb | 17 +++++++++++++++ db/schema.rb | 4 +++- 12 files changed, 56 insertions(+), 25 deletions(-) delete mode 100644 app/views/stack_pass_users/edit.html.erb create mode 100644 db/migrate/20250710003327_add_processed_by_id_to_stack_requests.rb diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 04a6582b..c60e0957 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -23,6 +23,7 @@ def callback @user = User.from_omniauth(auth_params).tap do |user| sign_in(user) log_signin(user) + update_name(user, auth_params['extra']['displayName']) end redirect_to request.env['omniauth.origin'] || home_path @@ -46,4 +47,9 @@ def auth_params def log_signin(user) logger.debug({ msg: 'Signed in user', user: }) end + + def update_name(user, name) + framework_user = FrameworkUsers.find_by_lcasid(user.uid) + framework_user.update(name:) if framework_user.present? + end end diff --git a/app/controllers/stack_pass_forms_controller.rb b/app/controllers/stack_pass_forms_controller.rb index 15374579..e7bdb3ce 100644 --- a/app/controllers/stack_pass_forms_controller.rb +++ b/app/controllers/stack_pass_forms_controller.rb @@ -49,7 +49,7 @@ def update @form = StackPassForm.find(params[:id]) @form.approvedeny = params[:stack_pass_][:approve_deny] - @form.processed_by = params[:processed_by] + @form.processed_by_id = FrameworkUsers.find_by_lcasid(current_user.uid).id if @form.approvedeny == false deny_reason = params[:denial_reason] diff --git a/app/controllers/stack_pass_users_controller.rb b/app/controllers/stack_pass_users_controller.rb index 6d2309e9..dbdabe00 100644 --- a/app/controllers/stack_pass_users_controller.rb +++ b/app/controllers/stack_pass_users_controller.rb @@ -1,7 +1,7 @@ # Manipulate stack pass / reference card admins. class StackPassUsersController < ApplicationController before_action :auth_as_admin! - before_action :set_user, only: %i[edit update destroy] + before_action :set_user, only: %i[update destroy] def index @users = Role.stackpass_admin.framework_users.order(:name) @@ -27,17 +27,6 @@ def create redirect_to stack_pass_users_path, flash: { success: "Added #{@user.name} as an administrator" } end - def edit; end - - def update - @user.name = user_params[:name] - if @user.save - redirect_to stack_pass_users_path, flash: { success: "Saved new name for #{@user.name}" } - else - render :edit, status: :unprocessable_entity - end - end - def destroy user_assignment = Assignment.where(framework_users: @user, role: Role.stackpass_admin).first user_assignment.destroy diff --git a/app/models/framework_users.rb b/app/models/framework_users.rb index 5497d7bc..e8600f43 100644 --- a/app/models/framework_users.rb +++ b/app/models/framework_users.rb @@ -21,6 +21,10 @@ class FrameworkUsers < ActiveRecord::Base validates :role, presence: true + def name_for_lcasid(lcasid) + find_by(lcasid:)&.name + end + class << self # Hardcoded admins - so if for some reason all of the # admins in the DB are deleted, we still have a way of diff --git a/app/models/stack_request.rb b/app/models/stack_request.rb index 131410c1..320c13b5 100644 --- a/app/models/stack_request.rb +++ b/app/models/stack_request.rb @@ -10,6 +10,27 @@ class StackRequest < ActiveRecord::Base presence: true, email: true + # Return the human-friendly status for this request. + def status + case approvedeny + when nil + 'Unprocessed' + when true + 'Approved' + when false + 'Denied' + end + end + + # Return the name of the person who processed this request. + def processor_name + if processed_by_id.present? + FrameworkUsers.name_from_lcasid(processed_by_id) + else + processed_by + end + end + private # For Stack Pass: diff --git a/app/views/reference_card_forms/show.html.erb b/app/views/reference_card_forms/show.html.erb index c01c0a75..c4432d3c 100644 --- a/app/views/reference_card_forms/show.html.erb +++ b/app/views/reference_card_forms/show.html.erb @@ -72,8 +72,7 @@
- <%= label_tag(:processed_by, "Processed by (enter your name):", class: "control-label") %>
- <%= text_field_tag(:processed_by, @current_user.display_name, placeholder: "Enter your name", class: "form-control", required: true) %> + Processed by: <%= @current_user.display_name %>
diff --git a/app/views/stack_pass_forms/show.html.erb b/app/views/stack_pass_forms/show.html.erb index 91fcf0d6..afd170e4 100644 --- a/app/views/stack_pass_forms/show.html.erb +++ b/app/views/stack_pass_forms/show.html.erb @@ -118,7 +118,7 @@ Processed By: - <%= @req.processed_by %> + <%= @req.processor_name %>
diff --git a/app/views/stack_pass_users/edit.html.erb b/app/views/stack_pass_users/edit.html.erb deleted file mode 100644 index 96e4972b..00000000 --- a/app/views/stack_pass_users/edit.html.erb +++ /dev/null @@ -1,6 +0,0 @@ - -

<%= t('.page_title') %>

- <%= link_to 'Admin Home', forms_stack_pass_admin_path, class: "title-link text-nowrap" %> - - -<%= render 'form', action_url: stack_pass_user_url(@user.lcasid) %> diff --git a/app/views/stack_pass_users/index.html.erb b/app/views/stack_pass_users/index.html.erb index 2e9bd880..93311066 100644 --- a/app/views/stack_pass_users/index.html.erb +++ b/app/views/stack_pass_users/index.html.erb @@ -24,7 +24,6 @@ <%= u.lcasid %> - <%= link_to 'Edit', edit_stack_pass_user_path(u.lcasid), class: 'btn btn-sm btn-warning', title: 'Edit Administrator' %> <%= link_to 'Remove', stack_pass_user_path(u.lcasid), method: :delete, data: {confirm: "Are you sure you want to remove #{u.name} as an administrator?"}, diff --git a/config/routes.rb b/config/routes.rb index 4e0f83b1..81a0fc6b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -60,7 +60,7 @@ get '/forms/stack-pass-admin/', to: 'stack_pass_admin#admin' get '/forms/stack-pass-admin/stack-passes', to: 'stack_pass_admin#stackpasses' get '/forms/stack-pass-admin/reference-cards', to: 'stack_pass_admin#refcards' - resources :stack_pass_users, path: '/forms/stack-pass-admin/users', except: [:show] + resources :stack_pass_users, path: '/forms/stack-pass-admin/users', except: [:show, :edit, :update] # Proxy Borrower Form (DSP and Faculty) Routes: get '/forms/proxy-borrower/dsp', to: 'proxy_borrower_forms#dsp_form' diff --git a/db/migrate/20250710003327_add_processed_by_id_to_stack_requests.rb b/db/migrate/20250710003327_add_processed_by_id_to_stack_requests.rb new file mode 100644 index 00000000..598af45a --- /dev/null +++ b/db/migrate/20250710003327_add_processed_by_id_to_stack_requests.rb @@ -0,0 +1,17 @@ +class AddProcessedByIdToStackRequests < ActiveRecord::Migration[7.0] + def up + add_column :stack_requests, :processed_by_id, :integer + add_foreign_key :stack_requests, :framework_users, column: :processed_by_id + + StackRequest.find_each do |request| + request.processed_by_id = FrameworkUsers.find_by_name(request.processed_by) + end + end + + def down + StackRequest.where(processed_by: nil).find_each do |request| + request.processed_by = FrameworkUsers.name_for_lcasid(request.processed_by_id) + end + delete_column :stack_requests, :processed_by_id, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 87caa02f..07e17223 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2025_04_04_180042) do +ActiveRecord::Schema[7.0].define(version: 2025_07_10_003327) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -235,6 +235,7 @@ t.string "affiliation" t.text "research_desc" t.date "pass_date_end" + t.integer "processed_by_id" end create_table "tind_validators", force: :cascade do |t| @@ -250,4 +251,5 @@ add_foreign_key "host_bib_linked_bibs", "linked_bibs" add_foreign_key "host_bibs", "host_bib_tasks" add_foreign_key "location_records", "location_requests" + add_foreign_key "stack_requests", "framework_users", column: "processed_by_id" end