Moved OpenAccessLocation creation out of existing? block in the Law i…

…mporters so they can be added to pre-existing publications. Still idempotent though. Fixed specs to test these changes. (#599)
psu-libraries · Nov 2, 2022 · e087322 · e087322
1 parent 7b1afc3
commit e087322
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 17 deletions.
diff --git a/app/importers/oai_creator.rb b/app/importers/oai_creator.rb
@@ -14,7 +14,7 @@ def last_name
 
   def first_name
     fn = text.split(',')[1]
-    fn.strip.split.first.strip if fn
+    fn.strip.split.first.strip if fn.present?
   end
 
   def user_match

diff --git a/app/importers/oai_importer.rb b/app/importers/oai_importer.rb
@@ -11,13 +11,13 @@ def call
 
       if rr.any_user_matches?
         ActiveRecord::Base.transaction do
-          existing_import = PublicationImport.find_by(source: import_source,
-                                                      source_identifier: rr.identifier)
+          pi = PublicationImport.find_by(source: import_source, source_identifier: rr.identifier) ||
+            PublicationImport.new
 
           # For now we don't anticipate that source data that we've already imported
           # will change, so we only need to create new records, and we don't need to
           # update existing records.
-          unless existing_import
+          unless pi.persisted?
             p = Publication.new
             p.title = rr.title
             p.journal_title = rr.source
@@ -28,7 +28,6 @@ def call
             p.status = 'Published'
             p.save!
 
-            pi = PublicationImport.new
             pi.publication = p
             pi.source = import_source
             pi.source_identifier = rr.identifier
@@ -62,18 +61,22 @@ def call
               end
             end
 
-            oal = OpenAccessLocation.new
-            oal.publication = p
-            oal.url = rr.url
-            oal.source = location_source # TODO return to import_source once import sources are refactored
-            oal.save!
-
             DuplicatePublicationGroup.group_duplicates_of(p)
 
             if p.reload.duplicate_group
               p.update!(visible: false)
             end
           end
+
+          pub = pi.reload.publication
+
+          if OpenAccessLocation.find_by(publication: pub, source: location_source).blank?
+            oal = OpenAccessLocation.new
+            oal.publication = pub
+            oal.url = rr.url
+            oal.source = location_source # TODO return to import_source once import sources are refactored
+            oal.save!
+          end
         end
       end
     end

diff --git a/spec/component/importers/psu_dickinson_publication_importer_spec.rb b/spec/component/importers/psu_dickinson_publication_importer_spec.rb
@@ -16,7 +16,8 @@
                     any_user_matches?: true,
                     identifier: 'existing-identifier',
                     publisher: 'Test Publisher',
-                    source: 'Test Source' }
+                    source: 'Test Source',
+                    url: 'https://example.com/article' }
   let(:r3) { double 'record 3',
                     any_user_matches?: true,
                     identifier: 'non-existing-identifier',
@@ -74,8 +75,8 @@
       expect { importer.call }.to change(ContributorName, :count).by 2
     end
 
-    it "creates new open access locations for records that are importable and that don't already exist" do
-      expect { importer.call }.to change(OpenAccessLocation, :count).by 1
+    it 'creates new open access locations for records that are importable' do
+      expect { importer.call }.to change(OpenAccessLocation, :count).by 2
     end
 
     it 'is idempotent in terms of creating publication imports' do

diff --git a/spec/component/importers/psu_law_school_publication_importer_spec.rb b/spec/component/importers/psu_law_school_publication_importer_spec.rb
@@ -16,7 +16,8 @@
                     any_user_matches?: true,
                     identifier: 'existing-identifier',
                     publisher: 'Test Publisher',
-                    source: 'Test Source' }
+                    source: 'Test Source',
+                    url: 'https://example.com/article' }
   let(:r3) { double 'record 3',
                     any_user_matches?: true,
                     identifier: 'non-existing-identifier',
@@ -74,8 +75,8 @@
       expect { importer.call }.to change(ContributorName, :count).by 2
     end
 
-    it "creates new open access locations for records that are importable and that don't already exist" do
-      expect { importer.call }.to change(OpenAccessLocation, :count).by 1
+    it 'creates new open access locations for records that are importable' do
+      expect { importer.call }.to change(OpenAccessLocation, :count).by 2
     end
 
     it 'is idempotent in terms of creating publication imports' do