From c4a879ac0dffcdb7d4afbb0d2d4ba9688112c7b1 Mon Sep 17 00:00:00 2001 From: jazairi <16103405+jazairi@users.noreply.github.com> Date: Fri, 15 Sep 2023 13:49:24 -0400 Subject: [PATCH] Ensure theses are not publishable if their degree period has no accession number Why these changes are being introduced: Because we want the accession number to be used in the preservation workflow, we do not want a thesis without an accession number to be published. Relevant ticket(s): https://mitlibraries.atlassian.net/browse/ETD-596 How this addresses that need: This adds an `accession_number` convenience method to the Thesis model that can look up an accession based on a thesis' degree period. This method is included in the publication checks. The thesis processing form now shows whether it can find an accession number. If no accession number exists, the form will render a hint that prompts the user to create an accession number for the given degree period. Side effects of this change: * A future state of this application is for the Thesis model to belong to the Degree Period model. For now, there is no direct assocation, so we are using ActiveRecord lookups based on the thesis' graduation year and graduation month. * Some fixtures have been added to allow tests related to publication status to pass. (Certain thesis fixtures had no degree periods or accession numbers, they could not be put into publication review.) * The `new` action on the Archivematica Accessions dashboard has been overwritten to prefill the degree period ID from params. --- .../archivematica_accessions_controller.rb | 7 ++ app/models/thesis.rb | 16 +++- app/views/thesis/process_theses.html.erb | 8 ++ test/fixtures/archivematica_accessions.yml | 12 +++ test/fixtures/degree_periods.yml | 12 +++ .../admin_archivematica_accession_test.rb | 14 ++++ test/models/thesis_test.rb | 77 +++++++++++++++++++ 7 files changed, 145 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/archivematica_accessions_controller.rb b/app/controllers/admin/archivematica_accessions_controller.rb index 559b7940..ee8795e4 100644 --- a/app/controllers/admin/archivematica_accessions_controller.rb +++ b/app/controllers/admin/archivematica_accessions_controller.rb @@ -8,6 +8,13 @@ class ArchivematicaAccessionsController < Admin::ApplicationController # send_foo_updated_email(requested_resource) # end + def new + resource = ArchivematicaAccession.new(degree_period_id: params[:degree_period_id]) + render locals: { + page: Administrate::Page::Form.new(dashboard, resource) + } + end + # Override this method to specify custom lookup behavior. # This will be used to set the resource for the `show`, `edit`, and `update` # actions. diff --git a/app/models/thesis.rb b/app/models/thesis.rb index 18826619..ecd64ea7 100644 --- a/app/models/thesis.rb +++ b/app/models/thesis.rb @@ -146,6 +146,19 @@ class Thesis < ApplicationRecord enum proquest_exported: ['Not exported', 'Full harvest', 'Partial harvest'] + # Looks up the thesis' accession number based on its degree period. + def accession_number + degree_period = look_up_degree_period + return if degree_period.nil? + return if degree_period.archivematica_accession.nil? + + degree_period.archivematica_accession.accession_number + end + + def look_up_degree_period + DegreePeriod.find_by(grad_year: graduation_year, grad_month: graduation_month) + end + # Returns a true/false value (rendered as "yes" or "no") if there are any # holds with a status of either 'active' or 'expired'. A false/"No" is # only returned if all holds are 'released'. @@ -189,7 +202,8 @@ def evaluate_status no_active_holds?, authors_graduated?, departments_have_dspace_name?, - degrees_have_types? + degrees_have_types?, + accession_number.present? ].all? end diff --git a/app/views/thesis/process_theses.html.erb b/app/views/thesis/process_theses.html.erb index 5dc33e59..d4cc6482 100644 --- a/app/views/thesis/process_theses.html.erb +++ b/app/views/thesis/process_theses.html.erb @@ -47,6 +47,14 @@ hint_html: { style: 'display: block' }, hint: link_to('See details in admin interface', admin_thesis_path(f.object), target: :_blank) %> +
  • + <%= f.input :accession_number?, as: :string, + readonly: true, + label_html: { style: 'width: 50%' }, + input_html: { class: 'disabled', style: 'width: 40%', value: f.object.accession_number.present?? 'Yes' : 'No' }, + hint_html: { style: 'display: block' }, + hint: (link_to('Create new accession number', new_admin_archivematica_accession_path(degree_period_id: f.object.look_up_degree_period), target: :_blank) if f.object.accession_number.nil?) %> +
  • Issues found diff --git a/test/fixtures/archivematica_accessions.yml b/test/fixtures/archivematica_accessions.yml index fe355190..a38e55ee 100644 --- a/test/fixtures/archivematica_accessions.yml +++ b/test/fixtures/archivematica_accessions.yml @@ -16,3 +16,15 @@ valid_number_and_degree_period: accession_number: '2023_001' degree_period: june_2023 + +september_2017_001: + accession_number: '2017_001' + degree_period: september_2017 + +june_2018_001: + accession_number: '2018_001' + degree_period: june_2018 + +june_2021_001: + accession_number: '2021_001' + degree_period: june_2021 diff --git a/test/fixtures/degree_periods.yml b/test/fixtures/degree_periods.yml index 4e71b5e8..ce4e81eb 100644 --- a/test/fixtures/degree_periods.yml +++ b/test/fixtures/degree_periods.yml @@ -16,3 +16,15 @@ june_2023: no_archivematica_accessions: grad_month: 'May' grad_year: '2024' + +september_2017: + grad_month: 'September' + grad_year: '2017' + +june_2018: + grad_month: 'June' + grad_year: '2018' + +june_2021: + grad_month: 'June' + grad_year: '2021' diff --git a/test/integration/admin/admin_archivematica_accession_test.rb b/test/integration/admin/admin_archivematica_accession_test.rb index 60c0bd30..608f8f37 100644 --- a/test/integration/admin/admin_archivematica_accession_test.rb +++ b/test/integration/admin/admin_archivematica_accession_test.rb @@ -115,4 +115,18 @@ def teardown delete admin_archivematica_accession_path(archivematica_accession) assert_not ArchivematicaAccession.exists?(archivematica_accession_id) end + + test 'new form can be prefilled with degree period' do + mock_auth users(:thesis_admin) + get new_admin_archivematica_accession_path, params: { degree_period_id: degree_periods(:june_2023).id } + assert_select 'select#archivematica_accession_degree_period_id', count: 1 + assert_select 'option', text: 'June 2023', count: 1 + end + + test 'new form can be loaded with no prefilled degree period' do + mock_auth users(:thesis_admin) + get new_admin_archivematica_accession_path + assert_select 'select#archivematica_accession_degree_period_id', count: 1 + assert_select 'option', text: '', count: 1 + end end diff --git a/test/models/thesis_test.rb b/test/models/thesis_test.rb index aa251a3c..2987dbc5 100644 --- a/test/models/thesis_test.rb +++ b/test/models/thesis_test.rb @@ -610,6 +610,7 @@ def attach_file_with_purpose_to(thesis, purpose = 'thesis_pdf') assert_equal true, thesis.no_active_holds? assert_equal true, thesis.departments_have_dspace_name? assert_equal true, thesis.degrees_have_types? + assert_equal true, thesis.accession_number.present? assert_equal 'Publication review', thesis.publication_status # Attempting to set a different status will be overwritten by the update_status method thesis.publication_status = 'Not ready for publication' @@ -1442,4 +1443,80 @@ def attach_file_with_purpose_to(thesis, purpose = 'thesis_pdf') assert_not_includes Thesis.ready_for_proquest_export, wrong_term_full_export_thesis assert_not_includes Thesis.ready_for_proquest_export, no_export_thesis end + + test 'can look up accession number' do + t = theses(:one) + + # a thesis needs a grad date to have an accession number + assert_not t.grad_date.nil? + assert_not t.accession_number.nil? + end + + test 'accession number matches expectations' do + t = theses(:one) + assert t.accession_number.starts_with? t.graduation_year + end + + test 'can look up degree period' do + thesis = theses(:one) + + # Ensure the thesis has a degree period + assert_not_nil DegreePeriod.find_by(grad_year: thesis.graduation_year, grad_month: thesis.graduation_month) + + # Ensure that degree period lookup returns the appropriate record + degree_period = thesis.look_up_degree_period + assert_equal thesis.graduation_year, degree_period.grad_year + assert_equal thesis.graduation_month, degree_period.grad_month + end + + test 'returns nil on degree period lookup if no degree period exists' do + thesis = theses(:one) + + # Ensure the thesis has no degree period + thesis.graduation_year = '3000' + thesis.save + assert_nil DegreePeriod.find_by(grad_year: thesis.graduation_year, grad_month: thesis.graduation_month) + + # Ensure that degree period lookup also returns nil + assert_nil thesis.look_up_degree_period + end + + test 'bachelor theses cannot be put into publication review without accession number' do + t = theses(:bachelor) + t.save + t.reload + assert_equal 'Publication review', t.publication_status + + t.graduation_year = '3000' + t.save + t.reload + assert_nil t.accession_number + assert_not_equal 'Publication review', t.publication_status + end + + test 'master theses cannot be put into publication review without an accession number' do + t = theses(:master) + t.save + t.reload + assert_equal 'Publication review', t.publication_status + + t.graduation_year = '3000' + t.save + t.reload + assert_nil t.accession_number + assert_not_equal 'Publication review', t.publication_status + end + + test 'doctoral theses cannot be put into publication review without an accession number' do + t = theses(:doctor) + t.save + t.reload + assert_equal 'Publication review', t.publication_status + + t.graduation_year = '3000' + t.save + t.reload + assert_nil t.accession_number + assert_not_equal 'Publication review', t.publication_status + end end