🐛 Normalizing export column names (#847)

Prior to this commit, when exporting a FileSet and GenericWork we'd see
a "creator_1" and "creator" column.  The "creator" column might look as
follows: `["Sandra Samvera"]`.  Which meant that creator was not an
`ActiveTriples::Relation` (based on the prior logic).  Yet the "creator"
field was stored as multi-value.  Perhaps having been previously coerced
into an Array.

With this commit, we're favoring the Bulkrax parser field mapping
`"join"` configuration over whether or not the object is an
=ActiveTriples::Relation=.  That is to say, if you told us to join the
field, we're going to do that regardless of the data we have.

Likewise, if the field's value is not an enumerable, we're not going to
introduce an ordinal suffix (e.g. "creator_1" when we have a scalar
creator).

In other words don't do the join logic if we don't have an "array" or we
weren't told to join arrays.

Perhaps we could interrogate the model to ask if it's single value or
not?  But this reduces the implementation knowledge of the properties by
looking at a more primitive level (is the data multi-valued or not).

Related to:

- notch8/palni-palci#624
- #839

Author: jeremyf

app/models/bulkrax/csv_entry.rb

@@ -243,20 +243,17 @@ def build_object(_key, value)
       object_metadata(Array.wrap(data))
     end
 
-    def build_value(key, value)
-      return unless hyrax_record.respond_to?(key.to_s)
-
-      data = hyrax_record.send(key.to_s)
-      if data.is_a?(ActiveTriples::Relation)
-        if value['join']
-          self.parsed_metadata[key_for_export(key)] = data.map { |d| prepare_export_data(d) }.join(Bulkrax.multi_value_element_join_on).to_s
-        else
-          data.each_with_index do |d, i|
-            self.parsed_metadata["#{key_for_export(key)}_#{i + 1}"] = prepare_export_data(d)
-          end
-        end
+    def build_value(property_name, mapping_config)
+      return unless hyrax_record.respond_to?(property_name.to_s)
+
+      data = hyrax_record.send(property_name.to_s)
+
+      if mapping_config['join'] || !data.is_a?(Enumerable)
+        self.parsed_metadata[key_for_export(property_name)] = prepare_export_data_with_join(data)
       else
-        self.parsed_metadata[key_for_export(key)] = prepare_export_data(data)
+        data.each_with_index do |d, i|
+          self.parsed_metadata["#{key_for_export(property_name)}_#{i + 1}"] = prepare_export_data(d)
+        end
       end
     end
 
@@ -269,6 +266,14 @@ def key_for_export(key)
       "#{unnumbered_key}#{key.sub(clean_key, '')}"
     end
 
+    def prepare_export_data_with_join(data)
+      # Yes...it's possible we're asking to coerce a multi-value but only have a single value.
+      return data.to_s unless data.is_a?(Enumerable)
+      return "" if data.empty?
+
+      data.map { |d| prepare_export_data(d) }.join(Bulkrax.multi_value_element_join_on).to_s
+    end
+
     def prepare_export_data(datum)
       if datum.is_a?(ActiveTriples::Resource)
         datum.to_uri.to_s

spec/models/bulkrax/csv_entry_spec.rb

@@ -916,6 +916,7 @@ class ::Avacado < Work
           allow(work_obj).to receive(:member_work_ids).and_return([])
           allow(work_obj).to receive(:multiple_objects).and_return(source_id_rel)
           allow(source_id_rel).to receive(:is_a?).with(ActiveTriples::Relation).and_return(true)
+          allow(source_id_rel).to receive(:is_a?).with(Enumerable).and_return(true)
           allow(source_id_rel).to receive(:to_a).and_return(['test_work_source_id'])
           allow(source_id_rel).to receive(:each_with_index).and_return(['test_work_source_id', 0])
         end