From 7d985dda63bb027fb9f1cb7416709a54a420968a Mon Sep 17 00:00:00 2001
From: Kirk Wang <kirk.wang@scientist.com>
Date: Mon, 16 Sep 2024 16:46:14 -0700
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fix=20importers=20sorting=20for?=
 =?UTF-8?q?=20last=20run=20and=20next=20run?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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:
- https://github.com/samvera/bulkrax/issues/956
---
 app/models/bulkrax/importer.rb                | 25 +++++++++++++++++++
 app/models/bulkrax/importer_run.rb            | 11 ++++++++
 ...d_last_imported_at_to_bulkrax_importers.rb |  5 ++++
 ...add_next_import_at_to_bulkrax_importers.rb |  5 ++++
 4 files changed, 46 insertions(+)
 create mode 100644 db/migrate/20240916182737_add_last_imported_at_to_bulkrax_importers.rb
 create mode 100644 db/migrate/20240916182823_add_next_import_at_to_bulkrax_importers.rb

diff --git a/app/models/bulkrax/importer.rb b/app/models/bulkrax/importer.rb
index 4ef79f2e..3e5d0196 100644
--- a/app/models/bulkrax/importer.rb
+++ b/app/models/bulkrax/importer.rb
@@ -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
 
@@ -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
diff --git a/app/models/bulkrax/importer_run.rb b/app/models/bulkrax/importer_run.rb
index 132cdd1d..e9e139b6 100644
--- a/app/models/bulkrax/importer_run.rb
+++ b/app/models/bulkrax/importer_run.rb
@@ -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
@@ -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
diff --git a/db/migrate/20240916182737_add_last_imported_at_to_bulkrax_importers.rb b/db/migrate/20240916182737_add_last_imported_at_to_bulkrax_importers.rb
new file mode 100644
index 00000000..3cd782d0
--- /dev/null
+++ b/db/migrate/20240916182737_add_last_imported_at_to_bulkrax_importers.rb
@@ -0,0 +1,5 @@
+class AddLastImportedAtToBulkraxImporters < ActiveRecord::Migration[5.1]
+  def change
+    add_column :bulkrax_importers, :last_imported_at, :datetime
+  end
+end
diff --git a/db/migrate/20240916182823_add_next_import_at_to_bulkrax_importers.rb b/db/migrate/20240916182823_add_next_import_at_to_bulkrax_importers.rb
new file mode 100644
index 00000000..26a76ce0
--- /dev/null
+++ b/db/migrate/20240916182823_add_next_import_at_to_bulkrax_importers.rb
@@ -0,0 +1,5 @@
+class AddNextImportAtToBulkraxImporters < ActiveRecord::Migration[5.1]
+  def change
+    add_column :bulkrax_importers, :next_import_at, :datetime
+  end
+end