From 044b7f50a732691354114d93feae547985033d01 Mon Sep 17 00:00:00 2001 From: Adam Jazairi Date: Wed, 18 Jan 2023 14:13:00 -0500 Subject: [PATCH] Fix bug in consented_to_proquest scope ignoring null values Why these changes are being introduced: The consented_to_proquest scope returns thesis for which all authors have a proquest_allowed value of true and not false. This ignores author records with a null proquest_allowed value, so if a thesis has multiple authors and at least one of them has not selected an opt-in status, it will incorrectly be flagged as ready for ProQuest export. Relevant ticket(s): https://mitlibraries.atlassian.net/browse/ETD-588 https://mitlibraries.atlassian.net/browse/ETD-603 How this addresses that need: This chains an additional `excluding` call to the scope that checks for authors with null proquest_allowed, and adds regression tests to confirm the correct behavior. Side effects of this change: The additional `excluding` call is a bit ugly, but after fiddling with it for a while I decided it was more important to land a working solution than an elegant one. --- app/models/thesis.rb | 2 ++ test/models/thesis_test.rb | 19 ++++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/app/models/thesis.rb b/app/models/thesis.rb index c4158d4b..1c03363a 100644 --- a/app/models/thesis.rb +++ b/app/models/thesis.rb @@ -118,6 +118,8 @@ class Thesis < ApplicationRecord includes(authors: :user).where(authors: { proquest_allowed: true }) .excluding(includes(authors: :user) .where(authors: { proquest_allowed: false })) + .excluding(includes(authors: :user) + .where(authors: { proquest_allowed: nil })) } scope :not_consented_to_proquest, lambda { excluding(includes(authors: :user).where(authors: { proquest_allowed: true })) diff --git a/test/models/thesis_test.rb b/test/models/thesis_test.rb index 77f46feb..e9b4f48d 100644 --- a/test/models/thesis_test.rb +++ b/test/models/thesis_test.rb @@ -1269,9 +1269,26 @@ def attach_file_with_purpose_to(thesis, purpose = 'thesis_pdf') thesis = theses(:with_hold) assert_not_includes Thesis.consented_to_proquest, thesis - # multi-author thesis with conflicting opt-in statuses is excluded + # multi-author thesis with conflicting opt-in statuses (true, false) is excluded thesis = theses(:doctor) assert_not_includes Thesis.consented_to_proquest, thesis + + # multi-author thesis with conflicting opt-in statuses (true, nil) is excluded + first_author = thesis.authors.first + second_author = thesis.authors.second + assert_equal 2, thesis.authors.count + assert_equal true, first_author.proquest_allowed + + second_author.proquest_allowed = nil + second_author.save + assert_nil second_author.proquest_allowed + assert_not_includes Thesis.consented_to_proquest, thesis + + # multi-author thesis with conflicting opt-in statuses (false, nil) is excluded + first_author.proquest_allowed = false + thesis.save + assert_equal false, first_author.proquest_allowed + assert_not_includes Thesis.consented_to_proquest, thesis end test 'only certain proquest_exported values are valid' do