diff --git a/app/controllers/bulkrax/importers_controller.rb b/app/controllers/bulkrax/importers_controller.rb index d088fdb15..6d4b7bf4d 100644 --- a/app/controllers/bulkrax/importers_controller.rb +++ b/app/controllers/bulkrax/importers_controller.rb @@ -54,6 +54,14 @@ def entry_table end end + def statuses_table + @statuses = @importer.statuses.order(table_order).page(table_page).per(table_per_page) + @statuses = @statuses.where(status_table_search) if status_table_search.present? + respond_to do |format| + format.json { render json: format_statuses(@statuses, @importer) } + end + end + # GET /importers/new def new @importer = Importer.new diff --git a/app/controllers/concerns/bulkrax/datatables_behavior.rb b/app/controllers/concerns/bulkrax/datatables_behavior.rb index 570ac18db..d15bb25a1 100644 --- a/app/controllers/concerns/bulkrax/datatables_behavior.rb +++ b/app/controllers/concerns/bulkrax/datatables_behavior.rb @@ -132,6 +132,7 @@ def format_entries(entries, item) status_message: status_message_for(e), type: e.type, updated_at: e.updated_at, + # refactor this errors: e.status_message == 'Failed' ? view_context.link_to(e.error_class, view_context.item_entry_path(item, e)) : "", actions: entry_util_links(e, item) } @@ -143,6 +144,26 @@ def format_entries(entries, item) } end + def format_statuses(statuses, item) + result = statuses.map do |s| + { + identifier: view_context.link_to(s.identifier, view_context.item_status_path(item, s)), + id: s.id, + status_message: s.status_message, + error_class: s.error_class, + created_at: s.created_at, + updated_at: s.updated_at, + runnable_id: view_context.link_to(s.runnable_id, view_context.importer_path(item)), + actions: status_util_links(s, item) + } + end + { + data: result, + recordsTotal: item.statuses.size, + recordsFiltered: item.statuses.size + } + end + def entry_util_links(e, item) links = [] links << view_context.link_to(view_context.raw(''), view_context.item_entry_path(item, e)) @@ -151,6 +172,13 @@ def entry_util_links(e, item) links.join(" ") end + def status_util_links(_s, item) + links = [] + links << view_context.link_to(view_context.raw(''), view_context.item_status_path(item, e)) + links << "" if view_context.an_importer?(item) + links.join(" ") + end + def status_message_for(e) if e.status_message == "Complete" " #{e.status_message}" diff --git a/app/matchers/bulkrax/application_matcher.rb b/app/matchers/bulkrax/application_matcher.rb index 4a4472163..aa8296ee0 100644 --- a/app/matchers/bulkrax/application_matcher.rb +++ b/app/matchers/bulkrax/application_matcher.rb @@ -16,9 +16,8 @@ def initialize(args) def result(_parser, content) return nil if self.excluded == true || Bulkrax.reserved_properties.include?(self.to) - # rubocop:disable Style/RedundantParentheses - return nil if self.if && (!self.if.is_a?(Array) && self.if.length != 2) - # rubocop:enable Style/RedundantParentheses + return nil if self.if && !self.if.is_a?(Array) && self.if.length != 2 + if self.if return unless content.send(self.if[0], Regexp.new(self.if[1])) end diff --git a/app/models/bulkrax/csv_entry.rb b/app/models/bulkrax/csv_entry.rb index 65666890b..2e56771ac 100644 --- a/app/models/bulkrax/csv_entry.rb +++ b/app/models/bulkrax/csv_entry.rb @@ -83,6 +83,10 @@ def build_metadata add_local self.parsed_metadata + rescue => e + self.save! + self.set_status_info(e, self) + raise => e end def validate_record diff --git a/app/models/bulkrax/importer.rb b/app/models/bulkrax/importer.rb index be4925146..2c90cabf3 100644 --- a/app/models/bulkrax/importer.rb +++ b/app/models/bulkrax/importer.rb @@ -209,7 +209,7 @@ def import_objects(types_array = nil) existing_entries? ? parser.rebuild_entries(types) : parser.create_objects(types) mark_unseen_as_skipped rescue StandardError => e - set_status_info(e) + set_status_info(e, self) end # After an import any entries we did not touch are skipped. diff --git a/app/models/concerns/bulkrax/import_behavior.rb b/app/models/concerns/bulkrax/import_behavior.rb index 8e6e9e354..77d5c431d 100644 --- a/app/models/concerns/bulkrax/import_behavior.rb +++ b/app/models/concerns/bulkrax/import_behavior.rb @@ -16,6 +16,7 @@ def build_for_importer child_jobs if self.parsed_metadata[related_children_parsed_mapping]&.join.present? end rescue RSolr::Error::Http, CollectionsCreatedError => e + set_status_info(e) raise e rescue StandardError => e set_status_info(e) diff --git a/app/models/concerns/bulkrax/status_info.rb b/app/models/concerns/bulkrax/status_info.rb index c48dc58ea..e7a4adee6 100644 --- a/app/models/concerns/bulkrax/status_info.rb +++ b/app/models/concerns/bulkrax/status_info.rb @@ -50,12 +50,49 @@ def set_status_info(e = nil, current_run = nil) else self.statuses.create!(status_message: 'Failed', runnable: runnable, error_class: e.class.to_s, error_message: e.message, error_backtrace: e.backtrace) end + rescue => e + save_with_placeholder_to_capture_status(e, runnable) end alias status_info set_status_info deprecation_deprecate status_info: "Favor Bulkrax::StatusInfo.set_status_info. We will be removing .status_info in Bulkrax v6.0.0" + def save_with_placeholder_to_capture_status(e, runnable) + case e.class.to_s + when 'ActiveRecord::RecordInvalid' + runnable.user = current_or_placeholder_user if runnable.user.nil? + runnable.admin_set_id = placeholder_admin_set_id if runnable.admin_set_id.nil? + runnable.name = 'Placeholder Name' if runnable.name.nil? + runnable.parser_klass = Bulkrax::CsvParser if runnable.parser_klass.nil? + runnable.save! + runnable.errors.each do |error| + set_status_info(error, runnable) + end + when 'ActiveRecord::RecordNotSaved' + runnable.user = current_or_placeholder_user if runnable.user.nil? + runnable.admin_set_id = placeholder_admin_set_id if runnable.admin_set_id.nil? + runnable.name = 'Placeholder Name' if runnable.name.nil? + runnable.parser_klass = Bulkrax::CsvParser if runnable.parser_klass.nil? + runnable.save! + runnable.errors.each do |error| + set_status_info(error, runnable) + end + end + end + + def current_or_placeholder_user + placeholder_user = User.new(display_name: 'Placeholder User') + placeholder_user.save! + @current_user.presence || placeholder_user + end + + def placeholder_admin_set_id + placeholder_admin_set = AdminSet.new(title: ['Placeholder Admin Set']) + placeholder_admin_set.save! + placeholder_admin_set.id + end + # api compatible with previous error structure def last_error return unless current_status && current_status.error_class.present? diff --git a/app/parsers/bulkrax/bagit_parser.rb b/app/parsers/bulkrax/bagit_parser.rb index 369e542b3..cc26d1f81 100644 --- a/app/parsers/bulkrax/bagit_parser.rb +++ b/app/parsers/bulkrax/bagit_parser.rb @@ -30,6 +30,10 @@ def import_fields raise StandardError, 'No metadata files were found' if metadata_paths.blank? @import_fields ||= metadata_paths.sample(10).map do |path| entry_class.fields_from_data(entry_class.read_data(path)) + rescue => e + importer = Importer.find(entry_class.runnable_id) + importer.set_status_info(e, importer) + raise => e end.flatten.compact.uniq end @@ -41,6 +45,10 @@ def records(_opts = {}) raise StandardError, 'No metadata files were found' if path.blank? data = entry_class.read_data(path) get_data(bag, data) + rescue => e + importer = Importer.find(entry_class.runnable_id) + importer.set_status_info(e, importer) + raise => e end @records = @records.flatten diff --git a/app/parsers/bulkrax/csv_parser.rb b/app/parsers/bulkrax/csv_parser.rb index 820c10c52..2115bfc46 100644 --- a/app/parsers/bulkrax/csv_parser.rb +++ b/app/parsers/bulkrax/csv_parser.rb @@ -20,6 +20,9 @@ def records(_opts = {}) importer.save @records = csv_data.map { |record_data| entry_class.data_for_entry(record_data, nil, self) } + rescue => e + importer.set_status_info(e, importer) + raise => e end # rubocop:disable Metrics/AbcSize diff --git a/app/parsers/bulkrax/xml_parser.rb b/app/parsers/bulkrax/xml_parser.rb index 0e9a33259..9f6f12604 100644 --- a/app/parsers/bulkrax/xml_parser.rb +++ b/app/parsers/bulkrax/xml_parser.rb @@ -62,6 +62,10 @@ def records(_opts = {}) # Retrieve all records elements = entry_class.read_data(md).xpath("//#{record_element}") r += elements.map { |el| entry_class.data_for_entry(el, source_identifier, self) } + rescue => e + importer = Importer.find(entry_class.runnable_id) + importer.set_status_info(e, importer) + raise => e end # Flatten because we may have multiple records per array r.compact.flatten @@ -69,6 +73,10 @@ def records(_opts = {}) metadata_paths.map do |md| data = entry_class.read_data(md).xpath("//#{record_element}").first # Take only the first record entry_class.data_for_entry(data, source_identifier, self) + rescue => e + importer = Importer.find(entry_class.runnable_id) + importer.set_status_info(e, importer) + raise => e end.compact # No need to flatten because we take only the first record end end diff --git a/app/views/bulkrax/shared/_bulkrax_errors.html.erb b/app/views/bulkrax/shared/_bulkrax_errors.html.erb index 5cdeba35f..5414450ba 100644 --- a/app/views/bulkrax/shared/_bulkrax_errors.html.erb +++ b/app/views/bulkrax/shared/_bulkrax_errors.html.erb @@ -1,8 +1,12 @@ -<% if item.failed? %> +<% if item.succeeded? %> +

