I tried adding barcode in Global System Preferences - Acquisitions - UniqueItemFields as barcode but this feature does not seem to work. Even same accession number is provided to new book it does not give any error message and it does not add that same number again but keeps that item as blank. *** Bug 9185 has been marked as a duplicate of this bug. *** Merging with duplicate bug 9185: Settings: AcqCreateItem = on receive UniqueItemFields = barcode 1) Create a new order with one or more order lines 2) Close basket 3) Create a new invoice/shipment 4) Receive one item, use existing barcode Problem: - on saving, no warning appears - item creation silently fails Log: ... [Sat Dec 01 23:11:05 2012] [error] [client 127.0.0.1] [Sat Dec 1 23:11:05 2012] finishreceive.pl: Indicator in 952 is empty at /home/katrin/kohaclone/C4/Biblio.pm line 2285., referer: http://localhost:8080/cgi-bin/koha/acqui/orderreceive.pl?ordernumber=25&invoiceid=3 [Sat Dec 01 23:11:05 2012] [error] [client 127.0.0.1] [Sat Dec 1 23:11:05 2012] finishreceive.pl: DBD::mysql::st execute failed: Duplicate entry '3' for key 'itembarcodeidx' at /home/katrin/kohaclone/C4/Items.pm line 2105., referer: http://localhost:8080/cgi-bin/koha/acqui/orderreceive.pl?ordernumber=25&invoiceid=3 [Sat Dec 01 23:11:05 2012] [error] [client 127.0.0.1] [Sat Dec 1 23:11:05 2012] finishreceive.pl: DBD::mysql::st execute failed: Column 'itemnumber' cannot be null at /home/katrin/kohaclone/C4/Acquisition.pm line 1094., referer: http://localhost:8080/cgi-bin/koha/acqui/orderreceive.pl?ordernumber=25&invoiceid=3 ... The checks for duplicate values within the order work, but it's not correctly checking the database. (In reply to Katrin Fischer from comment #4) > The checks for duplicate values within the order work, but it's not > correctly checking the database. What did you mean with "not correctly"? Does this still valid? Hm, thinking about it, this is probably what I meant: - if you have duplicates in the forms on the order page (2 items, same barcode) it is found. - if you have a barcode in an item, that already exists in the database, it isn't found I haven't retested, but I think that's what happened. Ok confirmed, still valid on master. Actually introduced by bug 11425 (pushed after this bug report has been opened, hum...). Created attachment 37545 [details] [review] Bug 8684: Fix duplicate check on creating items in the acquisition module The script acqui/check_uniqueness.pl has been introduced by bug 7178. But bug 11425 added a call to a new subroutine SearchItemsByField in order to refactore some code. This subroutine calls SearchItems with an arrayref, which is not what SearchItems is expecting. This broke the duplicate check done on creating items in the acquisition module. To reproduce the issue: 1/ Fill the pref UniqueItemFields with "barcode" and AcqCreateItem with "placing an order" 2/ Create a new basket 3/ Create a new order 4/ Close the basket 5/ Receive items and set barcode with one already in your DB 6/ Save 7/ Nothing happen on the interface, but an error should have been added in your Koha log file: Can't use an undefined value as an ARRAY reference at /home/koha/src/acqui/check_uniqueness.pl line 48. Test plan: 1/ Repeat steps 1-6 2/ You should see a warning on the interface Note that this should also happen with AcqCreateItem set to "creating an order". Comment on attachment 37545 [details] [review] Bug 8684: Fix duplicate check on creating items in the acquisition module Review of attachment 37545 [details] [review]: ----------------------------------------------------------------- Secondary thought: $ git grep SearchItemsByField | grep \.t: -- Nothing. Perhaps including a test would (a) increase testing coverage (always good, but a pain to do), and (b) prevent brokenness like this, because people would trigger tests for C4::Items and it would be covered. Suggested test would be different commit. Apply before patch, run test, it fails. Apply patch, run test, it passes. As is, I believe the patch is right, but I have not tested it sufficiently (actually triggered it) to sign off. ::: C4/Items.pm @@ -2591,5 @@ > > - my $filters = [ { > - field => $field, > - query => $value, > - } ]; I see why someone may have been half awake reading the if code of the next function and thinking @{$filter->{filters}} when putting those square brackets there. But this isn't that structure, and clearly triggers the else which requires a hash ref, not an array. Now if only I could actually trigger this code, I'd sign this off. But I must be failing at following the test plan. :( Created attachment 37712 [details] [review] Bug 8684: Add regression tests for SearchItemsByField Created attachment 40790 [details] [review] Bug 8684: Fix duplicate check on creating items in the acquisition module Works well, no errors Created attachment 40791 [details] [review] Bug 8684: Add regression tests for SearchItemsByField Created attachment 41171 [details] [review] [PASSED QA] Bug 8684: Fix duplicate check on creating items in the acquisition module The script acqui/check_uniqueness.pl has been introduced by bug 7178. But bug 11425 added a call to a new subroutine SearchItemsByField in order to refactore some code. This subroutine calls SearchItems with an arrayref, which is not what SearchItems is expecting. This broke the duplicate check done on creating items in the acquisition module. To reproduce the issue: 1/ Fill the pref UniqueItemFields with "barcode" and AcqCreateItem with "placing an order" 2/ Create a new basket 3/ Create a new order 4/ Close the basket 5/ Receive items and set barcode with one already in your DB 6/ Save 7/ Nothing happen on the interface, but an error should have been added in your Koha log file: Can't use an undefined value as an ARRAY reference at /home/koha/src/acqui/check_uniqueness.pl line 48. Test plan: 1/ Repeat steps 1-6 2/ You should see a warning on the interface Note that this should also happen with AcqCreateItem set to "creating an order". Works well, no errors. Signed-off-by: Amit Gupta <amit.gupta@informaticsglobal.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 41172 [details] [review] [PASSED QA] Bug 8684: Add regression tests for SearchItemsByField Signed-off-by: Amit Gupta <amit.gupta@informaticsglobal.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Patches pushed to master. Good job Jonathan! Pushed to 3.20.x will be in 3.20.3 |
When creating items for an order, the duplicate check based on UniqueItemFields doesn't work any longer. To test: UniqueItemFields: barcode AcqCreateItem: placing an order Autobarcode: 1, 2, 3... 1) Create a new order 2) Add 2 items with the same barcode 3) Save the order > only 1 item is created, no warning shown Turning off Autobarcode (do not generate) didn't solve the problem. There is a javascript error that you can see when saving the order.