From 1bee634a59c907a34829af1f461c064908194c8f Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 21 May 2024 14:31:11 -0400 Subject: [PATCH 01/30] tweaking ingest service, introducing reporting service --- .../tasks/dimensions_ingest_service.rb | 2 +- .../tasks/dimensions_reporting_service.rb | 34 +++++++++++++++++++ .../tasks/dimensions_ingest_service_spec.rb | 7 ++-- 3 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 app/services/tasks/dimensions_reporting_service.rb diff --git a/app/services/tasks/dimensions_ingest_service.rb b/app/services/tasks/dimensions_ingest_service.rb index a3815064e..e3493c192 100644 --- a/app/services/tasks/dimensions_ingest_service.rb +++ b/app/services/tasks/dimensions_ingest_service.rb @@ -19,7 +19,7 @@ def initialize(config) def ingest_publications(publications) time = Time.now Rails.logger.info('Ingesting publications from Dimensions.') - res = {ingested: [], failed: [], time: time} + res = {ingested: [], failed: [], time: time, admin_set_title: @admin_set.title.first, depositor: @config['depositor_onyen']} publications.each.with_index do |publication, index| begin diff --git a/app/services/tasks/dimensions_reporting_service.rb b/app/services/tasks/dimensions_reporting_service.rb new file mode 100644 index 000000000..df0081d68 --- /dev/null +++ b/app/services/tasks/dimensions_reporting_service.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true +module Tasks + class DimensionsReportingService + def initialize(ingested_publications) + @ingested_publications = ingested_publications + end + + def report + Rails.logger.info("Reporting publications from dimensions ingest at #{ingested_publications[:time]} by #{ingested_publications[:depositor]}.") + Rails.logger.info("Admin Set: #{@ingested_publications[:admin_set_title]}") + Rails.logger.info("Depositor: #{@ingested_publications[:depositor]}") + extracted_info = extract_publication_info() + Rails.logger.info("Successfully Ingested:\n") + Rails.logger.info("#{extracted_info[:successfully_ingested].join("\n")}") + Rails.logger.info("Marked for Review:\n") + Rails.logger.info("#{extracted_info[:marked_for_review].join("\n")}") + Rails.logger.info("Failed to Ingest:\n") + Rails.logger.info("#{extracted_info[:failed_to_ingest].join("\n")}") + end + + def extract_publication_info + publication_info = {successfully_ingested: [], failed_to_ingest: [], marked_for_review: []} + @ingested_publications[:ingested].map do |publication| + if publication['marked_for_review'] + publication_info[:marked_for_review] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: #{publication['url']}, PDF Attached: #{publication['pdf_attached'] : 'Yes' : 'No'}" + else + publication_info[:successfully_ingested] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: #{publication['url']}, PDF Attached: #{publication['pdf_attached'] : 'Yes' : 'No'}" + end + end + @ingested_publications[:failed].map do |publication| + publication_info[:failed_to_ingest] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: #{publication['url']}, Error: #{publication['error'][0]} - #{publication['error'][1]}" + end + end + end \ No newline at end of file diff --git a/spec/services/tasks/dimensions_ingest_service_spec.rb b/spec/services/tasks/dimensions_ingest_service_spec.rb index c5eebf76e..1d001649a 100644 --- a/spec/services/tasks/dimensions_ingest_service_spec.rb +++ b/spec/services/tasks/dimensions_ingest_service_spec.rb @@ -138,11 +138,6 @@ describe '#ingest_publications' do it 'processes each publication and handles failures' do failing_publication = test_publications.first - # expected_trace = [ - # "/opt/rh/rh-ruby27/root/usr/share/gems/gems/rspec-mocks-3.12.6/lib/rspec/mocks/message_expectation.rb:188:in `block in and_raise'", - # "/opt/rh/rh-ruby27/root/usr/share/gems/gems/rspec-mocks-3.12.6/lib/rspec/mocks/message_expectation.rb:761:in `block in call'", - # "/opt/rh/rh-ruby27/root/usr/share/gems/gems/rspec-mocks-3.12.6/lib/rspec/mocks/message_expectation.rb:760:in `map'" - # ] test_err_msg = 'Test error' expected_log_outputs = [ "Error ingesting publication '#{failing_publication['title']}'", @@ -158,6 +153,8 @@ expect(Rails.logger).to receive(:error).with(include(expected_log_outputs[1])) expect { res = service.ingest_publications(test_publications) + expect(res[:admin_set_title]).to eq('Open_Access_Articles_and_Book_Chapters') + expect(res[:depositor]).to eq('admin') expect(res[:failed].count).to eq(1) expect(res[:failed].first[:publication]).to eq(failing_publication) expect(res[:failed].first[:error]).to eq([StandardError.to_s, test_err_msg]) From 82489336b48ddbfe80c23ee56c205b3bae84783a Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 21 May 2024 14:40:49 -0400 Subject: [PATCH 02/30] syntax, including test suite --- .../tasks/dimensions_reporting_service.rb | 7 +- .../dimensions_reporting_service_spec.rb | 92 +++++++++++++++++++ 2 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 spec/services/tasks/dimensions_reporting_service_spec.rb diff --git a/app/services/tasks/dimensions_reporting_service.rb b/app/services/tasks/dimensions_reporting_service.rb index df0081d68..d565d194d 100644 --- a/app/services/tasks/dimensions_reporting_service.rb +++ b/app/services/tasks/dimensions_reporting_service.rb @@ -22,13 +22,14 @@ def extract_publication_info publication_info = {successfully_ingested: [], failed_to_ingest: [], marked_for_review: []} @ingested_publications[:ingested].map do |publication| if publication['marked_for_review'] - publication_info[:marked_for_review] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: #{publication['url']}, PDF Attached: #{publication['pdf_attached'] : 'Yes' : 'No'}" + publication_info[:marked_for_review] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: #{publication['url']}, PDF Attached: #{publication['pdf_attached'] ? 'Yes' : 'No'}" else - publication_info[:successfully_ingested] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: #{publication['url']}, PDF Attached: #{publication['pdf_attached'] : 'Yes' : 'No'}" + publication_info[:successfully_ingested] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: #{publication['url']}, PDF Attached: #{publication['pdf_attached'] ? 'Yes' : 'No'}" end end @ingested_publications[:failed].map do |publication| publication_info[:failed_to_ingest] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: #{publication['url']}, Error: #{publication['error'][0]} - #{publication['error'][1]}" end end - end \ No newline at end of file + end +end \ No newline at end of file diff --git a/spec/services/tasks/dimensions_reporting_service_spec.rb b/spec/services/tasks/dimensions_reporting_service_spec.rb new file mode 100644 index 000000000..f694933a3 --- /dev/null +++ b/spec/services/tasks/dimensions_reporting_service_spec.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Tasks::DimensionsReportingService do + let(:config) { + { + 'admin_set' => 'Open_Access_Articles_and_Book_Chapters', + 'depositor_onyen' => 'admin' + } + } + let(:dimensions_ingest_test_fixture) do + File.read(File.join(Rails.root, '/spec/fixtures/files/dimensions_ingest_test_fixture.json')) + end + let(:admin) { FactoryBot.create(:admin) } + let(:service) { described_class.new(config) } + + let(:admin_set) do + FactoryBot.create(:admin_set, title: ['Open_Access_Articles_and_Book_Chapters']) + end + let(:permission_template) do + FactoryBot.create(:permission_template, source_id: admin_set.id) + end + let(:workflow) do + FactoryBot.create(:workflow, permission_template_id: permission_template.id, active: true) + end + let(:workflow_state) do + FactoryBot.create(:workflow_state, workflow_id: workflow.id, name: 'deposited') + end + let(:pdf_content) { File.binread(File.join(Rails.root, '/spec/fixtures/files/sample_pdf.pdf')) } + + + # Retrieving fixture publications and randomly assigning the marked_for_review attribute + let(:test_publications) do + JSON.parse(dimensions_ingest_test_fixture)['publications'] + end + + + before do + ActiveFedora::Cleaner.clean! + admin_set + permission_template + workflow + workflow_state + allow(User).to receive(:find_by).with(uid: 'admin').and_return(admin) + allow(AdminSet).to receive(:where).with(title: 'Open_Access_Articles_and_Book_Chapters').and_return([admin_set]) + stub_request(:head, 'https://test-url.com/') + .to_return(status: 200, headers: { 'Content-Type' => 'application/pdf' }) + stub_request(:get, 'https://test-url.com/') + .to_return(body: pdf_content, status: 200, headers: { 'Content-Type' => 'application/pdf' }) + # stub virus checking + allow(Hyrax::VirusCheckerService).to receive(:file_has_virus?) { false } + # stub longleaf job + allow(RegisterToLongleafJob).to receive(:perform_later).and_return(nil) + # stub FITS characterization + allow(CharacterizeJob).to receive(:perform_later) + end + + describe '#initialize' do + it 'successfully initializes the service' do + expect { described_class.new([]) }.not_to raise_error + end + end + +# describe '#ingest_publications' do +# it 'processes each publication and handles failures' do +# failing_publication = test_publications.first +# test_err_msg = 'Test error' +# expected_log_outputs = [ +# "Error ingesting publication '#{failing_publication['title']}'", +# [StandardError.to_s, test_err_msg].join($RS) +# ] +# ingested_publications = test_publications[1..-1] + +# # Stub the process_publication method to raise an error for the first publication only +# allow(service).to receive(:process_publication).and_call_original +# allow(service).to receive(:process_publication).with(failing_publication).and_raise(StandardError, test_err_msg) + +# expect(Rails.logger).to receive(:error).with(expected_log_outputs[0]) +# expect(Rails.logger).to receive(:error).with(include(expected_log_outputs[1])) +# expect { +# res = service.ingest_publications(test_publications) +# expect(res[:admin_set_title]).to eq('Open_Access_Articles_and_Book_Chapters') +# expect(res[:depositor]).to eq('admin') +# expect(res[:failed].count).to eq(1) +# expect(res[:failed].first[:publication]).to eq(failing_publication) +# expect(res[:failed].first[:error]).to eq([StandardError.to_s, test_err_msg]) +# expect(res[:ingested]).to match_array(ingested_publications) +# expect(res[:time]).to be_a(Time) +# }.to change { Article.count }.by(ingested_publications.size) +# end +# end +end From 96ebbbe796cd93ac5373a6eed767c44024f2f27c Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 21 May 2024 15:44:24 -0400 Subject: [PATCH 03/30] small changes to reporting service and ingest --- .../tasks/dimensions_ingest_service.rb | 2 +- .../tasks/dimensions_reporting_service.rb | 4 +- .../tasks/dimensions_ingest_service_spec.rb | 15 ++-- .../dimensions_reporting_service_spec.rb | 73 ++++++++++--------- 4 files changed, 52 insertions(+), 42 deletions(-) diff --git a/app/services/tasks/dimensions_ingest_service.rb b/app/services/tasks/dimensions_ingest_service.rb index e3493c192..9106799b3 100644 --- a/app/services/tasks/dimensions_ingest_service.rb +++ b/app/services/tasks/dimensions_ingest_service.rb @@ -27,7 +27,7 @@ def ingest_publications(publications) process_publication(publication) res[:ingested] << publication rescue StandardError => e - res[:failed] << { publication: publication, error: [e.class.to_s, e.message] } + res[:failed] << publication.merge('error' => [e.class.to_s, e.message]) Rails.logger.error("Error ingesting publication '#{publication['title']}'") Rails.logger.error [e.class.to_s, e.message, *e.backtrace].join($RS) end diff --git a/app/services/tasks/dimensions_reporting_service.rb b/app/services/tasks/dimensions_reporting_service.rb index d565d194d..f2b2f0c63 100644 --- a/app/services/tasks/dimensions_reporting_service.rb +++ b/app/services/tasks/dimensions_reporting_service.rb @@ -6,7 +6,7 @@ def initialize(ingested_publications) end def report - Rails.logger.info("Reporting publications from dimensions ingest at #{ingested_publications[:time]} by #{ingested_publications[:depositor]}.") + Rails.logger.info("Reporting publications from dimensions ingest at #{@ingested_publications[:time]} by #{@ingested_publications[:depositor]}.") Rails.logger.info("Admin Set: #{@ingested_publications[:admin_set_title]}") Rails.logger.info("Depositor: #{@ingested_publications[:depositor]}") extracted_info = extract_publication_info() @@ -28,8 +28,10 @@ def extract_publication_info end end @ingested_publications[:failed].map do |publication| + puts "Inspecting failed publication: #{publication}" publication_info[:failed_to_ingest] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: #{publication['url']}, Error: #{publication['error'][0]} - #{publication['error'][1]}" end + publication_info end end end \ No newline at end of file diff --git a/spec/services/tasks/dimensions_ingest_service_spec.rb b/spec/services/tasks/dimensions_ingest_service_spec.rb index 1d001649a..5caa0888e 100644 --- a/spec/services/tasks/dimensions_ingest_service_spec.rb +++ b/spec/services/tasks/dimensions_ingest_service_spec.rb @@ -28,8 +28,6 @@ end let(:pdf_content) { File.binread(File.join(Rails.root, '/spec/fixtures/files/sample_pdf.pdf')) } - - # Retrieving fixture publications and randomly assigning the marked_for_review attribute let(:test_publications) do JSON.parse(dimensions_ingest_test_fixture)['publications'] end @@ -137,27 +135,30 @@ describe '#ingest_publications' do it 'processes each publication and handles failures' do - failing_publication = test_publications.first + expected_failing_publication = test_publications.first test_err_msg = 'Test error' expected_log_outputs = [ - "Error ingesting publication '#{failing_publication['title']}'", + "Error ingesting publication '#{expected_failing_publication['title']}'", [StandardError.to_s, test_err_msg].join($RS) ] ingested_publications = test_publications[1..-1] # Stub the process_publication method to raise an error for the first publication only allow(service).to receive(:process_publication).and_call_original - allow(service).to receive(:process_publication).with(failing_publication).and_raise(StandardError, test_err_msg) + allow(service).to receive(:process_publication).with(expected_failing_publication).and_raise(StandardError, test_err_msg) expect(Rails.logger).to receive(:error).with(expected_log_outputs[0]) expect(Rails.logger).to receive(:error).with(include(expected_log_outputs[1])) expect { res = service.ingest_publications(test_publications) + actual_failed_publication = res[:failed].first + actual_failed_publication_error = res[:failed].first['error'] + actual_failed_publication.delete('error') expect(res[:admin_set_title]).to eq('Open_Access_Articles_and_Book_Chapters') expect(res[:depositor]).to eq('admin') expect(res[:failed].count).to eq(1) - expect(res[:failed].first[:publication]).to eq(failing_publication) - expect(res[:failed].first[:error]).to eq([StandardError.to_s, test_err_msg]) + expect(actual_failed_publication).to eq(expected_failing_publication) + expect(actual_failed_publication_error).to eq([StandardError.to_s, test_err_msg]) expect(res[:ingested]).to match_array(ingested_publications) expect(res[:time]).to be_a(Time) }.to change { Article.count }.by(ingested_publications.size) diff --git a/spec/services/tasks/dimensions_reporting_service_spec.rb b/spec/services/tasks/dimensions_reporting_service_spec.rb index f694933a3..a5ad213a9 100644 --- a/spec/services/tasks/dimensions_reporting_service_spec.rb +++ b/spec/services/tasks/dimensions_reporting_service_spec.rb @@ -12,7 +12,7 @@ File.read(File.join(Rails.root, '/spec/fixtures/files/dimensions_ingest_test_fixture.json')) end let(:admin) { FactoryBot.create(:admin) } - let(:service) { described_class.new(config) } + let(:ingest_service) { Tasks::DimensionsIngestService.new(config) } let(:admin_set) do FactoryBot.create(:admin_set, title: ['Open_Access_Articles_and_Book_Chapters']) @@ -55,38 +55,45 @@ allow(CharacterizeJob).to receive(:perform_later) end - describe '#initialize' do - it 'successfully initializes the service' do - expect { described_class.new([]) }.not_to raise_error - end - end - -# describe '#ingest_publications' do -# it 'processes each publication and handles failures' do -# failing_publication = test_publications.first -# test_err_msg = 'Test error' -# expected_log_outputs = [ -# "Error ingesting publication '#{failing_publication['title']}'", -# [StandardError.to_s, test_err_msg].join($RS) -# ] -# ingested_publications = test_publications[1..-1] + describe '#report' do + it 'generates a report for ingest dimensions publications' do + # Splitting the test publications into three groups: failing, marked_for_review, and ingested + failing_publication_sample = test_publications[0..2] + marked_for_review_sample = test_publications[3..5] + marked_for_review_sample.each { |pub| pub['marked_for_review'] = true } + ingested_publications = test_publications[5..-1] + test_err_msg = 'Test error' + + # expected_log_outputs = failing_publication_sample.flat_map do |pub| + # "Error ingesting publication '#{pub['title']}'", + # [StandardError.to_s, test_err_msg].join($RS) + # end + # expected_log_outputs = [ + # "Error ingesting publication '#{failing_publication['title']}'", + # [StandardError.to_s, test_err_msg].join($RS) + # ] + + # Stub the process_publication method to raise an error for the three first publications + allow(ingest_service).to receive(:process_publication).and_call_original + allow(ingest_service).to receive(:process_publication).with(satisfy { |pub| failing_publication_sample.include?(pub) }).and_raise(StandardError, test_err_msg) + + ingested_publications = ingest_service.ingest_publications(test_publications) + described_class.new(ingested_publications).report -# # Stub the process_publication method to raise an error for the first publication only -# allow(service).to receive(:process_publication).and_call_original -# allow(service).to receive(:process_publication).with(failing_publication).and_raise(StandardError, test_err_msg) -# expect(Rails.logger).to receive(:error).with(expected_log_outputs[0]) -# expect(Rails.logger).to receive(:error).with(include(expected_log_outputs[1])) -# expect { -# res = service.ingest_publications(test_publications) -# expect(res[:admin_set_title]).to eq('Open_Access_Articles_and_Book_Chapters') -# expect(res[:depositor]).to eq('admin') -# expect(res[:failed].count).to eq(1) -# expect(res[:failed].first[:publication]).to eq(failing_publication) -# expect(res[:failed].first[:error]).to eq([StandardError.to_s, test_err_msg]) -# expect(res[:ingested]).to match_array(ingested_publications) -# expect(res[:time]).to be_a(Time) -# }.to change { Article.count }.by(ingested_publications.size) -# end -# end + # expected_log_outputs.each do |expected_log_output| + # expect(Rails.logger).to receive(:error).with(include(expected_log_output)) + # end + # expect { + # res = service.ingest_publications(test_publications) + # expect(res[:admin_set_title]).to eq('Open_Access_Articles_and_Book_Chapters') + # expect(res[:depositor]).to eq('admin') + # expect(res[:failed].count).to eq(1) + # expect(res[:failed].first[:publication]).to eq(failing_publication) + # expect(res[:failed].first[:error]).to eq([StandardError.to_s, test_err_msg]) + # expect(res[:ingested]).to match_array(ingested_publications) + # expect(res[:time]).to be_a(Time) + # }.to change { Article.count }.by(ingested_publications.size) + end + end end From d78b5c8d24141c20e5146372d0f850dbe98ed425 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 21 May 2024 18:40:22 -0400 Subject: [PATCH 04/30] tests for report generation helper function --- .../tasks/dimensions_reporting_service.rb | 24 +++-- .../dimensions_reporting_service_spec.rb | 101 ++++++++++-------- 2 files changed, 71 insertions(+), 54 deletions(-) diff --git a/app/services/tasks/dimensions_reporting_service.rb b/app/services/tasks/dimensions_reporting_service.rb index f2b2f0c63..7cecc5a05 100644 --- a/app/services/tasks/dimensions_reporting_service.rb +++ b/app/services/tasks/dimensions_reporting_service.rb @@ -5,17 +5,20 @@ def initialize(ingested_publications) @ingested_publications = ingested_publications end - def report - Rails.logger.info("Reporting publications from dimensions ingest at #{@ingested_publications[:time]} by #{@ingested_publications[:depositor]}.") - Rails.logger.info("Admin Set: #{@ingested_publications[:admin_set_title]}") - Rails.logger.info("Depositor: #{@ingested_publications[:depositor]}") + def generate_report + res = [] extracted_info = extract_publication_info() - Rails.logger.info("Successfully Ingested:\n") - Rails.logger.info("#{extracted_info[:successfully_ingested].join("\n")}") - Rails.logger.info("Marked for Review:\n") - Rails.logger.info("#{extracted_info[:marked_for_review].join("\n")}") - Rails.logger.info("Failed to Ingest:\n") - Rails.logger.info("#{extracted_info[:failed_to_ingest].join("\n")}") + formatted_time = @ingested_publications[:time].strftime("%B %d, %Y at %I:%M %p %Z") + res << "Reporting dimensions ingest results from job at #{formatted_time} by #{@ingested_publications[:depositor]}." + res << "Admin Set: #{@ingested_publications[:admin_set_title]}" + res << "Total Publications: #{extracted_info[:successfully_ingested].length + extracted_info[:failed_to_ingest].length + extracted_info[:marked_for_review].length}" + res << "\nSuccessfully Ingested: (#{extracted_info[:successfully_ingested].length} Publications)" + res << extracted_info[:successfully_ingested].join("\n") + res << "\nMarked for Review: (#{extracted_info[:marked_for_review].length} Publications)" + res << extracted_info[:marked_for_review].join("\n") + res << "\nFailed to Ingest: (#{extracted_info[:failed_to_ingest].length} Publications)" + res << extracted_info[:failed_to_ingest].join("\n") + res.join("\n") end def extract_publication_info @@ -28,7 +31,6 @@ def extract_publication_info end end @ingested_publications[:failed].map do |publication| - puts "Inspecting failed publication: #{publication}" publication_info[:failed_to_ingest] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: #{publication['url']}, Error: #{publication['error'][0]} - #{publication['error'][1]}" end publication_info diff --git a/spec/services/tasks/dimensions_reporting_service_spec.rb b/spec/services/tasks/dimensions_reporting_service_spec.rb index a5ad213a9..830a4bcff 100644 --- a/spec/services/tasks/dimensions_reporting_service_spec.rb +++ b/spec/services/tasks/dimensions_reporting_service_spec.rb @@ -13,7 +13,6 @@ end let(:admin) { FactoryBot.create(:admin) } let(:ingest_service) { Tasks::DimensionsIngestService.new(config) } - let(:admin_set) do FactoryBot.create(:admin_set, title: ['Open_Access_Articles_and_Book_Chapters']) end @@ -27,12 +26,20 @@ FactoryBot.create(:workflow_state, workflow_id: workflow.id, name: 'deposited') end let(:pdf_content) { File.binread(File.join(Rails.root, '/spec/fixtures/files/sample_pdf.pdf')) } - - - # Retrieving fixture publications and randomly assigning the marked_for_review attribute let(:test_publications) do - JSON.parse(dimensions_ingest_test_fixture)['publications'] + res = JSON.parse(dimensions_ingest_test_fixture)['publications'] + res[3..5].each { |pub| pub['marked_for_review'] = true } + res + end + let(:ingested_publications) do + ingest_service.ingest_publications(test_publications) end + let(:test_err_msg) { 'Test error' } + let(:service) { described_class.new(ingested_publications) } + let(:failing_publication_sample) { test_publications[0..2] } + let(:marked_for_review_sample) { test_publications[3..5] } + let(:successful_publication_sample) { test_publications[6..-1] } + before do @@ -41,12 +48,20 @@ permission_template workflow workflow_state + failing_publication_sample + marked_for_review_sample + successful_publication_sample allow(User).to receive(:find_by).with(uid: 'admin').and_return(admin) allow(AdminSet).to receive(:where).with(title: 'Open_Access_Articles_and_Book_Chapters').and_return([admin_set]) stub_request(:head, 'https://test-url.com/') .to_return(status: 200, headers: { 'Content-Type' => 'application/pdf' }) stub_request(:get, 'https://test-url.com/') .to_return(body: pdf_content, status: 200, headers: { 'Content-Type' => 'application/pdf' }) + # Splitting the test publications into three groups: failing, marked_for_review, and ingested + allow(ingest_service).to receive(:process_publication).and_call_original + allow(ingest_service).to receive(:process_publication).with(satisfy { |pub| failing_publication_sample.include?(pub) }).and_raise(StandardError, test_err_msg) + ingested_publications + service # stub virus checking allow(Hyrax::VirusCheckerService).to receive(:file_has_virus?) { false } # stub longleaf job @@ -55,45 +70,45 @@ allow(CharacterizeJob).to receive(:perform_later) end - describe '#report' do + describe '#generate_report' do it 'generates a report for ingest dimensions publications' do - # Splitting the test publications into three groups: failing, marked_for_review, and ingested - failing_publication_sample = test_publications[0..2] - marked_for_review_sample = test_publications[3..5] - marked_for_review_sample.each { |pub| pub['marked_for_review'] = true } - ingested_publications = test_publications[5..-1] - test_err_msg = 'Test error' - - # expected_log_outputs = failing_publication_sample.flat_map do |pub| - # "Error ingesting publication '#{pub['title']}'", - # [StandardError.to_s, test_err_msg].join($RS) - # end - # expected_log_outputs = [ - # "Error ingesting publication '#{failing_publication['title']}'", - # [StandardError.to_s, test_err_msg].join($RS) - # ] - - # Stub the process_publication method to raise an error for the three first publications - allow(ingest_service).to receive(:process_publication).and_call_original - allow(ingest_service).to receive(:process_publication).with(satisfy { |pub| failing_publication_sample.include?(pub) }).and_raise(StandardError, test_err_msg) - - ingested_publications = ingest_service.ingest_publications(test_publications) - described_class.new(ingested_publications).report - + report = service.generate_report + puts "Report: #{report}" + # expect(report).to include("Reporting publications from dimensions ingest at #{ingested_publications[:time]} by admin.") + # expect(report).to include("Admin Set: Open_Access_Articles_and_Book_Chapters") + # expect(report).to include("Depositor: admin") + # expect(report).to include({"Successfully Ingested:" => failing_publication_sample}) + # expect(report).to include("Marked for Review:") + # expect(report).to include("Failed to Ingest:") + # for failing_publication in failing_publication_sample + # expect(report).to include("Title: #{failing_publication['title']}, ID: #{failing_publication['id']}, URL: #{failing_publication['url']}, Error: StandardError - #{test_err_msg}") + # end + # expect(report).to include("Title: #{marked_for_review_sample[0]['title']}, ID: #{marked_for_review_sample[0]['id']}, URL: #{marked_for_review_sample[0]['url']}, PDF Attached: No") + # expect(report).to include("Title: #{ingested_publications[:ingested][0]['title']}, ID: #{ingested_publications[:ingested][0]['id']}, URL: #{ingested_publications[:ingested][0]['url']}, PDF Attached: Yes") + end + end + + describe '#extract_publication_info' do + def expect_publication_info(info_array, sample_array, additional_check = nil) + info_array.each_with_index do |info, i| + expect(info).to include("Title: #{sample_array[i]['title']}") + expect(info).to include("ID: #{sample_array[i]['id']}") + expect(info).to include("URL: #{sample_array[i]['url']}") + additional_check.call(info, i) if additional_check + end + end - # expected_log_outputs.each do |expected_log_output| - # expect(Rails.logger).to receive(:error).with(include(expected_log_output)) - # end - # expect { - # res = service.ingest_publications(test_publications) - # expect(res[:admin_set_title]).to eq('Open_Access_Articles_and_Book_Chapters') - # expect(res[:depositor]).to eq('admin') - # expect(res[:failed].count).to eq(1) - # expect(res[:failed].first[:publication]).to eq(failing_publication) - # expect(res[:failed].first[:error]).to eq([StandardError.to_s, test_err_msg]) - # expect(res[:ingested]).to match_array(ingested_publications) - # expect(res[:time]).to be_a(Time) - # }.to change { Article.count }.by(ingested_publications.size) + it 'extracts publication information for the report' do + extracted_info = service.extract_publication_info + expect(extracted_info[:successfully_ingested].length).to eq(4) + expect(extracted_info[:marked_for_review].length).to eq(3) + expect(extracted_info[:failed_to_ingest].length).to eq(3) + + expect_publication_info(extracted_info[:successfully_ingested], successful_publication_sample) + expect_publication_info(extracted_info[:marked_for_review], marked_for_review_sample) + expect_publication_info(extracted_info[:failed_to_ingest], failing_publication_sample, lambda { |info, i| + expect(info).to include("Error: StandardError - #{test_err_msg}") + }) end end -end +end \ No newline at end of file From e1b8e2a3e2ba5af48a72e70e304e1bcc78ca4c50 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 21 May 2024 20:04:14 -0400 Subject: [PATCH 05/30] slight phrasing change, updating test --- .../tasks/dimensions_reporting_service.rb | 2 +- .../dimensions_reporting_service_spec.rb | 19 ++++++++----------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/app/services/tasks/dimensions_reporting_service.rb b/app/services/tasks/dimensions_reporting_service.rb index 7cecc5a05..32be2b3e0 100644 --- a/app/services/tasks/dimensions_reporting_service.rb +++ b/app/services/tasks/dimensions_reporting_service.rb @@ -9,7 +9,7 @@ def generate_report res = [] extracted_info = extract_publication_info() formatted_time = @ingested_publications[:time].strftime("%B %d, %Y at %I:%M %p %Z") - res << "Reporting dimensions ingest results from job at #{formatted_time} by #{@ingested_publications[:depositor]}." + res << "Reporting publications from dimensions ingest at #{formatted_time} by #{@ingested_publications[:depositor]}." res << "Admin Set: #{@ingested_publications[:admin_set_title]}" res << "Total Publications: #{extracted_info[:successfully_ingested].length + extracted_info[:failed_to_ingest].length + extracted_info[:marked_for_review].length}" res << "\nSuccessfully Ingested: (#{extracted_info[:successfully_ingested].length} Publications)" diff --git a/spec/services/tasks/dimensions_reporting_service_spec.rb b/spec/services/tasks/dimensions_reporting_service_spec.rb index 830a4bcff..1297e3c19 100644 --- a/spec/services/tasks/dimensions_reporting_service_spec.rb +++ b/spec/services/tasks/dimensions_reporting_service_spec.rb @@ -51,6 +51,8 @@ failing_publication_sample marked_for_review_sample successful_publication_sample + fixed_time = Time.new(2024, 5, 21, 10, 0, 0) + allow(Time).to receive(:now).and_return(fixed_time) allow(User).to receive(:find_by).with(uid: 'admin').and_return(admin) allow(AdminSet).to receive(:where).with(title: 'Open_Access_Articles_and_Book_Chapters').and_return([admin_set]) stub_request(:head, 'https://test-url.com/') @@ -74,17 +76,12 @@ it 'generates a report for ingest dimensions publications' do report = service.generate_report puts "Report: #{report}" - # expect(report).to include("Reporting publications from dimensions ingest at #{ingested_publications[:time]} by admin.") - # expect(report).to include("Admin Set: Open_Access_Articles_and_Book_Chapters") - # expect(report).to include("Depositor: admin") - # expect(report).to include({"Successfully Ingested:" => failing_publication_sample}) - # expect(report).to include("Marked for Review:") - # expect(report).to include("Failed to Ingest:") - # for failing_publication in failing_publication_sample - # expect(report).to include("Title: #{failing_publication['title']}, ID: #{failing_publication['id']}, URL: #{failing_publication['url']}, Error: StandardError - #{test_err_msg}") - # end - # expect(report).to include("Title: #{marked_for_review_sample[0]['title']}, ID: #{marked_for_review_sample[0]['id']}, URL: #{marked_for_review_sample[0]['url']}, PDF Attached: No") - # expect(report).to include("Title: #{ingested_publications[:ingested][0]['title']}, ID: #{ingested_publications[:ingested][0]['id']}, URL: #{ingested_publications[:ingested][0]['url']}, PDF Attached: Yes") + expect(report).to include("Reporting publications from dimensions ingest at May 21, 2024 at 10:00 AM UTC by admin.") + .and include("Admin Set: Open_Access_Articles_and_Book_Chapters") + .and include("Total Publications: #{test_publications.length}") + .and include("Successfully Ingested: (#{successful_publication_sample.length} Publications)") + .and include("Marked for Review: (#{marked_for_review_sample.length} Publications)") + .and include("Failed to Ingest: (#{failing_publication_sample.length} Publications)") end end From 31a7cecd43f32cfae4bc076330bc9a65f8f73001 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Thu, 23 May 2024 15:36:29 -0400 Subject: [PATCH 06/30] changes to ingest and reporting service to include cdr urls in email reports --- .../tasks/dimensions_ingest_service.rb | 5 ++- .../tasks/dimensions_reporting_service.rb | 6 +-- .../dimensions_reporting_service_spec.rb | 42 ++++++++----------- 3 files changed, 23 insertions(+), 30 deletions(-) diff --git a/app/services/tasks/dimensions_ingest_service.rb b/app/services/tasks/dimensions_ingest_service.rb index 9106799b3..3ca242b3c 100644 --- a/app/services/tasks/dimensions_ingest_service.rb +++ b/app/services/tasks/dimensions_ingest_service.rb @@ -24,8 +24,8 @@ def ingest_publications(publications) publications.each.with_index do |publication, index| begin next unless publication.presence - process_publication(publication) - res[:ingested] << publication + article = process_publication(publication) + res[:ingested] << publication.merge('article_id' => article.id) rescue StandardError => e res[:failed] << publication.merge('error' => [e.class.to_s, e.message]) Rails.logger.error("Error ingesting publication '#{publication['title']}'") @@ -54,6 +54,7 @@ def article_with_metadata(publication) article.visibility = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PRIVATE article.permissions_attributes = group_permissions(@admin_set) article.save! + # puts "Article Inspection: #{article.inspect}" article end diff --git a/app/services/tasks/dimensions_reporting_service.rb b/app/services/tasks/dimensions_reporting_service.rb index 32be2b3e0..d6c11e9a7 100644 --- a/app/services/tasks/dimensions_reporting_service.rb +++ b/app/services/tasks/dimensions_reporting_service.rb @@ -25,13 +25,13 @@ def extract_publication_info publication_info = {successfully_ingested: [], failed_to_ingest: [], marked_for_review: []} @ingested_publications[:ingested].map do |publication| if publication['marked_for_review'] - publication_info[:marked_for_review] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: #{publication['url']}, PDF Attached: #{publication['pdf_attached'] ? 'Yes' : 'No'}" + publication_info[:marked_for_review] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: https://cdr.lib.unc.edu/concern/articles/#{publication['article_id']}?locale=en, PDF Attached: #{publication['pdf_attached'] ? 'Yes' : 'No'}" else - publication_info[:successfully_ingested] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: #{publication['url']}, PDF Attached: #{publication['pdf_attached'] ? 'Yes' : 'No'}" + publication_info[:successfully_ingested] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: https://cdr.lib.unc.edu/concern/articles/#{publication['article_id']}?locale=en, PDF Attached: #{publication['pdf_attached'] ? 'Yes' : 'No'}" end end @ingested_publications[:failed].map do |publication| - publication_info[:failed_to_ingest] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: #{publication['url']}, Error: #{publication['error'][0]} - #{publication['error'][1]}" + publication_info[:failed_to_ingest] << "Title: #{publication['title']}, ID: #{publication['id']}, Error: #{publication['error'][0]} - #{publication['error'][1]}" end publication_info end diff --git a/spec/services/tasks/dimensions_reporting_service_spec.rb b/spec/services/tasks/dimensions_reporting_service_spec.rb index 1297e3c19..97c8c2d7f 100644 --- a/spec/services/tasks/dimensions_reporting_service_spec.rb +++ b/spec/services/tasks/dimensions_reporting_service_spec.rb @@ -8,6 +8,7 @@ 'depositor_onyen' => 'admin' } } + let(:service) { described_class.new(ingested_publications) } let(:dimensions_ingest_test_fixture) do File.read(File.join(Rails.root, '/spec/fixtures/files/dimensions_ingest_test_fixture.json')) end @@ -26,20 +27,19 @@ FactoryBot.create(:workflow_state, workflow_id: workflow.id, name: 'deposited') end let(:pdf_content) { File.binread(File.join(Rails.root, '/spec/fixtures/files/sample_pdf.pdf')) } - let(:test_publications) do - res = JSON.parse(dimensions_ingest_test_fixture)['publications'] - res[3..5].each { |pub| pub['marked_for_review'] = true } - res + let(:test_err_msg) { 'Test error' } + let(:fixed_time) { Time.new(2024, 5, 21, 10, 0, 0) } + let(:test_publications) { JSON.parse(dimensions_ingest_test_fixture)['publications'] } + let(:failing_publication_sample) { test_publications[0..2] } + let(:successful_publication_sample) { test_publications[6..-1] } + let(:marked_for_review_sample) do + test_publications[3..5].each { |pub| pub['marked_for_review'] = true } + test_publications[3..5] end let(:ingested_publications) do ingest_service.ingest_publications(test_publications) end - let(:test_err_msg) { 'Test error' } - let(:service) { described_class.new(ingested_publications) } - let(:failing_publication_sample) { test_publications[0..2] } - let(:marked_for_review_sample) { test_publications[3..5] } - let(:successful_publication_sample) { test_publications[6..-1] } - + before do @@ -48,10 +48,6 @@ permission_template workflow workflow_state - failing_publication_sample - marked_for_review_sample - successful_publication_sample - fixed_time = Time.new(2024, 5, 21, 10, 0, 0) allow(Time).to receive(:now).and_return(fixed_time) allow(User).to receive(:find_by).with(uid: 'admin').and_return(admin) allow(AdminSet).to receive(:where).with(title: 'Open_Access_Articles_and_Book_Chapters').and_return([admin_set]) @@ -59,11 +55,10 @@ .to_return(status: 200, headers: { 'Content-Type' => 'application/pdf' }) stub_request(:get, 'https://test-url.com/') .to_return(body: pdf_content, status: 200, headers: { 'Content-Type' => 'application/pdf' }) - # Splitting the test publications into three groups: failing, marked_for_review, and ingested allow(ingest_service).to receive(:process_publication).and_call_original allow(ingest_service).to receive(:process_publication).with(satisfy { |pub| failing_publication_sample.include?(pub) }).and_raise(StandardError, test_err_msg) + marked_for_review_sample ingested_publications - service # stub virus checking allow(Hyrax::VirusCheckerService).to receive(:file_has_virus?) { false } # stub longleaf job @@ -75,7 +70,6 @@ describe '#generate_report' do it 'generates a report for ingest dimensions publications' do report = service.generate_report - puts "Report: #{report}" expect(report).to include("Reporting publications from dimensions ingest at May 21, 2024 at 10:00 AM UTC by admin.") .and include("Admin Set: Open_Access_Articles_and_Book_Chapters") .and include("Total Publications: #{test_publications.length}") @@ -86,12 +80,12 @@ end describe '#extract_publication_info' do - def expect_publication_info(info_array, sample_array, additional_check = nil) + def expect_publication_info(info_array, sample_array, failed) info_array.each_with_index do |info, i| expect(info).to include("Title: #{sample_array[i]['title']}") expect(info).to include("ID: #{sample_array[i]['id']}") - expect(info).to include("URL: #{sample_array[i]['url']}") - additional_check.call(info, i) if additional_check + expect(info).to include("URL: https://cdr.lib.unc.edu/concern/articles/#{sample_array[i]['article_id']}?locale=en") if !failed + expect(info).to include("Error: StandardError - #{test_err_msg}") if failed end end @@ -101,11 +95,9 @@ def expect_publication_info(info_array, sample_array, additional_check = nil) expect(extracted_info[:marked_for_review].length).to eq(3) expect(extracted_info[:failed_to_ingest].length).to eq(3) - expect_publication_info(extracted_info[:successfully_ingested], successful_publication_sample) - expect_publication_info(extracted_info[:marked_for_review], marked_for_review_sample) - expect_publication_info(extracted_info[:failed_to_ingest], failing_publication_sample, lambda { |info, i| - expect(info).to include("Error: StandardError - #{test_err_msg}") - }) + expect_publication_info(extracted_info[:successfully_ingested], ingested_publications[:ingested].select { |pub| !pub['marked_for_review'] }, false) + expect_publication_info(extracted_info[:marked_for_review], ingested_publications[:ingested].select { |pub| pub['marked_for_review'] }, false) + expect_publication_info(extracted_info[:failed_to_ingest], ingested_publications[:failed], true) end end end \ No newline at end of file From 668b778e7a640f18bfe68a593645af8e8cdb0138 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Fri, 24 May 2024 17:47:06 -0400 Subject: [PATCH 07/30] configuration for action mailer, slight refactor to report generation --- app/mailers/dimensions_report_mailer.rb | 6 ++ .../tasks/dimensions_reporting_service.rb | 29 +++--- .../report_email.html.erb | 90 +++++++++++++++++++ config/environments/development.rb | 11 +++ .../dimensions_report_mailer_preview.rb | 17 ++++ 5 files changed, 139 insertions(+), 14 deletions(-) create mode 100644 app/mailers/dimensions_report_mailer.rb create mode 100644 app/views/dimensions_report_mailer/report_email.html.erb create mode 100644 spec/mailers/previews/dimensions_report_mailer_preview.rb diff --git a/app/mailers/dimensions_report_mailer.rb b/app/mailers/dimensions_report_mailer.rb new file mode 100644 index 000000000..5870cc13d --- /dev/null +++ b/app/mailers/dimensions_report_mailer.rb @@ -0,0 +1,6 @@ +class DimensionsReportMailer < ApplicationMailer + def report_email(report) + @report = report + mail(to: 'recipient@example.com', subject: report[:subject]) + end +end diff --git a/app/services/tasks/dimensions_reporting_service.rb b/app/services/tasks/dimensions_reporting_service.rb index d6c11e9a7..918a38ec5 100644 --- a/app/services/tasks/dimensions_reporting_service.rb +++ b/app/services/tasks/dimensions_reporting_service.rb @@ -6,32 +6,33 @@ def initialize(ingested_publications) end def generate_report - res = [] + report = { successfully_ingested_rows: [], marked_for_review_rows: [], failed_to_ingest_rows: [], subject: [], headers: { }} extracted_info = extract_publication_info() formatted_time = @ingested_publications[:time].strftime("%B %d, %Y at %I:%M %p %Z") - res << "Reporting publications from dimensions ingest at #{formatted_time} by #{@ingested_publications[:depositor]}." - res << "Admin Set: #{@ingested_publications[:admin_set_title]}" - res << "Total Publications: #{extracted_info[:successfully_ingested].length + extracted_info[:failed_to_ingest].length + extracted_info[:marked_for_review].length}" - res << "\nSuccessfully Ingested: (#{extracted_info[:successfully_ingested].length} Publications)" - res << extracted_info[:successfully_ingested].join("\n") - res << "\nMarked for Review: (#{extracted_info[:marked_for_review].length} Publications)" - res << extracted_info[:marked_for_review].join("\n") - res << "\nFailed to Ingest: (#{extracted_info[:failed_to_ingest].length} Publications)" - res << extracted_info[:failed_to_ingest].join("\n") - res.join("\n") + report[:subject] = "Dimensions Ingest Report for #{formatted_time}" + report[:headers][:reporting_message] = "Reporting publications from dimensions ingest at #{formatted_time} by #{@ingested_publications[:depositor]}." + report[:headers][:admin_set] = "Admin Set: #{@ingested_publications[:admin_set_title]}" + report[:headers][:total_publications] = "Total Publications: #{extracted_info[:successfully_ingested].length + extracted_info[:failed_to_ingest].length + extracted_info[:marked_for_review].length}" + report[:headers][:successfully_ingested] = "\nSuccessfully Ingested: (#{extracted_info[:successfully_ingested].length} Publications)" + report[:headers][:marked_for_review] = "\nMarked for Review: (#{extracted_info[:marked_for_review].length} Publications)" + report[:headers][:failed_to_ingest] = "\nFailed to Ingest: (#{extracted_info[:failed_to_ingest].length} Publications)" + report[:successfully_ingested_rows] = extracted_info[:successfully_ingested] + report[:marked_for_review_rows] = extracted_info[:marked_for_review] + report[:failed_to_ingest_rows] = extracted_info[:failed_to_ingest] + report end def extract_publication_info publication_info = {successfully_ingested: [], failed_to_ingest: [], marked_for_review: []} @ingested_publications[:ingested].map do |publication| if publication['marked_for_review'] - publication_info[:marked_for_review] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: https://cdr.lib.unc.edu/concern/articles/#{publication['article_id']}?locale=en, PDF Attached: #{publication['pdf_attached'] ? 'Yes' : 'No'}" + publication_info[:marked_for_review] << { title: publication['title'], id: publication['id'], url: "https://cdr.lib.unc.edu/concern/articles/#{publication['article_id']}?locale=en", pdf_attached: publication['pdf_attached'] ? 'Yes' : 'No' } else - publication_info[:successfully_ingested] << "Title: #{publication['title']}, ID: #{publication['id']}, URL: https://cdr.lib.unc.edu/concern/articles/#{publication['article_id']}?locale=en, PDF Attached: #{publication['pdf_attached'] ? 'Yes' : 'No'}" + publication_info[:successfully_ingested] << { title: publication['title'], id: publication['id'], url: "https://cdr.lib.unc.edu/concern/articles/#{publication['article_id']}?locale=en", pdf_attached: publication['pdf_attached'] ? 'Yes' : 'No' } end end @ingested_publications[:failed].map do |publication| - publication_info[:failed_to_ingest] << "Title: #{publication['title']}, ID: #{publication['id']}, Error: #{publication['error'][0]} - #{publication['error'][1]}" + publication_info[:failed_to_ingest] << { title: publication['title'], id: publication['id'], error: "#{publication['error'][0]} - #{publication['error'][1]}" } end publication_info end diff --git a/app/views/dimensions_report_mailer/report_email.html.erb b/app/views/dimensions_report_mailer/report_email.html.erb new file mode 100644 index 000000000..31006dbc6 --- /dev/null +++ b/app/views/dimensions_report_mailer/report_email.html.erb @@ -0,0 +1,90 @@ + + + + + + + + +

