[Bug 8684] New: Duplicate check for creating items on order broken
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Priority: P5 - low Change sponsored?: --- Bug ID: 8684 Assignee: koha-bugs@lists.koha-community.org Summary: Duplicate check for creating items on order broken Severity: enhancement Classification: Unclassified OS: All Reporter: katrin.fischer@bsz-bw.de Hardware: All Status: NEW Version: master Component: Acquisitions Product: Koha 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. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Sunita <sunitabarve@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Priority|P5 - low |P1 - high CC| |sunitabarve@gmail.com Version|master |3.8 --- Comment #1 from Sunita <sunitabarve@gmail.com> --- 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. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |major -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|3.8 |master -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** Bug 9185 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 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 ... -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- The checks for duplicate values within the order work, but it's not correctly checking the database. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=10483 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 marjorie barry-vila <marjorie.barry-vila@ccsr.qc.ca> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |marjorie.barry-vila@ccsr.qc | |.ca -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- (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? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 --- Comment #6 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 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. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 --- Comment #7 from Jonathan Druart <jonathan.druart@biblibre.com> --- Ok confirmed, still valid on master. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |7178 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |jonathan.druart@biblibre.co |ity.org |m Depends on| |11425 --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Actually introduced by bug 11425 (pushed after this bug report has been opened, hum...). -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 37545 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=37545&action=edit 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". -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 --- Comment #10 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 37545 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=37545 Bug 8684: Fix duplicate check on creating items in the acquisition module Review of attachment 37545: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=8684&attachment=37545) ----------------------------------------------------------------- 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. :( -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 37712 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=37712&action=edit Bug 8684: Add regression tests for SearchItemsByField -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Amit <amitddng135@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |amitddng135@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Amit <amitddng135@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #37545|0 |1 is obsolete| | --- Comment #12 from Amit <amitddng135@gmail.com> --- Created attachment 40790 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40790&action=edit Bug 8684: Fix duplicate check on creating items in the acquisition module Works well, no errors -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Amit <amitddng135@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #37712|0 |1 is obsolete| | --- Comment #13 from Amit <amitddng135@gmail.com> --- Created attachment 40791 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40791&action=edit Bug 8684: Add regression tests for SearchItemsByField -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Amit <amitddng135@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40790|0 |1 is obsolete| | Attachment #40791|0 |1 is obsolete| | --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 41171 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41171&action=edit [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> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 41172 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41172&action=edit [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> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Status|Passed QA |Pushed to Master --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Good job Jonathan! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8684 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz Status|Pushed to Master |Pushed to Stable --- Comment #17 from Chris Cormack <chris@bigballofwax.co.nz> --- Pushed to 3.20.x will be in 3.20.3 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org