Bug 3164 - OPAC-placed holds broken in SingleBranchMode
Summary: OPAC-placed holds broken in SingleBranchMode
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Galen Charlton
QA Contact: Bugs List
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2009-04-27 07:30 UTC by Chris Cormack
Modified: 2012-10-25 22:52 UTC (History)
4 users (show)

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


Attachments
The patch Ryan submitted 2009-05-30 (9.52 KB, patch)
2009-09-10 14:15 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2010-05-21 01:07:12 UTC


---- Reported by rch@liblime.com 2009-04-27 19:30:40 ----

Patrons cannot place holds on titles if SingleBranchMode is ON 
because the template does not supply the branchcode to opac-reserve.pl.

The script returns the error 'bad_data', which displays as
"ERROR: Internal error: incomplete hold request."

Offending code is around line 196 of opac/opac-reserve.pl,
but would be fixed by just keeping the branchcode as a param in the tmpl even if singlebranchmode is on.



---- Additional Comments From oleonard@myacpl.org 2009-05-02 19:08:35 ----

Ryan, are you saying that if the template includes a hidden form field with the user's default branch the bug would be fixed? I tried this:

<!-- TMPL_UNLESS NAME="singleBranchMode" -->
  ... normal branch select ...
<!-- TMPL_ELSE -->
<!-- TMPL_LOOP NAME="branchChoicesLoop" -->
<!-- TMPL_IF name="selected" --><input type="hidden" name="branch" value="<!-- TMPL_VAR NAME="value" -->" /><!-- /TMPL_IF -->
<!-- /TMPL_LOOP -->
<!-- /TMPL_UNLESS -->

...but I got the same error.



---- Additional Comments From rch@liblime.com 2009-06-01 03:08:10 ----

I think that it should have worked if the conditional was left out, but after glancing at the script, I think it was also worth making a couple other changes.  Patch submitted.



---- Additional Comments From oleonard@myacpl.org 2009-09-10 14:15:37 ----

Created an attachment
The patch Ryan submitted 2009-05-30





---- Additional Comments From oleonard@myacpl.org 2009-09-10 14:17:18 ----

This was Galen's response on the patches list following Ryan's submission:

On Sat, May 30, 2009 at 7:03 PM, Ryan Higgins <rch@liblime.com> wrote:
> +    my @selectedItems = split(/\//, $selectedItems, $MAX_SIMULTANEOUS_RESERVES);

Seems like $MAX_SIMULTANEOUS_RESERVES * 3 would be more accurate.

> -        my $branch    = shift(@selectedItems); # i.e., branch code, not name
> -
> -        my $singleBranchMode = $template->param('singleBranchMode');
> -        if ($singleBranchMode) {
> -            $branch = $borr->{'branchcode'};
> -        }
> +        my $branch    = ($singleBranchMode) ? $borr->{'branchcode'} : shift(@selectedItems);

This breaks parsing of the selected items parameter.  Prior to this
patch, three values ($biblionumber, $itemnumber, and $branch) always
got shifted off of the @selectedItems array during each loop
iteration, whereas now only two get snarfed if you're in single branch
mode.  However, the form doesn't know that, and still supplies a
triplet for each request.

Please fix and resubmit.

Regards,

Galen



---- Additional Comments From katrin.fischer@bsz-bw.de 2010-05-10 12:56:48 ----

This is still a problem in 3.01.00.133

With singleBranchMode ON placing holds in OPAC does not work:

Item level hold:
ERROR: No biblionumber received.

Bib level hold (next available):
ERROR: Internal error: incomplete hold request.



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:07 UTC  ---

This bug was previously known as _bug_ 3164 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=3164
Imported an attachment (id=1134)

Actual time not defined. Setting to 0.0
The original reporter of this bug does not have
   an account here. Reassigning to the person who moved
   it here: chris@bigballofwax.co.nz.
   Previous reporter was rch@liblime.com.
CC member bchurch@ptfs.com does not have an account here
CC member jstanley@hpplnj.org does not have an account here
CC member jwagner@ptfs.com does not have an account here
CC member maria.laude@liblime.com does not have an account here
CC member sedwards@alloycomputing.com does not have an account here
The original submitter of attachment 1134 [details] [review] is unknown.
   Reassigning to the person who moved it here: chris@bigballofwax.co.nz.

Comment 1 Katrin Fischer 2010-07-05 16:15:57 UTC
Retested with Koha Version 3.01.00.142

Now the error message is always:
ERROR: Internal error: incomplete hold request.

AllowOnShelfHolds ON/Allow
AllowHoldsOnDamagedItems ON/Allow
OpacItemHolds ON/Allow
HomeOrHoldingBranch the library the item was checked out from
ReservesControlBranch item's home library
canreservefromotherbranches Allow
maxreserves 50

Both patron and item are from the same branch.
Comment 2 Galen Charlton 2010-10-17 17:19:41 UTC
Pushed patch resolving this bug.