Bug 6963

Summary: When creating a new order, the item isn't added if its barcode already exists in the items table.
Product: Koha Reporter: Frédérick Capovilla <fcapovilla>
Component: AcquisitionsAssignee: Frédérick Capovilla <fcapovilla>
Status: CLOSED FIXED QA Contact: Bugs List <koha-bugs>
Severity: normal    
Priority: PATCH-Sent (DO NOT USE) CC: katrin.fischer, paul.poulain
Version: 3.6   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 21182    
Attachments: Corrects a problem when a new order is created with a duplicate barcode.
[SIGNED-OFF] Corrects a problem when a new order is created with a duplicate barcode.
New version of the initial patch with empty barcodes ignored.
Follow-up patch : use the subfield associated with items.barcode
[SIGNED-OFF] Follow-up patch : use the subfield associated with items.barcode
[SIGNED-OFF] Bug 6963: Corrects a problem when a new order is created with a duplicate barcode.
[SIGNED-OFF] Follow-up patch : use the subfield associated with items.barcode
Follow-up patch : use the subfield associated with items.barcode

Description Frédérick Capovilla 2011-10-03 19:43:50 UTC Comment hidden (obsolete)
Comment 1 Frédérick Capovilla 2011-10-03 19:45:31 UTC
Patch sponsored by the CCSR. ( http://www.ccsr.qc.ca/ )
Comment 2 Katrin Fischer 2011-10-08 20:19:27 UTC
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
Comment 3 Katrin Fischer 2011-10-08 20:20:15 UTC
Oh no... I looked at the wrong record. Checked again - all is well. Will sign-off.
Comment 4 Katrin Fischer 2011-10-08 20:22:51 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2011-10-08 21:51:17 UTC
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.
Comment 6 Frédérick Capovilla 2011-10-11 13:43:48 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2011-10-13 05:59:04 UTC
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
Comment 8 Frédérick Capovilla 2011-10-13 17:38:46 UTC
(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.
Comment 9 Katrin Fischer 2011-10-17 19:28:06 UTC
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.
Comment 10 Frédérick Capovilla 2011-10-17 20:09:23 UTC Comment hidden (obsolete)
Comment 11 Katrin Fischer 2011-10-17 20:47:57 UTC Comment hidden (obsolete)
Comment 12 Katrin Fischer 2011-10-17 20:48:37 UTC
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.
Comment 13 Katrin Fischer 2011-10-17 20:48:44 UTC Comment hidden (obsolete)
Comment 14 Paul Poulain 2011-10-18 14:48:53 UTC
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.
Comment 15 Paul Poulain 2011-10-18 14:49:24 UTC
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>
Comment 16 Frédérick Capovilla 2011-10-19 14:32:15 UTC
> 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.
Comment 17 Paul Poulain 2011-11-04 09:24:44 UTC
Patch pushed, please test