From 6014b9699e921312f38848543b2eaf72a419dfeb 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] 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

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests, QA script and new tests.
Works as described, tested with:
.../cgi-bin/koha/opac-modrequest.pl?reserve_id=XXX

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Conflicts:
	t/db_dependent/Reserves.t

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
(cherry picked from commit c0ab862f4a902ce261b9b8870c169f6fe27711e1)

Conflicts:
	opac/opac-modrequest-suspend.pl
---
 C4/Reserves.pm            | 24 ++++++++++++++++++++++++
 opac/opac-modrequest.pl   |  4 +---
 t/db_dependent/Reserves.t | 31 ++++++++++++++++++++++++++++++-
 3 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index d4059d2f17..8df27db2b9 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -118,6 +118,7 @@ BEGIN {
         &CheckReserves
         &CanBookBeReserved
 	&CanItemBeReserved
+        &CanReserveBeCanceledFromOpac
         &CancelReserve
         &CancelExpiredReserves
 
@@ -556,6 +557,29 @@ sub CanItemBeReserved{
 
     return 1;
 }
+
+=head2 CanReserveBeCanceledFromOpac
+
+    $number = CanReserveBeCanceledFromOpac($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 CanReserveBeCanceledFromOpac {
+    my ($reserve_id, $borrowernumber) = @_;
+
+    my $reserve = GetReserve($reserve_id);
+
+    return 0 unless $reserve->{borrowernumber} == $borrowernumber;
+    return 0 if ( $reserve->{found} eq 'W' ) or ( $reserve->{found} eq 'T' );
+
+    return 1;
+
+}
+
 #--------------------------------------------------------------------------------
 =head2 GetReserveCount
 
diff --git a/opac/opac-modrequest.pl b/opac/opac-modrequest.pl
index 2f63df02ca..710a9fa3cd 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 CanReserveBeCanceledFromOpac($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 af9d7a9f53..d2fe834485 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,35 @@ $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));
+AddReserve('CPL',  $requesters{'CPL'}, $item_bibnum,
+           $constraint, $bibitems,  1, undef, $expdate, $notes,
+           $title,      $checkitem, '');
+my (undef, $canres, undef) = CheckReserves($itemnumber);
+
+my $cancancel = CanReserveBeCanceledFromOpac($canres->{reserve_id}, $requesters{'CPL'});
+is($cancancel, 1, 'Can user cancel its own reserve');
+
+$cancancel = CanReserveBeCanceledFromOpac($canres->{reserve_id}, $requesters{'FPL'});
+is($cancancel, 0, 'Other user cant cancel reserve');
+
+ModReserveAffect($itemnumber, $requesters{'CPL'}, 1);
+$cancancel = CanReserveBeCanceledFromOpac($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 = CanReserveBeCanceledFromOpac($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 {
-- 
2.39.5