Bugzilla – Attachment 150363 Details for
Bug 32804
Importing and replacing items causes inconsistency when itemnumber match and biblio match disagree
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 32804: [22.05.x] Do not adjust biblionumber when replacing items during import
Bug-32804-2205x-Do-not-adjust-biblionumber-when-re.patch (text/plain), 9.17 KB, created by
Nick Clemens (kidclamp)
on 2023-04-28 11:02:12 UTC
(
hide
)
Description:
Bug 32804: [22.05.x] Do not adjust biblionumber when replacing items during import
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2023-04-28 11:02:12 UTC
Size:
9.17 KB
patch
obsolete
>From 52d5a52ae588ea6174470ed24cad9310d4b073ed Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Thu, 2 Feb 2023 20:16:49 +0000 >Subject: [PATCH] Bug 32804: [22.05.x] Do not adjust biblionumber when > replacing items during import > >This patch adjust the item matching at import to confirm that for a duplicate itemnumber or barcode >matches an existing item in the DB and uses the original biblionumber when updating the item. > >When ordering in a consortium the items may be moved around, duplicate biblios added, and various matches found. >We should not allow importing of items to move them from 1 biblio to another, but we should allow the imports >to succeed and assume itemnumber or barcode matches are authoritative. The responsibility for correct matching of items to biblio should fall on the creator of the files > >To test: > 1 - Be using the sample data in koha testing docker > 2 - Stage the sample file on this report > 3 - Match on KohaBiblio(999$c) / Item processing: replace > 4 - Note the biblio match is "The complete novels / Austen, Jane" > 5 - View the staged marc, note the barcode 39999000004090 in an item > 6 - Search for that barcode > 7 - You find biblio "Five novels : complete and unabridged / Gustave Flaubert" > 8 - Import the file > 9 - Check the db: > SELECT * FROM items WHERE biblionumber != biblioitemnumber; >10 - Note the mismatch >11 - Fix the item and set it as 'Music' type > UPDATE items SET biblionumber = biblioitemnumber, itype='MU' WHERE biblionumber != biblioitemnumber; >12 - Apply patch, restart all >13 - Stage and import the file with the same settings >14 - Confirm the item is modified on its original biblio (99) and that item type is Book again >15 - Change itemnumber to avoid itemnumber match and reset type > UPDATE items SET itype='MU', itemnumber=999 WHERE itemnumber=212; >16 - Stage and import with the same setttings >17 - Confirm the marcode match worked and item is updated to book on original record >18 - Delete the original item >19 - Stage and import the file with the same settings >20 - The item is successfully created >21 - Stage and import, but item processing option is 'add' >22 - Confirm 1 item ignored >23 - Check the db > SELECT * FROM import_items WHERE barcode=39999000004090 >24 - Confirm there is a line with 'error' and duplicate_barcode > >JD amended patch >- # We assume that when replaicing tiems we do not want to move them - the onus is on the importer to >+ # We assume that when replacing items we do not want to move them - the onus is on the importer to > >Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> > >Bug 32804: Add unit tests > >JD amended patch >- is( $item1->homebranch, $library->branchcode, "Item was overlayed succesfully" ); >+ is( $item1->homebranch, $library->branchcode, "Item was overlaid successfully" ); > >Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> > >Bug 32804: (follow-up) Test that id's have not changed > >Rather than assuming biblionumber and biblioitemnumber are the same, we >should test that they have not changed form their original values > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> > >Bug 32804: (QA follow-up) Typo ahs and fix ImportBatch.t > >Resolve: > # Failed test 'Item's biblioitemnumber has not changed' > # at t/db_dependent/ImportBatch.t line 407. > # > # got: '4261' > # expected: '2371' > >Do not compare $item1->biblionumber with $original_biblioitemnumber :) > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >--- > C4/ImportBatch.pm | 28 +++++++++++++++----- > t/db_dependent/ImportBatch.t | 51 +++++++++++++++++++++++++++++++++++- > 2 files changed, 71 insertions(+), 8 deletions(-) > >diff --git a/C4/ImportBatch.pm b/C4/ImportBatch.pm >index 1dd75382f9..c146671b94 100644 >--- a/C4/ImportBatch.pm >+++ b/C4/ImportBatch.pm >@@ -716,13 +716,20 @@ sub BatchCommitItems { > > my $item = TransformMarcToKoha( $item_marc ); > >- my $duplicate_barcode = exists( $item->{'barcode'} ) && Koha::Items->find({ barcode => $item->{'barcode'} }); >+ my $item_match; >+ my $duplicate_barcode = exists( $item->{'barcode'} ); > my $duplicate_itemnumber = exists( $item->{'itemnumber'} ); > >+ # We assume that when replacing items we do not want to move them - the onus is on the importer to >+ # ensure the correct items/records are being updated > my $updsth = $dbh->prepare("UPDATE import_items SET status = ?, itemnumber = ?, import_error = ? WHERE import_items_id = ?"); >- if ( $action eq "replace" && $duplicate_itemnumber ) { >+ if ( >+ $action eq "replace" && >+ $duplicate_itemnumber && >+ ( $item_match = Koha::Items->find( $item->{itemnumber} )) >+ ) { > # Duplicate itemnumbers have precedence, that way we can update barcodes by overlaying >- ModItemFromMarc( $item_marc, $biblionumber, $item->{itemnumber} ); >+ ModItemFromMarc( $item_marc, $item_match->biblionumber, $item->{itemnumber}); > $updsth->bind_param( 1, 'imported' ); > $updsth->bind_param( 2, $item->{itemnumber} ); > $updsth->bind_param( 3, undef ); >@@ -730,9 +737,12 @@ sub BatchCommitItems { > $updsth->execute(); > $updsth->finish(); > $num_items_replaced++; >- } elsif ( $action eq "replace" && $duplicate_barcode ) { >- my $itemnumber = $duplicate_barcode->itemnumber; >- ModItemFromMarc( $item_marc, $biblionumber, $itemnumber ); >+ } elsif ( >+ $action eq "replace" && >+ $duplicate_barcode && >+ ( $item_match = Koha::Items->find({ barcode => $item->{'barcode'} }) ) >+ ) { >+ ModItemFromMarc( $item_marc, $item_match->biblionumber, $item_match->itemnumber ); > $updsth->bind_param( 1, 'imported' ); > $updsth->bind_param( 2, $item->{itemnumber} ); > $updsth->bind_param( 3, undef ); >@@ -740,7 +750,11 @@ sub BatchCommitItems { > $updsth->execute(); > $updsth->finish(); > $num_items_replaced++; >- } elsif ($duplicate_barcode) { >+ } elsif ( >+ # We aren't replacing, but the incoming file has a barcode, we need to check if it exists >+ $duplicate_barcode && >+ ( $item_match = Koha::Items->find({ barcode => $item->{'barcode'} }) ) >+ ) { > $updsth->bind_param( 1, 'error' ); > $updsth->bind_param( 2, undef ); > $updsth->bind_param( 3, 'duplicate item barcode' ); >diff --git a/t/db_dependent/ImportBatch.t b/t/db_dependent/ImportBatch.t >index c93bc53019..414ffdd5db 100755 >--- a/t/db_dependent/ImportBatch.t >+++ b/t/db_dependent/ImportBatch.t >@@ -1,7 +1,7 @@ > #!/usr/bin/perl > > use Modern::Perl; >-use Test::More tests => 18; >+use Test::More tests => 19; > use utf8; > use File::Basename; > use File::Temp qw/tempfile/; >@@ -328,6 +328,55 @@ subtest "_get_commit_action" => sub { > > }; > >+ >+subtest "Do not adjust biblionumber when replacing items during import" => sub { >+ plan tests => 7; >+ >+ my $item1 = $builder->build_sample_item; >+ my $original_biblionumber = $item1->biblionumber; >+ my $original_biblioitemnumber = $item1->biblioitemnumber; >+ my $item2 = $builder->build_sample_item; >+ >+ my $library = $builder->build_object({ class => 'Koha::Libraries' }); >+ >+ my $import_item = $builder->build_object({ class => 'Koha::Import::Record::Items', value => { >+ marcxml => qq{<?xml version="1.0" encoding="UTF-8"?> >+<collection >+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" >+ xsi:schemaLocation="http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd" >+ xmlns="http://www.loc.gov/MARC21/slim"> >+ >+<record> >+ <leader>00000 a </leader> >+ <datafield tag="952" ind1=" " ind2=" "> >+ <subfield code="a">${\($library->branchcode)}</subfield> >+ <subfield code="b">${\($library->branchcode)}</subfield> >+ <subfield code="c">GEN</subfield> >+ <subfield code="p">${\($item1->barcode)}</subfield> >+ <subfield code="y">BK</subfield> >+ </datafield> >+</record> >+</collection> >+ }, >+ }}); >+ >+ isnt( $item1->homebranch, $library->branchcode, "Item's homebranch is currently not the same as our created branch's branchcode" ); >+ >+ my ( $num_items_added, $num_items_replaced, $num_items_errored ) = >+ C4::ImportBatch::BatchCommitItems( $import_item->import_record_id, $item2->biblionumber, 'replace' ); >+ >+ $item1->discard_changes(); >+ >+ is( $num_items_errored, 0, 'Item was replaced' ); >+ $import_item->discard_changes(); >+ is( $import_item->status, 'imported', 'Import was successful'); >+ is( $import_item->import_error, undef, 'No error was reported' ); >+ >+ is( $item1->biblionumber, $original_biblionumber, "Item's biblionumber has not changed" ); >+ is( $item1->biblioitemnumber, $original_biblioitemnumber, "Item's biblioitemnumber has not changed" ); >+ is( $item1->homebranch, $library->branchcode, "Item was overlaid successfully" ); >+}; >+ > sub get_import_record { > my $id_import_batch = shift; > return $dbh->do('SELECT * FROM import_records WHERE import_batch_id = ?', undef, $id_import_batch); >-- >2.30.2
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 32804
:
146034
|
146037
|
146292
|
146293
|
146345
|
146346
|
146579
|
147134
|
147135
|
147668
|
147669
|
147690
|
148314
|
148315
|
148316
|
148317
| 150363