From f855e23092abd99adc24545e8774518f9b7691ab Mon Sep 17 00:00:00 2001
From: "Michael J. Giarlo" <mjgiarlo@stanford.edu>
Date: Fri, 6 Dec 2019 11:33:05 -0800
Subject: [PATCH] Stop adding successful results to errors array when creating
 virtual objects

Refs sul-dlss/argo#1728
---
 app/jobs/create_virtual_objects_job.rb       |  5 ++-
 spec/jobs/create_virtual_objects_job_spec.rb | 36 ++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/app/jobs/create_virtual_objects_job.rb b/app/jobs/create_virtual_objects_job.rb
index 6f59dfe4d..0777dd282 100644
--- a/app/jobs/create_virtual_objects_job.rb
+++ b/app/jobs/create_virtual_objects_job.rb
@@ -13,7 +13,10 @@ def perform(virtual_objects:, background_job_result:)
     virtual_objects.each do |virtual_object|
       parent_id, child_ids = virtual_object.values_at(:parent_id, :child_ids)
       # Update the constituent relationship between the parent and child druids
-      errors << ConstituentService.new(parent_druid: parent_id).add(child_druids: child_ids)
+      result = ConstituentService.new(parent_druid: parent_id).add(child_druids: child_ids)
+      # Do not add `nil`s to the errors array as they signify successful
+      # creation of the virtual object
+      errors << result if result.present?
     rescue ActiveFedora::ObjectNotFoundError, Rubydora::FedoraInvalidRequest, Dor::Exception => e
       errors << { parent_id => [e.message] }
     rescue StandardError => e
diff --git a/spec/jobs/create_virtual_objects_job_spec.rb b/spec/jobs/create_virtual_objects_job_spec.rb
index 3eb84986e..a259f8f20 100644
--- a/spec/jobs/create_virtual_objects_job_spec.rb
+++ b/spec/jobs/create_virtual_objects_job_spec.rb
@@ -62,4 +62,40 @@
       expect(result.output[:errors].first[parent_id]).to match_array(['One thing was not combinable', 'And another'])
     end
   end
+
+  context 'with a mix of errors and successful results' do
+    let(:other_child1_id) { 'druid:bar' }
+    let(:other_child2_id) { 'druid:baz' }
+    let(:other_parent_id) { 'druid:foo' }
+    let(:virtual_objects) do
+      [
+        { parent_id: parent_id, child_ids: [child1_id, child2_id] },
+        { parent_id: other_parent_id, child_ids: [other_child1_id, other_child2_id] }
+      ]
+    end
+
+    before do
+      allow(ConstituentService).to receive(:new).with(parent_druid: other_parent_id).and_return(service)
+      allow(service).to receive(:add).and_return(nil, parent_id => ['One thing was not combinable', 'And another'])
+      described_class.perform_now(virtual_objects: virtual_objects,
+                                  background_job_result: result)
+    end
+
+    it 'marks the job as processing' do
+      expect(result).to have_received(:processing!).once
+    end
+
+    it 'invokes the constituent service to do the virtual object creation' do
+      expect(service).to have_received(:add).with(child_druids: [child1_id, child2_id]).once
+      expect(service).to have_received(:add).with(child_druids: [other_child1_id, other_child2_id]).once
+    end
+
+    it 'marks the job as complete' do
+      expect(result).to be_complete
+    end
+
+    it 'has output with errors' do
+      expect(result.output[:errors].first[parent_id]).to match_array(['One thing was not combinable', 'And another'])
+    end
+  end
 end