From fb97ed7c0abbfc91580297c8d558d29ca75e2586 Mon Sep 17 00:00:00 2001 From: jazairi <16103405+jazairi@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:55:26 -0700 Subject: [PATCH 1/3] Add missing validations to the Hold model Why these changes are being introduced: Thesis and Hold Source are required fields on the Hold model. This is defined in the db schema, but not at the model layer. Because administrate checks model validations to identify required fields, this means that these fields don't appear as required in the admin forms. Relevant ticket(s): - [ETD-691](https://mitlibraries.atlassian.net/browse/ETD-691) How this addresses that need: This adds validations for Thesis and Hold Source to the Hold model. Side effects of this change: None. --- app/models/hold.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/models/hold.rb b/app/models/hold.rb index ceece1bb..0cc2211f 100644 --- a/app/models/hold.rb +++ b/app/models/hold.rb @@ -36,7 +36,9 @@ class Hold < ApplicationRecord validates :date_requested, presence: true validates :date_start, presence: true validates :date_end, presence: true + validates :hold_source, presence: true validates :status, presence: true + validates :thesis, presence: true after_save :update_thesis_status From 9036134da5ba9b3c6fc13c69123c64dfaa71522b Mon Sep 17 00:00:00 2001 From: jazairi <16103405+jazairi@users.noreply.github.com> Date: Wed, 29 Jul 2026 10:20:45 -0700 Subject: [PATCH 2/3] Add regression coverage for missing validations --- test/models/hold_test.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/models/hold_test.rb b/test/models/hold_test.rb index 90395222..4d7cde5d 100644 --- a/test/models/hold_test.rb +++ b/test/models/hold_test.rb @@ -43,6 +43,24 @@ class HoldTest < ActiveSupport::TestCase assert(hold.invalid?) end + test 'invalid without thesis' do + hold = holds(:valid) + assert hold.thesis.present? + assert hold.valid? + + hold.thesis = nil + assert_not hold.valid? + end + + test 'invalid without hold source' do + hold = holds(:valid) + assert hold.hold_source.present? + assert hold.valid? + + hold.hold_source = nil + assert_not hold.valid? + end + test 'valid date_start' do hold = holds(:valid) assert(hold.valid?) From e423a53656c1d1034bcc3c04802cd159f25daca8 Mon Sep 17 00:00:00 2001 From: jazairi <16103405+jazairi@users.noreply.github.com> Date: Wed, 29 Jul 2026 10:40:03 -0700 Subject: [PATCH 3/3] Remove summary table from report dashboard Why these changes are being introduced: The report summary table is extremely inefficient and probably not very useful. Relevant ticket(s): - [ETD-623](https://mitlibraries.atlassian.net/browse/ETD-623) How this addresses that need: This removes the summary table, so the report index view just lists links to the individual reports. Side effects of this change: - Link sidebar has been removed from the dashboard, as it is now redundant. - The report dashboard is now a visual design abomination, but that feels better than a million N+1 queries. --- app/controllers/report_controller.rb | 6 +-- app/views/report/index.html.erb | 51 ++++++---------------- test/controllers/report_controller_test.rb | 22 +++------- 3 files changed, 21 insertions(+), 58 deletions(-) diff --git a/app/controllers/report_controller.rb b/app/controllers/report_controller.rb index d1f4074f..807861f2 100644 --- a/app/controllers/report_controller.rb +++ b/app/controllers/report_controller.rb @@ -88,11 +88,7 @@ def holds_by_source @list = filter_holds_by_source term_filtered end - def index - report = Report.new - @terms = Thesis.pluck(:grad_date).uniq.sort - @data = report.index_data - end + def index; end def term term = params[:graduation] ? params[:graduation].to_s : 'all' diff --git a/app/views/report/index.html.erb b/app/views/report/index.html.erb index 347a19f9..411b4790 100644 --- a/app/views/report/index.html.erb +++ b/app/views/report/index.html.erb @@ -2,44 +2,19 @@
-

Thesis dashboard

+

Report dashboard

-
-

The tables below summarize information about the thesis records which have been supplied to this application across academic terms.

-

Click on a column heading to see more detailed information about the theses from that term.

-
- - <%= render 'shared/whodunnit_accuracy_statement' %> - - <% @data.each do |table| %> - " style="margin-top: 4em;"> - - - - - <% @terms.each do |term| %> - - <% end %> - - - - <% table[1].each do |row| %> - - - <% if row[:data] %> - <% row[:data].each do |cell| %> - - <% end %> - <% end %> - - <% end %> - -
<%= table[0].gsub("-", " ").capitalize %>
 <%= link_to( term.in_time_zone('Eastern Time (US & Canada)').strftime('%b %Y'), report_term_path(:graduation => term) )%>
<%= row[:label] %><%= cell[1] %>
- <% end %> - +

Available reports

+
- -
diff --git a/test/controllers/report_controller_test.rb b/test/controllers/report_controller_test.rb index bf639be9..1f652adc 100644 --- a/test/controllers/report_controller_test.rb +++ b/test/controllers/report_controller_test.rb @@ -49,20 +49,20 @@ def create_thesis_with_whodunnit(user) end # ~~~~~~~~~~~~~~~~~~~~ Report dashboard ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - test 'summary report exists' do + test 'report dashboard exists' do sign_in users(:admin) get report_index_path assert_response :success end - test 'anonymous users are prompted to log in by summary report' do + test 'anonymous users are prompted to log in by report dashboard' do # Note that nobody is signed in. get report_index_path assert_response :redirect assert_redirected_to '/login' end - test 'basic users cannot see summary report' do + test 'basic users cannot see report dashboard' do sign_in users(:basic) get report_index_path assert_redirected_to '/' @@ -70,7 +70,7 @@ def create_thesis_with_whodunnit(user) assert_select 'div.alert', text: 'Not authorized.', count: 1 end - test 'submitters cannot see summary report' do + test 'submitters cannot see report dashboard' do sign_in users(:transfer_submitter) get report_index_path assert_redirected_to '/' @@ -78,32 +78,24 @@ def create_thesis_with_whodunnit(user) assert_select 'div.alert', text: 'Not authorized.', count: 1 end - test 'processors can see summary report' do + test 'processors can see report dashboard' do sign_in users(:processor) get report_index_path assert_response :success end - test 'thesis_admins can see summary report' do + test 'thesis_admins can see report dashboard' do sign_in users(:thesis_admin) get report_index_path assert_response :success end - test 'admins can see summary report' do + test 'admins can see report dashboard' do sign_in users(:admin) get report_index_path assert_response :success end - # ~~~~ Dashboard features - - test 'summary report has links to term-specific pages for all terms' do - sign_in users(:processor) - get report_index_path - assert_select 'table:first-of-type thead a', count: Thesis.pluck(:grad_date).uniq.count - end - # ~~~~~~~~~~~~~~~~~~~~ Report empty theses ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ test 'empty theses report exists' do sign_in users(:admin)