Bug 31224: Move item field removal up to $metadata->record

This patch addresse the fact Koha needs to strip out stray item field in
MARC records coming from the DB. While this is not ideal, it has worked
so far, limiting the negative effects of bad data to Koha users.

I put a FIXME because I think it deserves to be revisited colectively at
some point. I filed bug 38406 to track the discussion around this.

The `marcmarcrecord2csv.t` tests cover this behavior and I thought
removing it was not in the scope of this bug.

I decided to move the removal one step up in the call chain so it
applies to all calls to `$metadata->record`, not only those that ask to
include items. This is the right thing to do while we keep this
behavior.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Record/marcrecord2csv.t
=> FAIL: Item information gets extracted from more items than expected
(i.e. the item in the MARC record is not stripped out so 'Withdrawn'
shows 3 times instead of 2).
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Run:
  k$ prove t/db_dependent/Koha/Biblio/Metadata.t
=> SUCCESS: Tests pass! The behavior change in $metadata->record works
as intended!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This commit is contained in:
Tomas Cohen Arazi 2024-11-07 21:14:32 -03:00 committed by Katrin Fischer
parent dd79a55a34
commit fd90de9e49
No known key found for this signature in database
GPG Key ID: 0EF6E2C03357A834
2 changed files with 30 additions and 7 deletions

View File

@ -129,6 +129,12 @@ sub record {
'Koha::Biblio::Metadata->record called on unhandled format: ' . $format );
}
# FIXME: Remove existing items from the MARC record. This should be handled
# at store() time or pre-filtering in {Add|Mod}Biblio. Remove the FIXME
# once we reach some consensus on how to handle this.
my ( $itemtag, $itemsubfield ) = C4::Biblio::GetMarcFromKohaField("items.itemnumber");
$record->delete_field( ( $record->field($itemtag) ) );
if ( $embed_items ) {
$self->_embed_items({ %$params, format => $format, record => $record });
}
@ -223,12 +229,7 @@ sub _embed_items {
if ( $format eq 'marcxml' ) {
# First remove the existing items from the MARC record
my ( $itemtag, $itemsubfield ) = C4::Biblio::GetMarcFromKohaField( "items.itemnumber" );
foreach my $field ( $record->field($itemtag) ) {
$record->delete_field($field);
}
my $biblio = Koha::Biblios->find($biblionumber);
my $items = $biblio->items;

View File

@ -19,6 +19,7 @@ use Modern::Perl;
use Test::More tests => 5;
use Test::Exception;
use Test::MockModule;
use Test::Warn;
use t::lib::TestBuilder;
@ -36,7 +37,7 @@ my $builder = t::lib::TestBuilder->new;
subtest 'record() tests' => sub {
plan tests => 9;
plan tests => 11;
$schema->storage->txn_begin;
@ -45,12 +46,33 @@ subtest 'record() tests' => sub {
# Create a valid record
my $record = MARC::Record->new();
my $field = MARC::Field->new( '245', '', '', 'a' => $title );
$record->append_fields($field);
my $f952_1 = MARC::Field->new(
'952', '', '', 0 => '1',
y => 'BK',
c => 'GEN',
d => '2001-06-25',
);
my $f952_2 = MARC::Field->new(
'952', '', '', 0 => '1',
y => 'BK',
c => 'GEN',
d => '2001-06-25',
);
$record->append_fields( $field, $f952_1, $f952_2 );
my ($biblio_id) = C4::Biblio::AddBiblio( $record, '' );
my @fields_952 = $record->field('952');
is( scalar @fields_952, 2, 'The record to be inserted contains 2 item fields' );
my $c4_biblio = Test::MockModule->new('C4::Biblio');
$c4_biblio->mock( 'GetMarcFromKohaField', sub { return '952'; } );
my $metadata = Koha::Biblios->find($biblio_id)->metadata;
my $record2 = $metadata->record;
@fields_952 = $record2->field('952');
is( scalar @fields_952, 0, 'Item fields stripped out then calling $metadata->record' );
is( ref $record2, 'MARC::Record', 'Method record() returned a MARC::Record object' );
is( $record2->field('245')->subfield("a"),
$title, 'Title in 245$a matches title from original record object' );