<%= @report[:headers][:@reporting_message] %>

+

<%= @report[:headers][:admin_set] %>

+

<%= @report[:headers][:total_publications] %>

+ +

<%= @report[:headers][:successfully_ingested] %>

+ + + + + + + + + + + <% @report[:successfully_ingested_rows].each do |publication| %> + + + + + + + <% end %> + +
TitleIDURLPDF Attached
<%= publication[:title] %><%= publication[:id] %><%= publication[:url] %><%= publication[:pdf_attached] %>
+ +

<%= @report[:headers][:marked_for_review] %>

+ + + + + + + + + + + <% @report[:marked_for_review_rows].each do |publication| %> + + + + + + + <% end %> + +
TitleIDURLPDF Attached
<%= publication[:title] %><%= publication[:id] %><%= publication[:url] %><%= publication[:pdf_attached] %>
+ +

<%= @report[:headers][:failed_to_ingest] %>

+ + + + + + + + + + <% @report[:failed_to_ingest_rows].each do |publication| %> + + + + + + <% end %> + +
TitleIDError
<%= publication[:title] %><%= publication[:id] %><%= publication[:error] %>
+ + diff --git a/config/environments/development.rb b/config/environments/development.rb index 3c190cb92..ec713a73c 100755 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -89,5 +89,16 @@ 'web' # Allow this to be addressed when running in containers via docker-compose.yml. ] + config.action_mailer.delivery_method = :smtp + config.action_mailer.smtp_settings = { + address: 'smtp.office365.com', + port: 587, + domain: 'localhost', + user_name: ENV['EMAIL_USERNAME'], + password: ENV['EMAIL_PASSWORD'], + authentication: 'plain', + enable_starttls_auto: true + } + config.action_mailer.default_url_options = { host: 'localhost', port: 3000 } config.log_level = LogService.log_level end diff --git a/spec/mailers/previews/dimensions_report_mailer_preview.rb b/spec/mailers/previews/dimensions_report_mailer_preview.rb new file mode 100644 index 000000000..4fd70c656 --- /dev/null +++ b/spec/mailers/previews/dimensions_report_mailer_preview.rb @@ -0,0 +1,17 @@ +# Preview all emails at http://localhost:3000/rails/mailers/dimensions_report_mailer +class DimensionsReportMailerPreview < ActionMailer::Preview + def report_email + # WIP: Ensuring template works after an ingest with a test fixture, removing later + dimensions_ingest_test_fixture = File.read(File.join(Rails.root, '/spec/fixtures/files/dimensions_ingest_test_fixture.json')) + test_publications = JSON.parse(dimensions_ingest_test_fixture)['publications'] + config = { + 'admin_set' => 'default', + 'depositor_onyen' => 'admin' + } + dimensions_ingest_service = Tasks::DimensionsIngestService.new(config) + ingested_publications = dimensions_ingest_service.ingest_publications(test_publications) + dimensions_reporting_service = Tasks::DimensionsReportingService.new(ingested_publications) + report = dimensions_reporting_service.generate_report + DimensionsReportMailer.report_email(report) + end +end From a4857f1134eb398693c52a05905a9b25868cb931 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Fri, 24 May 2024 17:48:11 -0400 Subject: [PATCH 08/30] syntax --- app/views/dimensions_report_mailer/report_email.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/dimensions_report_mailer/report_email.html.erb b/app/views/dimensions_report_mailer/report_email.html.erb index 31006dbc6..eb2fbf708 100644 --- a/app/views/dimensions_report_mailer/report_email.html.erb +++ b/app/views/dimensions_report_mailer/report_email.html.erb @@ -19,7 +19,7 @@ -

