Skip to content

Commit

Permalink
🐛 Fix importers sorting for last run and next run
Browse files Browse the repository at this point in the history
This commit will fix the sorting error that happens when the user is on
the importers index and attempts to sort by the last run or next run. We
add some migrations to add fields that the datatables can use so it can
sort properly. Previously, this was not working because the
last_imported_at and next_import_at fields were actually methods on the
importer_run object and not the importer object.  We are adding a few
callbacks to the importer and importer_run models to ensure that the
fields are properly set when they are called from either the web or the
worker.

Ref:
- #956
  • Loading branch information
kirkkwang committed Sep 16, 2024
1 parent b511db6 commit 7d985dd
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 0 deletions.
25 changes: 25 additions & 0 deletions app/models/bulkrax/importer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ class Importer < ApplicationRecord # rubocop:disable Metrics/ClassLength

delegate :create_parent_child_relationships, :valid_import?, :write_errored_entries_file, :visibility, to: :parser

after_save :set_last_imported_at_from_importer_run
after_save :set_next_import_at_from_importer_run

attr_accessor :only_updates, :file_style, :file
attr_writer :current_run

Expand Down Expand Up @@ -247,5 +250,27 @@ def path_string
rescue
"#{self.id}_#{self.created_at.strftime('%Y%m%d%H%M%S')}"
end

private

# Adding this here since we can update the importer without running the importer.
# When we simply save the importer (as in just updating the importer from the options),
# it does not trigger the after_save callback in the importer_run.
def set_last_imported_at_from_importer_run
return if @skip_set_last_imported_at # Prevent infinite loop

@skip_set_last_imported_at = true
importer_runs.last&.set_last_imported_at
@skip_set_last_imported_at = false
end

# @see #set_last_imported_at_from_importer_run
def set_next_import_at_from_importer_run
return if @skip_set_next_import_at # Prevent infinite loop

@skip_set_next_import_at = true
importer_runs.last&.set_next_import_at
@skip_set_next_import_at = false
end
end
end
11 changes: 11 additions & 0 deletions app/models/bulkrax/importer_run.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ class ImporterRun < ApplicationRecord
has_many :statuses, as: :runnable, dependent: :destroy
has_many :pending_relationships, dependent: :destroy

after_save :set_last_imported_at
after_save :set_next_import_at

def parents
pending_relationships.pluck(:parent_id).uniq
end
Expand All @@ -15,5 +18,13 @@ def user
# fallback to the configured user.
importer.user || Bulkrax.fallback_user_for_importer_exporter_processing
end

def set_last_imported_at
importer.update(last_imported_at: importer.last_imported_at)
end

def set_next_import_at
importer.update(next_import_at: importer.next_import_at)
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddLastImportedAtToBulkraxImporters < ActiveRecord::Migration[5.1]
def change
add_column :bulkrax_importers, :last_imported_at, :datetime
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddNextImportAtToBulkraxImporters < ActiveRecord::Migration[5.1]
def change
add_column :bulkrax_importers, :next_import_at, :datetime
end
end

0 comments on commit 7d985dd

Please sign in to comment.