Created attachment 5682 [details] [review] Corrects a problem when a new order is created with a duplicate barcode. The problem happens if the AcqCreateItem preference is set to "ordering". If we try to add a new order with a barcode that's already used in the items table, no error is returned, but an invalid itemnumber is saved in the aqorders_items table and the new item is never created. Patch attached that adds a duplicate barcode verification in neworderempty.pl and modifies _koha_add_item so it won't return an invalid ID when an item can't be added.
Patch sponsored by the CCSR. ( http://www.ccsr.qc.ca/ )
Hi Frédérick, this didn't work for me. I did: 1) Set AcqCreateItem on 'on order' 2) Made sure barcode was available in my ACQ framework 3) Created a new basket, first order, barcode 'barc' 4) created a second order in that basket, tried again with 'barc' > Got a error message reporting the dublicate barcode 5) Changed the barcode to be something else and saved Problem: Order saved without error message, but no item was created
Oh no... I looked at the wrong record. Checked again - all is well. Will sign-off.
Created attachment 5806 [details] [review] [SIGNED-OFF] Corrects a problem when a new order is created with a duplicate barcode. If the AcqCreateItem preference is set to "ordering" and the barcode for the new item is already in use, no error is returned, but an invalid itemnumber is saved in the aqorders_items table and the item is never created. This patch adds a duplicate barcode verification in neworderempty.pl _koha_add_item is also modified so it won't return an invalid ID when an item can't be added. http://bugs.koha-community.org/show_bug.cgi?id=6963 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> 1) Set AcqCreateItem on 'on order' 2) Make sure barcode was available in my ACQ framework 3) Create a new basket, first order, barcode 'barc' 4) create a second order in that basket, tried again with 'barc' -> Alert message informs about the duplicate barcode 5) Change the barcode to be something else and saved New record with item should be correctly created
Ok, now I spotted a real problem, while doing some other tests. The check also detects empty barcodes. Test plan: 1) Create new basket 2) Create order with 2 items, leave both barcodes empty > The error message will show up. There is no way to save the items with empty barcodes.
Created attachment 5838 [details] [review] New version of the initial patch with empty barcodes ignored. Here is a new version of the patch. Empty barcodes should now be ignored.
Hi Frédérick, I still see the problem. My test plan is: AcqCreateItem is set to 'on order' 1) Create new basket 2) Create new order (I did from suggestion) 3) Order 2 items, fill out itemtype, leave barcode blank On saving I get following alert: Form not submitted because of the following problem(s) ------------------------------------------------------------------- - The barcode is used more than once in the form. Every barcode must be unique
(In reply to comment #7) > Hi Frédérick, > > I still see the problem. My test plan is: > > AcqCreateItem is set to 'on order' > 1) Create new basket > 2) Create new order (I did from suggestion) > 3) Order 2 items, fill out itemtype, leave barcode blank > > On saving I get following alert: > > Form not submitted because of the following problem(s) > ------------------------------------------------------------------- > - The barcode is used more than once in the form. Every barcode must be unique I tried the same test plan after applying the newest patch on a branch based on 3.6 and I get no error when creating an order with two items with blank barcodes. Are you sure your barcodes were really blank? (no whitespaces) Did you apply the newest patch? The old one is obsolete.
Hi Frederick, I can't reproduce the problem with saving empty barcodes now. So this seems ok. But I found another problem with your patch - it looks like it's hardcoded to subfield 'p', while UNIMARC is using 995$f for barcodes. NORMARC uses the same mappings as MARC21. I think checking for the field mapped to items.barcode would be the best solution.
Created attachment 5944 [details] [review] Follow-up patch : use the subfield associated with items.barcode You're right, the barcode won't always be in subfield $p, so using the value of items.barcode is the right way to do it. Added a follow-up patch so the value of items.barcode is now used instead of 'p'. Thanks for the feedback.
Created attachment 5945 [details] [review] [SIGNED-OFF] Follow-up patch : use the subfield associated with items.barcode Instead of using the 'p' subfield directly, use the subfield letter associated with the items.barcode column. http://bugs.koha-community.org/show_bug.cgi?id=6963 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Test cases tested: - 1 item, barcode empty - 2 items, barcodes both empty - 1 item, existing barcode - 1 item, new barcode - 2 items, one barcode empty, one existing All test cases worked nicely and gave correct error messages.
Created attachment 5946 [details] [review] [SIGNED-OFF] Bug 6963: Corrects a problem when a new order is created with a duplicate barcode. If the AcqCreateItem preference is set to "ordering" and the barcode for the new item is already in use, no error is returned, but an invalid itemnumber is saved in the aqorders_items table and the item is never created. This patch adds a duplicate barcode verification in neworderempty.pl _koha_add_item is also modified so it won't return an invalid ID when an item can't be added. http://bugs.koha-community.org/show_bug.cgi?id=6963 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Test plan on second patch.
Created attachment 5947 [details] [review] [SIGNED-OFF] Follow-up patch : use the subfield associated with items.barcode Instead of using the 'p' subfield directly, use the subfield letter associated with the items.barcode column. http://bugs.koha-community.org/show_bug.cgi?id=6963 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Test cases tested: - 1 item, barcode empty - 2 items, barcodes both empty - 1 item, existing barcode - 1 item, new barcode - 2 items, one barcode empty, one existing All test cases worked nicely and gave correct error messages.
QA comment: * this patch fixes the described problem. BUT : the same problem exist also if AcqCreateItem='Recieving' (ie = placing barcode when recieving). (No problem/bug for AcqCreateItem='Cataloguing') * Thanks katrin for the hardcoded 'p' = in UNIMARC, the barcode is in 995$f ! From a functionnal point of view, I can't see a case where you define the barcode when ordering the biblio (ie : you don't have it yet). When recieving it's more common. So more libraries will face the bug in the 'Recieving' case than in the 'Ordering' one. Would you agree to send a follow-up to have a fully working fix ? Proposing passed QA for the 2 first patches, bug please, don't close the bug until a 3rd patch has been submitted.
Created attachment 5954 [details] [review] Follow-up patch : use the subfield associated with items.barcode Instead of using the 'p' subfield directly, use the subfield letter associated with the items.barcode column. http://bugs.koha-community.org/show_bug.cgi?id=6963 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Test cases tested: - 1 item, barcode empty - 2 items, barcodes both empty - 1 item, existing barcode - 1 item, new barcode - 2 items, one barcode empty, one existing All test cases worked nicely and gave correct error messages. Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
> Would you agree to send a follow-up to have a fully working fix ? Unfortunately, our client doesn't need this improved fix for now, so I can't devote time on a third patch. If anybody could work on an improved fix, it would be greatly appreciated.
Patch pushed, please test