+ Succeeded At: <%= item.status_at %> +

+<% elsif item.failed? %>
-<% elsif item.succeeded? %> -

- Succeeded At: <%= item.status_at %> -

-<% else %> -

- Succeeded At: Item has not yet been <%= @importer.present? ? 'imported' : 'exported' %> successfully -

<% end %> + +
+ +
+
+
+ +
+ + +
+ +
+ <% item.statuses.map do |item_status| %> +
+ Occured at: <%= item_status.updated_at %>

+ Link to Status Page: <%= <%= link_to "Status Details", statuses_path(item_status.id) %>%> + Message: <%= item_status.error_class %> - <%= item_status.error_message %>

+
+
+ Occured at: <%= item_status.updated_at %>

+ Link to Status Page: <%= <%= link_to "Status Details", statuses_path(item_status.id) %>%> + Message: <%= item_status.error_class %> - <%= item_status.error_message %>

+
+ <% end %> +
+
+
+
+
diff --git a/app/views/bulkrax/statuses/show.html.erb b/app/views/bulkrax/statuses/show.html.erb new file mode 100644 index 000000000..5414450ba --- /dev/null +++ b/app/views/bulkrax/statuses/show.html.erb @@ -0,0 +1,85 @@ +<% if item.succeeded? %> +

+ Succeeded At: <%= item.status_at %> +

