[Koha-patches] [PATCH] Bugfix: Corrects bad behavior in opac-reserve.pl

Galen Charlton gmcharlt at gmail.com
Wed Feb 3 01:17:46 CET 2010


Hi,

This does not apply on HEAD.  Please resubmit, and if possible put the
whitespace-only parts of the change in a separate patch.

Regards,

Galen

On Mon, Feb 1, 2010 at 4:07 PM, Chris Nighswonger
<cnighswonger at foundations.edu> wrote:
> When AllowOnShelfHolds is enabled, but maxreserves is NULL, attempting to reserve
> an item results in a basically blank page being returned with no error message.
>
> This patch adds a check to see if the check for maxreserves has returned a value and
> bypasses the application of it if it has not.
> ---
>  opac/opac-reserve.pl |   66 +++++++++++++++++++++----------------------------
>  1 files changed, 28 insertions(+), 38 deletions(-)
>
> diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
> index be3c77a..782e0a6 100755
> --- a/opac/opac-reserve.pl
> +++ b/opac/opac-reserve.pl
> @@ -47,8 +47,8 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
>  );
>
>  sub get_out ($$$) {
> -       output_html_with_http_headers(shift,shift,shift); # $query, $cookie, $template->output;
> -       exit;
> +    output_html_with_http_headers(shift,shift,shift); # $query, $cookie, $template->output;
> +    exit;
>  }
>
>  # get borrower information ....
> @@ -96,16 +96,6 @@ $template->param( branch => $branch );
>  my $CGIbranchloop = GetBranchesLoop($branch);
>  $template->param( CGIbranch => $CGIbranchloop );
>
> -#Debug
> -#output_html_with_http_headers($query,$cookie,"<html><head></head><body> @biblionumbers </body></html>\n");
> -#exit;
> -#my %bibdata;
> -#my $rank;
> -#my $biblionumber;
> -#my $bibdata;
> -#my %itemhash;
> -#my $forloan;
> -
>  #
>  #
>  # Build hashes of the requested biblio(item)s and items.
> @@ -151,7 +141,7 @@ foreach my $biblioNumber (@biblionumbers) {
>  #
>  if ( $query->param('place_reserve') ) {
>     my $notes = $query->param('notes');
> -       my $canreserve=0;
> +    my $canreserve=0;
>
>     # List is composed of alternating biblio/item/branch
>     my $selectedItems = $query->param('selecteditems');
> @@ -167,7 +157,7 @@ if ( $query->param('place_reserve') ) {
>         my $branch = $query->param('branch');
>         $selectedItems = "$bib/$item/$branch/";
>     }
> -
> +
>     my @selectedItems = split /\//, $selectedItems;
>
>     # Make sure there is a biblionum/itemnum/branch triplet for each item.
> @@ -190,21 +180,21 @@ if ( $query->param('place_reserve') ) {
>
>         my $biblioData = $biblioDataHash{$biblioNum};
>         my $found;
> -
> -       # Check for user supplied reserve date
> -       my $startdate;
> -       if (
> -           C4::Context->preference( 'AllowHoldDateInFuture' ) &&
> -           C4::Context->preference( 'OPACAllowHoldDateInFuture' )
> -           ) {
> -           $startdate = $query->param("reserve_date_$biblioNum");
> -       }
> +
> +    # Check for user supplied reserve date
> +    my $startdate;
> +    if (
> +        C4::Context->preference( 'AllowHoldDateInFuture' ) &&
> +        C4::Context->preference( 'OPACAllowHoldDateInFuture' )
> +        ) {
> +        $startdate = $query->param("reserve_date_$biblioNum");
> +    }
>
>         # If a specific item was selected and the pickup branch is the same as the
>         # holdingbranch, force the value $rank and $found.
>         my $rank = $biblioData->{rank};
>         if ($itemNum ne ''){
> -               $canreserve = 1 if CanItemBeReserved($borrowernumber,$itemNum);
> +            $canreserve = 1 if CanItemBeReserved($borrowernumber,$itemNum);
>             $rank = '0' unless C4::Context->preference('ReservesNeedReturns');
>             my $item = GetItem($itemNum);
>             if ( $item->{'holdingbranch'} eq $branch ){
> @@ -212,11 +202,11 @@ if ( $query->param('place_reserve') ) {
>             }
>         }
>         else {
> -               $canreserve = 1 if CanBookBeReserved($borrowernumber,$biblioNum);
> +            $canreserve = 1 if CanBookBeReserved($borrowernumber,$biblioNum);
>             # Inserts a null into the 'itemnumber' field of 'reserves' table.
>             $itemNum = undef;
>         }
> -
> +
>         # Here we actually do the reserveration. Stage 3.
>         AddReserve($branch, $borrowernumber, $biblioNum, 'a', [$biblioNum], $rank, $startdate, $notes,
>                    $biblioData->{'title'}, $itemNum, $found) if ($canreserve);
> @@ -264,7 +254,8 @@ if ( $borr->{debarred} && ($borr->{debarred} eq 1) ) {
>
>  my @reserves = GetReservesFromBorrowernumber( $borrowernumber );
>  $template->param( RESERVES => \@reserves );
> -if ( scalar(@reserves) >= $MAXIMUM_NUMBER_OF_RESERVES ) {
> +if ( $MAXIMUM_NUMBER_OF_RESERVES && scalar(@reserves) >= $MAXIMUM_NUMBER_OF_RESERVES ) {
> +#if ( scalar(@reserves) >= $MAXIMUM_NUMBER_OF_RESERVES ) {
>     $template->param( message => 1 );
>     $noreserves = 1;
>     $template->param( too_many_reserves => scalar(@reserves));
> @@ -333,12 +324,12 @@ foreach my $biblioNum (@biblionumbers) {
>         my $fee = GetReserveFee(undef, $borrowernumber, $itemInfo->{'biblionumber'}, 'a',
>                                 ( $itemInfo->{'biblioitemnumber'} ) );
>         $itemInfo->{'reservefee'} = sprintf "%.02f", ($fee ? $fee : 0.0);
> -
> +
>         if ($itemLevelTypes && $itemInfo->{itype}) {
>             $itemInfo->{description} = $itemTypes->{$itemInfo->{itype}}{description};
>             $itemInfo->{imageurl} = getitemtypeimagesrc() . "/". $itemTypes->{$itemInfo->{itype}}{imageurl};
>         }
> -
> +
>         if (!$itemInfo->{'notforloan'} && !($itemInfo->{'itemnotforloan'} > 0)) {
>             $biblioLoopIter{forloan} = 1;
>         }
> @@ -435,27 +426,26 @@ foreach my $biblioNum (@biblionumbers) {
>                 ( $branchitemrule->{'holdallowed'} == 1 && $borr->{'branchcode'} ne $itemInfo->{'homebranch'} ) ) {
>             $policy_holdallowed = 0;
>         }
> -
>         if (IsAvailableForItemLevelRequest($itemNum) and $policy_holdallowed and CanItemBeReserved($borrowernumber,$itemNum)) {
>             $itemLoopIter->{available} = 1;
>             $numCopiesAvailable++;
>         }
>
> -       # FIXME: move this to a pm
> +    # FIXME: move this to a pm
>         my $dbh = C4::Context->dbh;
>         my $sth2 = $dbh->prepare("SELECT * FROM reserves WHERE borrowernumber=? AND itemnumber=? AND found='W'");
>         $sth2->execute($itemLoopIter->{ReservedForBorrowernumber}, $itemNum);
>         while (my $wait_hashref = $sth2->fetchrow_hashref) {
>             $itemLoopIter->{waitingdate} = format_date($wait_hashref->{waitingdate});
>         }
> -       $itemLoopIter->{imageurl} = getitemtypeimagelocation( 'opac', $itemTypes->{ $itemInfo->{itype} }{imageurl} );
> -
> +    $itemLoopIter->{imageurl} = getitemtypeimagelocation( 'opac', $itemTypes->{ $itemInfo->{itype} }{imageurl} );
> +
>     # Show serial enumeration when needed
>     if ($itemLoopIter->{enumchron}) {
> -        $itemdata_enumchron = 1;
> +        $itemdata_enumchron = 1;
>     }
>     $template->param( itemdata_enumchron => $itemdata_enumchron );
> -
> +
>         push @{$biblioLoopIter{itemLoop}}, $itemLoopIter;
>     }
>
> @@ -496,9 +486,9 @@ if (
>     C4::Context->preference( 'OPACAllowHoldDateInFuture' )
>     ) {
>     $template->param(
> -       reserve_in_future         => 1,
> -       DHTMLcalendar_dateformat  => C4::Dates->DHTMLcalendar(),
> -       );
> +    reserve_in_future         => 1,
> +    DHTMLcalendar_dateformat  => C4::Dates->DHTMLcalendar(),
> +    );
>  }
>
>  output_html_with_http_headers $query, $cookie, $template->output;
> --
> 1.6.0.4
>
> _______________________________________________
> Koha-patches mailing list
> Koha-patches at lists.koha.org
> http://lists.koha.org/mailman/listinfo/koha-patches
>



-- 
Galen Charlton
gmcharlt at gmail.com



More information about the Koha-patches mailing list