Attachment 68606 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), 20.83 KB, created by Kyle M Hall (khall) on 2017-10-26 13:27:18 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2017-10-26 13:27:18 UTC

Size: 20.83 KB

patch

obsolete

>From 593a296c22e4ddb6f4bb0bc089a5c14345ecfd03 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
>---
> 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           |  38 +++++-
> members/merge-patrons.pl                           |  58 +++++++++
> t/db_dependent/Patrons.t                           |  48 +++++++-
> 6 files changed, 376 insertions(+), 7 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 263cec7..3c53e38 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
>@@ -183,6 +210,54 @@ sub anonymise_issue_history {
>     $old_issues_to_anonymise->update( { 'old_issues.borrowernumber' => $anonymous_patron } );
> }
> 
>+=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 @@
> <script type="text/javascript">
> //<![CDATA[
> $(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();
> 
>@@ -359,9 +376,10 @@ function filterByFirstLetterSurname(letter) {
>             <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>
>@@ -385,8 +403,17 @@ function filterByFirstLetterSurname(letter) {
> 
>                           <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">
>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 0000000..3c79dee
>--- /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 c06997a..65fa7a1 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
>@@ -45,9 +45,9 @@ $(document).ready(function() {
>         });
>     });
> 
>-    $("#remove_patrons").submit(function(){
>+    $('.list-remove').on('click', function(){
>         var checkedItems = $("input:checked");
>-        if ($(checkedItems).size() == 0) {
>+        if ($(checkedItems).length == 0) {
>             alert(_("You must select one or more patrons to remove"));
>             return false;
>         }
>@@ -59,6 +59,28 @@ $(document).ready(function() {
>             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;
>+        }
>+    });
> });
> 
> //]]>
>@@ -107,6 +129,10 @@ $(document).ready(function() {
>                 <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">
>@@ -127,7 +153,10 @@ $(document).ready(function() {
>                 <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 %]
>@@ -152,7 +181,8 @@ $(document).ready(function() {
>             </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>
>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 <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 a1c7ad9..ec492c9 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;
>@@ -103,5 +103,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.10.2

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