Bug 15239

Summary: Add multiple items fail on barcode
Product: Koha Reporter: Hugo Agud <hagud>
Component: CatalogingAssignee: Charles Farmer <charles.farmer>
Status: RESOLVED DUPLICATE QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: charles.farmer, jonathan.druart, kachomanic, katrin.fischer, m.de.rooy, magnus, philippe.blouin, tomascohen
Version: master   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14110
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19113
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: screenshot of items list
Bug15239: feed branch to Barcode constructor, in case of hbyymmincr

Description Hugo Agud 2015-11-23 11:44:06 UTC
Created attachment 45104 [details]
screenshot of items list

we have identified that at least koha 3.20 on ubuntu 14.04 when you try to add multiple items and you have configured  autoBarcode to '¡<<branchcode>>yymmdd it only applies to the first item, the rest are just yymmdd

It only happens with <<branchode>>yymmdd.
Comment 1 Hugo Agud 2015-11-23 11:44:32 UTC
It also happens in koha 3_22 beta
Comment 2 Magnus Enger 2015-12-01 10:11:54 UTC
Could this be related to/similar to Bug 15042? Just a hunch...
Comment 3 Jonathan Druart 2016-01-06 12:10:27 UTC
The module looks completely buggy, indeed.

There is the following note in the module:
> This format is deprecated and SHOULD NOT BE USED.

With an explanation of why it should not be used.
Comment 4 Ruben Norori 2017-08-11 16:27:39 UTC
I have a solution, you must go to the route:
/usr /share/koha/intranet/cgi-bin/cataloging  search the additem.pl file and open it with your text editor.

