From e9eef04b95e7cb0dadda862f07c0d34a31f5eb6a Mon Sep 17 00:00:00 2001 From: Rafal Kopaczka 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 Signed-off-by: Katrin Fischer 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 --- C4/Reserves.pm | 24 ++++++++++++++++++++++++ opac/opac-modrequest-suspend.pl | 2 +- opac/opac-modrequest.pl | 4 +--- t/db_dependent/Reserves.t | 32 +++++++++++++++++++++++++++++++- 4 files changed, 57 insertions(+), 5 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index baa99ef460..4875c9cfbc 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -120,6 +120,7 @@ BEGIN { &CheckReserves &CanBookBeReserved &CanItemBeReserved + &CanReserveBeCanceledFromOpac &CancelReserve &CancelExpiredReserves @@ -572,6 +573,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 41ee5ad3f1..d39e22031a 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 e55a8861a7..4cefe21a9f 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 8b5a12ad86..04a9819793 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -2,7 +2,8 @@ use Modern::Perl; -use Test::More tests => 43; +use Test::More tests => 47; + use MARC::Record; use DateTime::Duration; @@ -414,6 +415,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