Bug 9302: Add ability to merge patron records
authorKyle M Hall <kyle@bywatersolutions.com>
Tue, 29 Aug 2017 17:55:59 +0000 (13:55 -0400)
committerJonathan Druart <jonathan.druart@bugs.koha-community.org>
Fri, 20 Apr 2018 16:34:41 +0000 (13:34 -0300)
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 <oleonard@myacpl.org>

Signed-off-by: Ed Veal <eveal@mckinneytexas.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Koha/Patrons.pm
koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt
koha-tmpl/intranet-tmpl/prog/en/modules/members/merge-patrons.tt [new file with mode: 0644]
koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/list.tt
members/merge-patrons.pl [new file with mode: 0755]
t/db_dependent/Patrons.t

index 1e37159..6d51bf0 100644 (file)
@@ -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
@@ -207,6 +234,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
index 866dd27..a52160a 100644 (file)
             <div id="searchheader">
               <h3>Patrons found for: <span id="searchpattern">[% IF searchmember %] for '[% searchmember | html %]'[% END %]</span></h3>
             </div>
-            [% IF CAN_user_tools_manage_patron_lists %]
+            [% IF CAN_user_tools_manage_patron_lists || CAN_user_borrowers %]
               <div id="searchheader">
                   <div>
+                    [% IF CAN_user_tools_manage_patron_lists %]
                       <a href="#" id="select_all"><i class="fa fa-check"></i> Select all</a>
                       |
                       <a href="#" id="clear_all"><i class="fa fa-remove"></i> Clear all</a>
 
                           <input id="add_to_patron_list_submit" type="submit" class="submit" value="Save">
                       </span>
+                    [% END %]
+
+                    [% IF CAN_user_tools_manage_patron_lists && CAN_user_borrowers %]
+                        |
+                    [% END %]
+
+                    [% IF CAN_user_borrowers %]
+                          <button id="merge-patrons" type="submit">Merge selected patrons</button>
+                    [% END %]
                   </div>
