From ff9ed873aa289bc633c5539cf133904ac4d1b5ef Mon Sep 17 00:00:00 2001 From: Adam Jazairi Date: Thu, 19 Jan 2023 12:38:24 -0500 Subject: [PATCH] Broaden set of theses returned by not_consented_to_proquest scope Why these changes are being introduced: This is a similar issue to the one solved in #1108, but in the not_consented_to_proquest scope. Here, the scope excludes all theses with an author ProQuest consent value of 'true'. This works in most cases, except when one author has opted in and another hasn't, that thesis should still be partially harvested. With the current logic, it will not show up in the export preview because the scope sees a 'true' consent value and thus excludes the thesis. Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/ETD-603 How this addresses that need: This inverts the logic of the not_consented_to_proquest scope. Instead of excluding theses with at least one ProQuest consent value of true, it includes theses with at least one ProQuest consent value of false or nil, using the 'or' method to return the union. Side effects of this change: Since 'includes' is not compatible with the 'or' method, the scope now uses 'left_joins' to load the author records. This also means that the the result will be an ActiveRecord assocation rather than an array. ActiveJob does not accept associations as params, so we are now casting the params as arrays. I've done with this with both the full and partial export sets to avoid confusion, even though it's only needed at this stage for the partial export set. --- app/controllers/thesis_controller.rb | 2 +- app/models/thesis.rb | 3 ++- test/models/thesis_test.rb | 26 +++++++++++++++++++++----- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/app/controllers/thesis_controller.rb b/app/controllers/thesis_controller.rb index 11b5cc0d..1b9c0d8a 100644 --- a/app/controllers/thesis_controller.rb +++ b/app/controllers/thesis_controller.rb @@ -141,7 +141,7 @@ def proquest_export_preview # TODO: we need to generate and send a budget report CSV for partially harvested theses (spec TBD). def proquest_export if Thesis.ready_for_proquest_export.any? - ProquestExportJob.perform_later(Thesis.partial_proquest_export, Thesis.full_proquest_export) + ProquestExportJob.perform_later(Thesis.partial_proquest_export.to_a, Thesis.full_proquest_export.to_a) flash[:success] = 'The theses you selected will be exported. ' \ 'Status updates are not immediate.' else diff --git a/app/models/thesis.rb b/app/models/thesis.rb index 1c03363a..5c43e96f 100644 --- a/app/models/thesis.rb +++ b/app/models/thesis.rb @@ -122,7 +122,8 @@ class Thesis < ApplicationRecord .where(authors: { proquest_allowed: nil })) } scope :not_consented_to_proquest, lambda { - excluding(includes(authors: :user).where(authors: { proquest_allowed: true })) + left_joins(authors: :user).where(authors: { proquest_allowed: nil }) + .or(where(authors: { proquest_allowed: false })) } scope :exported_to_proquest, -> { where(proquest_exported: ['Partial harvest', 'Full harvest']) } scope :not_exported_to_proquest, -> { where(proquest_exported: 'Not exported') } diff --git a/test/models/thesis_test.rb b/test/models/thesis_test.rb index e9b4f48d..c77d964a 100644 --- a/test/models/thesis_test.rb +++ b/test/models/thesis_test.rb @@ -1318,17 +1318,33 @@ def attach_file_with_purpose_to(thesis, purpose = 'thesis_pdf') thesis = theses(:one) assert_not_includes Thesis.not_consented_to_proquest, thesis + # multi-author thesis that has opted in is excluded + thesis = theses(:two) + assert_not_includes Thesis.not_consented_to_proquest, thesis + # multi-author thesis with all opt-outs is included thesis = theses(:with_hold) assert_includes Thesis.not_consented_to_proquest, thesis - # multi-author thesis with one opt-in and one opt-out is excluded + # multi-author thesis with one opt-in and one opt-out is included thesis = theses(:doctor) - assert_not_includes Thesis.not_consented_to_proquest, thesis + assert_includes Thesis.not_consented_to_proquest, thesis - # multi-author thesis that has opted in is excluded - thesis = theses(:two) - assert_not_includes Thesis.not_consented_to_proquest, thesis + # multi-author thesis with one opt-in and one null is included + assert_equal 2, thesis.authors.count + first_author = thesis.authors.first + second_author = thesis.authors.second + assert_equal true, first_author.proquest_allowed + assert_equal false, second_author.proquest_allowed + + second_author.proquest_allowed = nil + second_author.save + assert_includes Thesis.not_consented_to_proquest, thesis + + # multi-author thesis with one opt-out and one null is included + first_author.proquest_allowed = false + first_author.save + assert_includes Thesis.not_consented_to_proquest, thesis end test 'exported_to_proquest scope includes theses flagged for partial harvest' do