Attachment 73660 Details for Bug 9302 – Bug 9302: Add ability to merge patron records

[patch] Bug 9302: Add ability to merge patron records

Bug-9302-Add-ability-to-merge-patron-records.patch (text/plain), 21.19 KB, created by Tomás Cohen Arazi (tcohen) on 2018-04-04 19:58:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2018-04-04 19:58:33 UTC

Size: 21.19 KB

patch

obsolete

>From e9a7e48e8b51e3de6d59dd6a15f8e4b44e3d7160 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>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 <oleonard@myacpl.org>
>
>Signed-off-by: Ed Veal <eveal@mckinneytexas.org>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> 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 1e37159d5a..6d51bf0c24 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
>@@ -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
>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 b40bb0337f..cf648ab22e 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt
>@@ -52,9 +52,10 @@
>             <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>
>@@ -78,8 +79,17 @@
> 
>                           <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">
>@@ -227,6 +237,23 @@
>     <script type="text/javascript" src="[% interface %]/[% theme %]/js/members-menu_[% KOHA_VERSION %].js"></script>
>     <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
>index 0000000000..78a61d97c3
>--- /dev/null
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/merge-patrons.tt
>@@ -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' %]
>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 935a409d7b..e844d552a1 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
>@@ -106,6 +106,10 @@
>                 <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">
>@@ -126,7 +130,10 @@
>                 <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 %]
>@@ -151,7 +158,8 @@
>             </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>
>@@ -227,6 +235,28 @@
>                 $("#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
>index 0000000000..8ad2cfbd9d
>--- /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 <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;
>diff --git a/t/db_dependent/Patrons.t b/t/db_dependent/Patrons.t
>index f0d116a7bc..53ee4b4e01 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.17.0

Actions: View | Diff | Splinter Review

Attachments on bug 9302: 66615 | 66617 | 68606 | 70474 | 70477 | 70478 | 70479 | 70482 | 70483 | 72082 | 72083 | 73645 | 73646 | 73647 | 73648 | 73649 | 73660 | 73661 | 73662 | 73663 | 73664 | 73668 | 73679 | 74439 | 74440 | 74441 | 74442 | 74443 | 74444 | 74445 | 74446 | 74447 | 74448 | 74449 | 74450 | 74451 | 74585 | 74653