-              </div>
+                </div>
             [% END %]
 
             <table id="memberresultst">
     [% Asset.js("js/members-menu.js") %]
     <script type="text/javascript">
         $(document).ready(function() {
+            $('#merge-patrons').prop('disabled', true);
+            $('#memberresultst').on('change', 'input.selection', function() {
+                if ( $('.selection:checked').length > 1 ) {
+                    $('#merge-patrons').prop('disabled', false);
+                } else {
+                    $('#merge-patrons').prop('disabled', true);
+                }
+            });
+            $('#merge-patrons').on('click', function() {
+                var merge_patrons_url = 'merge-patrons.pl?' + $('.selection:checked')
+                    .map(function() {
+                       return "id=" + $(this).val()
+                    }).get().join('&');
+
+                window.location.href = merge_patrons_url;
+            });
+
             $('#add_to_patron_list_submit').prop('disabled', true);
             $('#new_patron_list').hide();
 
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/merge-patrons.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/merge-patrons.tt
new file mode 100644 (file)
index 0000000..78a61d9
--- /dev/null
@@ -0,0 +1,133 @@
+[% USE Branches %]
+[% USE Categories %]
+[% USE KohaDates %]
+[% INCLUDE 'doc-head-open.inc' %]
+<title>Koha &rsaquo; Patrons &rsaquo; Merge patron records</title>
+[% INCLUDE 'doc-head-close.inc' %]
+
+<script type="text/javascript">
+//<![CDATA[
+$(document).ready(function() {
+    $('#merge-patrons').prop('disabled', true);
+    $('#patron-merge-table').on('change', 'input', function() {
+        if ( $('.keeper:checked').length > 0 ) {
+            $('#merge-patrons').prop('disabled', false);
+        } else {
+            $('#merge-patrons').prop('disabled', true);
+        }
+    });
+});
+//]]>
+</script>
+
+</head>
+<body id="pat_merge" class="pat">
+[% 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 %]
+
+<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> &rsaquo; <a href="/cgi-bin/koha/members/members-home.pl">Patrons</a> &rsaquo; Merge patron records</div>
+
+<div id="doc2" class="yui-t7">
+   <div id="bd">
+        <div id="yui-main">
+            <h3>Merge patron records</h3>
+
+            [% IF action == 'show' %]
+                <p>Select patron to keep. Data from the other patrons will be transferred to this patron record and the remaining patron records will be deleted.</p>
+                <form type="post" action="merge-patrons.pl">
+                    <table id="patron-merge-table" class="datatable">
+                        <thead>
+                            <tr>
+                                <th>&nbsp;</th>
+                                <th>Card</th>
+                                <th>Name</th>
+                                <th>Date of birth</th>
+                                <th>Category</th>
+                                <th>Library</th>
+                                <th>Expires on</th>
+                            </tr>
+                        </thead>
+
+                        <tbody>
+                            [% FOREACH p IN patrons %]
+                                <tr>
+                                    <td><input class='keeper' type='radio' name='keeper' value='[% p.id %]' /></td>
+                                    <td>[% p.cardnumber | html %]</td>
+                                    <td>[% p.firstname | html %] [% p.surname | html %]</td>
+                                    <td>[% p.dateofbirth | $KohaDates %]</td>
+                                    <td>[% Categories.GetName( p.categorycode ) %] ([% p.categorycode %])</td>
+                                    <td>[% Branches.GetName( p.branchcode ) %]</td>
+                                    <td>[% p.dateexpiry | $KohaDates %]</td>
+                            [% END %]
+                        </tbody>
+                    </table>
+
+                    [% FOREACH p IN patrons %]
+                        <input type="hidden" name="id" value="[% p.id %]" />
+                    [% END %]
+
+                    <p/>
+
+                    <input type="hidden" name="action" value="merge" />
+                    <input id="merge-patrons" type="submit" value="Merge patrons" />
+                </form>
+            [% ELSIF action == 'merge' %]
+                <h4>Results</h4>
+
+                <p>
+                    Patron records merged into <a href="moremember.pl?borrowernumber=[% results.keeper.id %]">[% results.keeper.firstname %] [% results.keeper.surname %] ([% results.keeper.cardnumber | html %])</a>
+                </p>
+
+                [% FOREACH pair IN results.merged.pairs %]
+                    [% SET patron = pair.value.patron %]
+
+                    <h5>[% patron.firstname %] [% patron.surname %] ([% patron.cardnumber %])</h5>
+
+                    [% USE Dumper %]
+                    [% FOREACH r IN pair.value.updated.pairs %]
+                        [% SET name = r.key %]
+                        [% SET count = r.value %]
+                        [% IF count %]
+                            <p>
+                                [% count %] [% PROCESS display_names rs = name %] transferred.
+                                [% IF name == 'Reserve' %]
+                                    <strong>It is advisable to check for and resolve duplicate holds due to merging.</strong>
+                                [% END %]
+                            </p>
+                        [% END %]
+                    [% END %]
+                [% END %]
+            [% END %]
+        </div>
+    </div>
+[% INCLUDE 'intranet-bottom.inc' %]
index cde655d..fe539ea 100644 (file)
                 <div class="btn-group">
                     <button class="btn btn-default btn-xs list-remove" type="submit"><i class="fa fa-trash"></i> Remove selected</button>
                 </div>
+                |
+                <div class="btn-group">
+                    <button class="btn btn-default btn-xs merge-patrons"><i class="fa fa-compress"></i> Merge selected patrons</button>
+                </div>
             </div>
 
             <table id="patron-list-table">
                 <tbody>
                     [% FOREACH p IN list.patron_list_patrons %]
                         <tr>
-                            <td><input type="checkbox" name="patrons_to_remove" value="[% p.patron_list_patron_id %]" /></td>
+                            <td>
+                                <input type="checkbox" name="patrons_to_remove" class="selection" value="[% p.patron_list_patron_id %]" />
+                                <input type="hidden" id="borrowernumber_[% p.patron_list_patron_id %]" value="[% p.borrowernumber.id %]" />
+                            </td>
                             <td>
                                 <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% p.borrowernumber.borrowernumber %]">
                                     [% p.borrowernumber.cardnumber %]
             </table>
 
             <input type="hidden" name="patron_list_id" value="[% list.patron_list_id %]" />
-            <button type="submit" class="btn btn-default btn-sm"><i class="fa fa-trash" aria-hidden="true"></i> Remove selected patrons</button>
+            <button type="submit" class="btn btn-default btn-sm list-remove"><i class="fa fa-trash" aria-hidden="true"></i> Remove selected patrons</button>
+            <button class="btn btn-default btn-sm merge-patrons" type="submit"><i class="fa fa-compress"></i> Merge selected patrons</button>
         </form>
 
             </div>
                 $("#add_patrons_by_barcode, #patron_search_line").show();
                 $("#add_patrons_by_search, #patron_barcodes_line, #patron_barcodes_submit").hide();
             });
+
+            $('.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;
+                }
+            });
         });
     </script>
 [% END %]
diff --git a/members/merge-patrons.pl b/members/merge-patrons.pl
new file mode 100755 (executable)
index 0000000..8ad2cfb
--- /dev/null
@@ -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 <http://www.gnu.org/licenses>.
+
+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;
index f0d116a..53ee4b4 100755 (executable)
@@ -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();