<%= @report[:headers][:@reporting_message] %>

+

<%= @report[:headers][:reporting_message] %>

<%= @report[:headers][:admin_set] %>

<%= @report[:headers][:total_publications] %>

From 3a7a21eb9ed01add7b72ef6168d075cd1d2db988 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Fri, 24 May 2024 18:14:43 -0400 Subject: [PATCH 09/30] test refactoring --- .../dimensions_reporting_service_spec.rb | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/spec/services/tasks/dimensions_reporting_service_spec.rb b/spec/services/tasks/dimensions_reporting_service_spec.rb index 97c8c2d7f..d44d0307e 100644 --- a/spec/services/tasks/dimensions_reporting_service_spec.rb +++ b/spec/services/tasks/dimensions_reporting_service_spec.rb @@ -70,22 +70,24 @@ describe '#generate_report' do it 'generates a report for ingest dimensions publications' do report = service.generate_report - expect(report).to include("Reporting publications from dimensions ingest at May 21, 2024 at 10:00 AM UTC by admin.") - .and include("Admin Set: Open_Access_Articles_and_Book_Chapters") - .and include("Total Publications: #{test_publications.length}") - .and include("Successfully Ingested: (#{successful_publication_sample.length} Publications)") - .and include("Marked for Review: (#{marked_for_review_sample.length} Publications)") - .and include("Failed to Ingest: (#{failing_publication_sample.length} Publications)") + headers = report[:headers] + expect(report[:subject]).to eq("Dimensions Ingest Report for May 21, 2024 at 10:00 AM UTC") + expect(headers[:reporting_message]).to eq("Reporting publications from dimensions ingest at May 21, 2024 at 10:00 AM UTC by admin.") + expect(headers[:admin_set]).to eq("Admin Set: Open_Access_Articles_and_Book_Chapters") + expect(headers[:total_publications]).to eq("Total Publications: #{test_publications.length}") + expect(headers[:successfully_ingested]).to eq("\nSuccessfully Ingested: (#{successful_publication_sample.length} Publications)") + expect(headers[:marked_for_review]).to eq("\nMarked for Review: (#{marked_for_review_sample.length} Publications)") + expect(headers[:failed_to_ingest]).to eq("\nFailed to Ingest: (#{failing_publication_sample.length} Publications)") end end describe '#extract_publication_info' do def expect_publication_info(info_array, sample_array, failed) info_array.each_with_index do |info, i| - expect(info).to include("Title: #{sample_array[i]['title']}") - expect(info).to include("ID: #{sample_array[i]['id']}") - expect(info).to include("URL: https://cdr.lib.unc.edu/concern/articles/#{sample_array[i]['article_id']}?locale=en") if !failed - expect(info).to include("Error: StandardError - #{test_err_msg}") if failed + expect(info[:title]).to eq(sample_array[i]['title']) + expect(info[:id]).to eq(sample_array[i]['id']) + expect(info[:url]).to eq("https://cdr.lib.unc.edu/concern/articles/#{sample_array[i]['article_id']}?locale=en") if !failed + expect(info[:error]).to eq("StandardError - #{test_err_msg}") if failed end end From d6b76faeda9fe63a9af36aa648482281da6b80ab Mon Sep 17 00:00:00 2001 From: David Campbell Date: Fri, 24 May 2024 19:16:12 -0400 Subject: [PATCH 10/30] rubocop, mailer preview and tests --- app/mailers/application_mailer.rb | 2 +- app/mailers/dimensions_report_mailer.rb | 8 +- .../tasks/dimensions_reporting_service.rb | 68 +++++------ spec/mailers/dimensions_report_mailer_spec.rb | 111 ++++++++++++++++++ .../dimensions_report_mailer_preview.rb | 28 ++--- .../tasks/dimensions_ingest_service_spec.rb | 2 +- .../dimensions_reporting_service_spec.rb | 47 ++++---- 7 files changed, 192 insertions(+), 74 deletions(-) create mode 100644 spec/mailers/dimensions_report_mailer_spec.rb diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 24289009a..ed90960d6 100755 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true class ApplicationMailer < ActionMailer::Base - default from: 'from@example.com' + default from: 'no-reply@example.com' layout 'mailer' end diff --git a/app/mailers/dimensions_report_mailer.rb b/app/mailers/dimensions_report_mailer.rb index 5870cc13d..da98ea2fe 100644 --- a/app/mailers/dimensions_report_mailer.rb +++ b/app/mailers/dimensions_report_mailer.rb @@ -1,6 +1,6 @@ class DimensionsReportMailer < ApplicationMailer - def report_email(report) - @report = report - mail(to: 'recipient@example.com', subject: report[:subject]) - end + def report_email(report) + @report = report + mail(to: 'recipient@example.com', subject: report[:subject]) + end end diff --git a/app/services/tasks/dimensions_reporting_service.rb b/app/services/tasks/dimensions_reporting_service.rb index 918a38ec5..7a53f7971 100644 --- a/app/services/tasks/dimensions_reporting_service.rb +++ b/app/services/tasks/dimensions_reporting_service.rb @@ -1,40 +1,40 @@ # frozen_string_literal: true module Tasks - class DimensionsReportingService - def initialize(ingested_publications) - @ingested_publications = ingested_publications - end + class DimensionsReportingService + def initialize(ingested_publications) + @ingested_publications = ingested_publications + end - def generate_report - report = { successfully_ingested_rows: [], marked_for_review_rows: [], failed_to_ingest_rows: [], subject: [], headers: { }} - extracted_info = extract_publication_info() - formatted_time = @ingested_publications[:time].strftime("%B %d, %Y at %I:%M %p %Z") - report[:subject] = "Dimensions Ingest Report for #{formatted_time}" - report[:headers][:reporting_message] = "Reporting publications from dimensions ingest at #{formatted_time} by #{@ingested_publications[:depositor]}." - report[:headers][:admin_set] = "Admin Set: #{@ingested_publications[:admin_set_title]}" - report[:headers][:total_publications] = "Total Publications: #{extracted_info[:successfully_ingested].length + extracted_info[:failed_to_ingest].length + extracted_info[:marked_for_review].length}" - report[:headers][:successfully_ingested] = "\nSuccessfully Ingested: (#{extracted_info[:successfully_ingested].length} Publications)" - report[:headers][:marked_for_review] = "\nMarked for Review: (#{extracted_info[:marked_for_review].length} Publications)" - report[:headers][:failed_to_ingest] = "\nFailed to Ingest: (#{extracted_info[:failed_to_ingest].length} Publications)" - report[:successfully_ingested_rows] = extracted_info[:successfully_ingested] - report[:marked_for_review_rows] = extracted_info[:marked_for_review] - report[:failed_to_ingest_rows] = extracted_info[:failed_to_ingest] - report - end + def generate_report + report = { successfully_ingested_rows: [], marked_for_review_rows: [], failed_to_ingest_rows: [], subject: [], headers: { }} + extracted_info = extract_publication_info + formatted_time = @ingested_publications[:time].strftime('%B %d, %Y at %I:%M %p %Z') + report[:subject] = "Dimensions Ingest Report for #{formatted_time}" + report[:headers][:reporting_message] = "Reporting publications from dimensions ingest at #{formatted_time} by #{@ingested_publications[:depositor]}." + report[:headers][:admin_set] = "Admin Set: #{@ingested_publications[:admin_set_title]}" + report[:headers][:total_publications] = "Total Publications: #{extracted_info[:successfully_ingested].length + extracted_info[:failed_to_ingest].length + extracted_info[:marked_for_review].length}" + report[:headers][:successfully_ingested] = "\nSuccessfully Ingested: (#{extracted_info[:successfully_ingested].length} Publications)" + report[:headers][:marked_for_review] = "\nMarked for Review: (#{extracted_info[:marked_for_review].length} Publications)" + report[:headers][:failed_to_ingest] = "\nFailed to Ingest: (#{extracted_info[:failed_to_ingest].length} Publications)" + report[:successfully_ingested_rows] = extracted_info[:successfully_ingested] + report[:marked_for_review_rows] = extracted_info[:marked_for_review] + report[:failed_to_ingest_rows] = extracted_info[:failed_to_ingest] + report + end - def extract_publication_info - publication_info = {successfully_ingested: [], failed_to_ingest: [], marked_for_review: []} - @ingested_publications[:ingested].map do |publication| - if publication['marked_for_review'] - publication_info[:marked_for_review] << { title: publication['title'], id: publication['id'], url: "https://cdr.lib.unc.edu/concern/articles/#{publication['article_id']}?locale=en", pdf_attached: publication['pdf_attached'] ? 'Yes' : 'No' } - else - publication_info[:successfully_ingested] << { title: publication['title'], id: publication['id'], url: "https://cdr.lib.unc.edu/concern/articles/#{publication['article_id']}?locale=en", pdf_attached: publication['pdf_attached'] ? 'Yes' : 'No' } - end - end - @ingested_publications[:failed].map do |publication| - publication_info[:failed_to_ingest] << { title: publication['title'], id: publication['id'], error: "#{publication['error'][0]} - #{publication['error'][1]}" } - end - publication_info + def extract_publication_info + publication_info = {successfully_ingested: [], failed_to_ingest: [], marked_for_review: []} + @ingested_publications[:ingested].map do |publication| + if publication['marked_for_review'] + publication_info[:marked_for_review] << { title: publication['title'], id: publication['id'], url: "https://cdr.lib.unc.edu/concern/articles/#{publication['article_id']}?locale=en", pdf_attached: publication['pdf_attached'] ? 'Yes' : 'No' } + else + publication_info[:successfully_ingested] << { title: publication['title'], id: publication['id'], url: "https://cdr.lib.unc.edu/concern/articles/#{publication['article_id']}?locale=en", pdf_attached: publication['pdf_attached'] ? 'Yes' : 'No' } end + end + @ingested_publications[:failed].map do |publication| + publication_info[:failed_to_ingest] << { title: publication['title'], id: publication['id'], error: "#{publication['error'][0]} - #{publication['error'][1]}" } + end + publication_info end -end \ No newline at end of file + end +end diff --git a/spec/mailers/dimensions_report_mailer_spec.rb b/spec/mailers/dimensions_report_mailer_spec.rb new file mode 100644 index 000000000..8027b4f41 --- /dev/null +++ b/spec/mailers/dimensions_report_mailer_spec.rb @@ -0,0 +1,111 @@ +# spec/mailers/dimensions_report_mailer_spec.rb +require "rails_helper" + +RSpec.describe DimensionsReportMailer, type: :mailer do + let(:config) { + { + 'admin_set' => 'Open_Access_Articles_and_Book_Chapters', + 'depositor_onyen' => 'admin' + } + } + let(:dimensions_ingest_test_fixture) do + File.read(File.join(Rails.root, '/spec/fixtures/files/dimensions_ingest_test_fixture.json')) + end + + let(:admin) { FactoryBot.create(:admin) } + let(:admin_set) do + FactoryBot.create(:admin_set, title: ['Open_Access_Articles_and_Book_Chapters']) + end + let(:permission_template) do + FactoryBot.create(:permission_template, source_id: admin_set.id) + end + let(:workflow) do + FactoryBot.create(:workflow, permission_template_id: permission_template.id, active: true) + end + let(:workflow_state) do + FactoryBot.create(:workflow_state, workflow_id: workflow.id, name: 'deposited') + end + + let(:pdf_content) { File.binread(File.join(Rails.root, '/spec/fixtures/files/sample_pdf.pdf')) } + let(:test_err_msg) { 'Test error' } + + let(:fixed_time) { Time.new(2024, 5, 21, 10, 0, 0) } + let(:test_publications) { JSON.parse(dimensions_ingest_test_fixture)['publications'] } + + let(:failing_publication_sample) { test_publications[0..2] } + let(:marked_for_review_sample) do + test_publications[3..5].each { |pub| pub['marked_for_review'] = true } + test_publications[3..5] + end + let(:successful_publication_sample) { test_publications[6..-1] } + + let(:ingest_service) { Tasks::DimensionsIngestService.new(config) } + let(:ingested_publications) do + ingest_service.ingest_publications(test_publications) + end + let(:report) { Tasks::DimensionsReportingService.new(ingested_publications).generate_report } + + before do + ActiveFedora::Cleaner.clean! + admin_set + permission_template + workflow + workflow_state + allow(Time).to receive(:now).and_return(fixed_time) + allow(User).to receive(:find_by).with(uid: 'admin').and_return(admin) + allow(AdminSet).to receive(:where).with(title: 'Open_Access_Articles_and_Book_Chapters').and_return([admin_set]) + stub_request(:head, 'https://test-url.com/') + .to_return(status: 200, headers: { 'Content-Type' => 'application/pdf' }) + stub_request(:get, 'https://test-url.com/') + .to_return(body: pdf_content, status: 200, headers: { 'Content-Type' => 'application/pdf' }) + allow(ingest_service).to receive(:process_publication).and_call_original + allow(ingest_service).to receive(:process_publication).with(satisfy { |pub| failing_publication_sample.include?(pub) }).and_raise(StandardError, test_err_msg) + marked_for_review_sample + ingested_publications + # stub virus checking + allow(Hyrax::VirusCheckerService).to receive(:file_has_virus?) { false } + # stub longleaf job + allow(RegisterToLongleafJob).to receive(:perform_later).and_return(nil) + # stub FITS characterization + allow(CharacterizeJob).to receive(:perform_later) + end + + describe 'report_email' do + let(:mail) { DimensionsReportMailer.report_email(report) } + + it 'renders the headers' do + expect(mail.subject).to eq(report[:subject]) + expect(mail.to).to eq(['recipient@example.com']) + expect(mail.from).to eq(['no-reply@example.com']) + end + + it 'renders the body' do + expect(mail.body.encoded).to include(report[:headers][:reporting_message]) + .and include(report[:headers][:admin_set]) + .and include(report[:headers][:total_publications]) + .and include(report[:headers][:successfully_ingested]) + .and include(report[:headers][:marked_for_review]) + .and include(report[:headers][:failed_to_ingest]) + + report[:successfully_ingested_rows].each do |publication| + expect(mail.body.encoded).to include(publication[:title]) + .and include(publication[:id]) + .and include(publication[:url]) + .and include(publication[:pdf_attached]) + end + + report[:marked_for_review_rows].each do |publication| + expect(mail.body.encoded).to include(publication[:title]) + .and include(publication[:id]) + .and include(publication[:url]) + .and include(publication[:pdf_attached]) + end + + report[:failed_to_ingest_rows].each do |publication| + expect(mail.body.encoded).to include(publication[:title]) + .and include(publication[:id]) + .and include(publication[:error]) + end + end + end +end diff --git a/spec/mailers/previews/dimensions_report_mailer_preview.rb b/spec/mailers/previews/dimensions_report_mailer_preview.rb index 4fd70c656..e9ac10aeb 100644 --- a/spec/mailers/previews/dimensions_report_mailer_preview.rb +++ b/spec/mailers/previews/dimensions_report_mailer_preview.rb @@ -1,17 +1,17 @@ # Preview all emails at http://localhost:3000/rails/mailers/dimensions_report_mailer class DimensionsReportMailerPreview < ActionMailer::Preview - def report_email - # WIP: Ensuring template works after an ingest with a test fixture, removing later - dimensions_ingest_test_fixture = File.read(File.join(Rails.root, '/spec/fixtures/files/dimensions_ingest_test_fixture.json')) - test_publications = JSON.parse(dimensions_ingest_test_fixture)['publications'] - config = { - 'admin_set' => 'default', - 'depositor_onyen' => 'admin' - } - dimensions_ingest_service = Tasks::DimensionsIngestService.new(config) - ingested_publications = dimensions_ingest_service.ingest_publications(test_publications) - dimensions_reporting_service = Tasks::DimensionsReportingService.new(ingested_publications) - report = dimensions_reporting_service.generate_report - DimensionsReportMailer.report_email(report) - end + def report_email + # Ensuring template works after an ingest with a test fixture + dimensions_ingest_test_fixture = File.read(File.join(Rails.root, '/spec/fixtures/files/dimensions_ingest_test_fixture.json')) + test_publications = JSON.parse(dimensions_ingest_test_fixture)['publications'] + config = { + 'admin_set' => 'default', + 'depositor_onyen' => 'admin' + } + dimensions_ingest_service = Tasks::DimensionsIngestService.new(config) + ingested_publications = dimensions_ingest_service.ingest_publications(test_publications) + dimensions_reporting_service = Tasks::DimensionsReportingService.new(ingested_publications) + report = dimensions_reporting_service.generate_report + DimensionsReportMailer.report_email(report) + end end diff --git a/spec/services/tasks/dimensions_ingest_service_spec.rb b/spec/services/tasks/dimensions_ingest_service_spec.rb index 5caa0888e..1a228d26d 100644 --- a/spec/services/tasks/dimensions_ingest_service_spec.rb +++ b/spec/services/tasks/dimensions_ingest_service_spec.rb @@ -153,7 +153,7 @@ res = service.ingest_publications(test_publications) actual_failed_publication = res[:failed].first actual_failed_publication_error = res[:failed].first['error'] - actual_failed_publication.delete('error') + actual_failed_publication.delete('error') expect(res[:admin_set_title]).to eq('Open_Access_Articles_and_Book_Chapters') expect(res[:depositor]).to eq('admin') expect(res[:failed].count).to eq(1) diff --git a/spec/services/tasks/dimensions_reporting_service_spec.rb b/spec/services/tasks/dimensions_reporting_service_spec.rb index d44d0307e..b6da06533 100644 --- a/spec/services/tasks/dimensions_reporting_service_spec.rb +++ b/spec/services/tasks/dimensions_reporting_service_spec.rb @@ -8,12 +8,11 @@ 'depositor_onyen' => 'admin' } } - let(:service) { described_class.new(ingested_publications) } let(:dimensions_ingest_test_fixture) do File.read(File.join(Rails.root, '/spec/fixtures/files/dimensions_ingest_test_fixture.json')) end + let(:admin) { FactoryBot.create(:admin) } - let(:ingest_service) { Tasks::DimensionsIngestService.new(config) } let(:admin_set) do FactoryBot.create(:admin_set, title: ['Open_Access_Articles_and_Book_Chapters']) end @@ -26,20 +25,28 @@ let(:workflow_state) do FactoryBot.create(:workflow_state, workflow_id: workflow.id, name: 'deposited') end + let(:pdf_content) { File.binread(File.join(Rails.root, '/spec/fixtures/files/sample_pdf.pdf')) } - let(:test_err_msg) { 'Test error' } + let(:test_err_msg) { 'Test error' } + let(:fixed_time) { Time.new(2024, 5, 21, 10, 0, 0) } let(:test_publications) { JSON.parse(dimensions_ingest_test_fixture)['publications'] } + let(:failing_publication_sample) { test_publications[0..2] } - let(:successful_publication_sample) { test_publications[6..-1] } let(:marked_for_review_sample) do test_publications[3..5].each { |pub| pub['marked_for_review'] = true } test_publications[3..5] end + let(:successful_publication_sample) { test_publications[6..-1] } + + let(:ingest_service) { Tasks::DimensionsIngestService.new(config) } let(:ingested_publications) do ingest_service.ingest_publications(test_publications) end - + + let(:service) { described_class.new(ingested_publications) } + + before do @@ -71,35 +78,35 @@ it 'generates a report for ingest dimensions publications' do report = service.generate_report headers = report[:headers] - expect(report[:subject]).to eq("Dimensions Ingest Report for May 21, 2024 at 10:00 AM UTC") - expect(headers[:reporting_message]).to eq("Reporting publications from dimensions ingest at May 21, 2024 at 10:00 AM UTC by admin.") - expect(headers[:admin_set]).to eq("Admin Set: Open_Access_Articles_and_Book_Chapters") + expect(report[:subject]).to eq('Dimensions Ingest Report for May 21, 2024 at 10:00 AM UTC') + expect(headers[:reporting_message]).to eq('Reporting publications from dimensions ingest at May 21, 2024 at 10:00 AM UTC by admin.') + expect(headers[:admin_set]).to eq('Admin Set: Open_Access_Articles_and_Book_Chapters') expect(headers[:total_publications]).to eq("Total Publications: #{test_publications.length}") expect(headers[:successfully_ingested]).to eq("\nSuccessfully Ingested: (#{successful_publication_sample.length} Publications)") expect(headers[:marked_for_review]).to eq("\nMarked for Review: (#{marked_for_review_sample.length} Publications)") expect(headers[:failed_to_ingest]).to eq("\nFailed to Ingest: (#{failing_publication_sample.length} Publications)") end end - + describe '#extract_publication_info' do def expect_publication_info(info_array, sample_array, failed) - info_array.each_with_index do |info, i| + info_array.each_with_index do |info, i| expect(info[:title]).to eq(sample_array[i]['title']) expect(info[:id]).to eq(sample_array[i]['id']) expect(info[:url]).to eq("https://cdr.lib.unc.edu/concern/articles/#{sample_array[i]['article_id']}?locale=en") if !failed expect(info[:error]).to eq("StandardError - #{test_err_msg}") if failed - end + end end it 'extracts publication information for the report' do - extracted_info = service.extract_publication_info - expect(extracted_info[:successfully_ingested].length).to eq(4) - expect(extracted_info[:marked_for_review].length).to eq(3) - expect(extracted_info[:failed_to_ingest].length).to eq(3) - - expect_publication_info(extracted_info[:successfully_ingested], ingested_publications[:ingested].select { |pub| !pub['marked_for_review'] }, false) - expect_publication_info(extracted_info[:marked_for_review], ingested_publications[:ingested].select { |pub| pub['marked_for_review'] }, false) - expect_publication_info(extracted_info[:failed_to_ingest], ingested_publications[:failed], true) + extracted_info = service.extract_publication_info + expect(extracted_info[:successfully_ingested].length).to eq(4) + expect(extracted_info[:marked_for_review].length).to eq(3) + expect(extracted_info[:failed_to_ingest].length).to eq(3) + + expect_publication_info(extracted_info[:successfully_ingested], ingested_publications[:ingested].select { |pub| !pub['marked_for_review'] }, false) + expect_publication_info(extracted_info[:marked_for_review], ingested_publications[:ingested].select { |pub| pub['marked_for_review'] }, false) + expect_publication_info(extracted_info[:failed_to_ingest], ingested_publications[:failed], true) end end -end \ No newline at end of file +end From 862ed0f0a1987c3f29c1f2f7504a250eb4d12a57 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Fri, 24 May 2024 19:17:30 -0400 Subject: [PATCH 11/30] removing comment --- app/services/tasks/dimensions_ingest_service.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/services/tasks/dimensions_ingest_service.rb b/app/services/tasks/dimensions_ingest_service.rb index 3ca242b3c..3f7c44edc 100644 --- a/app/services/tasks/dimensions_ingest_service.rb +++ b/app/services/tasks/dimensions_ingest_service.rb @@ -54,7 +54,6 @@ def article_with_metadata(publication) article.visibility = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PRIVATE article.permissions_attributes = group_permissions(@admin_set) article.save! - # puts "Article Inspection: #{article.inspect}" article end From 5db8bab3171988d2638b79890e4ea03f59ff3502 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Fri, 24 May 2024 19:57:53 -0400 Subject: [PATCH 12/30] rubocop, renaming some stuff, updating preview class --- app/mailers/dimensions_report_mailer.rb | 2 +- ...l.html.erb => dimensions_report_email.erb} | 2 +- spec/mailers/dimensions_report_mailer_spec.rb | 6 +++--- .../dimensions_report_mailer_preview.rb | 21 ++++++++++++++++--- 4 files changed, 23 insertions(+), 8 deletions(-) rename app/views/dimensions_report_mailer/{report_email.html.erb => dimensions_report_email.erb} (96%) diff --git a/app/mailers/dimensions_report_mailer.rb b/app/mailers/dimensions_report_mailer.rb index da98ea2fe..f86dedbd3 100644 --- a/app/mailers/dimensions_report_mailer.rb +++ b/app/mailers/dimensions_report_mailer.rb @@ -1,5 +1,5 @@ class DimensionsReportMailer < ApplicationMailer - def report_email(report) + def dimensions_report_email(report) @report = report mail(to: 'recipient@example.com', subject: report[:subject]) end diff --git a/app/views/dimensions_report_mailer/report_email.html.erb b/app/views/dimensions_report_mailer/dimensions_report_email.erb similarity index 96% rename from app/views/dimensions_report_mailer/report_email.html.erb rename to app/views/dimensions_report_mailer/dimensions_report_email.erb index eb2fbf708..70ab9faf6 100644 --- a/app/views/dimensions_report_mailer/report_email.html.erb +++ b/app/views/dimensions_report_mailer/dimensions_report_email.erb @@ -1,4 +1,4 @@ - + diff --git a/spec/mailers/dimensions_report_mailer_spec.rb b/spec/mailers/dimensions_report_mailer_spec.rb index 8027b4f41..b44e088ea 100644 --- a/spec/mailers/dimensions_report_mailer_spec.rb +++ b/spec/mailers/dimensions_report_mailer_spec.rb @@ -1,5 +1,5 @@ # spec/mailers/dimensions_report_mailer_spec.rb -require "rails_helper" +require 'rails_helper' RSpec.describe DimensionsReportMailer, type: :mailer do let(:config) { @@ -70,8 +70,8 @@ allow(CharacterizeJob).to receive(:perform_later) end - describe 'report_email' do - let(:mail) { DimensionsReportMailer.report_email(report) } + describe 'dimensions_report_email' do + let(:mail) { DimensionsReportMailer.dimensions_report_email(report) } it 'renders the headers' do expect(mail.subject).to eq(report[:subject]) diff --git a/spec/mailers/previews/dimensions_report_mailer_preview.rb b/spec/mailers/previews/dimensions_report_mailer_preview.rb index e9ac10aeb..28f3e4025 100644 --- a/spec/mailers/previews/dimensions_report_mailer_preview.rb +++ b/spec/mailers/previews/dimensions_report_mailer_preview.rb @@ -1,17 +1,32 @@ # Preview all emails at http://localhost:3000/rails/mailers/dimensions_report_mailer class DimensionsReportMailerPreview < ActionMailer::Preview - def report_email - # Ensuring template works after an ingest with a test fixture + def dimensions_report_email + # Ensuring template works with a report generated after an ingest with a test fixture dimensions_ingest_test_fixture = File.read(File.join(Rails.root, '/spec/fixtures/files/dimensions_ingest_test_fixture.json')) test_publications = JSON.parse(dimensions_ingest_test_fixture)['publications'] + + # Marking some publications for review + test_publications[3..5].each { |pub| pub['marked_for_review'] = true } + marked_for_review_sample = test_publications[3..5] + # successful_publication_sample = test_publications[6..-1] config = { 'admin_set' => 'default', 'depositor_onyen' => 'admin' } dimensions_ingest_service = Tasks::DimensionsIngestService.new(config) ingested_publications = dimensions_ingest_service.ingest_publications(test_publications) + + # Moving publications in the failing publication sample to the failed array and adding an error message + failing_publication_sample = test_publications[0..2] + ingested_publications[:ingested] = ingested_publications[:ingested].reject do |pub| + failing_publication_sample.any? { |failing_pub| failing_pub['id'] == pub['id'] } + end + ingested_publications[:failed] = failing_publication_sample.map do |pub| + pub.merge('error' => ['Test error', 'Test error message']) + end + dimensions_reporting_service = Tasks::DimensionsReportingService.new(ingested_publications) report = dimensions_reporting_service.generate_report - DimensionsReportMailer.report_email(report) + DimensionsReportMailer.dimensions_report_email(report) end end From 05504646dbcacc2b4eed5efcede25d1f87abb5b7 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Fri, 24 May 2024 21:23:10 -0400 Subject: [PATCH 13/30] adding comment --- config/environments/development.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/environments/development.rb b/config/environments/development.rb index ec713a73c..d145d8e1a 100755 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -89,6 +89,8 @@ 'web' # Allow this to be addressed when running in containers via docker-compose.yml. ] + # Configure the mailer for development + # Note: Configuration likely needs to be changed later config.action_mailer.delivery_method = :smtp config.action_mailer.smtp_settings = { address: 'smtp.office365.com', From 467f8b4b7c97f75ae31a789e923f8861fb1639c7 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Fri, 24 May 2024 21:43:53 -0400 Subject: [PATCH 14/30] code climate --- app/services/tasks/dimensions_reporting_service.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/services/tasks/dimensions_reporting_service.rb b/app/services/tasks/dimensions_reporting_service.rb index 7a53f7971..5f5125a5d 100644 --- a/app/services/tasks/dimensions_reporting_service.rb +++ b/app/services/tasks/dimensions_reporting_service.rb @@ -25,10 +25,11 @@ def generate_report def extract_publication_info publication_info = {successfully_ingested: [], failed_to_ingest: [], marked_for_review: []} @ingested_publications[:ingested].map do |publication| + publication_item = { title: publication['title'], id: publication['id'], url: "https://cdr.lib.unc.edu/concern/articles/#{publication['article_id']}?locale=en", pdf_attached: publication['pdf_attached'] ? 'Yes' : 'No' } if publication['marked_for_review'] - publication_info[:marked_for_review] << { title: publication['title'], id: publication['id'], url: "https://cdr.lib.unc.edu/concern/articles/#{publication['article_id']}?locale=en", pdf_attached: publication['pdf_attached'] ? 'Yes' : 'No' } + publication_info[:marked_for_review] << publication_item else - publication_info[:successfully_ingested] << { title: publication['title'], id: publication['id'], url: "https://cdr.lib.unc.edu/concern/articles/#{publication['article_id']}?locale=en", pdf_attached: publication['pdf_attached'] ? 'Yes' : 'No' } + publication_info[:successfully_ingested] << publication_item end end @ingested_publications[:failed].map do |publication| From b1a3369bcf18da078b215869ef54df8021a234a9 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Fri, 24 May 2024 21:48:34 -0400 Subject: [PATCH 15/30] rubocop --- app/mailers/dimensions_report_mailer.rb | 1 + spec/mailers/dimensions_report_mailer_spec.rb | 1 + spec/mailers/previews/dimensions_report_mailer_preview.rb | 1 + 3 files changed, 3 insertions(+) diff --git a/app/mailers/dimensions_report_mailer.rb b/app/mailers/dimensions_report_mailer.rb index f86dedbd3..c2ec64216 100644 --- a/app/mailers/dimensions_report_mailer.rb +++ b/app/mailers/dimensions_report_mailer.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true class DimensionsReportMailer < ApplicationMailer def dimensions_report_email(report) @report = report diff --git a/spec/mailers/dimensions_report_mailer_spec.rb b/spec/mailers/dimensions_report_mailer_spec.rb index b44e088ea..f2219ae58 100644 --- a/spec/mailers/dimensions_report_mailer_spec.rb +++ b/spec/mailers/dimensions_report_mailer_spec.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true # spec/mailers/dimensions_report_mailer_spec.rb require 'rails_helper' diff --git a/spec/mailers/previews/dimensions_report_mailer_preview.rb b/spec/mailers/previews/dimensions_report_mailer_preview.rb index 28f3e4025..be765dc87 100644 --- a/spec/mailers/previews/dimensions_report_mailer_preview.rb +++ b/spec/mailers/previews/dimensions_report_mailer_preview.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true # Preview all emails at http://localhost:3000/rails/mailers/dimensions_report_mailer class DimensionsReportMailerPreview < ActionMailer::Preview def dimensions_report_email From 86d40d24a4c5940ebb47a4805ca4c6094ce90711 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Mon, 27 May 2024 16:47:16 -0400 Subject: [PATCH 16/30] addressing failing test --- app/services/tasks/dimensions_ingest_service.rb | 1 + spec/services/tasks/dimensions_ingest_service_spec.rb | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/services/tasks/dimensions_ingest_service.rb b/app/services/tasks/dimensions_ingest_service.rb index 3f7c44edc..0727d3556 100644 --- a/app/services/tasks/dimensions_ingest_service.rb +++ b/app/services/tasks/dimensions_ingest_service.rb @@ -27,6 +27,7 @@ def ingest_publications(publications) article = process_publication(publication) res[:ingested] << publication.merge('article_id' => article.id) rescue StandardError => e + publication.delete('pdf_attached') res[:failed] << publication.merge('error' => [e.class.to_s, e.message]) Rails.logger.error("Error ingesting publication '#{publication['title']}'") Rails.logger.error [e.class.to_s, e.message, *e.backtrace].join($RS) diff --git a/spec/services/tasks/dimensions_ingest_service_spec.rb b/spec/services/tasks/dimensions_ingest_service_spec.rb index 1a228d26d..40bc1043e 100644 --- a/spec/services/tasks/dimensions_ingest_service_spec.rb +++ b/spec/services/tasks/dimensions_ingest_service_spec.rb @@ -27,7 +27,6 @@ FactoryBot.create(:workflow_state, workflow_id: workflow.id, name: 'deposited') end let(:pdf_content) { File.binread(File.join(Rails.root, '/spec/fixtures/files/sample_pdf.pdf')) } - let(:test_publications) do JSON.parse(dimensions_ingest_test_fixture)['publications'] end @@ -141,7 +140,9 @@ "Error ingesting publication '#{expected_failing_publication['title']}'", [StandardError.to_s, test_err_msg].join($RS) ] - ingested_publications = test_publications[1..-1] + ingested_publications = test_publications[1..-1].map do |pub| + pub.merge('pdf_attached' => true) + end # Stub the process_publication method to raise an error for the first publication only allow(service).to receive(:process_publication).and_call_original @@ -159,6 +160,10 @@ expect(res[:failed].count).to eq(1) expect(actual_failed_publication).to eq(expected_failing_publication) expect(actual_failed_publication_error).to eq([StandardError.to_s, test_err_msg]) + # Removing article_id from the ingested publications for comparison + res[:ingested].each_with_index do |ingested_pub, index| + ingested_pub.delete('article_id') + end expect(res[:ingested]).to match_array(ingested_publications) expect(res[:time]).to be_a(Time) }.to change { Article.count }.by(ingested_publications.size) From 4d73750c22e072e8116ea5c84abe72dd9f850f81 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 28 May 2024 10:39:45 -0400 Subject: [PATCH 17/30] logs for debugging pr --- app/services/tasks/dimensions_ingest_service.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/services/tasks/dimensions_ingest_service.rb b/app/services/tasks/dimensions_ingest_service.rb index 0727d3556..b7a1f1103 100644 --- a/app/services/tasks/dimensions_ingest_service.rb +++ b/app/services/tasks/dimensions_ingest_service.rb @@ -33,6 +33,9 @@ def ingest_publications(publications) Rails.logger.error [e.class.to_s, e.message, *e.backtrace].join($RS) end end + # Debug PR: Inspecting res + Rails.logger.error("Inspecting res: #{res.inspect}") + puts "Inspecting res: #{res.inspect}" res end From 901b598bf13414881d2443f9a9661c4d4974b6b7 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 28 May 2024 10:41:15 -0400 Subject: [PATCH 18/30] another log statement --- app/services/tasks/dimensions_ingest_service.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/services/tasks/dimensions_ingest_service.rb b/app/services/tasks/dimensions_ingest_service.rb index b7a1f1103..b5105d48d 100644 --- a/app/services/tasks/dimensions_ingest_service.rb +++ b/app/services/tasks/dimensions_ingest_service.rb @@ -20,6 +20,9 @@ def ingest_publications(publications) time = Time.now Rails.logger.info('Ingesting publications from Dimensions.') res = {ingested: [], failed: [], time: time, admin_set_title: @admin_set.title.first, depositor: @config['depositor_onyen']} + # Debug PR: Sanity check, publications length + Rails.logger.error("Publications Length: #{publications.length}") + puts "Publications Length: #{publications.length}" publications.each.with_index do |publication, index| begin From 58dc740e50690c99fc51a7bfb01621bb3cab65b6 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 28 May 2024 10:42:56 -0400 Subject: [PATCH 19/30] modified build step to save time --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f2a2c4aed..5e2dbf42c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -131,7 +131,7 @@ jobs: run: docker exec solr_container solr create -c hydra-test -d /tmp/solr_config - name: Run rspec tests - run: bundle exec rspec + run: bundle exec rspec spec/services/tasks/dimensions_reporting_service_spec.rb env: REDIS_URL: redis://redis POSTGRES_USER: hyrax From f63ff6ef49783ff72b18cea3b33d8807f80419ac Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 28 May 2024 10:52:24 -0400 Subject: [PATCH 20/30] modified logging for debugging --- app/services/tasks/dimensions_ingest_service.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/services/tasks/dimensions_ingest_service.rb b/app/services/tasks/dimensions_ingest_service.rb index b5105d48d..7dbdf8016 100644 --- a/app/services/tasks/dimensions_ingest_service.rb +++ b/app/services/tasks/dimensions_ingest_service.rb @@ -21,8 +21,8 @@ def ingest_publications(publications) Rails.logger.info('Ingesting publications from Dimensions.') res = {ingested: [], failed: [], time: time, admin_set_title: @admin_set.title.first, depositor: @config['depositor_onyen']} # Debug PR: Sanity check, publications length - Rails.logger.error("Publications Length: #{publications.length}") - puts "Publications Length: #{publications.length}" + Rails.logger.error("E Publications Length: #{publications.length}") + puts "P Publications Length: #{publications.length}" publications.each.with_index do |publication, index| begin @@ -37,8 +37,9 @@ def ingest_publications(publications) end end # Debug PR: Inspecting res - Rails.logger.error("Inspecting res: #{res.inspect}") - puts "Inspecting res: #{res.inspect}" + Rails.logger.error("E Failed Array Length: #{res[:failed].length}") + puts "P Failed Array Length: #{res[:failed].length}" + puts "P Inspecting res failed array: #{res[:failed]}" res end From 38cc7a236f8bcf5a97d5bfafa1b74156a058f12e Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 28 May 2024 11:02:16 -0400 Subject: [PATCH 21/30] temporarily adding env variable to build yaml, logging --- .github/workflows/build.yml | 1 + app/services/tasks/dimensions_ingest_service.rb | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5e2dbf42c..92c7dbbc3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -12,6 +12,7 @@ jobs: runs-on: ubuntu-latest env: + LOG_LEVEL: 'debug' CC_TEST_REPORTER_ID: ab9f6d96726a23c491c04cdad5cc4959551f10716196aaf2260bdf72ca1a3d0b ALLOW_NOTIFICATIONS: true CLAMD_TCP_HOST: 'localhost' diff --git a/app/services/tasks/dimensions_ingest_service.rb b/app/services/tasks/dimensions_ingest_service.rb index 7dbdf8016..af53684b5 100644 --- a/app/services/tasks/dimensions_ingest_service.rb +++ b/app/services/tasks/dimensions_ingest_service.rb @@ -34,12 +34,14 @@ def ingest_publications(publications) res[:failed] << publication.merge('error' => [e.class.to_s, e.message]) Rails.logger.error("Error ingesting publication '#{publication['title']}'") Rails.logger.error [e.class.to_s, e.message, *e.backtrace].join($RS) + puts "P Error ingesting publication '#{publication['title']}'" + puts [e.class.to_s, e.message, *e.backtrace].join($RS) end end # Debug PR: Inspecting res Rails.logger.error("E Failed Array Length: #{res[:failed].length}") puts "P Failed Array Length: #{res[:failed].length}" - puts "P Inspecting res failed array: #{res[:failed]}" + puts "P Inspecting res failed array: #{res[:failed].map { |pub| [pub['title'],pub['error']] }}" res end From 4554b5860486613923126fdb90b74663792f1ecf Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 28 May 2024 11:04:37 -0400 Subject: [PATCH 22/30] rubocop --- app/services/tasks/dimensions_ingest_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/tasks/dimensions_ingest_service.rb b/app/services/tasks/dimensions_ingest_service.rb index af53684b5..7c3ac59cc 100644 --- a/app/services/tasks/dimensions_ingest_service.rb +++ b/app/services/tasks/dimensions_ingest_service.rb @@ -41,7 +41,7 @@ def ingest_publications(publications) # Debug PR: Inspecting res Rails.logger.error("E Failed Array Length: #{res[:failed].length}") puts "P Failed Array Length: #{res[:failed].length}" - puts "P Inspecting res failed array: #{res[:failed].map { |pub| [pub['title'],pub['error']] }}" + puts "P Inspecting res failed array: #{res[:failed].map { |pub| [pub['title'], pub['error']] }}" res end From c71489a84cbe866dcf4ec3d1d231071e9e3a2a18 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 28 May 2024 11:16:32 -0400 Subject: [PATCH 23/30] moving ingested publications to after virus checker stubbing --- spec/services/tasks/dimensions_reporting_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/services/tasks/dimensions_reporting_service_spec.rb b/spec/services/tasks/dimensions_reporting_service_spec.rb index b6da06533..1f9dfb2f0 100644 --- a/spec/services/tasks/dimensions_reporting_service_spec.rb +++ b/spec/services/tasks/dimensions_reporting_service_spec.rb @@ -65,13 +65,13 @@ allow(ingest_service).to receive(:process_publication).and_call_original allow(ingest_service).to receive(:process_publication).with(satisfy { |pub| failing_publication_sample.include?(pub) }).and_raise(StandardError, test_err_msg) marked_for_review_sample - ingested_publications # stub virus checking allow(Hyrax::VirusCheckerService).to receive(:file_has_virus?) { false } # stub longleaf job allow(RegisterToLongleafJob).to receive(:perform_later).and_return(nil) # stub FITS characterization allow(CharacterizeJob).to receive(:perform_later) + ingested_publications end describe '#generate_report' do From cb0d54b176d0d3b57c376f96291237872da26efe Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 28 May 2024 11:48:37 -0400 Subject: [PATCH 24/30] removing configuration stuff and debug logs, moving dimensions ingest in mailer test, adding host to url interpolation --- .github/workflows/build.yml | 3 +-- app/services/tasks/dimensions_ingest_service.rb | 9 --------- app/services/tasks/dimensions_reporting_service.rb | 2 +- .../dimensions_report_email.erb | 4 ++-- config/environments/development.rb | 13 ------------- spec/mailers/dimensions_report_mailer_spec.rb | 2 +- 6 files changed, 5 insertions(+), 28 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 92c7dbbc3..6ec43bc65 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -12,7 +12,6 @@ jobs: runs-on: ubuntu-latest env: - LOG_LEVEL: 'debug' CC_TEST_REPORTER_ID: ab9f6d96726a23c491c04cdad5cc4959551f10716196aaf2260bdf72ca1a3d0b ALLOW_NOTIFICATIONS: true CLAMD_TCP_HOST: 'localhost' @@ -132,7 +131,7 @@ jobs: run: docker exec solr_container solr create -c hydra-test -d /tmp/solr_config - name: Run rspec tests - run: bundle exec rspec spec/services/tasks/dimensions_reporting_service_spec.rb + run: bundle exec rspec spec/mailers/dimensions_mailer_spec.rb env: REDIS_URL: redis://redis POSTGRES_USER: hyrax diff --git a/app/services/tasks/dimensions_ingest_service.rb b/app/services/tasks/dimensions_ingest_service.rb index 7c3ac59cc..0727d3556 100644 --- a/app/services/tasks/dimensions_ingest_service.rb +++ b/app/services/tasks/dimensions_ingest_service.rb @@ -20,9 +20,6 @@ def ingest_publications(publications) time = Time.now Rails.logger.info('Ingesting publications from Dimensions.') res = {ingested: [], failed: [], time: time, admin_set_title: @admin_set.title.first, depositor: @config['depositor_onyen']} - # Debug PR: Sanity check, publications length - Rails.logger.error("E Publications Length: #{publications.length}") - puts "P Publications Length: #{publications.length}" publications.each.with_index do |publication, index| begin @@ -34,14 +31,8 @@ def ingest_publications(publications) res[:failed] << publication.merge('error' => [e.class.to_s, e.message]) Rails.logger.error("Error ingesting publication '#{publication['title']}'") Rails.logger.error [e.class.to_s, e.message, *e.backtrace].join($RS) - puts "P Error ingesting publication '#{publication['title']}'" - puts [e.class.to_s, e.message, *e.backtrace].join($RS) end end - # Debug PR: Inspecting res - Rails.logger.error("E Failed Array Length: #{res[:failed].length}") - puts "P Failed Array Length: #{res[:failed].length}" - puts "P Inspecting res failed array: #{res[:failed].map { |pub| [pub['title'], pub['error']] }}" res end diff --git a/app/services/tasks/dimensions_reporting_service.rb b/app/services/tasks/dimensions_reporting_service.rb index 5f5125a5d..35c2f91c6 100644 --- a/app/services/tasks/dimensions_reporting_service.rb +++ b/app/services/tasks/dimensions_reporting_service.rb @@ -25,7 +25,7 @@ def generate_report def extract_publication_info publication_info = {successfully_ingested: [], failed_to_ingest: [], marked_for_review: []} @ingested_publications[:ingested].map do |publication| - publication_item = { title: publication['title'], id: publication['id'], url: "https://cdr.lib.unc.edu/concern/articles/#{publication['article_id']}?locale=en", pdf_attached: publication['pdf_attached'] ? 'Yes' : 'No' } + publication_item = { title: publication['title'], id: publication['id'], url: "#{ENV['HYRAX_HOST']}/concern/articles/#{publication['article_id']}?locale=en", pdf_attached: publication['pdf_attached'] ? 'Yes' : 'No' } if publication['marked_for_review'] publication_info[:marked_for_review] << publication_item else diff --git a/app/views/dimensions_report_mailer/dimensions_report_email.erb b/app/views/dimensions_report_mailer/dimensions_report_email.erb index 70ab9faf6..2aff45b8b 100644 --- a/app/views/dimensions_report_mailer/dimensions_report_email.erb +++ b/app/views/dimensions_report_mailer/dimensions_report_email.erb @@ -50,7 +50,7 @@ Title - ID + Dimensions ID URL PDF Attached @@ -72,7 +72,7 @@ Title - ID + Dimensions ID Error diff --git a/config/environments/development.rb b/config/environments/development.rb index d145d8e1a..3c190cb92 100755 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -89,18 +89,5 @@ 'web' # Allow this to be addressed when running in containers via docker-compose.yml. ] - # Configure the mailer for development - # Note: Configuration likely needs to be changed later - config.action_mailer.delivery_method = :smtp - config.action_mailer.smtp_settings = { - address: 'smtp.office365.com', - port: 587, - domain: 'localhost', - user_name: ENV['EMAIL_USERNAME'], - password: ENV['EMAIL_PASSWORD'], - authentication: 'plain', - enable_starttls_auto: true - } - config.action_mailer.default_url_options = { host: 'localhost', port: 3000 } config.log_level = LogService.log_level end diff --git a/spec/mailers/dimensions_report_mailer_spec.rb b/spec/mailers/dimensions_report_mailer_spec.rb index f2219ae58..64e46a136 100644 --- a/spec/mailers/dimensions_report_mailer_spec.rb +++ b/spec/mailers/dimensions_report_mailer_spec.rb @@ -62,13 +62,13 @@ allow(ingest_service).to receive(:process_publication).and_call_original allow(ingest_service).to receive(:process_publication).with(satisfy { |pub| failing_publication_sample.include?(pub) }).and_raise(StandardError, test_err_msg) marked_for_review_sample - ingested_publications # stub virus checking allow(Hyrax::VirusCheckerService).to receive(:file_has_virus?) { false } # stub longleaf job allow(RegisterToLongleafJob).to receive(:perform_later).and_return(nil) # stub FITS characterization allow(CharacterizeJob).to receive(:perform_later) + ingested_publications end describe 'dimensions_report_email' do From 958d32d10334bea861e7005b1cab8a9aedc31a90 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 28 May 2024 13:45:35 -0400 Subject: [PATCH 25/30] simulating missing pdfs in email report preview and reporting service tests --- spec/mailers/dimensions_report_mailer_spec.rb | 9 +++- .../dimensions_report_mailer_preview.rb | 11 ++-- .../dimensions_reporting_service_spec.rb | 50 ++++++++++++------- 3 files changed, 49 insertions(+), 21 deletions(-) diff --git a/spec/mailers/dimensions_report_mailer_spec.rb b/spec/mailers/dimensions_report_mailer_spec.rb index 64e46a136..33bec9295 100644 --- a/spec/mailers/dimensions_report_mailer_spec.rb +++ b/spec/mailers/dimensions_report_mailer_spec.rb @@ -31,7 +31,14 @@ let(:test_err_msg) { 'Test error' } let(:fixed_time) { Time.new(2024, 5, 21, 10, 0, 0) } - let(:test_publications) { JSON.parse(dimensions_ingest_test_fixture)['publications'] } + # Removing linkout pdf from some publications to simulate missing pdfs + let(:test_publications) { + all_publications = JSON.parse(dimensions_ingest_test_fixture)['publications'] + all_publications.each_with_index do |pub, index| + pub.delete('linkout') if index.even? + end + all_publications + } let(:failing_publication_sample) { test_publications[0..2] } let(:marked_for_review_sample) do diff --git a/spec/mailers/previews/dimensions_report_mailer_preview.rb b/spec/mailers/previews/dimensions_report_mailer_preview.rb index be765dc87..14085612d 100644 --- a/spec/mailers/previews/dimensions_report_mailer_preview.rb +++ b/spec/mailers/previews/dimensions_report_mailer_preview.rb @@ -5,17 +5,22 @@ def dimensions_report_email # Ensuring template works with a report generated after an ingest with a test fixture dimensions_ingest_test_fixture = File.read(File.join(Rails.root, '/spec/fixtures/files/dimensions_ingest_test_fixture.json')) test_publications = JSON.parse(dimensions_ingest_test_fixture)['publications'] - # Marking some publications for review test_publications[3..5].each { |pub| pub['marked_for_review'] = true } - marked_for_review_sample = test_publications[3..5] - # successful_publication_sample = test_publications[6..-1] config = { 'admin_set' => 'default', 'depositor_onyen' => 'admin' } + dimensions_ingest_service = Tasks::DimensionsIngestService.new(config) ingested_publications = dimensions_ingest_service.ingest_publications(test_publications) + # Marking some successfully ingested publications as having attached PDFs, workaround to stubbing requests for them + # Odd publications marked as pdf_attached for consistency with tests + ingested_publications[:ingested].each_with_index do |pub, index| + if index.odd? + pub['pdf_attached'] = 'Yes' + end + end # Moving publications in the failing publication sample to the failed array and adding an error message failing_publication_sample = test_publications[0..2] diff --git a/spec/services/tasks/dimensions_reporting_service_spec.rb b/spec/services/tasks/dimensions_reporting_service_spec.rb index 1f9dfb2f0..b2de99d81 100644 --- a/spec/services/tasks/dimensions_reporting_service_spec.rb +++ b/spec/services/tasks/dimensions_reporting_service_spec.rb @@ -30,15 +30,24 @@ let(:test_err_msg) { 'Test error' } let(:fixed_time) { Time.new(2024, 5, 21, 10, 0, 0) } - let(:test_publications) { JSON.parse(dimensions_ingest_test_fixture)['publications'] } - - let(:failing_publication_sample) { test_publications[0..2] } + # Removing linkout pdf from some publications to simulate missing pdfs + let(:test_publications) { + all_publications = JSON.parse(dimensions_ingest_test_fixture)['publications'] + all_publications.each_with_index do |pub, index| + pub.delete('linkout') if index.even? + end + all_publications + } + let(:failing_publication_sample) { + { publications: test_publications[0..2], test_fixture_start_index: 0 } + } let(:marked_for_review_sample) do test_publications[3..5].each { |pub| pub['marked_for_review'] = true } - test_publications[3..5] + { publications: test_publications[3..5], test_fixture_start_index: 3 } end - let(:successful_publication_sample) { test_publications[6..-1] } - + let(:successful_publication_sample) { + { publications: test_publications[6..-1], test_fixture_start_index: 6 } + } let(:ingest_service) { Tasks::DimensionsIngestService.new(config) } let(:ingested_publications) do ingest_service.ingest_publications(test_publications) @@ -47,8 +56,6 @@ let(:service) { described_class.new(ingested_publications) } - - before do ActiveFedora::Cleaner.clean! admin_set @@ -63,7 +70,7 @@ stub_request(:get, 'https://test-url.com/') .to_return(body: pdf_content, status: 200, headers: { 'Content-Type' => 'application/pdf' }) allow(ingest_service).to receive(:process_publication).and_call_original - allow(ingest_service).to receive(:process_publication).with(satisfy { |pub| failing_publication_sample.include?(pub) }).and_raise(StandardError, test_err_msg) + allow(ingest_service).to receive(:process_publication).with(satisfy { |pub| failing_publication_sample[:publications].include?(pub) }).and_raise(StandardError, test_err_msg) marked_for_review_sample # stub virus checking allow(Hyrax::VirusCheckerService).to receive(:file_has_virus?) { false } @@ -82,19 +89,28 @@ expect(headers[:reporting_message]).to eq('Reporting publications from dimensions ingest at May 21, 2024 at 10:00 AM UTC by admin.') expect(headers[:admin_set]).to eq('Admin Set: Open_Access_Articles_and_Book_Chapters') expect(headers[:total_publications]).to eq("Total Publications: #{test_publications.length}") - expect(headers[:successfully_ingested]).to eq("\nSuccessfully Ingested: (#{successful_publication_sample.length} Publications)") - expect(headers[:marked_for_review]).to eq("\nMarked for Review: (#{marked_for_review_sample.length} Publications)") - expect(headers[:failed_to_ingest]).to eq("\nFailed to Ingest: (#{failing_publication_sample.length} Publications)") + expect(headers[:successfully_ingested]).to eq("\nSuccessfully Ingested: (#{successful_publication_sample[:publications].length} Publications)") + expect(headers[:marked_for_review]).to eq("\nMarked for Review: (#{marked_for_review_sample[:publications].length} Publications)") + expect(headers[:failed_to_ingest]).to eq("\nFailed to Ingest: (#{failing_publication_sample[:publications].length} Publications)") end end describe '#extract_publication_info' do - def expect_publication_info(info_array, sample_array, failed) + def expect_publication_info(info_array, sample_array, failed, sample_start_index) info_array.each_with_index do |info, i| + expect(info[:title]).to eq(sample_array[i]['title']) expect(info[:id]).to eq(sample_array[i]['id']) - expect(info[:url]).to eq("https://cdr.lib.unc.edu/concern/articles/#{sample_array[i]['article_id']}?locale=en") if !failed + expect(info[:url]).to eq("#{ENV['HYRAX_HOST']}/concern/articles/#{sample_array[i]['article_id']}?locale=en") if !failed expect(info[:error]).to eq("StandardError - #{test_err_msg}") if failed + if failed + expect(info[:pdf_attached]).to be_nil + else + # Offsetting the index if the sample start index is odd + offset_index = sample_start_index.even? ? i : i - 1 + expect(info[:pdf_attached]).to eq('No') if offset_index.even? + expect(info[:pdf_attached]).to eq('Yes') if offset_index.odd? + end end end @@ -104,9 +120,9 @@ def expect_publication_info(info_array, sample_array, failed) expect(extracted_info[:marked_for_review].length).to eq(3) expect(extracted_info[:failed_to_ingest].length).to eq(3) - expect_publication_info(extracted_info[:successfully_ingested], ingested_publications[:ingested].select { |pub| !pub['marked_for_review'] }, false) - expect_publication_info(extracted_info[:marked_for_review], ingested_publications[:ingested].select { |pub| pub['marked_for_review'] }, false) - expect_publication_info(extracted_info[:failed_to_ingest], ingested_publications[:failed], true) + expect_publication_info(extracted_info[:successfully_ingested], ingested_publications[:ingested].select { |pub| !pub['marked_for_review'] }, false, successful_publication_sample[:test_fixture_start_index]) + expect_publication_info(extracted_info[:marked_for_review], ingested_publications[:ingested].select { |pub| pub['marked_for_review'] }, false, marked_for_review_sample[:test_fixture_start_index]) + expect_publication_info(extracted_info[:failed_to_ingest], ingested_publications[:failed], true, failing_publication_sample[:test_fixture_start_index]) end end end From a4e975f52d4b04eadcfbe91bb5ff621995308fc7 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 28 May 2024 13:48:17 -0400 Subject: [PATCH 26/30] updating build yml --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 6ec43bc65..bb15e3b2c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -131,7 +131,7 @@ jobs: run: docker exec solr_container solr create -c hydra-test -d /tmp/solr_config - name: Run rspec tests - run: bundle exec rspec spec/mailers/dimensions_mailer_spec.rb + run: bundle exec rspec spec/mailers/dimensions_report_mailer_spec.rb env: REDIS_URL: redis://redis POSTGRES_USER: hyrax From f0a7f93808fdf85daea4895925cf5a1099196344 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 28 May 2024 13:50:57 -0400 Subject: [PATCH 27/30] updating column title --- app/views/dimensions_report_mailer/dimensions_report_email.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/dimensions_report_mailer/dimensions_report_email.erb b/app/views/dimensions_report_mailer/dimensions_report_email.erb index 2aff45b8b..c2d9e723e 100644 --- a/app/views/dimensions_report_mailer/dimensions_report_email.erb +++ b/app/views/dimensions_report_mailer/dimensions_report_email.erb @@ -28,7 +28,7 @@ Title - ID + Dimensions ID URL PDF Attached From 2973f65ae68b78a4fecba970973bad1d11b3f830 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 28 May 2024 13:55:35 -0400 Subject: [PATCH 28/30] rolling back debugging related change to build yml --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bb15e3b2c..f2a2c4aed 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -131,7 +131,7 @@ jobs: run: docker exec solr_container solr create -c hydra-test -d /tmp/solr_config - name: Run rspec tests - run: bundle exec rspec spec/mailers/dimensions_report_mailer_spec.rb + run: bundle exec rspec env: REDIS_URL: redis://redis POSTGRES_USER: hyrax From d1de3f4dc7619ccca0cb19f69f061691d556ce00 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Wed, 29 May 2024 10:44:19 -0400 Subject: [PATCH 29/30] implement suggested review changes --- .../tasks/dimensions_reporting_service.rb | 2 +- .../tasks/dimensions_ingest_service_spec.rb | 36 +++++++++++-------- .../dimensions_reporting_service_spec.rb | 6 ++-- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/app/services/tasks/dimensions_reporting_service.rb b/app/services/tasks/dimensions_reporting_service.rb index 35c2f91c6..c4f802355 100644 --- a/app/services/tasks/dimensions_reporting_service.rb +++ b/app/services/tasks/dimensions_reporting_service.rb @@ -10,7 +10,7 @@ def generate_report extracted_info = extract_publication_info formatted_time = @ingested_publications[:time].strftime('%B %d, %Y at %I:%M %p %Z') report[:subject] = "Dimensions Ingest Report for #{formatted_time}" - report[:headers][:reporting_message] = "Reporting publications from dimensions ingest at #{formatted_time} by #{@ingested_publications[:depositor]}." + report[:headers][:reporting_message] = "Reporting publications from dimensions ingest on #{formatted_time} by #{@ingested_publications[:depositor]}." report[:headers][:admin_set] = "Admin Set: #{@ingested_publications[:admin_set_title]}" report[:headers][:total_publications] = "Total Publications: #{extracted_info[:successfully_ingested].length + extracted_info[:failed_to_ingest].length + extracted_info[:marked_for_review].length}" report[:headers][:successfully_ingested] = "\nSuccessfully Ingested: (#{extracted_info[:successfully_ingested].length} Publications)" diff --git a/spec/services/tasks/dimensions_ingest_service_spec.rb b/spec/services/tasks/dimensions_ingest_service_spec.rb index 40bc1043e..a5d4581b6 100644 --- a/spec/services/tasks/dimensions_ingest_service_spec.rb +++ b/spec/services/tasks/dimensions_ingest_service_spec.rb @@ -150,23 +150,29 @@ expect(Rails.logger).to receive(:error).with(expected_log_outputs[0]) expect(Rails.logger).to receive(:error).with(include(expected_log_outputs[1])) + expect { - res = service.ingest_publications(test_publications) - actual_failed_publication = res[:failed].first - actual_failed_publication_error = res[:failed].first['error'] - actual_failed_publication.delete('error') - expect(res[:admin_set_title]).to eq('Open_Access_Articles_and_Book_Chapters') - expect(res[:depositor]).to eq('admin') - expect(res[:failed].count).to eq(1) - expect(actual_failed_publication).to eq(expected_failing_publication) - expect(actual_failed_publication_error).to eq([StandardError.to_s, test_err_msg]) - # Removing article_id from the ingested publications for comparison - res[:ingested].each_with_index do |ingested_pub, index| - ingested_pub.delete('article_id') - end - expect(res[:ingested]).to match_array(ingested_publications) - expect(res[:time]).to be_a(Time) + @res = service.ingest_publications(test_publications) }.to change { Article.count }.by(ingested_publications.size) + + actual_failed_publication = @res[:failed].first + actual_failed_publication_error = @res[:failed].first['error'] + # Removing error from the failed publication for comparison + actual_failed_publication.delete('error') + expect(actual_failed_publication).to eq(expected_failing_publication) + expect(actual_failed_publication_error).to eq([StandardError.to_s, test_err_msg]) + + expect(@res[:admin_set_title]).to eq('Open_Access_Articles_and_Book_Chapters') + expect(@res[:depositor]).to eq('admin') + expect(@res[:failed].count).to eq(1) + + # Removing article_id from the ingested publications for comparison + @res[:ingested].each_with_index do |ingested_pub, index| + ingested_pub.delete('article_id') + end + + expect(@res[:ingested]).to match_array(ingested_publications) + expect(@res[:time]).to be_a(Time) end end diff --git a/spec/services/tasks/dimensions_reporting_service_spec.rb b/spec/services/tasks/dimensions_reporting_service_spec.rb index b2de99d81..1034a3377 100644 --- a/spec/services/tasks/dimensions_reporting_service_spec.rb +++ b/spec/services/tasks/dimensions_reporting_service_spec.rb @@ -86,7 +86,7 @@ report = service.generate_report headers = report[:headers] expect(report[:subject]).to eq('Dimensions Ingest Report for May 21, 2024 at 10:00 AM UTC') - expect(headers[:reporting_message]).to eq('Reporting publications from dimensions ingest at May 21, 2024 at 10:00 AM UTC by admin.') + expect(headers[:reporting_message]).to eq('Reporting publications from dimensions ingest on May 21, 2024 at 10:00 AM UTC by admin.') expect(headers[:admin_set]).to eq('Admin Set: Open_Access_Articles_and_Book_Chapters') expect(headers[:total_publications]).to eq("Total Publications: #{test_publications.length}") expect(headers[:successfully_ingested]).to eq("\nSuccessfully Ingested: (#{successful_publication_sample[:publications].length} Publications)") @@ -101,11 +101,11 @@ def expect_publication_info(info_array, sample_array, failed, sample_start_index expect(info[:title]).to eq(sample_array[i]['title']) expect(info[:id]).to eq(sample_array[i]['id']) - expect(info[:url]).to eq("#{ENV['HYRAX_HOST']}/concern/articles/#{sample_array[i]['article_id']}?locale=en") if !failed - expect(info[:error]).to eq("StandardError - #{test_err_msg}") if failed if failed + expect(info[:error]).to eq("StandardError - #{test_err_msg}") expect(info[:pdf_attached]).to be_nil else + expect(info[:url]).to eq("#{ENV['HYRAX_HOST']}/concern/articles/#{sample_array[i]['article_id']}?locale=en") # Offsetting the index if the sample start index is odd offset_index = sample_start_index.even? ? i : i - 1 expect(info[:pdf_attached]).to eq('No') if offset_index.even? From d6702ef50df878f920b994026758bf1c55b05762 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Wed, 29 May 2024 11:25:14 -0400 Subject: [PATCH 30/30] updating view template --- .../dimensions_report_email.erb | 67 +++++++++++++------ 1 file changed, 45 insertions(+), 22 deletions(-) diff --git a/app/views/dimensions_report_mailer/dimensions_report_email.erb b/app/views/dimensions_report_mailer/dimensions_report_email.erb index c2d9e723e..d76b2e788 100644 --- a/app/views/dimensions_report_mailer/dimensions_report_email.erb +++ b/app/views/dimensions_report_mailer/dimensions_report_email.erb @@ -16,6 +16,21 @@ th { background-color: #f2f2f2; } + .title-col { + width: 30%; + } + .id-col { + width: 20%; + } + .url-col { + width: 30%; + } + .pdf-col { + width: 5%; + } + .error-col { + width: 15%; + } @@ -27,19 +42,21 @@ - - - - + + + + + <% @report[:successfully_ingested_rows].each do |publication| %> - - - - + + + + + <% end %> @@ -49,19 +66,21 @@
TitleDimensions IDURLPDF AttachedTitleDimensions IDURLPDF AttachedError
<%= publication[:title] %><%= publication[:id] %><%= publication[:url] %><%= publication[:pdf_attached] %><%= publication[:title] %><%= publication[:id] %><%= publication[:url] %><%= publication[:pdf_attached] %>N/A
- - - - + + + + + <% @report[:marked_for_review_rows].each do |publication| %> - - - - + + + + + <% end %> @@ -71,17 +90,21 @@
TitleDimensions IDURLPDF AttachedTitleDimensions IDURLPDF AttachedError
<%= publication[:title] %><%= publication[:id] %><%= publication[:url] %><%= publication[:pdf_attached] %><%= publication[:title] %><%= publication[:id] %><%= publication[:url] %><%= publication[:pdf_attached] %>N/A
- - - + + + + + <% @report[:failed_to_ingest_rows].each do |publication| %> - - - + + + + + <% end %>
TitleDimensions IDErrorTitleDimensions IDURLPDF AttachedError
<%= publication[:title] %><%= publication[:id] %><%= publication[:error] %><%= publication[:title] %><%= publication[:id] %>N/AN/A<%= publication[:error] %>