From 0c07cc533b4b46db8c633d65b72b249f98227acc Mon Sep 17 00:00:00 2001 From: Fridolin Somers Date: Mon, 18 Jun 2018 09:04:19 +0200 Subject: [PATCH] Bug 20953: Prevent several discharge requests on OPAC On OPAC, a user can requested a discharge even if one is already pending. This generates several pending discharges in staff interface that can not be deleted. This is because request operation leads to page 'opac-discharge.pl?op=request' and user can refresh this page performing a new request. Perl code must check that operation is allowed. Patch reoganised the code so that the following FIXME is obsolete : 'FIXME looks like $available is not needed' Patch also replaces 'op' arg test to also check undef : input->param("op") // '' Test plan : 1) Set system preference 'useDischarge' to 'Allow' 2) Choose a patron without checkouts nor fines nor restrictions 3) Log at OPAC and go to patron page /cgi-bin/koha/opac-user.pl 4) Click on 'ask for a discharge' tab => You see /cgi-bin/koha/opac-discharge.pl with text 'What is a discharge? ...' 5) Click on 'Ask for a discharge' link => You see /cgi-bin/koha/opac-discharge.pl?op=request with text 'Your discharge request has been sent ...' 6) In a new browser tab/page, go to intranet on /cgi-bin/koha/members/discharges.pl => You see one discharge requets for the patron 7) Come back to OPAC and refresh page => You see /cgi-bin/koha/opac-discharge.pl with text 'Your discharge will be available on this page within a few days.' 8) Come back to intranet and refresh /cgi-bin/koha/opac-discharge.pl => There is still one requets for the patron 9) Come back to OPAC and enter URL /cgi-bin/koha/opac-discharge.pl?op=get => You see /cgi-bin/koha/opac-discharge.pl with text 'Your discharge will be available on this page within a few days.' 10) Come back to intranet and refresh /cgi-bin/koha/opac-discharge.pl => There is still one requets for the patron 11) Click on 'allow' on patron discharge request 12) Come back to OPAC and refresh /cgi-bin/koha/opac-discharge.pl => You see link 'Get your discharge' 13) enter URL /cgi-bin/koha/opac-discharge.pl?op=request => You see same page and no new discharge requets is created 14) Come back to intranet on patron details page 15) Remove the discharge restriction 16) Come back to OPAC and refresh /cgi-bin/koha/opac-discharge.pl => You see text 'What is a discharge?...' 17) enter URL /cgi-bin/koha/opac-discharge.pl?op=get => You see same page and no new discharge requets is created Signed-off-by: Charles Farmer Signed-off-by: Katrin Fischer Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize --- opac/opac-discharge.pl | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/opac/opac-discharge.pl b/opac/opac-discharge.pl index 7653dc6122..4e8381bef8 100755 --- a/opac/opac-discharge.pl +++ b/opac/opac-discharge.pl @@ -37,7 +37,7 @@ unless ( C4::Context->preference('useDischarge') ) { exit; } -my $op = $input->param("op") || ''; +my $op = $input->param("op") // ''; # Getting the template and auth my ( $template, $loggedinuser, $cookie ) = get_template_and_user({ @@ -52,11 +52,21 @@ if ($can_be_discharged == 0) { $template->param( has_checkouts => 1 ); } +my $pending = Koha::Patron::Discharge::count({ + borrowernumber => $loggedinuser, + pending => 1, +}); +my $available = Koha::Patron::Discharge::is_discharged({borrowernumber => $loggedinuser}); + if ( $op eq 'request' ) { + if ($pending || $available) { + # Request already done + print $input->redirect("/cgi-bin/koha/opac-discharge.pl"); + exit; + } my $success = Koha::Patron::Discharge::request({ borrowernumber => $loggedinuser, }); - if ($success) { $template->param( success => 1 ); } @@ -65,6 +75,11 @@ if ( $op eq 'request' ) { } } elsif ( $op eq 'get' ) { + unless ($available) { + # No valid discharge to get + print $input->redirect("/cgi-bin/koha/opac-discharge.pl"); + exit; + } eval { # Getting member data @@ -92,18 +107,8 @@ elsif ( $op eq 'get' ) { } } else { - my $pending = Koha::Patron::Discharge::count({ - borrowernumber => $loggedinuser, - pending => 1, - }); - # FIXME looks like $available is not needed - # If a user is discharged they have a validated discharge available - my $available = Koha::Patron::Discharge::count({ - borrowernumber => $loggedinuser, - validated => 1, - }); $template->param( - available => $available && Koha::Patron::Discharge::is_discharged({borrowernumber => $loggedinuser}), + available => $available, pending => $pending, ); } -- 2.39.5