+<% elsif item.failed? %> +
+ +
+
+
+ +
+ + +
+ +
+
+ Errored at: <%= item.status_at %>

+ Error: <%= item.current_status.error_class %> - <%= item.current_status.error_message %>

+ Error Trace:

+ <% item.current_status.error_backtrace.each do |v| %> + <%= coderay(v, { wrap: :page, css: :class, tab_width: 200, break_lines: true }) %> +
+ <% end %> +
+
+ Errored at: <%= item.status_at %>

+ Error: <%= item.current_status.error_class %> - <%= item.current_status.error_message %>

+ Error Trace:

+ <% item.current_status.error_backtrace.each do |v| %> + <%= coderay(v, { css: :class, tab_width: 0, break_lines: false }) %> +
+ <% end %> +
+
+
+
+
+
+<% end %> + +
+ +
+
+
+ +
+ + +
+ +
+ <% item.statuses.map do |item_status| %> +
+ Occured at: <%= item_status.updated_at %>

+ Link to Status Page: <%= <%= link_to "Status Details", statuses_path(item_status.id) %>%> + Message: <%= item_status.error_class %> - <%= item_status.error_message %>

+
+
+ Occured at: <%= item_status.updated_at %>

+ Link to Status Page: <%= <%= link_to "Status Details", statuses_path(item_status.id) %>%> + Message: <%= item_status.error_class %> - <%= item_status.error_message %>

