From fee60647f2ecbb9b0fe1656185f3557355eb45b5 Mon Sep 17 00:00:00 2001 From: Emmi 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 Signed-off-by: Owen Leonard Signed-off-by: Nadine Pierre Signed-off-by: Arthur Bousquet --- Koha/Hold.pm | 2 +- Koha/Patron.pm | 7 +++++-- koha-tmpl/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 bfd3434..30188ae 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 348ec18..f0ab2d4 100644 --- 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 --- 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 %]
+ [% IF ( ItemsOnHold ) %] +

Patron has [% ItemsOnHold | html %] hold(s). Deleting patron cancels all their holds.


+ [% END %]

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

diff --git a/members/deletemem.pl b/members/deletemem.pl index 3d8712e..440f9d0 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.7.4