Attachment 78424 Details for Bug 15565 – Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC

[patch] Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC

Bug-15565-Fixed-issues-preventing-any-item-level-h.patch (text/plain), 6.41 KB, created by Alex Buckley on 2018-09-04 21:13:53 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Buckley

Created: 2018-09-04 21:13:53 UTC

Size: 6.41 KB

patch

obsolete

>From ae39543bff4cf9633658bd077c293ae1893b9eb8 Mon Sep 17 00:00:00 2001
>From: Alex Buckley <alexbuckley@catalyst.net.nz>
>Date: Sun, 26 Aug 2018 15:16:47 +1200
>Subject: [PATCH] Bug 15565: Fixed issues preventing any item-level holds being
> placed in OPAC
>
>This commit fixes two bugs:
>
>1. C4::Reserves->CanItemBeReserved() returns an output in a hash in the format: status => 'output'
>
>However opac-reserve.pl is not accessing th output of this function
>correctly consequently no item is ever reservable.
>
>And so after a patron submits the reservation form on opac-reserve.pl it
>looks like the item-level reservation has successfully been placed
>however it has not and no error message is displayed to them.
>
>This commit stores the output of the aforementioned function in a hash
>and then checks the value of the status key. As a result opac-reserve.pl
>now correctly accesses the value returned and item level holds are
>successfully placed from the OPAC.
>
>2. If a biblio has previously been reserved then the opac-reserve.pl page
>shows the 'Next available item' and 'A specific item' radio buttons to be
>disabled to ensure patrons cannot place a reservation of a different
>type on the biblio.
>
>With both of these radio buttons being disabled it results in item level holds
>placed in the OPAC on a previously reserved item not being stored in the
>database. This is very confusing for users.
>
>I have implemented a conditional which checks what the forced_hold_type
>of the previous hold was. If it was 'item' then the $reqtype =
>'Specific' and if it was 'record' then $reqtype is 'Any'.
>
>This means there will always be a value for $reqtype, and this variable
>is checked if it is equal to 'Specific' and if it is then an item level
>hold is placed.
>
>Test plan:
>1. Ensure in your circulation rules that 'Item level holds' is enabled
>
>2. Apply all patches on this bug report except this commit
>
>3. In the OPAC try placing a item level hold (and notice you can submit
>the opac-reserve.pl page and no error messages are displayed. However
>the hold(s) are not being placed.
>
>4. Place an item level hold(s) on the biblio from staff client interface
>and then return to the opac-reserve.pl page for the biblio in the OPAC
>and notice both the 'Next available' and 'A specific item' radio buttons
>are disabled.
>
>5. Again try placing an item level hold in the OPAC and notice dispite
>no error/warning message being displayed the hold is not being placed.
>
>6. Apply this patch
>
>7. Restart memcached, and plack
>
>8. In the staff client remove the reservation you placed from the staff
>client.
>
>9. Repeat step 3 and notice when you go to your opac summary
>(opac-user.pl) page the holds are displayed. .i.e. You can now
>successfully place item level holds in the OPAC.
>
>10. Return to the opac-reserve.pl page for the biblio and notice that
>because you have already placed a hold on this biblio the 'Next
>available item' and 'A specific item' radiobuttons are disabled.
>
>11. Select several of the items checkboxes to place more item level
>holds and press the 'Confirm hold' button.
>
>12. Go to your opac summary (opac-user.pl) page again and notice the
>holds you placed in step 11 are shown there.
>
>i.e. you can place item level holds from the OPAC on biblios which have
>previously had reserves placed on them.
>
>Sponsored-By: Brimbank Library
>---
> opac/opac-reserve.pl | 28 +++++++++++++++++++++++++---
> 1 file changed, 25 insertions(+), 3 deletions(-)
>
>diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
>index bbfcc75..afdaae5 100755
>--- a/opac/opac-reserve.pl
>+++ b/opac/opac-reserve.pl
>@@ -44,6 +44,7 @@ use Koha::Libraries;
> use Koha::Patrons;
> use Date::Calc qw/Today Date_to_Days/;
> use List::MoreUtils qw/uniq/;
>+use Data::Dumper;
> 
> my $maxreserves = C4::Context->preference("maxreserves");
> 
>@@ -196,6 +197,24 @@ if ( $query->param('place_reserve') ) {
>         my @itemnumbers = $query->param("checkitem_$biblioNum");
>         my $branch = $query->param("branch_$biblioNum");
>         my $reqtype = $query->param("reqtype_$biblioNum");
>+        warn $biblioNum;
>+        if (!$reqtype) {
>+            #If no $reqtype value has been passed from the form this means both reqany and reqspecific radio buttons are disabled because a hold has been placed on this biblio previously and so a forced_hold_level exists.
>+            # Determine what the forced_hold_level is.
>+            my $holds = Koha::Holds->search(
>+                {
>+                     borrowernumber => $borrowernumber,
>+                     biblionumber   => $biblioNum,
>+                     found          => undef,
>+                }
>+            );
>+            my $forced_hold_level = $holds->forced_hold_level();
>+            if ($forced_hold_level eq "item") {
>+                $reqtype = "Specific";
>+            } elsif ($forced_hold_level eq "record") {
>+                $reqtype = "Any";
>+            }
>+        }
>         my $holds_to_place_count = $query->param("holds_to_place_count_$biblioNum") || 1;
>         my $notes = $query->param('notes_'.$biblioNum) || '';
>         my $nbRequested = $reqtype eq 'Specific' ? @itemnumbers : $holds_to_place_count;
>@@ -246,16 +265,18 @@ if ( $query->param('place_reserve') ) {
>                 if ( $hostbiblioNum ne $biblioNum ) {
>                     $biblionumber = $hostbiblioNum;
>                 }
>-
>-                $canreserve = 0 unless CanItemBeReserved( $borrowernumber, $itemnumber ) eq 'OK';
>+                my $reservestatus = CanItemBeReserved( $borrowernumber, $itemnumber);
>+                $canreserve = 0 if !($reservestatus->{status} eq 'OK');
>                 $rank = '0' unless C4::Context->preference('ReservesNeedReturns');
>                 my $item = GetItem($itemnumber);
>                 if ( $item->{'holdingbranch'} eq $branch ) {
>                     $found = 'W'
>                       unless C4::Context->preference('ReservesNeedReturns');
>                 }
>-
>+                warn $itemnumber;
>+                warn $canreserve;
>                 if ($canreserve) {
>+                    warn $canreserve;
>                     my $reserve_id = AddReserve(
>                         $branch,         $borrowernumber,
>                         $biblionumber,
>@@ -594,6 +615,7 @@ foreach my $biblioNum (@biblionumbers) {
>         }
>     );
>     my $forced_hold_level = $holds->forced_hold_level();
>+    warn $forced_hold_level;
>     if ($forced_hold_level) {
>         #$biblioLoopIter{force_hold}   = 1 if $forced_hold_level eq 'item';
>         #$biblioLoopIter{itemholdable} = 0 if $forced_hold_level eq 'record';
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 15565: 47527 | 47551 | 47552 | 50068 | 50069 | 50392 | 50393 | 56229 | 56230 | 56231 | 56232 | 56458 | 56510 | 56511 | 56512 | 56513 | 56514 | 56515 | 56517 | 56518 | 56519 | 56520 | 56521 | 56522 | 56523 | 56524 | 61244 | 61245 | 61246 | 61247 | 61248 | 61576 | 61577 | 61578 | 61579 | 61580 | 64590 | 64591 | 64592 | 64593 | 64594 | 66594 | 66595 | 66596 | 66597 | 66598 | 68759 | 68760 | 68761 | 68762 | 68763 | 71643 | 71644 | 71645 | 71646 | 71647 | 71648 | 72644 | 72645 | 72646 | 72647 | 72648 | 72649 | 78242 | 78243 | 78244 | 78245 | 78246 | 78247 | 78248 | 78266 | 78305 | 78417 | 78418 | 78419 | 78420 | 78421 | 78422 | 78423 | 78424 | 78425 | 78426 | 78712 | 78713 | 78714 | 78715 | 78716 | 78717 | 78718 | 78719 | 78720 | 78815 | 78816 | 78817 | 78818 | 78819 | 78820 | 78821 | 78822 | 78823 | 81419 | 81420 | 81421 | 81422 | 81423 | 81424 | 81425 | 81426 | 81427 | 81431 | 81449 | 81570 | 81571 | 81572 | 81573 | 81574 | 81575 | 81576 | 81577 | 81578 | 81579 | 81580 | 81581 | 82205 | 82206 | 82207 | 82208 | 82209 | 82210 | 82211 | 82212 | 82213 | 82214 | 82215 | 82216 | 82220 | 82221 | 83331 | 83332 | 83333 | 83334 | 83335 | 83336 | 83337 | 83338 | 83339 | 83340 | 83341 | 83342 | 83378 | 83380 | 83381 | 84541 | 84542 | 84543 | 84544 | 84545 | 84546 | 84547 | 84548 | 84549 | 84550 | 84551 | 84552 | 84553 | 87979 | 87980 | 87981 | 87982 | 87983 | 87984 | 87985 | 87986 | 87987 | 87988 | 87989 | 87990 | 87991 | 117128 | 117129 | 117130 | 117131 | 117132 | 117133 | 120365 | 120366 | 120367 | 120639 | 120640 | 120641 | 121941 | 122098 | 162071 | 162072 | 162073 | 162074 | 163052 | 163053 | 163054 | 163055 | 163092 | 163093 | 166005 | 166006 | 166007 | 166008 | 166009