Duplicate barcodes are generated when placing an Order in Acquisitions when AcqCreateItem = "placing an order" and Autobarcode is turned on. The problem is that you can accidentally attach 3 items to an order, but only 1 will be saved to the database. When you go to receive your order, you can only receive 1 item as the other two were never made, since the barcode wasn't unique. In 3.8.0, a software error comes up which prevent any item creation, I believe, but master (3.9.x) doesn't throw any warnings or errors.
Hm, this should not happen, but maybe it will help to set the UniqueItemFields to barcode? In my tests it worked really nicely, but perhaps something changed.
(In reply to comment #1) > Hm, this should not happen, but maybe it will help to set the > UniqueItemFields to barcode? In my tests it worked really nicely, but > perhaps something changed. No luck. Setting "barcode" in UniqueItemFields makes no change. When did you test it?
Hm, I think I signed off on the patch introducing it, so the bug history should tell.
(In reply to comment #3) > Hm, I think I signed off on the patch introducing it, so the bug history > should tell. Which patch is that, Katrin? --- Also, when I look at my standard install 3.8.0 or my dev install 3.8.0 branch, the clear button works in the item block. However, it doesn't work in 3.8.2 (maybe 3.8.1) or higher... Also, in my standard 3.8.0 install, duplicate barcodes creates this error: DBD::mysql::st execute failed: Duplicate entry '201180550469528' for key 'itembarcodeidx' at .../lib/C4/Items.pm line 2127. While in my dev install 3.8.0 branch, a nice JS window pops up to say: "Duplicate values detected. Please correct the errors and resubmit." As mentioned before, in 3.8.2+, no error message is generated and only the first item is created.
(In reply to comment #4) > (In reply to comment #3) > > Also, when I look at my standard install 3.8.0 or my dev install 3.8.0 > branch, the clear button works in the item block. However, it doesn't work > in 3.8.2 (maybe 3.8.1) or higher... > Hmm. I was a bit hasty there. The clear button clears all the fields except date and barcode... although I'm not sure that's a good thing...
Still not sure why this one is happening... The relevant code in neworderempty.tt looks identical. neworderempty.pl looks like it should be passing everything correctly to the template too. additems.js seems close enough, although check_uniqueness.pl is very different...but a quick test didn't show a difference. Even when I replace all these files with files from a version where the dupe check works, the JS still fails to pick up the duplication of the barcodes. I'll have to try git bisect again, but anyone have any ideas on this one? I figure the problem must be with additems.js and check_uniqueness.pl...
Hi David, I filed a separate bug for the problem with the clear link, it's bug 8683 now.
I can replicate the problem: When using Autobarcode items created on order will all get the same(!) barcode. Additionally the check on saving the order fails, so you get no notice and only one item is created. I think this is because Autobarcode checks for the next barcode in the database. We are creating more than one item at once, so there is a problem. I am not sure if that part of the problem can be solved easily, but the form should warn on saving when duplicate barcodes are detected. Settings: UniqueItemFields: barcode AcqCreateItem: placing an order Autobarcode: 1, 2, 3... Turning off Autobarcode (do not generate) didn't solve the problem. There is a javascript error that you can see when saving the order in Firebug. Filing another bug for the broken duplicate check.
oops sorry - wrong bug!
I'm not able to reproduce the problem -- I suspect it was fixed by the patch for bug 8495. David: could you check to see if you're still seeing this?
(In reply to Galen Charlton from comment #10) > I'm not able to reproduce the problem -- I suspect it was fixed by the patch > for bug 8495. > > David: could you check to see if you're still seeing this? Really? That's odd. Admittedly, I don't fully comprehend the black magic that is the AJAX item add form in Acquisitions but I'm still seeing this :/. The first item you add will have a new barcode ($nextnum in comparison to the database). However, each new item you add to an order will have the exact same barcode as that one since they're all using the same database data, right? I don't see any checks against the barcode for items already in the order... Steps to Reproduce: 1) Syspref "AcqCreateItem" to "placing an order" 2) Autobarcode to any of the options (although I do get a Javascript error for " generated in the form <branchcode>yymm0001.". So probably try the others for the time being...) 3) UniqueItemFields to "barcode" 4) Go to acquisitions > new basket 5) Click on the "barcode" field for a new item, and add that item 6) Repeat step #5 and observe that they all have the same barcode
The problem is that the items are not inserted in the DB when you click on the "Add item" button. So the item form generated just after does not know an item is on the road. Hopefully, there is now a check when you submit the complete order "Duplicate values detected. Please correct the errors and resubmit." The form is not submitted, there are identical barcodes. It seems quite tricky to fix this one.
Created attachment 169883 [details] [review] Bug 8425: Add an offset parameter for barcode generation in hbyymmincr barcode.pl In order to propose a patch for 8425 and 30056, we need to have the possibility to ask for multiple barcodes without storing a new barcode. See 8425 and 30056 for more information. Note: Since 30056 also needs this patch, these are splitted in order to rebase easier
Created attachment 169884 [details] [review] Bug 8425: Increment the barcode on each call of set_barcode() Duplicate barcodes are generated when placing an Order in Acquisitions when AcqCreateItem = "placing an order" and Autobarcode is turned on. The problem is that you can accidentally attach 3 items to an order, but only 1 will be saved to the database. When you go to receive your order, you can only receive 1 item as the other two were never made, since the barcode wasn't unique. In 3.8.0, a software error comes up which prevent any item creation, I believe, but master (3.9.x) doesn't throw any warnings or errors. Yes.. This tries and solve a bug from 3.8.0 /o/ Test plan: 1 - set syspref "autoBarcode" to generated in the form <branchcode> yymm001 2 - set the barcode field to "barcode.pl" in marc structure 3 - create a new basket 4 - add one item to this basket 5 - click on the barcode field -> it should have a barcode 6 - click on add item and click on the new barcode fiels, it should have the same value APPLY PATCH: 7 - click on the barcode field -> it should have a barcode 8 - click on add item and click on the new barcode fields, it should have an higher barcode. Note : The barcode is incremented each time the barcode with an empty value is clicked on and only on "hbyymmincr" mode. Therefore, if the librarian removes the value from barcode and click again, they could get another barcode. I do not know if it is an issue.
Note, the proposed approach does not solve any problem of concurrent accesses to barcode.pl but I do not think it could be resolved without refactoring barcode.pl.
Note: this should also resolve: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30056
Created attachment 171350 [details] [review] Bug 8425: Add an offset parameter for barcode generation in hbyymmincr barcode.pl In order to propose a patch for 8425 and 30056, we need to have the possibility to ask for multiple barcodes without storing a new barcode. See 8425 and 30056 for more information. Note: Since 30056 also needs this patch, these are splitted in order to rebase easier Signed-off-by: Sam Sowanick <sam.sowanick@corvallisoregon.gov>
Created attachment 171351 [details] [review] Bug 8425: Increment the barcode on each call of set_barcode() Duplicate barcodes are generated when placing an Order in Acquisitions when AcqCreateItem = "placing an order" and Autobarcode is turned on. The problem is that you can accidentally attach 3 items to an order, but only 1 will be saved to the database. When you go to receive your order, you can only receive 1 item as the other two were never made, since the barcode wasn't unique. In 3.8.0, a software error comes up which prevent any item creation, I believe, but master (3.9.x) doesn't throw any warnings or errors. Yes.. This tries and solve a bug from 3.8.0 /o/ Test plan: 1 - set syspref "autoBarcode" to generated in the form <branchcode> yymm001 2 - set the barcode field to "barcode.pl" in marc structure 3 - create a new basket 4 - add one item to this basket 5 - click on the barcode field -> it should have a barcode 6 - click on add item and click on the new barcode fiels, it should have the same value APPLY PATCH: 7 - click on the barcode field -> it should have a barcode 8 - click on add item and click on the new barcode fields, it should have an higher barcode. Note : The barcode is incremented each time the barcode with an empty value is clicked on and only on "hbyymmincr" mode. Therefore, if the librarian removes the value from barcode and click again, they could get another barcode. I do not know if it is an issue. Signed-off-by: Sam Sowanick <sam.sowanick@corvallisoregon.gov>
(In reply to Biblibre Sandboxes from comment #18) > In 3.8.0, a software error comes up which prevent any item creation, I > believe, but master (3.9.x) doesn't throw any warnings or errors. > > Yes.. This tries and solve a bug from 3.8.0 /o/ Am I really reading this in 2024 ??
get_barcode my $scr = qq~ let elt = \$("#"+id); let homebranch = elt.parents('fieldset.rows:first') .find('input[name="kohafield"][value="items.homebranch"]') .siblings("select") .val(); if(typeof offset == 'undefined'){ var offset = 0; } if ( \$(elt).val() == '' ) { \$(elt).val(homebranch + ($nextnum + offset)); } ~; TOGETHER WITH +function set_barcode(id, force, offset=0) { => Default is zero. Why are we checking the type then later? Seems useless? +if(typeof autobarcodetype == 'undefined') { + var autobarcodetype = "$autoBarcodeType"; + var attempt = -1; +} => This does not look good either? Why are we conditionally declaring vars here? Why are we fixing this only for a specific autobarcode type?
(In reply to Marcel de Rooy from comment #20) > get_barcode > my $scr = qq~ > let elt = \$("#"+id); > let homebranch = elt.parents('fieldset.rows:first') > > .find('input[name="kohafield"][value="items.homebranch"]') > .siblings("select") > .val(); > > if(typeof offset == 'undefined'){ > var offset = 0; > } > if ( \$(elt).val() == '' ) { > \$(elt).val(homebranch + ($nextnum + offset)); > } > ~; > TOGETHER WITH > +function set_barcode(id, force, offset=0) { > => Default is zero. Why are we checking the type then later? Seems useless? Tbh I'm very unsecure with this part of the code and I'm unable to prove that this body is not appended to any other function without this prototype, hence to secure the behavior I prefered adding this. If there is a way to be sure this function definition will not be appended to any other prototype, then this part of the code is useless. > +if(typeof autobarcodetype == 'undefined') { > + var autobarcodetype = "$autoBarcodeType"; > + var attempt = -1; > +} > => This does not look good either? Why are we conditionally declaring vars > here? The point here is that due to the way this works, the script is imported each time you click on "add item", hence this will reset the number of attempts and so one. This is also true if you put it in a document.ready. We might use attempts as condition and only do this for attempts, it might be more understandable. > Why are we fixing this only for a specific autobarcode type? Good question, I'm on it.
Created attachment 174602 [details] [review] Bug 8425: Increment the barcode on each call of set_barcode() Duplicate barcodes are generated when placing an Order in Acquisitions when AcqCreateItem = "placing an order" and Autobarcode is turned on. The problem is that you can accidentally attach 3 items to an order, but only 1 will be saved to the database. When you go to receive your order, you can only receive 1 item as the other two were never made, since the barcode wasn't unique. In 3.8.0, a software error comes up which prevent any item creation, I believe, but master (3.9.x) doesn't throw any warnings or errors. Yes.. This tries and solve a bug from 3.8.0 /o/ Test plan: 1 - set syspref "autoBarcode" to generated in the form <branchcode> yymm001 2 - set the barcode field to "barcode.pl" in marc structure 3 - create a new basket 4 - add one item to this basket 5 - click on the barcode field -> it should have a barcode 6 - click on add item and click on the new barcode fiels, it should have the same value APPLY PATCH: 7 - click on the barcode field -> it should have a barcode 8 - click on add item and click on the new barcode fields, it should have an higher barcode. Note : The barcode is incremented each time the barcode with an empty value is clicked on. Therefore, if the librarian removes the value from barcode and click again, they could get another barcode. I do not know if it is an issue. Note: This patch is not splitted in two parts anymore Signed-off-by: Sam Sowanick <sam.sowanick@corvallisoregon.gov>
> +if(typeof autobarcodetype == 'undefined') { > + var autobarcodetype = "$autoBarcodeType"; > + var attempt = -1; > +} > => This does not look good either? Why are we conditionally declaring vars here? @Marcel I did not address this point, tell me if you see a clean solution for this dirty code :)