From 13a61279523b35370f7b3adeb0f47ad30bfa937d Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 9 Aug 2016 22:18:14 +0100 Subject: [PATCH] Bug 17097: Add a confirmation page when deleting a patron It won't hurt to have a confirmation page when deleting a patron. Moreover it's the more easy way to protect against CSRF attacks :) Test plan: Make sure you get a confirmation page when deleting a patron Confirm that approving or denying the confirmation work as expected Signed-off-by: Chris Cormack Signed-off-by: Marcel de Rooy Signed-off-by: Kyle M Hall --- .../prog/en/includes/members-toolbar.inc | 10 +-------- .../prog/en/modules/members/deletemem.tt | 14 +++++++++++++ members/deletemem.pl | 21 ++++++++++--------- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc index 7f7fe5c4ea..9eb1f65f12 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc @@ -25,9 +25,7 @@ $(document).ready(function(){ }); [% ELSE %] $("#deletepatron").click(function(){ - confirm_deletion(); - $(".btn-group").removeClass("open"); - return false; + window.location='/cgi-bin/koha/members/deletemem.pl?member=[% borrowernumber | url%]'; }); [% END %] $("#renewpatron").click(function(){ @@ -71,12 +69,6 @@ $(document).ready(function(){ return false; }) }); -function confirm_deletion() { - var is_confirmed = window.confirm(_("Are you sure you want to delete this patron? This cannot be undone.")); - if (is_confirmed) { - window.location='/cgi-bin/koha/members/deletemem.pl?member=[% borrowernumber %]'; - } -} function confirm_local_deletion() { var is_confirmed = window.confirm(_("Are you sure you want to delete this patron from the local database? This cannot be undone.")); if (is_confirmed) { diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/deletemem.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/deletemem.tt index 4572f8240b..9e24bd6a46 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/deletemem.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/deletemem.tt @@ -28,6 +28,20 @@ [% END %] + [% ELSIF op == 'delete_confirm' and borrowernumber %] + [%# TODO add "patron does not exist" unless borrowernumber %] +
+

Are you sure you want to delete the patron [% firstname %] [% surname %]? This cannot be undone.

+
+ + + +
+
+ + +
+
[% END %] [% IF ( keeplocal ) %]
diff --git a/members/deletemem.pl b/members/deletemem.pl index fd53eccd70..173d70e334 100755 --- a/members/deletemem.pl +++ b/members/deletemem.pl @@ -101,13 +101,10 @@ if (C4::Context->preference("IndependentBranches")) { } } +my $op = $input->param('op') || 'delete_confirm'; my $dbh = C4::Context->dbh; -my $sth=$dbh->prepare("Select * from borrowers where guarantorid=?"); -$sth->execute($member); -my $data=$sth->fetchrow_hashref; -if ($countissues > 0 or $flags->{'CHARGES'} or $data->{'borrowernumber'} or $deletelocal == 0){ - # print $input->header; - +my $is_guarantor = $dbh->selectrow_array("SELECT COUNT(*) FROM borrowers WHERE guarantorid=?", undef, $member); +if ( $op eq 'delete_confirm' or $countissues > 0 or $flags->{'CHARGES'} or $is_guarantor or $deletelocal == 0){ my $patron_image = Koha::Patron::Images->find($bor->{borrowernumber}); $template->param( picture => 1 ) if $patron_image; @@ -137,19 +134,23 @@ if ($countissues > 0 or $flags->{'CHARGES'} or $data->{'borrowernumber'} or $de if ($flags->{'CHARGES'} ne '') { $template->param(charges => $flags->{'CHARGES'}->{'amount'}); } - if ($data->{'borrowernumber'}) { + if ($is_guarantor) { $template->param(guarantees => 1); } if ($deletelocal == 0) { $template->param(keeplocal => 1); } -output_html_with_http_headers $input, $cookie, $template->output; - -} else { + # This is silly written but reflect the same conditions as above + if ( not $countissues > 0 and not $flags->{CHARGES} ne '' and not $is_guarantor and not $deletelocal == 0 ) { + $template->param( op => 'delete_confirm' ); + } +}elsif ( $op eq 'delete_confirmed' ) { MoveMemberToDeleted($member); C4::Members::HandleDelBorrower($member); DelMember($member); + # TODO Tell the user everything went ok print $input->redirect("/cgi-bin/koha/members/members-home.pl"); } +output_html_with_http_headers $input, $cookie, $template->output; -- 2.39.5