The script runs item addition in an eval, and skips the record if there is an error. Before we deal with the error, however, we try to see if items were added by accessing a hash of itemnumebrs returned. When we throw an exception, the variable is not set. We should move that line after the error handling
Created attachment 169959 [details] [review] Bug 37550: Move item check after error handling To test: 1 - Grab the sample file on this bug report 2 - perl misc/migration_tools/bulkmarcimport.pl -b --file=spaceditems.pl -v 3 - An exception for 'Duplicate ID' is thrown and script dies 4 - Apply patch 5 - run script again 6 - The script finishes, no items are added, but record is
Created attachment 169960 [details] [review] Bug 37550: Unit test for CheckItemPreSave
Created attachment 169961 [details] [review] Bug 37550: CheckItemPreSave should run barcodes through barcodedecode In Koha::Item we run a barcode through barcodedecode before any save. We should do the same when checking barcodes to avoid a duplicate error when the barcode is cleaned before it is written to the DB To test: 1 - Follow previous test plan 2 - Note that after this patch is applied there is no exception All items are skipped as duplicate barcodes
I don't see the sample file.
Created attachment 170630 [details] Sample file
Created attachment 170648 [details] [review] Bug 37550: Move item check after error handling To test: 1 - Grab the sample file on this bug report 2 - perl misc/migration_tools/bulkmarcimport.pl -b --file=spaceditems.pl -v 3 - An exception for 'Duplicate ID' is thrown and script dies 4 - Apply patch 5 - run script again 6 - The script finishes, no items are added, but record is Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Created attachment 170649 [details] [review] Bug 37550: Unit test for CheckItemPreSave Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Created attachment 170650 [details] [review] Bug 37550: CheckItemPreSave should run barcodes through barcodedecode In Koha::Item we run a barcode through barcodedecode before any save. We should do the same when checking barcodes to avoid a duplicate error when the barcode is cleaned before it is written to the DB To test: 1 - Follow previous test plan 2 - Note that after this patch is applied there is no exception All items are skipped as duplicate barcodes Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Created attachment 173868 [details] [review] Bug 37550: Move item check after error handling To test: 1 - Grab the sample file on this bug report 2 - perl misc/migration_tools/bulkmarcimport.pl -b --file=spaceditems.pl -v 3 - An exception for 'Duplicate ID' is thrown and script dies 4 - Apply patch 5 - run script again 6 - The script finishes, no items are added, but record is Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 173869 [details] [review] Bug 37550: Unit test for CheckItemPreSave Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 173870 [details] [review] Bug 37550: CheckItemPreSave should run barcodes through barcodedecode In Koha::Item we run a barcode through barcodedecode before any save. We should do the same when checking barcodes to avoid a duplicate error when the barcode is cleaned before it is written to the DB To test: 1 - Follow previous test plan 2 - Note that after this patch is applied there is no exception All items are skipped as duplicate barcodes Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Pushed for 24.11! Well done everyone, thank you!