From d8dd31142c0bfd9b09f9fa8e02fc292cf9deed2c Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 17 Apr 2020 11:22:04 +0200 Subject: [PATCH] Bug 13518: Delete patron's modifications along with the patron The table borrower_modifications has no FK constraint on the borrowernumber and will remain untouched when the patron is deleted. If the borrowernumber doesn't exist in the database, the modification entry is no longer visible in Koha. The problem is that this table is used for the borrower modifications and the self-registration features. So far borrowernumber is the PK (int(11) NOT NULL DEFAULT '0'), for the self-registration feature we can have borrowernumber that is not defined (0 is used) Ideally we would like to have borrowernumber a DEFAULT NULL, and use NULL for self-reg, but then we will loose the PK (PK cannot be NULL). As we cannot keep the correct constraints at DB level anyway, we will need to handle consistency at code-level. Test plan: Create a new patron Do some modification at the OPAC Delete the patron Confirm that the modifications as been removed (directly in DB) Signed-off-by: Bernardo Gonzalez Kriegel Signed-off-by: Katrin Fischer Signed-off-by: Martin Renvoize --- Koha/Patron.pm | 5 +++++ t/db_dependent/Koha/Patrons.t | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 9948f8c35c..fe3e80aebb 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -42,6 +42,7 @@ use Koha::Patron::Categories; use Koha::Patron::HouseboundProfile; use Koha::Patron::HouseboundRole; use Koha::Patron::Images; +use Koha::Patron::Modifications; use Koha::Patron::Relationships; use Koha::Patrons; use Koha::Plugins; @@ -383,6 +384,10 @@ sub delete { # FIXME Could be $patron->get_lists $_->delete for Koha::Virtualshelves->search( { owner => $self->borrowernumber } ); + # We cannot have a FK on borrower_modifications.borrowernumber, the table is also used + # for patron selfreg + $_->delete for Koha::Patron::Modifications->search( { borrowernumber => $self->borrowernumber } ); + $self->SUPER::delete; logaction( "MEMBERS", "DELETE", $self->borrowernumber, "" ) if C4::Context->preference("BorrowersLog"); diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index d4582fcbd9..0ca5faa717 100644 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -466,7 +466,7 @@ subtest "move_to_deleted" => sub { }; subtest "delete" => sub { - plan tests => 6; + plan tests => 7; t::lib::Mocks::mock_preference( 'BorrowersLog', 1 ); my $patron = $builder->build( { source => 'Borrower' } ); my $retrieved_patron = Koha::Patrons->find( $patron->{borrowernumber} ); @@ -480,6 +480,7 @@ subtest "delete" => sub { value => { owner => $patron->{borrowernumber} } } ); + my $modification = $builder->build_object({ class => 'Koha::Patron::Modifications', value => { borrowernumber => $patron->{borrowernumber} } }); my $deleted = $retrieved_patron->delete; is( ref($deleted), 'Koha::Patron', 'Koha::Patron->delete should return the deleted patron object if the patron has been correctly deleted' ); @@ -492,6 +493,8 @@ subtest "delete" => sub { is( Koha::Virtualshelves->search( { owner => $patron->{borrowernumber} } )->count, 0, q|Koha::Patron->delete should have deleted patron's lists| ); + is( Koha::Patron::Modifications->search( { borrowernumber => $patron->{borrowernumber} } )->count, 0, q|Koha::Patron->delete should have deleted patron's modifications| ); + my $number_of_logs = $schema->resultset('ActionLog')->search( { module => 'MEMBERS', action => 'DELETE', object => $retrieved_patron->borrowernumber } )->count; is( $number_of_logs, 1, 'With BorrowerLogs, Koha::Patron->delete should have logged' ); }; -- 2.39.5