Bug 6963 - When creating a new order, the item isn't added if its barcode already exists in the items table.
Summary: When creating a new order, the item isn't added if its barcode already exists...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: 3.6
Hardware: All All
: PATCH-Sent (DO NOT USE) normal (vote)
Assignee: Frédérick Capovilla
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks: 21182
  Show dependency treegraph
 
Reported: 2011-10-03 19:43 UTC by Frédérick Capovilla
Modified: 2018-08-08 17:57 UTC (History)
2 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Corrects a problem when a new order is created with a duplicate barcode. (6.02 KB, patch)
2011-10-03 19:43 UTC, Frédérick Capovilla
Details | Diff | Splinter Review
[SIGNED-OFF] Corrects a problem when a new order is created with a duplicate barcode. (6.49 KB, patch)
2011-10-08 20:22 UTC, Katrin Fischer
Details | Diff | Splinter Review
New version of the initial patch with empty barcodes ignored. (6.24 KB, patch)
2011-10-11 13:43 UTC, Frédérick Capovilla
Details | Diff | Splinter Review
Follow-up patch : use the subfield associated with items.barcode (2.12 KB, patch)
2011-10-17 20:09 UTC, Frédérick Capovilla
Details | Diff | Splinter Review
[SIGNED-OFF] Follow-up patch : use the subfield associated with items.barcode (2.46 KB, patch)
2011-10-17 20:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6963: Corrects a problem when a new order is created with a duplicate barcode. (6.39 KB, patch)
2011-10-17 20:48 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Follow-up patch : use the subfield associated with items.barcode (2.46 KB, patch)
2011-10-17 20:48 UTC, Katrin Fischer
Details | Diff | Splinter Review
Follow-up patch : use the subfield associated with items.barcode (2.50 KB, patch)
2011-10-18 14:49 UTC, Paul Poulain
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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