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

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nadine Pierre <nadine.pierre@inLibro.com>
Signed-off-by: Arthur Bousquet <arthur.bousquet@inlibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/Koha/Hold.pm b/Koha/Hold.pm
index bfd3434..30188ae 100644 (file)
--- 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 348ec18..f0ab2d4 100644 (file)
--- a/Koha/Patron.pm
+++ b/Koha/Patron.pm
@@ -328,8 +328,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 f17df68..74cef2a 100644 (file)
--- 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 | html %] 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 3d8712e..440f9d0 100755 (executable)
--- 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(