From 2789058488e0436266bdc1ee969ff7c800406e3a Mon Sep 17 00:00:00 2001 From: David Campbell Date: Fri, 30 Aug 2024 11:47:16 -0400 Subject: [PATCH] updated logging and tests --- app/helpers/work_utils_helper.rb | 23 +++++-- spec/helpers/hyrax/work_utils_helper_spec.rb | 71 ++++++++++++-------- 2 files changed, 60 insertions(+), 34 deletions(-) diff --git a/app/helpers/work_utils_helper.rb b/app/helpers/work_utils_helper.rb index b62b1031e..1650a766f 100644 --- a/app/helpers/work_utils_helper.rb +++ b/app/helpers/work_utils_helper.rb @@ -8,15 +8,24 @@ def self.fetch_work_data_by_fileset_id(fileset_id) # If the admin set name is not nil, fetch the admin set # Set the admin set to an empty hash if the solr query returns nil admin_set = admin_set_name ? ActiveFedora::SolrService.get("title_tesim:#{admin_set_name}", { :rows => 1, 'df' => 'title_tesim'})['response']['docs'].first || {} : {} - warning_message = admin_set_name ? "No admin set found for title_tesim: #{admin_set_name}" : "No admin set found with fileset id: #{fileset_id}" - Rails.logger.warn(warning_message) if admin_set.blank? + Rails.logger.warn(self.generate_warning_message(admin_set_name, fileset_id)) if admin_set.blank? { - work_id: work['id'] || 'Unknown', - work_type: work.dig('has_model_ssim', 0) || 'Unknown', - title: work['title_tesim']&.first || 'Unknown', - admin_set_id: admin_set['id'] || 'Unknown', - admin_set_name: admin_set_name || 'Unknown' + work_id: work['id'], + work_type: work.dig('has_model_ssim', 0), + title: work['title_tesim']&.first, + admin_set_id: admin_set['id'], + admin_set_name: admin_set_name } end + + private_class_method + + def self.generate_warning_message(admin_set_name, fileset_id) + if admin_set_name.blank? + return "Could not find an admin set, the work with fileset id: #{fileset_id} has no admin set name." + else + return "No admin set found with title_tesim: #{admin_set_name}." + end + end end diff --git a/spec/helpers/hyrax/work_utils_helper_spec.rb b/spec/helpers/hyrax/work_utils_helper_spec.rb index 6c58c2fb9..ddbae9d90 100644 --- a/spec/helpers/hyrax/work_utils_helper_spec.rb +++ b/spec/helpers/hyrax/work_utils_helper_spec.rb @@ -4,16 +4,21 @@ require Rails.root.join('app/overrides/controllers/hyrax/downloads_controller_override.rb') RSpec.describe WorkUtilsHelper, type: :module do - let(:fileset_id) { 'file-set-id' } + let(:fileset_ids) { ['file-set-id', 'file-set-id-2'] } let(:admin_set_name) { 'Open_Access_Articles_and_Book_Chapters' } - let(:example_admin_set_id) { 'h128zk07m' } - let(:example_work_id) { '1z40m031g' } - let(:mock_record) { [{ + let(:mock_records) { [[{ 'has_model_ssim' => ['Article'], 'id' => '1z40m031g', 'title_tesim' => ['Key ethical issues discussed at CDC-sponsored international, regional meetings to explore cultural perspectives and contexts on pandemic influenza preparedness and response'], 'admin_set_tesim' => ['Open_Access_Articles_and_Book_Chapters']} + ], + [{ + 'has_model_ssim' => ['Article'], + 'id' => '1z40m031g-2', + 'title_tesim' => ['Placeholder Title'], + 'admin_set_tesim' => []} + ] ] } @@ -34,49 +39,61 @@ } before do - allow(ActiveFedora::SolrService).to receive(:get).with("file_set_ids_ssim:#{fileset_id}", rows: 1).and_return('response' => { 'docs' => mock_record }) - allow(ActiveFedora::SolrService).to receive(:get).with("title_tesim:#{admin_set_name}", rows: 1).and_return('response' => { 'docs' => mock_admin_set }) + allow(ActiveFedora::SolrService).to receive(:get).with("file_set_ids_ssim:#{fileset_ids[0]}", rows: 1).and_return('response' => { 'docs' => mock_records[0] }) + allow(ActiveFedora::SolrService).to receive(:get).with("title_tesim:#{admin_set_name}", {'df'=>'title_tesim', :rows=>1}).and_return('response' => { 'docs' => mock_admin_set }) end describe '#fetch_work_data_by_fileset_id' do it 'fetches the work data correctly' do - result = WorkUtilsHelper.fetch_work_data_by_fileset_id(fileset_id) + result = WorkUtilsHelper.fetch_work_data_by_fileset_id(fileset_ids[0]) expect(result).to eq(expected_work_data) end - it 'properly substitutes Unknown for missing values' do + it 'logs appropriate messages for missing values' do # Mock the solr response to simulate a work with missing values, if it somehow makes it past the initial nil check - allow(ActiveFedora::SolrService).to receive(:get).with("file_set_ids_ssim:#{fileset_id}", rows: 1).and_return('response' => { 'docs' => [{ 'placeholder-key' => 'placeholder-value' }] }) - allow(ActiveFedora::SolrService).to receive(:get).with('title_tesim:Unknown', rows: 1).and_return('response' => { 'docs' => [] }) - result = WorkUtilsHelper.fetch_work_data_by_fileset_id(fileset_id) - expect(result[:work_id]).to eq('Unknown') - expect(result[:work_type]).to eq('Unknown') - expect(result[:title]).to eq('Unknown') - expect(result[:admin_set_id]).to eq('Unknown') + allow(ActiveFedora::SolrService).to receive(:get).with("file_set_ids_ssim:#{fileset_ids[0]}", rows: 1).and_return('response' => { 'docs' => [] }) + allow(Rails.logger).to receive(:warn) + # allow(ActiveFedora::SolrService).to receive(:get).with('title_tesim:Unknown', rows: 1).and_return('response' => { 'docs' => [] }) + result = WorkUtilsHelper.fetch_work_data_by_fileset_id(fileset_ids[0]) + expect(Rails.logger).to have_received(:warn).with("No work found for fileset id: #{fileset_ids[0]}") + expect(Rails.logger).to have_received(:warn).with("Could not find an admin set, the work with fileset id: #{fileset_ids[0]} has no admin set name.") + expect(result[:work_id]).to be_nil + expect(result[:work_type]).to be_nil + expect(result[:title]).to be_nil + expect(result[:admin_set_id]).to be_nil end context 'when no work is found' do - before do - allow(ActiveFedora::SolrService).to receive(:get).with("file_set_ids_ssim:#{fileset_id}", rows: 1).and_return('response' => { 'docs' => [] }) - end - it 'logs a warning if no work is found' do + allow(ActiveFedora::SolrService).to receive(:get).with("file_set_ids_ssim:#{fileset_ids[1]}", rows: 1).and_return('response' => { 'docs' => [] }) allow(Rails.logger).to receive(:warn) - WorkUtilsHelper.fetch_work_data_by_fileset_id(fileset_id) - expect(Rails.logger).to have_received(:warn).with("No work found for fileset id: #{fileset_id}") + WorkUtilsHelper.fetch_work_data_by_fileset_id(fileset_ids[1]) + expect(Rails.logger).to have_received(:warn).with("No work found for fileset id: #{fileset_ids[1]}") end end context 'when admin set is not found' do - before do - allow(ActiveFedora::SolrService).to receive(:get).with("title_tesim:#{admin_set_name}", rows: 1).and_return('response' => { 'docs' => [] }) + # before do + # allow(ActiveFedora::SolrService).to receive(:get).with("title_tesim:#{admin_set_name}", {"df"=>"title_tesim", :rows=>1}).and_return('response' => { 'docs' => mock_records[1] }) + # end + + it 'logs an appropriate message if the work doesnt have an admin set title' do + # Using the mock record without an admin set title + allow(ActiveFedora::SolrService).to receive(:get).with("file_set_ids_ssim:#{fileset_ids[1]}", rows: 1).and_return('response' => { 'docs' => mock_records[1] }) + allow(Rails.logger).to receive(:warn) + result = WorkUtilsHelper.fetch_work_data_by_fileset_id(fileset_ids[1]) + expect(Rails.logger).to have_received(:warn).with("Could not find an admin set, the work with fileset id: #{fileset_ids[1]} has no admin set name.") + expect(result[:admin_set_id]).to be_nil end - it 'sets the admin_set_id to Unknown if admin set is not found' do + it 'logs an appropriate message if the query for an admin set returns nothing' do + # Using the mock record with an admin set title + allow(ActiveFedora::SolrService).to receive(:get).with("file_set_ids_ssim:#{fileset_ids[1]}", rows: 1).and_return('response' => { 'docs' => mock_records[0] }) + allow(ActiveFedora::SolrService).to receive(:get).with("title_tesim:#{admin_set_name}", {'df'=>'title_tesim', :rows=>1}).and_return('response' => { 'docs' => [] }) allow(Rails.logger).to receive(:warn) - result = WorkUtilsHelper.fetch_work_data_by_fileset_id(fileset_id) - expect(Rails.logger).to have_received(:warn).with("No admin set found for title_tesim: #{admin_set_name}") - expect(result[:admin_set_id]).to eq('Unknown') + result = WorkUtilsHelper.fetch_work_data_by_fileset_id(fileset_ids[1]) + expect(Rails.logger).to have_received(:warn).with("No admin set found with title_tesim: #{admin_set_name}.") + expect(result[:admin_set_id]).to be_nil end end end