From aa8ab8d0e789d1ece38206b069b2369778d58161 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 10 Jan 2018 16:49:33 -0300 Subject: [PATCH] Bug 18856: Don't show cancel option for waiting holds in OPAC This is the alternative patch of Kyle's """ If a hold is 'waiting' for the patron to collect then the patron should be prevented from cancelling the hold via their account in the opac. If a patron tries to cancel the hold, Koha will give an 'are you sure' alert and when you click Yes the page just refreshes and the hold remains. Staff can cancel the hold from the staff interface but they can then action the waiting hold. I think therefore that it is correct behaviour that a patron cannot cancel a hold when it reaches waiting state via the opac but it would be useful to either have a warning to prevent the cancellation or a useful message when they attempt to do so. The template was using a method that tells Koha if *staff* can cancel a hold, instead of patron. Test Plan: 1) Set up a waiting hold 2) Try to cancel it from the opac 3) Note you cannot 4) Apply this patch 5) Reload the page 6) Note the cancel button has disappeared for found holds """ It sounds better to keep the ->is_cancelable method, for readability Signed-off-by: Kyle M Hall Signed-off-by: Marcel de Rooy Signed-off-by: Jonathan Druart (cherry picked from commit 710c0e5df644731cee15f91b4ebc62924f7f85c9) Signed-off-by: Fridolin Somers --- Koha/Hold.pm | 10 ++++------ t/db_dependent/Hold.t | 6 +++--- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/Koha/Hold.pm b/Koha/Hold.pm index 84117a5f9e..0368a422bb 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -228,9 +228,9 @@ sub is_in_transit { Returns true if hold is a cancelable hold -Holds may be canceled if they not found, or -are found and waiting. A hold found but in -transit cannot be canceled. +Holds may be only canceled if they are found. + +This is used from the OPAC. =cut @@ -238,9 +238,7 @@ sub is_cancelable { my ($self) = @_; return 1 unless $self->is_found(); - return 0 if $self->is_in_transit(); - return 1 if $self->is_waiting(); - return 0; + return 0; # if ->is_in_transit or if ->is_waiting } =head3 is_at_destination diff --git a/t/db_dependent/Hold.t b/t/db_dependent/Hold.t index 7eb04abd5e..8078f2a932 100755 --- a/t/db_dependent/Hold.t +++ b/t/db_dependent/Hold.t @@ -123,11 +123,11 @@ ok( !$hold->is_in_transit, 'The hold is not in transit' ); # Test method is_cancelable $hold->found(undef); -ok( $hold->is_cancelable(), "Unfound hold is cancelable" ); +is( $hold->is_cancelable, 1, "Unfound hold is cancelable" ); $hold->found('W'); -ok( $hold->is_cancelable, "Waiting hold is cancelable" ); +is( $hold->is_cancelable, 0, "Waiting hold is not cancelable" ); $hold->found('T'); -ok( !$hold->is_cancelable, "In transit hold is not cancelable" ); +is( $hold->is_cancelable, 0, "In transit hold is not cancelable" ); # Test method is_at_destination $hold->found(undef); -- 2.39.5