From c1f183ae6cfeb3347d6c5f2a4745e4b764f3c486 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 Signed-off-by: Katrin Fischer --- 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 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 348ec181e2..f0ab2d4d3e 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 f17df687d4..74cef2af5a 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 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.11.0