Bug 6673 - Create Batch Label from Staged MARC Records fails after upgrade to 3.4.x
Summary: Create Batch Label from Staged MARC Records fails after upgrade to 3.4.x
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Label/patron card printing (show other bugs)
Version: 3.4
Hardware: All All
: PATCH-Sent (DO NOT USE) normal
Assignee: Ian Walls
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-05 13:16 UTC by Larry Baerveldt
Modified: 2013-12-05 20:04 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Proposed fix (1.41 KB, patch)
2011-08-05 16:52 UTC, Ian Walls
Details | Diff | Splinter Review
Bug 6673: Create Batch Label from Staged MARC Records fails (1.46 KB, patch)
2011-08-06 00:56 UTC, Chris Cormack
Details | Diff | Splinter Review
Expanded patch (4.04 KB, patch)
2011-09-21 15:13 UTC, Ian Walls
Details | Diff | Splinter Review
Signed-off patch (4.15 KB, patch)
2011-09-27 19:17 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Larry Baerveldt 2011-08-05 13:16:22 UTC
Several sites are reporting that after upgrading Koha 3.4.x, they are unable to run Create Batch Label, from the Staged MARC Records page. Error message is:

"Error attempting to create label batch. Please ask your system administrator to check the log for more details."
Comment 1 Ian Walls 2011-08-05 16:52:32 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2011-08-06 00:56:04 UTC Comment hidden (obsolete)
Comment 3 Chris Cormack 2011-08-06 00:56:47 UTC
Works well, is more robust. Signed Off
Comment 4 Owen Leonard 2011-08-08 15:27:13 UTC
I just tried testing this by doing the following:

- Apply the patch (attachment 4874 [details] [review]) to my test system
- Export a batch of MARC records from our live system
- Import the batch into my test system
- Find the batch listed on the Manage Staged MARC Records page
- Click 'Create Label Batch' link for that set

I still got "Error attempting to create label batch" message. In my logs I find this:

manage-marc-import.pl: create_labelbatch_from_importbatch() : Call to C4::ImportBatch::GetItemNumbersFromImportBatch returned no item number(s) from import batch #13. at /home/oleonard/kohaclone/tools/manage-marc-import.pl line 196.
Comment 5 Brendan Gallagher 2011-08-09 03:57:18 UTC
Owen - Can you try it with a new batch of items - instead of ones that are already in your system.  I just want to compare the differences. Looking at the database - I see the batches that are not adding items (are going into the database with a NULL itemnumber in the labels_database tables - since they are already in there).  It will just give me another spot to test on.
Comment 6 Owen Leonard 2011-08-24 13:17:35 UTC
Sorry, looks like I neglected to post the results of my last test: Adding a batch of items which I've gotten from an external source still result in the same error in my test system running current master.
Comment 7 Ian Walls 2011-09-21 02:16:07 UTC
Owen,


I've discovered that it's not possible to create a label batch if do not have a branch set, as is the default with the Koha database user.  The branchcode added to the system must be a valid entry.  Are you testing with that user, or with a specific patron account?

I've poked through this pretty thoroughly... not sure what else it could be
Comment 8 Ian Walls 2011-09-21 02:29:25 UTC
Further evidence:  all other invocations of the variable LoginBranchname work like this:

get_branch_code_from_name($template->{VARS}->{'LoginBranchname'})

Whereas (at least before the attached patch), manage-marc-import.pl does this:

get_branch_code_from_name($template->param('LoginBranchname'))

By default, LoginBranchname is defined as:

C4::Context->userenv?C4::Context->userenv->{"branchname"}:"insecure"
or
C4::Context->userenv?C4::Context->userenv->{"branchname"}:""

depending on what part of C4/Auth.pm you look at.

Since 'insecure' and '' are both invalid entries, it makes more sense to bypass the code=>name=>code jump, and just invoke the code in the standard fashion, as submitted in this patch.
Comment 9 Chris Cormack 2011-09-21 02:39:16 UTC
Furthermore

In 3.4.x and onwards, 
$template->param('LoginBranchname') always returns nothing 

$template->param('anything') will always return nothing. With the change to template::toolkit the param call is just a wrapper to set a variable for use by the template:

$template->param('fish' => 'food') == $template->{VARS}->{'fish'} = 'food';
$template->param('fish') == $template->{VARS}-{'fish'} = '';
 
The way to get a variable is $template->{VARS}->{'LoginBranchname'}

I agree, we may as well just fetch the branch for the userenv. The reason its failing now, is branch is never going to be set with the existing code
Comment 10 Ian Walls 2011-09-21 15:13:40 UTC Comment hidden (obsolete)
Comment 11 Owen Leonard 2011-09-27 19:17:43 UTC
Created attachment 5620 [details] [review]
Signed-off patch

I tested this batch on a fresh data set and found it to work. Before applying the patch, attempting to create a label batch from an imported MARC record batch (/cgi-bin/koha/tools/manage-marc-import.pl?import_batch_id=xx&op=create_labels) would result in an error. After applying the patch the label batch is created without errors.
Comment 12 Ian Walls 2011-09-27 19:23:30 UTC
I'm very confident of my marking this patch Passed QA, but if anyone would like another set of eyes on this before it gets committed, please speak up.
Comment 13 Chris Cormack 2011-09-28 07:36:33 UTC
Pushed, please test
Comment 14 Jared Camins-Esakov 2012-12-31 00:18:37 UTC
There have been no further reports of problems so I am marking this bug resolved.