From ba96138741d84fb6d5000536bea5f81ad18e9069 Mon Sep 17 00:00:00 2001
From: Emmi <emmi.takkinen@outlook.com>
Date: Tue, 2 Jul 2019 11:05:36 +0300
Subject: [PATCH] Bug 23219: Cancel patrons holds when patron delete

Currently deleting a patron deletes all their holds and leaves
no record to the "old_reserves" table.

 Steps to reproduce:
- Create a patron
- Add holds for patron
- Holds are recorded to "reserves" table
- Delete patron
- Confirm delete
=>Patron and all holds are deleted and no record of holds is
left in "old_reserves" table

This patch displays alert text notifying user that deleting patron
cancels all their holds. Holds are cancelled instead of deleting them.
This patch also writes stringified datetime to holds cancel log instead
of whole datetime object.

To test:
- Apply this patch
- Create a patron
- Add holds for patron
- Holds are recorded to "reserves" table
- Delete patron
- Alert text of holds is displayed
- Confirm patron delete
=> Patron is deleted, their holds are cancelled and moved to
"old_reserves" table
---
 Koha/Hold.pm                                               | 2 +-
 Koha/Patron.pm                                             | 7 +++++--
 .../intranet-tmpl/prog/en/modules/members/deletemem.tt     | 3 +++
 members/deletemem.pl                                       | 5 ++++-
 4 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/Koha/Hold.pm b/Koha/Hold.pm
index bfd3434026..30188ae69e 100644
--- a/Koha/Hold.pm
+++ b/Koha/Hold.pm
@@ -354,7 +354,7 @@ sub cancel {
     my ( $self, $params ) = @_;
     $self->_result->result_source->schema->txn_do(
         sub {
-            $self->cancellationdate(dt_from_string);
+            $self->cancellationdate( dt_from_string->strftime( '%Y-%m-%d %H:%M:%S' ) );
             $self->priority(0);
             $self->_move_to_old;
             $self->SUPER::delete(); # Do not add a DELETE log
diff --git a/Koha/Patron.pm b/Koha/Patron.pm
index 04b54a3ed6..2984f89bc1 100644
--- a/Koha/Patron.pm
+++ b/Koha/Patron.pm
@@ -324,8 +324,11 @@ sub delete {
     my $deleted;
     $self->_result->result_source->schema->txn_do(
         sub {
-            # Delete Patron's holds
-            $self->holds->delete;
+            # Cancel Patron's holds
+            my $holds = $self->holds;
+            while( my $hold = $holds->next ){
+                $hold->cancel;
+            }
 
             # Delete all lists and all shares of this borrower
             # Consistent with the approach Koha uses on deleting individual lists
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 f17df687d4..b458b64b2a 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/deletemem.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/deletemem.tt
@@ -37,6 +37,9 @@
     [% ELSIF op == 'delete_confirm' and patron %]
         [%# TODO add "patron does not exist" unless patron %]
         <div class="dialog alert">
+            [% IF ( ItemsOnHold ) %]
+            <h3>Patron has [% ItemsOnHold %] hold(s). Deleting patron cancels all their holds.</h3></br>
+            [% END %]
             <h3>Are you sure you want to delete the patron [% patron.firstname | html %] [% patron.surname | html %]? This cannot be undone.</h3>
             <form action="/cgi-bin/koha/members/deletemem.pl">
                 <input type="hidden" name="csrf_token" value="[% csrf_token | html %]" />
diff --git a/members/deletemem.pl b/members/deletemem.pl
index 3d8712ede7..440f9d0a40 100755
--- a/members/deletemem.pl
+++ b/members/deletemem.pl
@@ -85,6 +85,7 @@ if (C4::Context->preference("IndependentBranches")) {
 my $op = $input->param('op') || 'delete_confirm';
 my $dbh = C4::Context->dbh;
 my $is_guarantor = $dbh->selectrow_array("SELECT COUNT(*) FROM borrowers WHERE guarantorid=?", undef, $member);
+my $countholds = $dbh->selectrow_array("SELECT COUNT(*) FROM reserves WHERE borrowernumber=?", undef, $member);
 if ( $op eq 'delete_confirm' or $countissues > 0 or $charges or $is_guarantor ) {
 
     $template->param(
@@ -99,7 +100,9 @@ if ( $op eq 'delete_confirm' or $countissues > 0 or $charges or $is_guarantor )
     if ($is_guarantor) {
         $template->param(guarantees => 1);
     }
-
+    if($countholds > 0){
+        $template->param(ItemsOnHold => $countholds);
+    }
     # This is silly written but reflect the same conditions as above
     if ( not $countissues > 0 and not $charges and not $is_guarantor ) {
         $template->param(
-- 
2.17.1