Merge pull request #2681 from wellcomecollection/Prefer-MARC-tag-264-…

…over-260

Prefer MARC tag 264 over 260 for Sierra

pipeline/transformer/transformer_marc_common/src/main/scala/weco/pipeline/transformer/marc_common/transformers/MarcDataTransformer.scala

@@ -13,6 +13,12 @@ trait MarcDataTransformer {
   def apply(record: MarcRecord): Output
 }
 
+trait MarcProductionTransformer {
+  type Output
+
+  def apply(record: MarcRecord, prefer264Field: Boolean): Output
+}
+
 trait MarcDataTransformerWithLoggingContext {
   type Output
 

pipeline/transformer/transformer_marc_common/src/main/scala/weco/pipeline/transformer/marc_common/transformers/MarcProduction.scala

@@ -22,28 +22,26 @@ import weco.pipeline.transformer.transformers.{
 }
 
 object MarcProduction
-    extends MarcDataTransformer
+    extends MarcProductionTransformer
     with MarcFieldOps
     with ConceptsTransformer
     with Logging {
   type Output = List[ProductionEvent[IdState.Unminted]]
 
-  def apply(record: MarcRecord): List[ProductionEvent[IdState.Unminted]] = {
+  def apply(
+    record: MarcRecord,
+    prefer264Field: Boolean = false
+  ): List[ProductionEvent[IdState.Unminted]] = {
     val productionEvents = (
       getProductionFrom260Fields(record),
       getProductionFrom264Fields(record)
     ) match {
       case (Nil, Nil)     => Nil
       case (from260, Nil) => from260
       case (Nil, from264) => from264
-
-      // If both 260 and 264 are present we prefer the 260 fields
-      case (from260, _) => {
-        warn(
-          s"Record ${record.controlField("001")} has both 260 and 264 fields. Using 260 fields."
-        )
-        from260
-      }
+      case (from260, from264) =>
+        if (prefer264Field) from264
+        else from260
     }
 
     val marc008productionEvents = getProductionFrom008(record)

pipeline/transformer/transformer_marc_common/src/test/scala/weco/pipeline/transformer/marc_common/transformers/MarcProductionTest.scala

@@ -21,13 +21,78 @@ class MarcProductionTest
     with Matchers
     with RandomGenerators {
 
+
+  private val MARC_260_FIXTURE = MarcField(
+    marcTag = "260",
+    subfields = List(
+      MarcSubfield(tag = "a", content = "Paris")
+    )
+  )
+
+  private val MARC_264_FIXTURE = MarcField(
+    marcTag = "264",
+    indicator2 = "0",
+    subfields = List(
+      MarcSubfield(tag = "a", content = "London")
+    )
+  )
+
+  private val PRODUCTION_260_FIXTURE = ProductionEvent(
+    label = "Paris",
+    places = List(Place("Paris")),
+    agents = List(),
+    dates = List(),
+    function = None
+  )
+
+  private val PRODUCTION_264_FIXTURE =  ProductionEvent(
+    label = "London",
+    places = List(Place("London")),
+    agents = List(),
+    dates = List(),
+    function = Some(Concept(label="Production"))
+  )
+
   it("returns an empty list if neither 260 nor 264 are present") {
     MarcProduction(MarcTestRecord(fields = List())) shouldBe List()
   }
 
   describe("Both MARC field 260 and 264") {
     it(
-      "if both 260 and 264 are present, accept 260"
+      "if both 260 and 264 are present, accept 260 by default"
+    ) {
+      MarcProduction(
+        MarcTestRecord(
+          controlFields = List(
+            MarcControlField(
+              marcTag = "001",
+              content = randomAlphanumeric(length = 9)
+            )
+          ),
+          fields = List(MARC_260_FIXTURE, MARC_264_FIXTURE)
+        )
+      ) shouldBe List(PRODUCTION_260_FIXTURE)
+    }
+
+    it(
+      "if both 260 and 264 are present, accept 264 if preferred"
+    ) {
+      MarcProduction(
+        MarcTestRecord(
+          controlFields = List(
+            MarcControlField(
+              marcTag = "001",
+              content = randomAlphanumeric(length = 9)
+            )
+          ),
+          fields = List(MARC_260_FIXTURE, MARC_264_FIXTURE)
+        ),
+        prefer264Field = true
+      ) shouldBe List(PRODUCTION_264_FIXTURE)
+    }
+
+    it(
+      "if 264 is preferred but has an indicator2='4', accept 260"
     ) {
       MarcProduction(
         MarcTestRecord(
@@ -38,30 +103,89 @@ class MarcProductionTest
             )
           ),
           fields = List(
+            MARC_260_FIXTURE,
             MarcField(
-              marcTag = "260",
+              marcTag = "264",
+              indicator2 = "4",
+              subfields = List(
+                MarcSubfield(tag = "a", content = "London")
+              )
+            )
+          )
+        ),
+        prefer264Field = true
+      ) shouldBe List(PRODUCTION_260_FIXTURE)
+    }
+
+
+    it(
+      "if 260 doesn't exist, accept 264"
+    ) {
+      MarcProduction(
+        MarcTestRecord(
+          controlFields = List(
+            MarcControlField(
+              marcTag = "001",
+              content = randomAlphanumeric(length = 9)
+            )
+          ),
+          fields = List(MARC_260_FIXTURE)
+        ),
+        prefer264Field = true
+      ) shouldBe List(PRODUCTION_260_FIXTURE)
+    }
+
+    it(
+      "if 264 is preferred but doesn't exist, accept 260"
+    ) {
+      MarcProduction(
+        MarcTestRecord(
+          controlFields = List(
+            MarcControlField(
+              marcTag = "001",
+              content = randomAlphanumeric(length = 9)
+            )
+          ),
+          fields = List(MARC_260_FIXTURE)
+        ),
+        prefer264Field = true
+      ) shouldBe List(PRODUCTION_260_FIXTURE)
+    }
+
+    it(
+      "filters out 264 tags with indicator2='4' or indicator2=' '"
+    ) {
+      MarcProduction(
+        MarcTestRecord(
+          controlFields = List(
+            MarcControlField(
+              marcTag = "001",
+              content = randomAlphanumeric(length = 9)
+            )
+          ),
+          fields = List(
+            MARC_260_FIXTURE,
+            MARC_264_FIXTURE,
+            MarcField(
+              marcTag = "264",
+              indicator2 = "4",
               subfields = List(
-                MarcSubfield(tag = "a", content = "Paris")
+                MarcSubfield(tag = "a", content = "Test"),
+                MarcSubfield(tag = "b", content = "Test"),
+                MarcSubfield(tag = "c", content = "Test")
               )
             ),
             MarcField(
               marcTag = "264",
-              indicator2 = "0",
+              indicator2 = " ",
               subfields = List(
-                MarcSubfield(tag = "a", content = "London")
+                MarcSubfield(tag = "a", content = "Berlin")
               )
             )
           )
-        )
-      ) shouldBe List(
-        ProductionEvent(
-          label = "Paris",
-          places = List(Place("Paris")),
-          agents = List(),
-          dates = List(),
-          function = None
-        )
-      )
+        ),
+        prefer264Field = true
+      ) shouldBe List(PRODUCTION_264_FIXTURE)
     }
   }
 

pipeline/transformer/transformer_sierra/src/main/scala/weco/pipeline/transformer/sierra/transformers/SierraProduction.scala

@@ -17,5 +17,5 @@ object SierraProduction
     bibId: SierraBibNumber,
     bibData: SierraBibData
   ): List[ProductionEvent[IdState.Unminted]] =
-    MarcProduction(bibData)
+    MarcProduction(bibData, prefer264Field = true)
 }

pipeline/transformer/transformer_sierra/src/test/scala/weco/pipeline/transformer/sierra/transformers/SierraProductionTest.scala

@@ -466,7 +466,7 @@ class SierraProductionTest
 
   describe("Both MARC field 260 and 264") {
     it(
-      "if both 260 and 264 are present, accept 260"
+      "if both 260 and 264 are present, accept 264"
     ) {
       val bibId = createSierraBibNumber
 
@@ -494,11 +494,11 @@ class SierraProductionTest
 
       SierraProduction(bibId, bibData) shouldBe List(
         ProductionEvent(
-          label = "Paris",
-          places = List(Place("Paris")),
+          label = "London",
+          places = List(Place("London")),
           agents = List(),
           dates = List(),
-          function = None
+          function = Some(Concept(label="Production"))
         )
       )
     }