Merge pull request WGBH-MLA#728 from WGBH-MLA/i729-prevent-assets-mis…

…sing-children-from-being-pushed-to-aapb

Prevent Assets missing children from being pushed to AAPB (Validator)

app/actors/hyrax/actors/asset_actor.rb

@@ -12,6 +12,7 @@ def create(env)
         add_title_types(env)
         add_description_types(env)
         add_date_types(env)
+        set_validation_status(env)
 
         # queue indexing if we are importing
         env.curation_concern.reindex_extent = "queue#{env.importing.id}" if env.importing
@@ -23,6 +24,8 @@ def update(env)
         add_title_types(env)
         add_description_types(env)
         add_date_types(env)
+        set_validation_status(env)
+
         # queue indexing if we are importing
         env.curation_concern.reindex_extent = "queue#{env.importing.id}" if env.importing
         save_aapb_admin_data(env) && super && create_or_update_contributions(env, contributions)
@@ -220,6 +223,17 @@ def typed_value_present?(values)
         def get_typed_value(type, typed_values)
           typed_values.map { |v| v[:value] if v[:type] == type } .compact
         end
+
+        def set_validation_status(env)
+          intended_children_count = env.curation_concern.intended_children_count.to_i
+          current_children_count = env.curation_concern.all_members.size
+
+          if current_children_count != intended_children_count
+            env.curation_concern.validation_status_for_aapb = [Asset::VALIDATION_STATUSES[:missing_children]]
+          else
+            env.curation_concern.validation_status_for_aapb = [Asset::VALIDATION_STATUSES[:valid]]
+          end
+        end
     end
   end
 end

app/indexers/asset_indexer.rb

@@ -18,6 +18,7 @@ def generate_solr_document
       solr_doc['created_date_drsim'] = object.created_date if object.created_date
       solr_doc['copyright_date_drsim'] = object.copyright_date if object.copyright_date
       solr_doc[Solrizer.solr_name('bulkrax_identifier', :facetable)] = object.bulkrax_identifier
+      solr_doc['intended_children_count_isi'] = object.intended_children_count.to_i
 
       if object.admin_data
         # Index the admin_data_gid

app/models/asset.rb

@@ -5,6 +5,12 @@ class Asset < ActiveFedora::Base
   include ::AMS::CascadeDestroyMembers
   include ::AMS::AllMembers
 
+  # @see Push#add_status_error
+  VALIDATION_STATUSES = {
+    valid: 'valid',
+    missing_children: 'missing child record(s)'
+  }.freeze
+
   self.indexer = AssetIndexer
   before_save :save_admin_data
   # Change this to restrict which works can be added as a child.
@@ -204,6 +210,15 @@ def annotations
     index.as :stored_searchable
   end
 
+  # This property is intended to be an Integer, but ActiveFedora stores everything as Strings. We don't declare indexing
+  # rules in a block here because indexing for this property is handled manually.
+  # @see AssetIndexer#generate_solr_document
+  property :intended_children_count, predicate: ::RDF::URI("http://ams2.wgbh-mla.org/resource#intendedChildrenCount"), multiple: false
+
+  property :validation_status_for_aapb, predicate: ::RDF::URI("http://ams2.wgbh-mla.org/resource#validationStatusForAapb"), multiple: true do |index|
+    index.as :stored_searchable
+  end
+
   def admin_data_gid=(new_admin_data_gid)
     raise "Can't modify admin data of this asset" if persisted? && !admin_data_gid_was.nil? && admin_data_gid_was != new_admin_data_gid
     new_admin_data = AdminData.find_by_gid!(new_admin_data_gid)

app/models/push.rb

@@ -6,6 +6,7 @@ class Push < ApplicationRecord
     errors.add(:user, "is required") unless user.is_a? User
     missing_ids = ids_not_found
     errors.add(:pushed_id_csv, "The following IDs are not found in the repository: #{missing_ids.join(', ')}") unless missing_ids.empty?
+    validate_status
   end
 
   # NOTE: do not memoize
