Fix TypeError when parsing metadata (samvera#387)

* fix TypeError, do not add blank values to parsed metadata, always respect specified position when importing multiple_objects, update specs

* do not try to set position specifically (too complicated), cleanup

* generate rubocop todo

.rubocop_todo.yml

@@ -1,49 +1,49 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2021-12-03 16:15:13 -0800 using RuboCop version 0.85.1.
+# on 2021-12-14 15:16:54 -0800 using RuboCop version 0.85.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 4
+# Offense count: 6
 # Cop supports --auto-correct.
 # Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
 # URISchemes: http, https
 Layout/LineLength:
   Max: 301
 
-# Offense count: 18
+# Offense count: 17
 # Configuration parameters: IgnoredMethods.
 Metrics/AbcSize:
   Max: 42
 
 # Offense count: 6
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 198
+  Max: 197
 
 # Offense count: 10
 # Configuration parameters: IgnoredMethods.
 Metrics/CyclomaticComplexity:
-  Max: 11
+  Max: 12
 
-# Offense count: 25
+# Offense count: 26
 # Configuration parameters: CountComments, ExcludedMethods.
 Metrics/MethodLength:
   Max: 26
 
-# Offense count: 4
+# Offense count: 3
 # Configuration parameters: CountComments.
 Metrics/ModuleLength:
   Max: 131
 
-# Offense count: 11
+# Offense count: 10
 # Configuration parameters: IgnoredMethods.
 Metrics/PerceivedComplexity:
-  Max: 12
+  Max: 13
 
-# Offense count: 24
+# Offense count: 25
 RSpec/AnyInstance:
   Exclude:
     - 'spec/models/bulkrax/csv_entry_spec.rb'
@@ -66,13 +66,13 @@ RSpec/MessageChain:
     - 'spec/parsers/bulkrax/csv_parser_spec.rb'
     - 'spec/parsers/bulkrax/xml_parser_spec.rb'
 
-# Offense count: 76
+# Offense count: 80
 # Configuration parameters: .
 # SupportedStyles: have_received, receive
 RSpec/MessageSpies:
   EnforcedStyle: receive
 
-# Offense count: 132
+# Offense count: 178
 # Configuration parameters: IgnoreSharedExamples.
 RSpec/NamedSubject:
   Exclude:
@@ -86,3 +86,10 @@ RSpec/NamedSubject:
 RSpec/RepeatedDescription:
   Exclude:
     - 'spec/models/bulkrax/oai_entry_spec.rb'
+
+# Offense count: 2
+# Cop supports --auto-correct.
+Style/IfUnlessModifier:
+  Exclude:
+    - 'app/models/bulkrax/csv_entry.rb'
+    - 'lib/generators/bulkrax/templates/config/initializers/bulkrax.rb'

app/models/bulkrax/csv_entry.rb

@@ -68,8 +68,9 @@ def add_ingested_metadata
         'Creating Collections using the collection_field_mapping will no longer be supported as of version Bulkrax v2.' \
         ' Please configure Bulkrax to use related_parents_field_mapping and related_children_field_mapping instead.'
       )
-      record.each do |key, value|
+      record.sort.each do |key, value|
         next if self.parser.collection_field_mapping.to_s == key_without_numbers(key)
+        next if value.blank?
 
         index = key[/\d+/].to_i - 1 if key[/\d+/].to_i != 0
         add_metadata(key_without_numbers(key), value, index)

app/models/concerns/bulkrax/has_matchers.rb

@@ -52,11 +52,18 @@ def add_metadata(node_name, node_content, index = nil)
                   single_metadata(node_content)
                 end
 
-        set_parsed_data(object_multiple, object_name, name, index, value)
+        object_name.present? ? set_parsed_object_data(object_multiple, object_name, name, index, value) : set_parsed_data(name, value)
       end
     end
 
-    def set_parsed_data(object_multiple, object_name, name, index, value)
+    def set_parsed_data(name, value)
+      return parsed_metadata[name] = value unless multiple?(name)
+
+      parsed_metadata[name] ||= []
+      parsed_metadata[name] += Array.wrap(value).flatten
+    end
+
+    def set_parsed_object_data(object_multiple, object_name, name, index, value)
       if object_multiple
         index ||= 0
         parsed_metadata[object_name][index] ||= {}
@@ -66,22 +73,13 @@ def set_parsed_data(object_multiple, object_name, name, index, value)
         else
           parsed_metadata[object_name][index][name] = value
         end
-      elsif object_name
+      else
         parsed_metadata[object_name][name] ||= []
         if value.is_a?(Array)
           parsed_metadata[object_name][name] += value
         else
           parsed_metadata[object_name][name] = value
         end
-      else
-        parsed_metadata[name] ||= []
-        if value.is_a?(Array)
-          parsed_metadata[name] += value
-        elsif index.present? # if index is present, the key has multiple values
-          parsed_metadata[name] += [value].flatten
-        else
-          parsed_metadata[name] = value
-        end
       end
     end
 
@@ -153,7 +151,16 @@ def multiple?(field)
         'Creating Collections using the collection_field_mapping will no longer be supported as of version Bulkrax v2.' \
         ' Please configure Bulkrax to use related_parents_field_mapping and related_children_field_mapping instead.'
       )
-      return true if %W[file remote_files #{parser.collection_field_mapping}].include?(field)
+      @multiple_bulkrax_fields ||=
+        %W[
+          file
+          remote_files
+          #{parser.collection_field_mapping}
+          #{related_parents_parsed_mapping}
+          #{related_children_parsed_mapping}
+        ]
+
+      return true if @multiple_bulkrax_fields.include?(field)
       return false if field == 'model'
 
       field_supported?(field) && factory_class&.properties&.[](field)&.[]('multiple')

spec/factories/bulkrax_importers.rb

@@ -47,8 +47,8 @@
     trait :with_relationships_mappings do
       field_mapping do
         {
-          'parents' => { 'from' => ['parents_column'], related_parents_field_mapping: true },
-          'children' => { 'from' => ['children_column'], related_children_field_mapping: true }
+          'parents' => { 'from' => ['parents_column'], split: /\s*[|]\s*/, related_parents_field_mapping: true },
+          'children' => { 'from' => ['children_column'], split: /\s*[|]\s*/, related_children_field_mapping: true }
         }
       end
     end

spec/models/bulkrax/csv_entry_spec.rb

@@ -290,7 +290,7 @@ module Bulkrax
         it 'succeeds' do
           metadata = subject.build_metadata
           expect(metadata['multiple_objects'][0]['first_name']).to eq('Fake')
-          expect(metadata['multiple_objects'][0]['last_name']).to eq('')
+          expect(metadata['multiple_objects'][0]['last_name']).to eq(nil)
           expect(metadata['multiple_objects'][0]['position']).to include('Leader', 'Jester', 'Queen')
           expect(metadata['multiple_objects'][0]['language']).to eq('English')
           expect(metadata['multiple_objects'][1]['first_name']).to eq('Judge')