From ed1cadcd3df3379760190ef01fac48e907373722 Mon Sep 17 00:00:00 2001 From: Galen Charlton Date: Thu, 3 Jul 2008 14:40:59 -0500 Subject: [PATCH] bug 2297: improve ModBiblio() to avoid duplicate item fields Prior to this patch, ModBiblio() would append item tags from the previous version of the bib record to the incoming bib record before saving the results, even if the incoming bib record already has embedded item tags. For example, if a bib is retrieved using GetMarcBiblio() then saved using ModBiblio(), the caller was obliged to delete any item tags first to avoid duplication. ModBiblio() now deletes item tags supplied in the incoming MARC record. This eliminates the possibility of duplication, and removes any implication that ModBiblio() can or should be used to modify item records - ModItem() should be used for that. Signed-off-by: Joshua Ferraro --- C4/Biblio.pm | 26 ++++- t/lib/KohaTest/Biblio/ModBiblio.pm | 154 +++++++++++++++++++++++++++++ 2 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 t/lib/KohaTest/Biblio/ModBiblio.pm diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 22bf57843c..4031cb205e 100755 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -246,8 +246,25 @@ sub AddBiblio { =head2 ModBiblio +=over 4 + ModBiblio( $record,$biblionumber,$frameworkcode); - Exported function (core API) to modify a biblio + +=back + +Replace an existing bib record identified by C<$biblionumber> +with one supplied by the MARC::Record object C<$record>. The embedded +item, biblioitem, and biblionumber fields from the previous +version of the bib record replace any such fields of those tags that +are present in C<$record>. Consequently, ModBiblio() is not +to be used to try to modify item records. + +C<$frameworkcode> specifies the MARC framework to use +when storing the modified bib record; among other things, +this controls how MARC fields get mapped to display columns +in the C and C tables, as well as +which fields are used to store embedded item, biblioitem, +and biblionumber data for indexing. =cut @@ -265,6 +282,13 @@ sub ModBiblio { # get the items before and append them to the biblio before updating the record, atm we just have the biblio my ( $itemtag, $itemsubfield ) = GetMarcFromKohaField("items.itemnumber",$frameworkcode); my $oldRecord = GetMarcBiblio( $biblionumber ); + + # delete any item fields from incoming record to avoid + # duplication or incorrect data - use AddItem() or ModItem() + # to change items + foreach my $field ($record->field($itemtag)) { + $record->delete_field($field); + } # parse each item, and, for an unknown reason, re-encode each subfield # if you don't do that, the record will have encoding mixed diff --git a/t/lib/KohaTest/Biblio/ModBiblio.pm b/t/lib/KohaTest/Biblio/ModBiblio.pm new file mode 100644 index 0000000000..5b29ea8b61 --- /dev/null +++ b/t/lib/KohaTest/Biblio/ModBiblio.pm @@ -0,0 +1,154 @@ +package KohaTest::Biblio::ModBiblio; +use base qw( KohaTest::Biblio ); + +use strict; +use warnings; + +use Test::More; + +use C4::Biblio; +use C4::Items; + +=head2 STARTUP METHODS + +These get run once, before the main test methods in this module + +=head3 add_bib_to_modify + +=cut + +sub add_bib_to_modify : Test( startup => 3 ) { + my $self = shift; + + my $bib = MARC::Record->new(); + $bib->leader(' ngm a22 7a 4500'); + $bib->append_fields( + MARC::Field->new('100', ' ', ' ', a => 'Moffat, Steven'), + MARC::Field->new('245', ' ', ' ', a => 'Silence in the library'), + ); + + my ($bibnum, $bibitemnum) = AddBiblio($bib, ''); + $self->{'bib_to_modify'} = $bibnum; + + # add an item + my ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => 'CPL', holdingbranch => 'CPL' } , $bibnum); + + cmp_ok($item_bibnum, '==', $bibnum, "new item is linked to correct biblionumber"); + cmp_ok($item_bibitemnum, '==', $bibitemnum, "new item is linked to correct biblioitemnumber"); + + $self->reindex_marc(); + + my $marc = $self->fetch_bib($bibnum); + $self->sort_item_and_bibnumber_fields($marc); + $self->{'bib_to_modify_formatted'} = $marc->as_formatted(); # simple way to compare later +} + +=head2 TEST METHODS + +standard test methods + +=head3 bug_2297 + +Regression test for bug 2297 (saving a subscription duplicates MARC item fields) + +=cut + +sub bug_2297 : Test( 5 ) { + my $self = shift; + + my $bibnum = $self->{'bib_to_modify'}; + my $marc = $self->fetch_bib($bibnum); + $self->check_item_count($marc, 1); + + ModBiblio($marc, $bibnum, ''); # no change made to bib + + my $modified_marc = $self->fetch_bib($bibnum); + diag "checking item field count after null modification"; + $self->check_item_count($modified_marc, 1); + + $self->sort_item_and_bibnumber_fields($modified_marc); + is($modified_marc->as_formatted(), $self->{'bib_to_modify_formatted'}, "no change to bib after null modification"); +} + +=head2 HELPER METHODS + +These methods are used by other test methods, but +are not meant to be called directly. + +=cut + +=head3 fetch_bib + +=cut + +sub fetch_bib { # +1 to test count per call + my $self = shift; + my $bibnum = shift; + + my $marc = GetMarcBiblio($bibnum); + ok(defined($marc), "retrieved bib record $bibnum"); + + return $marc; +} + +=head3 check_item_count + +=cut + +sub check_item_count { # +1 to test count per call + my $self = shift; + my $marc = shift; + my $expected_items = shift; + + my ($itemtag, $itemsubfield) = GetMarcFromKohaField("items.itemnumber", ''); + my @item_fields = $marc->field($itemtag); + cmp_ok(scalar(@item_fields), "==", $expected_items, "exactly one item field"); +} + +=head3 sort_item_and_bibnumber_fields + +This method sorts the field containing the embedded item data +and the bibnumber - ModBiblio(), AddBiblio(), and ModItem() do +not guarantee that these fields will be sorted in tag order. + +=cut + +sub sort_item_and_bibnumber_fields { + my $self = shift; + my $marc = shift; + + my ($itemtag, $itemsubfield) = GetMarcFromKohaField("items.itemnumber", ''); + my ($bibnumtag, $bibnumsubfield) = GetMarcFromKohaField("biblio.biblionumber", ''); + + my @item_fields = (); + foreach my $field ($marc->field($itemtag)) { + push @item_fields, $field; + $marc->delete_field($field); + } + $marc->insert_fields_ordered(@item_fields) if scalar(@item_fields);; + + my @bibnum_fields = (); + foreach my $field ($marc->field($bibnumtag)) { + push @bibnum_fields, $field; + $marc->delete_field($field); + } + $marc->insert_fields_ordered(@bibnum_fields) if scalar(@bibnum_fields); + +} + +=head2 SHUTDOWN METHODS + +These get run once, after the main test methods in this module + +=head3 shutdown_clean_object + +=cut + +sub shutdown_clean_object : Test( shutdown => 0 ) { + my $self = shift; + + delete $self->{'bib_to_modify'}; + delete $self->{'bib_to_modify_formatted'}; +} + +1; -- 2.39.5