Bugzilla – Attachment 174262 Details for
Bug 31224
Koha::Biblio::Metadata->record should use the EmbedItems filter
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 31224: Move item field removal up to $metadata->record
Bug-31224-Move-item-field-removal-up-to-metadata-r.patch (text/plain), 4.68 KB, created by
Tomás Cohen Arazi (tcohen)
on 2024-11-08 00:23:40 UTC
(
hide
)
Description:
Bug 31224: Move item field removal up to $metadata->record
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2024-11-08 00:23:40 UTC
Size:
4.68 KB
patch
obsolete
>From 1190945a928b36b5897cc4e8073883fad4e75ba2 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Thu, 7 Nov 2024 21:14:32 -0300 >Subject: [PATCH] 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> >--- > Koha/Biblio/Metadata.pm | 11 ++++++----- > t/db_dependent/Koha/Biblio/Metadata.t | 26 ++++++++++++++++++++++++-- > 2 files changed, 30 insertions(+), 7 deletions(-) > >diff --git a/Koha/Biblio/Metadata.pm b/Koha/Biblio/Metadata.pm >index 1b6854f0bf4..ff14735162c 100644 >--- a/Koha/Biblio/Metadata.pm >+++ b/Koha/Biblio/Metadata.pm >@@ -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; >diff --git a/t/db_dependent/Koha/Biblio/Metadata.t b/t/db_dependent/Koha/Biblio/Metadata.t >index 172fd2bb041..67a09b40cec 100755 >--- a/t/db_dependent/Koha/Biblio/Metadata.t >+++ b/t/db_dependent/Koha/Biblio/Metadata.t >@@ -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' ); >-- >2.47.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 31224
:
138060
|
138061
|
138062
|
138063
|
138064
|
142137
|
142138
|
142139
|
142140
|
142141
|
142142
|
142143
|
161933
|
161934
|
161935
|
161936
|
161937
|
161938
|
161939
|
161940
|
161941
|
161942
|
161943
|
161944
|
173530
|
173531
|
173532
|
173533
|
173534
|
173535
|
173588
|
173597
|
173598
|
173636
|
173637
|
173638
|
173639
|
173640
|
173641
|
173642
|
173643
|
173644
| 174262 |
174263