Search this code:
# For the first iteration
            my $barcodevalue = $oldbarcode;
            my $exist_itemnumber;
           
            for (my $i = 0; $i < $number_of_copies;) {

                # If there is a barcode
                if ($barcodevalue) {

                    # Getting a new barcode (if it is not the first iteration or the barcode we tried already exists)
                    $barcodevalue = $barcodeobj->next_value($oldbarcode) if ($i > 0 || $exist_itemnumber);


Replace it with:
# For the first iteration
            my $barcodevalue = $oldbarcode;
            my $exist_itemnumber;
            my $valor = substr($barcodevalue, 0, 3);

            for (my $i = 0; $i < $number_of_copies;) {

                # If there is a barcode
                if ($barcodevalue) {

                    # Getting a new barcode (if it is not the first iteration or the barcode we tried already exists)
                    $barcodevalue = $valor.$barcodeobj->next_value($oldbarcode) if ($i > 0 || $exist_itemnumber);


Make a back up of the file before try this. Works for me.
Comment 5 Katrin Fischer 2017-08-12 10:11:40 UTC
Hi Ruben, thx for your suggestion. My guess is that this would work for a branchcode of 3 characters, but would have to changed a bit to check for the actual length in order to go in.

 my $valor = substr($barcodevalue, 0, 3);

Which version did you test with?
Comment 6 Ruben Norori 2017-08-12 12:21:13 UTC
I tried it in version 3.20.07
And you are right, I will update this code to check the length of branchcode and improve this solution.
Comment 7 Ruben Norori 2017-08-14 16:01:10 UTC
This is the final solution, this regular expression extracts only the alphabetical part of the string, and use it in the new item.

my $barcodevalue = $oldbarcode;
            my $exist_itemnumber;
            my $newv = $barcodevalue;
            $newv =~ s/[^a-zA-Z,]//g;

            for (my $i = 0; $i < $number_of_copies;) {

                # If there is a barcode
                if ($barcodevalue) {
                    # Getting a new barcode (if it is not the first iteration or the barcode we tried already exists)
                    $barcodevalue = $newv.$barcodeobj->next_value($oldbarcode) if ($i > 0 || $exist_itemnumber);
Comment 8 Katrin Fischer 2017-08-15 13:53:03 UTC
Hi Ruben,

thx for working on this, but I think we still need some changes here. 

I think you need to actually look up the branchcode, so you can cut off the right bit, as branchcodes can contain numbers too. For example, we have a library where the branches are number KAT01-KAT99. In your code, the KAT would be removed, but the number part would remain.

Also you might want to take a look at how to submit patches - if you could provide a formatted patch, this would be great:
https://wiki.koha-community.org/wiki/Submitting_A_Patch
Comment 9 Ruben Norori 2017-08-22 20:23:28 UTC
I have the definitive solution:
First we must add two new procedures, functions, methods, which consult in the database the registry homebranch.

#Original function
sub get_item_from_barcode {
    my ($barcode)=@_;
    my $dbh=C4::Context->dbh;
    my $result;
    my $rq=$dbh->prepare("SELECT itemnumber from items where items.barcode=?");
    $rq->execute($barcode);
    ($result)=$rq->fetchrow;
    return($result);
}

#New function
sub get_home_from_barcode {
    my ($barcode)=@_;
    my $dbh=C4::Context->dbh;
    my $result;
    my $rq=$dbh->prepare("SELECT homebranch from items where items.barcode=?");
    $rq->execute($barcode);
    ($result)=$rq->fetchrow;
    return($result);
}

#New function
sub get_home_from_item {
    my ($barcode)=@_;
    my $dbh=C4::Context->dbh;
    my $result;
    my $rq=$dbh->prepare("SELECT homebranch from items where items.itemnumber=?");
    $rq->execute($barcode);
    ($result)=$rq->fetchrow;
    return($result);
}

Then call the functions and save the result in a variable that we concate at the final result, also add an "if" to use one function in the first iteration and the other function executes it in the "else".

for (my $i = 0; $i < $number_of_copies;) {
                # If there is a barcode
                if ($barcodevalue) {
                    # Getting a new barcode (if it is not the first iteration or the barcode we tried already exists)
                    # For execute only in the first iteration
                    if($i<1){
                    # Call new function
                    my $newv = get_home_from_item($exist_itemnumber);
                    $barcodevalue = $newv.$barcodeobj->next_value($oldbarcode) if ($i > 0 || $exist_itemnumber);
                    }else{
                    # Call new function 
                    my $exin = get_home_from_barcode($barcodevalue);
                    $barcodevalue = $exin.$barcodeobj->next_value($oldbarcode) if ($i > 0 || $exist_itemnumber);
                    }
                    # Putting it into the record
                    if ($barcodevalue) {
                        $record->field($tagfield)->update($tagsubfield => $barcodevalue);
                    }
Comment 10 Tomás Cohen Arazi 2017-08-23 12:25:26 UTC
Hi Ruben,

(In reply to Ruben Norori from comment #9)
> I have the definitive solution:
> First we must add two new procedures, functions, methods, which consult in
> the database the registry homebranch.

I didn't go deep into the problem you're trying to solve, but I suggest you don't introduce new subs, but use the current api instead.

We have a (sort of) DAO class called Koha::Items that exposes a ->search method, which returns Koha::Item objects. So you can simply do something like this:

  use Koha::Items;

  my $item = Koha::Items->find( $barcode );

->find gets an id column to find the row and create the object. 'barcode' is a unique key, so this works.

Now you have a Koha::Item object, representing the item with the passed barcode. You can then use it like:

  my $homebranch = $item->homebranch;

Hope it clarifies! This conversations are easier on the koha-devel list or on the IRC channel.
Comment 11 Ruben Norori 2017-08-23 15:09:16 UTC
Hi Tomás.

Thank for your suggestion, try the long way because I didn't know another solution, but I will try your recomendation.
Comment 12 Charles Farmer 2018-03-30 14:18:38 UTC
Created attachment 73488 [details] [review]
Bug15239: feed branch to Barcode constructor, in case of hbyymmincr
Comment 13 Charles Farmer 2018-03-30 14:28:55 UTC
Because C4::Barcodes treats its arguments with shifts, there isn't any way to feed a branch to the constructor at all time without potentially clashing with other barcode types.

Work could be done to change shifts to $params->{whatever}, but I went with an 'if' test in additem.pl instead.

Also, as mentionned by Jonathan and as explained in the hbyymmincr class, this barcode format is far from perfect. At least, with this patch, it should now be correctly incrementing when autoBarcode is set and add_multiple_copies_submit is used to submit the form.
Comment 14 Owen Leonard 2018-05-10 13:56:55 UTC
Could we get a test plan for this?
Comment 15 Katrin Fischer 2019-11-02 21:56:18 UTC

*** This bug has been marked as a duplicate of bug 23851 ***