Bug 12911 - batch_id for new label batches can be assigned to several batches
Summary: batch_id for new label batches can be assigned to several batches
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
: 8042 (view as bug list)
Depends on:
Blocks: 14204
  Show dependency treegraph
 
Reported: 2014-09-11 19:04 UTC by Nick Clemens
Modified: 2017-06-14 22:10 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 12911 - batch_id for new label batches can be assigned to several batches -Added code to make a placeholder and thus reserve the batch number. -Added code to delete old placeholders everytime a new batch is made. -Modified code in SELECT query to disc (2.54 KB, patch)
2015-01-29 05:18 UTC, Thomas Wright
Details | Diff | Splinter Review
Bug 12911 - batch_id for new labels batches can be asssigned to several batches (4.51 KB, patch)
2015-02-07 18:00 UTC, Nick Clemens
Details | Diff | Splinter Review
[Signed-off] Bug 12911 - batch_id for new labels batches can be asssigned to several batches (4.68 KB, patch)
2015-02-10 16:14 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 12911 - batch_id for new labels batches can be asssigned to several batches (4.62 KB, patch)
2015-02-13 13:12 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2014-09-11 19:04:01 UTC
When adding a new label batch the MAX batch_id is retrieved from creator_batches and incremented, however, the updated value is not saved until new items are added to the batch.  

This means that if one library opens a new batch but does not add items before another library opens the batch they will both be assigned the same batch number

To recreate:
1 -In one browser window, go to tools->label creator and click the new batch button
2 - Before adding items, open a new browser, and go tools->label creator and click the new batch button
3 - Note that both batches have the same number listed
4 - Add an item to the first batch - you should now see one item in the batch
5 - Add an item to the second batch, you should see two items in the batch

I can also verify this happens pretty often in our shared system.


C4:Creators
44 sub new {
 45     my ($invocant) = shift;
 46     my $type = ref($invocant) || $invocant;
 47     my $self = {
 48         batch_id        => 0,
 49         items           => [],
 50         branch_code     => 'NB',
 51         batch_stat      => 0,   # False if any data has changed and the db has not been updated
 52         @_,
 53     };
 54     my $sth = C4::Context->dbh->prepare("SELECT MAX(batch_id) FROM creator_batches;");
 55     $sth->execute();
 56     my $batch_id = $sth->fetchrow_array;
 57     $self->{'batch_id'} = ++$batch_id unless $self->{'batch_id'} != 0;      # this allows batch_id to be passed in for individual label     printing
 58     bless ($self, $type);
 59     return $self;
 60 }
 61 
 62 sub add_item {
 63     my $self = shift;
 64     my $number = shift;
 65     ref($self) =~ m/C4::(.+)::.+$/;
 66     my $number_type = ($1 eq 'Patroncards' ? 'borrower_number' : 'item_number');
 67     my $query = "INSERT INTO creator_batches (batch_id, $number_type, branch_code, creator) VALUES (?,?,?,?);";
 68     my $sth = C4::Context->dbh->prepare($query);
 69 #    $sth->{'TraceLevel'} = 3;
 70     $sth->execute($self->{'batch_id'}, $number, $self->{'branch_code'}, $1);
 71     if ($sth->err) {
 72         warn sprintf('Database returned the following error on attempted INSERT: %s', $sth->errstr);
 73         return -1;
 74     }
 75     $query = "SELECT max(label_id) FROM creator_batches WHERE batch_id=? AND $number_type=? AND branch_code=?;";
 76     my $sth1 = C4::Context->dbh->prepare($query);
 77     $sth1->execute($self->{'batch_id'}, $number, $self->{'branch_code'});
 78     my $label_id = $sth1->fetchrow_array;
 79     push (@{$self->{'items'}}, {$number_type => $number, label_id => $label_id});
 80     $self->{'batch_stat'} = 1;
 81     return 0;
 82 }
Comment 1 Katrin Fischer 2014-11-02 11:44:07 UTC
*** Bug 8042 has been marked as a duplicate of this bug. ***
Comment 2 Thomas Wright 2015-01-29 05:18:12 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens 2015-02-03 21:48:06 UTC
I tried testing and got odd results with one number displaying as batch id, but the results saving into the DB with a higher batch id, I suspect there is some double increment due to the placeholder records.

I am wondering if instead of adding a placeholder at object creation the batch id could be assigned after the first items are added?  Maybe assign batch ID -1, don't display it in the add screen, and then in the add items function check for batch id -1 and create the new items then?
Comment 4 Nick Clemens 2015-02-07 18:00:29 UTC Comment hidden (obsolete)
Comment 5 Marc Véron 2015-02-10 16:14:42 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2015-02-13 13:12:12 UTC
Created attachment 35898 [details] [review]
[PASSED QA] Bug 12911 - batch_id for new labels batches can be asssigned to several batches

Currently batch_id is assigned upon creation of a new batch object.  This patch leaves batch_id as 0 at creation and adds a check when adding items.  If batch is new then batch_id is created then

Test plan:
1 -In one browser window, go to tools->label creator and click the new batch button
2 - Before adding items, open a new browser, and go tools->label creator and click the new batch button
3 - Note that both batches have the same number listed
4 - Add an item to the first batch - you should now see one item in the batch
5 - Add an item to the second batch, you should see two items in the batch
6 - Apply patch and repeat steps 1&2
7 - Note that neither batch lists a batch number
8 - Add an item to the first batch, you should see one item and a batch number
9 - Add an item to the second batch, you should see one item and a new batch number

Patch behaves as expected.
Signed-off-by: Marc Veron <veron@veron.ch>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Tomás Cohen Arazi 2015-05-14 14:18:21 UTC
Patch pushed to master.

Thanks Nick!
Comment 8 Chris Cormack 2015-05-17 21:56:05 UTC
Pushed to 3.18.x will be in 3.18.7
Comment 9 Jonathan Druart 2015-08-27 09:07:42 UTC
> Test plan:
> 1 -In one browser window, go to tools->label creator and click the new batch
> button
> 2 - Before adding items, open a new browser, and go tools->label creator and
> click the new batch button
> 3 - Note that both batches have the same number listed
> 4 - Add an item to the first batch - you should now see one item in the batch
> 5 - Add an item to the second batch, you should see two items in the batch
> 6 - Apply patch and repeat steps 1&2
> 7 - Note that neither batch lists a batch number
> 8 - Add an item to the first batch, you should see one item and a batch
> number
> 9 - Add an item to the second batch, you should see one item and a new batch
> number

But does not work if you add a second item!

See bug 14739 and please provide a quick fix.
This bug is much more annoying than the one fixed by this patch.