Attachment 31404 Details for Bug 12876 – Bug 12876 - Reserve in waiting/transfer status may be cancelled by user

[patch] Bug 12876 - Reserve in waiting/transfer status may be cancelled by user

Bug-12876---Reserve-in-waitingtransfer-status-may-.patch (text/plain), 5.09 KB, created by Rafal Kopaczka on 2014-09-05 14:35:41 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Rafal Kopaczka

Created: 2014-09-05 14:35:41 UTC

Size: 5.09 KB

patch

obsolete

>From cbcc249d5231d2b915afdf5cbe97b1510eb905bd 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
>---
> C4/Reserves.pm                  |   24 ++++++++++++++++++++++++
> opac/opac-modrequest-suspend.pl |    2 +-
> opac/opac-modrequest.pl         |    4 +---
> t/db_dependent/Reserves.t       |   31 ++++++++++++++++++++++++++++++-
> 4 files changed, 56 insertions(+), 5 deletions(-)
>
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index a98eed4..54b674d 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-suspend.pl b/opac/opac-modrequest-suspend.pl
>index 41ee5ad..d39e220 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 CanReserveBeCanceledFromOpac($reserve_id, $borrowernumber);
> }
> else {
>     SuspendAll(
>diff --git a/opac/opac-modrequest.pl b/opac/opac-modrequest.pl
>index e55a886..4cefe21 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 0698534..ed7e31e 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 {
>-- 
>1.7.10.4

Actions: View | Diff | Splinter Review

Attachments on bug 12876: 31393 | 31404 | 31406 | 31413 | 31414 | 31440 | 31441 | 31846