From d732d4bd72ff02853a1db6108854c59f93c6683b Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Tue, 29 Aug 2017 13:55:59 -0400 Subject: [PATCH] Bug 9302 - Add ability to merge patron records It would be great if there were a merge patrons feature. If you accidentally end up with one patron with two cards it would be nice to merge their records together so that you don't lose their history or holds or anything. This patch adds a basic patron merge feature. It attempts to relink all patron related tables from the patron(s) to be merged. It does not attempt to relink librarian account related tables at this time. This feature does not attempt to automatically resolve issues such as duplicate holds. Such a feature could build upon this one though. Test Plan: 1) Apply this patch 2) Find two or more patrons 3) Perform a patron search that will bring them up on the same page of results, or add them all to a list of patrons. 4) Use the 'merge' button to begin the merging process 5) Choose a patron to keep 6) Verify the deleted patrons data ( checkouts, holds, etc ) are now linked to the kept patron Signed-off-by: Owen Leonard Signed-off-by: Ed Veal --- Koha/Patrons.pm | 75 ++++++++++++ .../prog/en/modules/members/member.tt | 31 ++++- .../prog/en/modules/members/merge-patrons.tt | 133 +++++++++++++++++++++ .../prog/en/modules/patron_lists/list.tt | 34 +++++- members/merge-patrons.pl | 58 +++++++++ t/db_dependent/Patrons.t | 48 +++++++- 6 files changed, 374 insertions(+), 5 deletions(-) create mode 100644 koha-tmpl/intranet-tmpl/prog/en/modules/members/merge-patrons.tt create mode 100755 members/merge-patrons.pl diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm index fd2b997..1abcecf 100644 --- a/Koha/Patrons.pm +++ b/Koha/Patrons.pm @@ -31,6 +31,33 @@ use Koha::Patron; use base qw(Koha::Objects); +our $RESULTSET_PATRON_ID_MAPPING = { + Accountline => 'borrowernumber', + ArticleRequest => 'borrowernumber', + BorrowerAttribute => 'borrowernumber', + BorrowerDebarment => 'borrowernumber', + BorrowerFile => 'borrowernumber', + BorrowerModification => 'borrowernumber', + ClubEnrollment => 'borrowernumber', + Issue => 'borrowernumber', + ItemsLastBorrower => 'borrowernumber', + Linktracker => 'borrowernumber', + Message => 'borrowernumber', + MessageQueue => 'borrowernumber', + OldIssue => 'borrowernumber', + OldReserve => 'borrowernumber', + Rating => 'borrowernumber', + Reserve => 'borrowernumber', + Review => 'borrowernumber', + Statistic => 'borrowernumber', + SearchHistory => 'userid', + Suggestion => 'suggestedby', + TagAll => 'borrowernumber', + Virtualshelfcontent => 'borrowernumber', + Virtualshelfshare => 'borrowernumber', + Virtualshelve => 'owner', +}; + =head1 NAME Koha::Patron - Koha Patron Object class @@ -186,6 +213,54 @@ sub anonymise_issue_history { return $nb_rows; } +=head3 merge + + Koha::Patrons->search->merge( { keeper => $borrowernumber, patrons => \@borrowernumbers } ); + + This subroutine merges a list of patrons into another patron record. This is accomplished by finding + all related patron ids for the patrons to be merged in other tables and changing the ids to be that + of the keeper patron. + +=cut + +sub merge { + my ( $self, $params ) = @_; + + my $keeper = $params->{keeper}; + my @borrowernumbers = @{ $params->{patrons} }; + + my $patron_to_keep = Koha::Patrons->find( $keeper ); + return unless $patron_to_keep; + + # Ensure the keeper isn't in the list of patrons to merge + @borrowernumbers = grep { $_ ne $keeper } @borrowernumbers; + + my $schema = Koha::Database->new()->schema(); + + my $results; + + foreach my $borrowernumber (@borrowernumbers) { + my $patron = Koha::Patrons->find( $borrowernumber ); + + next unless $patron; + + # Unbless for safety, the patron will end up being deleted + $results->{merged}->{$borrowernumber}->{patron} = $patron->unblessed; + + while (my ($r, $field) = each(%$RESULTSET_PATRON_ID_MAPPING)) { + my $rs = $schema->resultset($r)->search({ $field => $borrowernumber} ); + $results->{merged}->{ $borrowernumber }->{updated}->{$r} = $rs->count(); + $rs->update( { $field => $keeper }); + } + + $patron->delete(); + } + + $results->{keeper} = $patron_to_keep; + + return $results; +} + =head3 type =cut diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt index 0c5e044..85743e2 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt @@ -11,6 +11,23 @@ + + + +[% INCLUDE 'header.inc' %] +[% INCLUDE 'patron-search.inc' %] + +[% BLOCK display_names %] + [% SWITCH rs %] + [% CASE 'Accountline' %]account lines + [% CASE 'ArticleRequest' %]article requests + [% CASE 'BorrowerAttribute' %]extended patron attributes + [% CASE 'BorrowerDebarment' %]patron restrictions + [% CASE 'BorrowerFile' %]patrons files + [% CASE 'BorrowerModification' %]patron modification requests + [% CASE 'ClubEnrollment' %]club enrollments + [% CASE 'Issue' %]checkouts + [% CASE 'ItemsLastBorrower' %]marks as last borrower of item + [% CASE 'Linktracker' %]tracked link clicks + [% CASE 'Message' %]patron messages + [% CASE 'MessageQueue' %]patron notices + [% CASE 'OldIssue' %]previous checkouts + [% CASE 'OldReserve' %]filled holds + [% CASE 'Rating' %]ratings + [% CASE 'Reserve' %]current holds + [% CASE 'Review' %]reviews + [% CASE 'Statistic' %]statistics + [% CASE 'SearchHistory' %]historical searches + [% CASE 'Suggestion' %]purchase suggestions + [% CASE 'TagAll' %]tags + [% CASE 'Virtualshelfcontent' %]list items + [% CASE 'Virtualshelfshare' %]list shares + [% CASE 'Virtualshelve' %]lists + [% CASE %][% rs %] + [% END %] +[% END %] + + + +
+
+
+

Merge patron records

+ + [% IF action == 'show' %] +

Select patron to keep. Data from the other patrons will be transferred to this patron record and the remaining patron records will be deleted.

+
+ + + + + + + + + + + + + + + [% FOREACH p IN patrons %] + + + + + + + + + [% END %] + +
 CardNameDate of birthCategoryLibraryExpires on
[% p.cardnumber | html %][% p.firstname | html %] [% p.surname | html %][% p.dateofbirth | $KohaDates %][% Categories.GetName( p.categorycode ) %] ([% p.categorycode %])[% Branches.GetName( p.branchcode ) %][% p.dateexpiry | $KohaDates %]
+ + [% FOREACH p IN patrons %] + + [% END %] + +

+ + + +

+ [% ELSIF action == 'merge' %] +

Results

+ +

+ Patron records merged into [% results.keeper.firstname %] [% results.keeper.surname %] ([% results.keeper.cardnumber | html %]) +

+ + [% FOREACH pair IN results.merged.pairs %] + [% SET patron = pair.value.patron %] + +
[% patron.firstname %] [% patron.surname %] ([% patron.cardnumber %])
+ + [% USE Dumper %] + [% FOREACH r IN pair.value.updated.pairs %] + [% SET name = r.key %] + [% SET count = r.value %] + [% IF count %] +

+ [% count %] [% PROCESS display_names rs = name %] transferred. + [% IF name == 'Reserve' %] + It is advisable to check for and resolve duplicate holds due to merging. + [% END %] +

+ [% END %] + [% END %] + [% END %] + [% END %] +
+
+[% INCLUDE 'intranet-bottom.inc' %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/list.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/list.tt index e0092fb..037026a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/list.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/list.tt @@ -48,6 +48,10 @@
+ | +
+ +
@@ -68,7 +72,10 @@ [% FOREACH p IN list.patron_list_patrons %] - +
+ + + [% p.borrowernumber.cardnumber %] @@ -93,7 +100,8 @@
- + + @@ -159,6 +167,28 @@ return false; } }); + + $('.merge-patrons').on('click', function() { + var checkedItems = $("input:checked"); + if ($(checkedItems).length < 2) { + alert(_("You must select one or more patrons to remove")); + return false; + } + $(checkedItems).parents('tr').addClass("warn"); + if (confirm(_("Are you sure you want to remove the selected patrons?"))) { + var merge_patrons_url = '/cgi-bin/koha/members/merge-patrons.pl?' + + $('.selection:checked') + .map(function() { + return "id=" + $( '#borrowernumber_' + $(this).val() ).val() + }).get().join('&'); + + window.location.href = merge_patrons_url; + return false; + } else { + $(checkedItems).parents('tr').removeClass("warn"); + return false; + } + }); }); [% END %] diff --git a/members/merge-patrons.pl b/members/merge-patrons.pl new file mode 100755 index 0000000..8ad2cfb --- /dev/null +++ b/members/merge-patrons.pl @@ -0,0 +1,58 @@ +#!/usr/bin/perl + +# Copyright ByWater Solutions 2017 +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use CGI qw ( -utf8 ); + +use C4::Auth; +use C4::Output; +use C4::Context; +use Koha::Patrons; + +my $cgi = new CGI; + +my ( $template, $loggedinuser, $cookie, $flags ) = get_template_and_user( + { + template_name => "members/merge-patrons.tt", + query => $cgi, + type => "intranet", + authnotrequired => 0, + flagsrequired => { borrowers => 1 }, + debug => 1, + } +); + +my $action = $cgi->param('action') || 'show'; +my @ids = $cgi->multi_param('id'); + +if ( $action eq 'show' ) { + my $patrons = + Koha::Patrons->search( { borrowernumber => { -in => \@ids } } ); + $template->param( patrons => $patrons ); +} elsif ( $action eq 'merge' ) { + my $keeper = $cgi->param('keeper'); + my $results = Koha::Patrons->merge( { keeper => $keeper, patrons => \@ids } ); + $template->param( results => $results ); +} + +$template->param( action => $action ); + +output_html_with_http_headers $cgi, $cookie, $template->output; + +1; diff --git a/t/db_dependent/Patrons.t b/t/db_dependent/Patrons.t index f0d116a..53ee4b4 100755 --- a/t/db_dependent/Patrons.t +++ b/t/db_dependent/Patrons.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 17; +use Test::More tests => 18; use Test::Warn; use C4::Context; @@ -104,5 +104,51 @@ foreach my $b ( $patrons->as_list() ) { is( $b->categorycode(), $categorycode, "Iteration returns a patron object" ); } +subtest 'Test Koha::Patrons::merge' => sub { + plan tests => 98; + + my $schema = Koha::Database->new()->schema(); + + my $resultsets = $Koha::Patrons::RESULTSET_PATRON_ID_MAPPING; + + my $keeper = $builder->build( { source => 'Borrower' } )->{borrowernumber}; + my $loser_1 = $builder->build( { source => 'Borrower' } )->{borrowernumber}; + my $loser_2 = $builder->build( { source => 'Borrower' } )->{borrowernumber}; + + while (my ($r, $field) = each(%$resultsets)) { + $builder->build( { source => $r, value => { $field => $keeper } } ); + $builder->build( { source => $r, value => { $field => $loser_1 } } ); + $builder->build( { source => $r, value => { $field => $loser_2 } } ); + + my $keeper_rs = + $schema->resultset($r)->search( { $field => $keeper } ); + is( $keeper_rs->count(), 1, "Found 1 $r rows for keeper" ); + + my $loser_1_rs = + $schema->resultset($r)->search( { $field => $loser_1 } ); + is( $loser_1_rs->count(), 1, "Found 1 $r rows for loser_1" ); + + my $loser_2_rs = + $schema->resultset($r)->search( { $field => $loser_2 } ); + is( $loser_2_rs->count(), 1, "Found 1 $r rows for loser_2" ); + } + + my $results = Koha::Patrons->merge( + { + keeper => $keeper, + patrons => [ $loser_1, $loser_2 ], + } + ); + + while (my ($r, $field) = each(%$resultsets)) { + my $keeper_rs = + $schema->resultset($r)->search( {$field => $keeper } ); + is( $keeper_rs->count(), 3, "Found 2 $r rows for keeper" ); + } + + is( Koha::Patrons->find($loser_1), undef, 'Loser 1 has been deleted' ); + is( Koha::Patrons->find($loser_2), undef, 'Loser 2 has been deleted' ); +}; + $schema->storage->txn_rollback(); -- 2.1.4