@@ -27,6 +28,24 @@ def ids_not_found
       push_ids - found_ids
     end
 
+    def validate_status
+      invalid_docs = export_search.solr_documents.reject do |doc|
+        doc.validation_status_for_aapb == [Asset::VALIDATION_STATUSES[:valid]]
+      end
+      return if invalid_docs.blank?
+
+      add_status_error(invalid_docs, Asset::VALIDATION_STATUSES[:missing_children])
+    end
+
+    def add_status_error(invalid_docs, status)
+      ids_matching_status = invalid_docs.select { |doc| doc.validation_status_for_aapb.include?(status) }.map(&:id)
+      # Prevents adding errors to docs that don't have a value
+      # in :validation_status_for_aapb, including all non-Assets.
+      return if ids_matching_status.blank?
+
+      errors.add(:pushed_id_csv, "The following IDs are #{status}: #{ids_matching_status.join(', ')}")
+    end
+
     # NOTE: do not memoize
     # @return [AMS::Export::Search::CombinedIDSearch] instance used for performing
     #   search and returning solr document results.

app/models/solr_document.rb

@@ -17,6 +17,9 @@ class SolrDocument
   SolrDocument.use_extension(AMS::CsvExportExtension)
   SolrDocument.use_extension(AMS::PbcoreXmlExportExtension)
 
+  attribute :intended_children_count, Solr::String, 'intended_children_count_isi'
+  attribute :validation_status_for_aapb, Solr::Array, 'validation_status_for_aapb_tesim'
+
   # DublinCore uses the semantic field mappings below to assemble an OAI-compliant Dublin Core document
   # Semantic mappings of solr stored fields. Fields may be multi or
   # single valued. See Blacklight::Document::SemanticFields#field_semantics
@@ -451,7 +454,7 @@ def aapb_preservation_disk
     self[Solrizer.solr_name('aapb_preservation_disk')]
   end
 
-  def bulkrax_importer_id 
+  def bulkrax_importer_id
     self[Solrizer.solr_name('bulkrax_importer_id')]
   end
 

app/parsers/pbcore_xml_parser.rb

@@ -152,6 +152,7 @@ def set_objects(file, index)
 
   def instantiation_rows(instantiations, xml_asset, asset, asset_id)
     xml_records = []
+    children_count = 0
     instantiations.each.with_index do |inst, i|
       instantiation_class =  'PhysicalInstantiation' if inst.physical
       instantiation_class ||= 'DigitalInstantiation' if inst.digital
@@ -170,6 +171,7 @@ def instantiation_rows(instantiations, xml_asset, asset, asset_id)
         parse_rows([xml_track], 'EssenceTrack', asset_id, asset, j+1)
         xml_record[:children] << xml_track[work_identifier]
         xml_tracks << xml_track
+        children_count += 1
       end
       parse_rows([xml_record], instantiation_class, asset_id, asset)
       add_object(xml_record)
@@ -178,7 +180,9 @@ def instantiation_rows(instantiations, xml_asset, asset, asset_id)
     end
     xml_records.each do |row|
       xml_asset[:children] << row[work_identifier]
+      children_count += 1
     end
+    xml_asset[:intended_children_count] = children_count
   end
 
   def parse_rows(rows, type, asset_id, parent_asset = nil, counter = nil)

app/views/pushes/new.html.erb

@@ -47,7 +47,8 @@
   function validate(id_field_val){
     $.post('/pushes/validate_ids', { id_field: id_field_val }, function(resp) {
       $('#loader').hide();
-
+      $('#valid-box').empty();
+
       var message;
       if(resp.error) {
         message = resp.error;
@@ -57,7 +58,10 @@
         $('#valid-box').addClass("green").removeClass("red");
       }
 
-      $('#valid-box').text(message);
+      messages_array = message.split("\n\n")
+      messages_array.forEach(msg => {
+        $('#valid-box').append('<p>' + msg + '</p>')
+      });
     });
   }
 </script>