+
+ <% end %> +
+
+
+
+
diff --git a/config/routes.rb b/config/routes.rb index c4dfc4c6b..286ea5343 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -8,6 +8,7 @@ get :exporter_table end resources :entries, only: %i[show update destroy] + resources :statuses, only: :show end resources :importers do put :continue @@ -19,6 +20,7 @@ post :external_sets end resources :entries, only: %i[show update destroy] + resources :statuses, only: :show get :upload_corrected_entries post :upload_corrected_entries_file end diff --git a/spec/controllers/concerns/bulkrax/datatables_behavior_spec.rb b/spec/controllers/concerns/bulkrax/datatables_behavior_spec.rb index abfca8849..38e185cf0 100644 --- a/spec/controllers/concerns/bulkrax/datatables_behavior_spec.rb +++ b/spec/controllers/concerns/bulkrax/datatables_behavior_spec.rb @@ -95,6 +95,33 @@ def current_user end end + describe '#format_statuses' do + let(:item) { FactoryBot.create(:bulkrax_importer) } + let(:entry_1) { FactoryBot.create(:bulkrax_entry, importerexporter: item) } + let(:entry_2) { FactoryBot.create(:bulkrax_entry, importerexporter: item) } + let(:entries) { [entry_1, entry_2] } + let(:status_1) { FactoryBot.create(:bulkrax_status) } + let(:status_2) { FactoryBot.create(:bulkrax_status) } + let(:status_3) { FactoryBot.create(:bulkrax_status) } + let(:statuses) { [status_1, status_2, status_3] } + + it 'returns a hash with the correct structure' do + get :index + result = controller.format_statuses(statuses, item) + expect(result).to be_a(Hash) + expect(result.keys).to contain_exactly(:data, :recordsTotal, :recordsFiltered) + expect(result[:data]).to be_a(Array) + expect(result[:data].first.keys).to contain_exactly(:identifier, :id, :status_message, :type, :updated_at, :errors, :actions) + end + + it 'returns the correct number of statuses' do + get :index + result = controller.format_statuses(statuses, item) + expect(result[:recordsTotal]).to eq(statuses.size) + expect(result[:recordsFiltered]).to eq(statuses.size) + end + end + describe '#entry_util_links' do include Bulkrax::Engine.routes.url_helpers @@ -124,6 +151,28 @@ def current_user end end + describe '#status_util_links' do + include Bulkrax::Engine.routes.url_helpers + + let(:item) { FactoryBot.create(:bulkrax_importer) } + let(:entry) { FactoryBot.create(:bulkrax_entry, importerexporter: item) } + let(:status) { FactoryBot.create(:bulkrax_status) } + + it 'returns a string of HTML links' do + get :index + result = controller.status_util_links(status, item) + expect(result).to be_a(String) + expect(result).to include('fa-info-circle') + expect(result).to include('fa-repeat') + end + + it 'includes a link to the status' do + get :index + result = controller.status_util_links(status, item) + expect(result).to include(importer_status_path(item, status)) + end + end + describe '#status_message_for' do let(:item) { FactoryBot.create(:bulkrax_importer) } diff --git a/spec/models/bulkrax/csv_entry_spec.rb b/spec/models/bulkrax/csv_entry_spec.rb index eb212e257..102212b08 100644 --- a/spec/models/bulkrax/csv_entry_spec.rb +++ b/spec/models/bulkrax/csv_entry_spec.rb @@ -118,7 +118,14 @@ class ::Avacado < Work end it 'fails and stores an error' do + expect(subject.status_message).to eq 'Pending' + expect(subject.error_class).to eq nil expect { subject.build_metadata }.to raise_error(StandardError) + expect(subject.status_message).to eq 'Failed' + expect(subject.statuses[0].status_message).to eq 'Failed' + expect(subject.statuses[0].error_message).to eq 'Missing required elements, missing element(s) are: title' + expect(subject.statuses[0].error_backtrace.nil?).to eq false + expect(subject.error_class).to eq 'StandardError' end end @@ -227,8 +234,12 @@ class ::Avacado < Work end it 'raises a StandardError' do + expect(subject.status_message).to eq 'Pending' + expect(subject.error_class).to eq nil expect { subject.build_metadata } .to raise_error(StandardError, %("hello world" is not a valid and/or active authority ID for the :rights_statement field)) + expect(subject.status_message).to eq 'Failed' + expect(subject.error_class).to eq 'StandardError' end end end @@ -282,8 +293,12 @@ class ::Avacado < Work end it 'raises a StandardError' do + expect(subject.status_message).to eq 'Pending' + expect(subject.error_class).to eq nil expect { subject.build_metadata } .to raise_error(StandardError, %("hello world" is not a valid and/or active authority ID for the :license field)) + expect(subject.status_message).to eq 'Failed' + expect(subject.error_class).to eq 'StandardError' end end end diff --git a/spec/models/bulkrax/status_spec.rb b/spec/models/bulkrax/status_spec.rb index f4dff9655..6d242f763 100644 --- a/spec/models/bulkrax/status_spec.rb +++ b/spec/models/bulkrax/status_spec.rb @@ -4,6 +4,117 @@ module Bulkrax RSpec.describe Status, type: :model do - pending "add some examples to (or delete) #{__FILE__}" + context 'for exporters' do + let(:exporter_without_errors) { FactoryBot.create(:bulkrax_exporter) } + let(:exporter_with_errors) { Bulkrax::Exporter.new } + context 'when there are no errors' do + it 'can display the current status' do + exporter_without_errors.save + expect(exporter_without_errors.statuses.count).to eq 0 + expect(exporter_without_errors.last_error_at).to eq nil + expect(exporter_without_errors.error_class).to eq nil + expect(exporter_without_errors.status).to eq 'Pending' + end + end + context 'when there are errors' do + it 'can display a history' do + end + end + end + context 'for importers' do + let(:importer_without_errors) { FactoryBot.create(:bulkrax_importer) } + let(:importer_with_errors) { Bulkrax::Importer.new } + context 'when there are no errors' do + it 'can display the current status' do + importer_without_errors.save + expect(importer_without_errors.statuses.count).to eq 0 + expect(importer_without_errors.last_error_at).to eq nil + expect(importer_without_errors.error_class).to eq nil + expect(importer_without_errors.status).to eq 'Pending' + end + end + context 'when there are errors' do + it 'can display a history' do + # expect { importer_with_errors.import_objects }.to raise_error(StandardError) + importer_with_errors.import_objects + expect(importer_with_errors.statuses.count).to eq 1 + expect(importer_with_errors.status_message).to eq 'Failed' + expect(importer_with_errors.statuses[0].status_message).to eq 'Failed' + expect(importer_with_errors.statuses[0].error_message).to eq 'Missing required elements, missing element(s) are: title' + expect(importer_with_errors.statuses[0].error_backtrace.nil?).to eq false + expect(importer_with_errors.error_class).to eq 'StandardError' + expect { importer_with_errors.import_objects }.to raise_error(StandardError) + expect(importer_with_errors.statuses.count).to eq 2 + end + end + end + context 'for csv entries' do + let(:importer) { FactoryBot.create(:bulkrax_importer_csv) } + let(:entry_without_errors) { FactoryBot.create(:bulkrax_csv_entry) } + let(:entry_with_errors) { Bulkrax::CsvEntry.new(importerexporter: importer) } + let(:collection) { FactoryBot.build(:collection) } + let(:hyrax_record) do + OpenStruct.new( + file_sets: [], + member_of_collections: [], + member_of_work_ids: [], + in_work_ids: [], + member_work_ids: [] + ) + end + + before do + allow(entry_with_errors).to receive(:hyrax_record).and_return(hyrax_record) + end + + context 'when there are no errors' do + it 'can display the current status' do + entry_without_errors.save + expect(entry_without_errors.statuses.count).to eq 0 + expect(entry_without_errors.last_error_at).to eq nil + expect(entry_without_errors.error_class).to eq nil + expect(entry_without_errors.status).to eq 'Pending' + end + end + context 'when there are errors' do + context 'for missing metadata' do + before do + allow_any_instance_of(Bulkrax::CsvEntry).to receive(:collections_created?).and_return(true) + allow_any_instance_of(Bulkrax::CsvEntry).to receive(:find_collection).and_return(collection) + allow(entry_with_errors).to receive(:raw_metadata).and_return(source_identifier: '1', some_field: 'some data') + end + it 'can display a history' do + expect { entry_with_errors.build_metadata }.to raise_error(StandardError) + expect(entry_with_errors.statuses.count).to eq 1 + expect(entry_with_errors.status_message).to eq 'Failed' + expect(entry_with_errors.statuses[0].status_message).to eq 'Failed' + expect(entry_with_errors.statuses[0].error_message).to eq 'Missing required elements, missing element(s) are: title' + expect(entry_with_errors.statuses[0].error_backtrace.nil?).to eq false + expect(entry_with_errors.error_class).to eq 'StandardError' + expect { entry_with_errors.build_metadata }.to raise_error(StandardError) + expect(entry_with_errors.statuses.count).to eq 2 + end + end + + context 'for missing collection' do + before do + allow_any_instance_of(Bulkrax::CsvEntry).to receive(:collections_created?).and_return(false) + allow_any_instance_of(Bulkrax::CsvEntry).to receive(:find_collection).and_return(nil) + allow(entry_with_errors).to receive(:raw_metadata).and_return(source_identifier: '1', some_field: 'some data', title: 'Missing Collection Example') + end + it 'can display a history' do + expect { entry_with_errors.build_metadata }.to raise_error(CollectionsCreatedError) + expect(entry_with_errors.statuses.count).to eq 1 + expect(entry_with_errors.status_message).to eq 'Failed' + expect(entry_with_errors.statuses[0].status_message).to eq 'Failed' + expect(entry_with_errors.statuses[0].error_message).to eq 'Missing required elements, missing element(s) are: title' + expect(entry_with_errors.statuses[0].error_backtrace.nil?).to eq false + expect(entry_with_errors.error_class).to eq 'StandardError' + expect { entry_with_errors.build_metadata }.to raise_error(StandardError) + expect(entry_with_errors.statuses.count).to eq 2 + end + end + end + end end end diff --git a/spec/test_app/db/schema.rb b/spec/test_app/db/schema.rb index f14b49ade..97b64f1da 100644 --- a/spec/test_app/db/schema.rb +++ b/spec/test_app/db/schema.rb @@ -104,6 +104,8 @@ t.integer "total_file_set_entries", default: 0 t.integer "processed_works", default: 0 t.integer "failed_works", default: 0 + t.integer "processed_children", default: 0 + t.integer "failed_children", default: 0 t.index ["importer_id"], name: "index_bulkrax_importer_runs_on_importer_id" end