diff --git a/pipeline/transformer/transformer_marc_common/src/main/scala/weco/pipeline/transformer/marc_common/transformers/MarcDataTransformer.scala b/pipeline/transformer/transformer_marc_common/src/main/scala/weco/pipeline/transformer/marc_common/transformers/MarcDataTransformer.scala index e92d29d748..8206f66e78 100644 --- a/pipeline/transformer/transformer_marc_common/src/main/scala/weco/pipeline/transformer/marc_common/transformers/MarcDataTransformer.scala +++ b/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 diff --git a/pipeline/transformer/transformer_marc_common/src/main/scala/weco/pipeline/transformer/marc_common/transformers/MarcProduction.scala b/pipeline/transformer/transformer_marc_common/src/main/scala/weco/pipeline/transformer/marc_common/transformers/MarcProduction.scala index a22bffd352..1aafe30a65 100644 --- a/pipeline/transformer/transformer_marc_common/src/main/scala/weco/pipeline/transformer/marc_common/transformers/MarcProduction.scala +++ b/pipeline/transformer/transformer_marc_common/src/main/scala/weco/pipeline/transformer/marc_common/transformers/MarcProduction.scala @@ -22,13 +22,16 @@ 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) @@ -36,14 +39,9 @@ object MarcProduction 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) diff --git a/pipeline/transformer/transformer_marc_common/src/test/scala/weco/pipeline/transformer/marc_common/transformers/MarcProductionTest.scala b/pipeline/transformer/transformer_marc_common/src/test/scala/weco/pipeline/transformer/marc_common/transformers/MarcProductionTest.scala index b478c188e4..f373ed8f79 100644 --- a/pipeline/transformer/transformer_marc_common/src/test/scala/weco/pipeline/transformer/marc_common/transformers/MarcProductionTest.scala +++ b/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) } } diff --git a/pipeline/transformer/transformer_sierra/src/main/scala/weco/pipeline/transformer/sierra/transformers/SierraProduction.scala b/pipeline/transformer/transformer_sierra/src/main/scala/weco/pipeline/transformer/sierra/transformers/SierraProduction.scala index 14e39f327a..f4d3fb3a45 100644 --- a/pipeline/transformer/transformer_sierra/src/main/scala/weco/pipeline/transformer/sierra/transformers/SierraProduction.scala +++ b/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) } diff --git a/pipeline/transformer/transformer_sierra/src/test/scala/weco/pipeline/transformer/sierra/transformers/SierraProductionTest.scala b/pipeline/transformer/transformer_sierra/src/test/scala/weco/pipeline/transformer/sierra/transformers/SierraProductionTest.scala index 99ceefe9ea..9cea00b38c 100644 --- a/pipeline/transformer/transformer_sierra/src/test/scala/weco/pipeline/transformer/sierra/transformers/SierraProductionTest.scala +++ b/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")) ) ) }