From d2dc843e3a5c934e49bbd74fe33c47f0b894bf98 Mon Sep 17 00:00:00 2001
From: Rafal Kopaczka <rkk0@poczta.onet.pl>
Date: Fri, 5 Sep 2014 14:50:15 +0200
Subject: [PATCH] [PATCH] Bug 12876 - Reserve in waiting/transfer status may
 be cancelled by user User may cancel his own reservation at
 waiting or in transit status through calling
 opac-modrequest.pl. Cancel button is disabled in interface
 but possibility to cancel should be checked also in
 opac-moderequest.pl, before calling CancelReserve().
 Similar situation is with opac-modrequest-suspend.pl This
 patch provides new soubroutine to chceck if user can cancel
 given reserve. It's possible only when he's owner of hold
 and hold isn't in transfer or waiting status. Additionaly
 there are new test for this function in Reserves.t

---
 C4/Reserves.pm                  |   30 ++++++++++++++++++++++++++++++
 opac/opac-modrequest-suspend.pl |    2 +-
 opac/opac-modrequest.pl         |    4 +---
 t/db_dependent/Reserves.t       |   33 ++++++++++++++++++++++++++++++++-
 4 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index a98eed4..6f27ae4 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -118,6 +118,7 @@ BEGIN {
         &CheckReserves
         &CanBookBeReserved
 	&CanItemBeReserved
+        &CanUserCancelReserve
         &CancelReserve
         &CancelExpiredReserves
 
@@ -556,6 +557,35 @@ sub CanItemBeReserved{
 
     return 1;
 }
+
+=head2 CanUserCancelReserve
+
+    $number = CanUserCancelReserve($reserve_id, $borrowernumber);
+
+    returns 1 if reserve can be cancelled by user from OPAC.
+    First check if reserve belongs to user, next checks if reserve is not in
+    transfer or waiting status
+
+=cut
+
+sub CanUserCancelReserve {
+    my ($reserve_id, $borrowernumber) = @_;
+
+    my $reserve = GetReserve($reserve_id);
+
+    return 0 unless $reserve->{borrowernumber} == $borrowernumber;
+    return 0 if $reserve->{found} eq 'W';
+
+    if ( $reserve->{itemnumber} ){
+        my ($transfertwhen, $transfertfrom, $transfertto) = C4::Circulation::GetTransfers( $reserve->{itemnumber} );
+
+        return 0 if $transfertwhen; #item is in transfer
+    }
+
+    return 1;
+
+}
+
 #--------------------------------------------------------------------------------
 =head2 GetReserveCount
 
diff --git a/opac/opac-modrequest-suspend.pl b/opac/opac-modrequest-suspend.pl
index 41ee5ad..5dfc574 100755
--- a/opac/opac-modrequest-suspend.pl
+++ b/opac/opac-modrequest-suspend.pl
@@ -39,7 +39,7 @@ my $suspend_until = $query->param('suspend_until') || undef;
 my $reserve_id    = $query->param('reserve_id');
 
 if ($reserve_id) {
-    ToggleSuspend( $reserve_id, $suspend_until );
+    ToggleSuspend( $reserve_id, $suspend_until ) if CanUserCancelReserve($reserve_id, $borrowernumber);
 }
 else {
     SuspendAll(
diff --git a/opac/opac-modrequest.pl b/opac/opac-modrequest.pl
index e55a886..b36e84a 100755
--- a/opac/opac-modrequest.pl
+++ b/opac/opac-modrequest.pl
@@ -45,9 +45,7 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
 my $reserve_id = $query->param('reserve_id');
 
 if ($reserve_id && $borrowernumber) {
-
-    my $reserve = GetReserve($reserve_id);
-    CancelReserve({ reserve_id => $reserve_id }) if $borrowernumber == $reserve->{borrowernumber} ;
+    CancelReserve({ reserve_id => $reserve_id }) if CanUserCancelReserve($reserve_id, $borrowernumber);
 }
 
 print $query->redirect("/cgi-bin/koha/opac-user.pl#opac-user-holds");
diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t
index 0698534..d3d7e36 100755
--- a/t/db_dependent/Reserves.t
+++ b/t/db_dependent/Reserves.t
@@ -2,7 +2,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 35;
+use Test::More tests => 39;
 
 use MARC::Record;
 use DateTime::Duration;
@@ -395,6 +395,37 @@ $p = C4::Reserves::CalculatePriority($bibnum, $resdate);
 is($p, 3, 'CalculatePriority should now return priority 3');
 # End of tests for bug 8918
 
+# Tests for cancel reserves by users from OPAC.
+$dbh->do('DELETE FROM reserves', undef, ($bibnum));
+$dbh->do('DELETE FROM branchtransfers', undef, undef);
+AddReserve('CPL',  $requesters{'CPL'}, $item_bibnum,
+           $constraint, $bibitems,  1, undef, $expdate, $notes,
+           $title,      $checkitem, '');
+my (undef, $canres, undef) = CheckReserves($itemnumber);
+
+my $cancancel = CanUserCancelReserve($canres->{reserve_id}, $requesters{'CPL'});
+is($cancancel, 1, 'Can user cancel its own reserve');
+
+$cancancel = CanUserCancelReserve($canres->{reserve_id}, $requesters{'FPL'});
+is($cancancel, 0, 'Other user cant cancel reserve');
+
+ModReserveAffect($itemnumber, $requesters{'CPL'}, 1);
+ModItemTransfer($itemnumber, 'CPL', 'FPL' );
+$cancancel = CanUserCancelReserve($canres->{reserve_id}, $requesters{'CPL'});
+is($cancancel, 0, 'Reserve in transfer status cant be canceled');
+
+$dbh->do('DELETE FROM reserves', undef, ($bibnum));
+AddReserve('CPL',  $requesters{'CPL'}, $item_bibnum,
+           $constraint, $bibitems,  1, undef, $expdate, $notes,
+           $title,      $checkitem, '');
+(undef, $canres, undef) = CheckReserves($itemnumber);
+
+ModReserveAffect($itemnumber, $requesters{'CPL'}, 0);
+$cancancel = CanUserCancelReserve($canres->{reserve_id}, $requesters{'CPL'});
+is($cancancel, 0, 'Reserve in waiting status cant be canceled');
+
+# End of tests for bug 12876
+
 $dbh->rollback;
 
 sub count_hold_print_messages {
-- 
1.7.10.4