🐛 Fix importers sorting for last run and next run (#977)

* 🐛 Fix importers sorting for last run and next run

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

* 🤖 Update upload-artifact and download-artifact

CI was getting errors because the versions we were previously using were
deprecated.  This commit updates the actions to the latest versions.

* 🐛 Remove Downloadable Files sorting

The downloadable files sorting was broken plus, it's not clear now a
downloadable file should be sorted.

* ⚙️ Add guard for new migrations

This commit will add a guard to the new migrations to ensure that they
do not run if the columns already exist in the database.

* 🤖 Add rake task to re save importers

This rake task will allow users to re save all their importers.  It
accounts for tenants if it is a Hyku application.

```sh
bundle exec rake bulkrax:resave_importers
```

.github/workflows/test.yml

@@ -43,10 +43,11 @@ jobs:
         run: bundle exec rake spec
 
       - name: Upload coverage results
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v4.4.0
         with:
           name: coverage-report-${{ matrix.ruby }}
-          path: coverage
+          path: coverage/**
+          include-hidden-files: true
 
   coverage:
     runs-on: ubuntu-latest
@@ -56,7 +57,7 @@ jobs:
 
     steps:
       - name: Download coverage report
-        uses: actions/download-artifact@v2
+        uses: actions/download-artifact@v4.1.8
         with:
           name: coverage-report-2.7
           path: coverage

app/assets/javascripts/bulkrax/datatables.js

@@ -67,7 +67,7 @@ Blacklight.onLoad(function() {
         { "data": "name" },
         { "data": "status_message" },
         { "data": "created_at" },
-        { "data": "download" },
+        { "data": "download", "orderable": false },
         { "data": "actions", "orderable": false }
       ],
       initComplete: function () {

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

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

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 unless column_exists?(:bulkrax_importers, :last_imported_at)
+  end
+end

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 unless column_exists?(:bulkrax_importers, :next_import_at)
+  end
+end

lib/tasks/bulkrax_tasks.rake

@@ -141,4 +141,25 @@ namespace :bulkrax do
   rescue => e
     puts "(#{e.message})"
   end
+
+  desc "Resave importers"
+  task resave_importers: :environment do
+    if defined?(::Hyku)
+      Account.find_each do |account|
+        next if account.name == "search"
+        switch!(account)
+        puts "=============== updating #{account.name} ============"
+
+        resave_importers
+
+        puts "=============== finished updating #{account.name} ============"
+      end
+    else
+      resave_importers
+    end
+  end
+
+  def resave_importers
+    Bulkrax::Importer.find_each(&:save!)
+  end
 end