Attachment 103176 Details for Bug 13518 – Bug 13518: Delete patron's modifications along with the patron

[patch] Bug 13518: Delete patron's modifications along with the patron

Bug-13518-Delete-patrons-modifications-along-with-.patch (text/plain), 3.91 KB, created by Bernardo Gonzalez Kriegel on 2020-04-17 17:37:26 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Bernardo Gonzalez Kriegel

Created: 2020-04-17 17:37:26 UTC

Size: 3.91 KB

patch

obsolete

>From 07b64bdd5fba0b16b8115787036cab171569f2eb Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>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 <bgkriegel@gmail.com>
>Modifications removed, test pass
>---
> 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.17.1

Actions: View | Diff | Splinter Review

Attachments on bug 13518: 103130 | 103176 | 103187