Refactors transaction step for saving admin data

app/models/admin_data.rb

@@ -1,21 +1,29 @@
 class AdminData < ApplicationRecord
+  self.table_name = "admin_data"
+
+  include ::EmptyDetection
+
   attr_reader :asset_error
 
   belongs_to  :hyrax_batch_ingest_batch, optional: true
   belongs_to  :bulkrax_importer, optional: true, class_name: 'Bulkrax::Importer'
   has_many    :annotations, dependent: :destroy
   accepts_nested_attributes_for :annotations, allow_destroy: true
-
-  self.table_name = "admin_data"
-  include ::EmptyDetection
-
   serialize :sonyci_id, Array
+  validate :validate_undeletable_fields
+
+  # TODO: Only used in AssetActor, which is deprecated. One AssetActor is
+  # removed, remove SERIALIZED_FIELD as well.
 
   SERIALIZED_FIELDS = [ :sonyci_id ]
 
+  # Mark fields that should not be deletable once set. These are used in validation.
+  UNDELETABLE_FIELDS = %w(bulkrax_importer_id hyrax_batch_ingest_batch_id)
+
   # Find the admin data associated with the Global Identifier (gid)
   # @param [String] gid - Global Identifier for this admin_data (e.g.gid://ams/admindata/1)
-  # @return [AdminData] if record matching gid is found, an instance of AdminData with id = the model_id portion of the gid (e.g. 1)
+  # @return [AdminData] if record matching gid is found, an instance of
+  #   AdminData with id = the model_id portion of the gid (e.g. 1)
   # @return [False] if record matching gid is not found
   def self.find_by_gid(gid)
     find(GlobalID.new(gid).model_id)
@@ -24,8 +32,10 @@ def self.find_by_gid(gid)
   end
 
   # Find the admin data associated with the Global Identifier (gid)
-  # @param [String] gid - Global Identifier for this adimindata (e.g. gid://ams/admindata/1)
-  # @return [AdminData] an instance of AdminData with id = the model_id portion of the gid (e.g. 1)
+  # @param [String] gid - Global Identifier for this adimindata\
+  #   (e.g. gid://ams/admindata/1)
+  # @return [AdminData] an instance of AdminData with id = the model_id portion
+  #   of the gid (e.g. 1)
   # @raise [ActiveRecord::RecordNotFound] if record matching gid is not found
   def self.find_by_gid!(gid)
     result = find_by_gid(gid)
@@ -35,11 +45,12 @@ def self.find_by_gid!(gid)
 
   # These are the attributes that could be edited through a form or through ingest.
   def self.attributes_for_update
-    (AdminData.attribute_names.dup - ['id', 'created_at', 'updated_at']).map(&:to_sym)
+    AdminData.attribute_names.dup - ['id', 'created_at', 'updated_at']
   end
 
   # Return the Global Identifier for this admin data.
-  # @return [String] Global Identifier (gid) for this AdminData (e.g.gid://ams/admindata/1)
+  # @return [String] Global Identifier (gid) for this AdminData
+  #   (e.g.gid://ams/admindata/1)
   def gid
     URI::GID.build(app: GlobalID.app, model_name: model_name.name.parameterize.to_sym, model_id: id).to_s if id
   end
@@ -78,4 +89,14 @@ def sonyci_records
   def sony_ci_api
     @sony_ci_api ||= SonyCiApi::Client.new('config/ci.yml')
   end
+
+  def validate_undeletable_fields
+    UNDELETABLE_FIELDS.each do |field|
+      # NOTE: the {field}_was helper comes from ActiveModel::Dirty
+      new_val, old_val = send(field), send("#{field}_was")
+      if new_val.blank? && old_val.present?
+        errors.add(field, "can't change from #{old_val} to blank")
+      end
+    end
+  end
 end

app/services/aapb/batch_ingest/csv_item_ingester.rb

@@ -213,8 +213,8 @@ def set_batch_ingest_id_on_related_asset(work_id, ability)
         def set_admin_data_attributes(admin_data, attributes)
           # add existing admin_data values so they're preserved in the AssetActor
           AdminData.attributes_for_update.each do |admin_attr|
-            next unless attributes.keys.include?(admin_attr.to_s)
-            admin_data.send("#{admin_attr}=", attributes[admin_attr.to_s])
+            next unless attributes.keys.include?(admin_attr)
+            admin_data.send("#{admin_attr}=", attributes[admin_attr])
           end
         end
 

app/transactions/ams/steps/create_aapb_admin_data.rb

@@ -51,22 +51,12 @@ def save_aapb_admin_data(change_set)
       end
 
       def set_admin_data_attributes(admin_data, change_set)
-        AdminData.attributes_for_update.each do |k|
-          # Some attributes are serialized on AdminData, so always send an array
-          k_string = k.to_s
-          if should_empty_admin_data_value?(k, change_set)
-            AdminData::SERIALIZED_FIELDS.include?(k) ? admin_data.send("#{k}=", Array.new) : admin_data.send("#{k}=", nil)
-          elsif change_set.fields[k_string].present?
-            AdminData::SERIALIZED_FIELDS.include?(k) ? admin_data.send("#{k}=", Array(change_set.fields[k_string])) : admin_data.send("#{k}=", change_set.fields[k_string].to_s)
-          end
+        admin_data_values = change_set.fields.slice(*AdminData.attributes_for_update).compact
+        # If Sony Ci IDs are present, ensure there are no blank ones, such as empty strings.
+        if admin_data_values.key?('sonyci_id')
+          admin_data_values['sonyci_id'] = Array(admin_data_values['sonyci_id']).reject(&:blank?)
         end
-      end
-
-      def should_empty_admin_data_value?(key, change_set)
-        return false if %i[bulkrax_importer_id hyrax_batch_ingest_batch_id].include?(key)
-
-        # The presence of the key with a "blank" value indicates we're intentionally emptying the value
-        change_set.fields.key?(key) && change_set.fields[key].blank?
+        admin_data.assign_attributes(admin_data_values)
       end
 
       def delete_removed_annotations(admin_data, change_set)