Bugzilla – Attachment 73668 Details for
Bug 9302
Add ability to merge patron records
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 9302: (QA follow-up) Consistency follow-up
Bug-9302-QA-follow-up-Consistency-follow-up.patch (text/plain), 14.22 KB, created by
Tomás Cohen Arazi (tcohen)
on 2018-04-04 22:43:56 UTC
(
hide
)
Description:
Bug 9302: (QA follow-up) Consistency follow-up
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2018-04-04 22:43:56 UTC
Size:
14.22 KB
patch
obsolete
>From 05394e14ce4641fb7b48cb912766efb50be48f16 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Wed, 4 Apr 2018 16:56:34 -0300 >Subject: [PATCH] Bug 9302: (QA follow-up) Consistency follow-up > >This patch moves the Koha::Patrons->merge method into >Koha::Patron->merge_with in the line of the discussed implementation for >bug 15336. I agree with that implementation so I provide this follow-up. > >Tests are adjusted, the controller script is adapted too. The behaviour >remains. > >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > Koha/Patron.pm | 75 +++++++++++++++++ > Koha/Patrons.pm | 80 +------------------ > .../prog/en/modules/members/merge-patrons.tt | 2 +- > members/merge-patrons.pl | 27 ++++--- > t/db_dependent/Koha/Patrons.t | 48 ++++++++++- > t/db_dependent/Patrons.t | 48 +---------- > 6 files changed, 140 insertions(+), 140 deletions(-) > >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index 2322131713..e81eaeee9b 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -41,6 +41,33 @@ use Koha::Account; > > use base qw(Koha::Object); > >+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 >@@ -200,6 +227,54 @@ sub siblings { > ); > } > >+=head3 merge_with >+ >+ my $patron = Koha::Patrons->find($id); >+ $patron->merge_with( \@patron_ids ); >+ >+ This subroutine merges a list of patrons into the 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_with { >+ my ( $self, $patron_ids ) = @_; >+ >+ my @patron_ids = @{ $patron_ids }; >+ >+ # Ensure the keeper isn't in the list of patrons to merge >+ @patron_ids = grep { $_ ne $self->id } @patron_ids; >+ >+ my $schema = Koha::Database->new()->schema(); >+ >+ my $results; >+ >+ $self->_result->result_source->schema->txn_do( sub { >+ foreach my $patron_id (@patron_ids) { >+ my $patron = Koha::Patrons->find( $patron_id ); >+ >+ next unless $patron; >+ >+ # Unbless for safety, the patron will end up being deleted >+ $results->{merged}->{$patron_id}->{patron} = $patron->unblessed; >+ >+ while (my ($r, $field) = each(%$RESULTSET_PATRON_ID_MAPPING)) { >+ my $rs = $schema->resultset($r)->search({ $field => $patron_id }); >+ $results->{merged}->{ $patron_id }->{updated}->{$r} = $rs->count(); >+ $rs->update({ $field => $self->id }); >+ } >+ >+ $patron->move_to_deleted(); >+ $patron->delete(); >+ } >+ }); >+ >+ return $results; >+} >+ >+ >+ > =head3 wants_check_for_previous_checkout > > $wants_check = $patron->wants_check_for_previous_checkout; >diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm >index 0ef5f4f0b9..0d2d9ec6b4 100644 >--- a/Koha/Patrons.pm >+++ b/Koha/Patrons.pm >@@ -31,33 +31,6 @@ 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 >@@ -234,58 +207,7 @@ 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; >- >- $self->_resultset->result_source->schema->txn_do( sub { >- 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->move_to_deleted(); >- $patron->delete(); >- } >- }); >- >- $results->{keeper} = $patron_to_keep; >- >- return $results; >-} >- >-=head3 type >+=head3 _type > > =cut > >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 >index 6ae668f517..354ffbf555 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/merge-patrons.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/merge-patrons.tt >@@ -108,7 +108,7 @@ $(document).ready(function() { > <div class="dialog alert">Merge failed! The following error was reported: [% error %].</div> > [% ELSE %] > <p> >- Patron records merged into <a href="moremember.pl?borrowernumber=[% results.keeper.id %]">[% results.keeper.firstname %] [% results.keeper.surname %] ([% results.keeper.cardnumber | html %])</a> >+ Patron records merged into <a href="moremember.pl?borrowernumber=[% keeper.id %]">[% keeper.firstname %] [% keeper.surname %] ([% keeper.cardnumber | html %])</a> > </p> > > [% FOREACH pair IN results.merged.pairs %] >diff --git a/members/merge-patrons.pl b/members/merge-patrons.pl >index a35fe83f29..2008fd4917 100755 >--- a/members/merge-patrons.pl >+++ b/members/merge-patrons.pl >@@ -19,6 +19,7 @@ > use Modern::Perl; > > use CGI qw ( -utf8 ); >+use Try::Tiny; > > use C4::Auth; > use C4::Output; >@@ -33,26 +34,30 @@ my ( $template, $loggedinuser, $cookie, $flags ) = get_template_and_user( > query => $cgi, > type => "intranet", > authnotrequired => 0, >- flagsrequired => { borrowers => 1 }, >- debug => 1, >+ flagsrequired => { borrowers => 1 } > } > ); > > my $action = $cgi->param('action') || 'show'; >-my @ids = $cgi->multi_param('id'); >+my @ids = $cgi->multi_param('id'); > > if ( $action eq 'show' ) { >- my $patrons = >- Koha::Patrons->search( { borrowernumber => { -in => \@ids } } ); >+ my $patrons = Koha::Patrons->search({ borrowernumber => { -in => \@ids } }); > $template->param( patrons => $patrons ); > } elsif ( $action eq 'merge' ) { >- my $keeper = $cgi->param('keeper'); >+ my $keeper_id = $cgi->param('keeper'); > my $results; >- eval { $results = Koha::Patrons->merge( { keeper => $keeper, patrons => \@ids } ); }; >- if ($@) { >- $template->param( results => $results ); >- } else { >- $template->param( error => $@ ); >+ >+ try { >+ my $keeper = Koha::Patrons->find( $keeper_id ); >+ $results = $keeper->merge_with( \@ids ); >+ $template->param( >+ keeper => $keeper, >+ results => $results >+ ); >+ } >+ catch { >+ $template->param( error => $_ ); > } > } > >diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t >index f0b1992054..8a020a6ad6 100644 >--- a/t/db_dependent/Koha/Patrons.t >+++ b/t/db_dependent/Koha/Patrons.t >@@ -19,7 +19,7 @@ > > use Modern::Perl; > >-use Test::More tests => 27; >+use Test::More tests => 28; > use Test::Warn; > use Time::Fake; > use DateTime; >@@ -1217,9 +1217,53 @@ subtest 'Log cardnumber change' => sub { > is( scalar @logs, 2, 'With BorrowerLogs, Change in cardnumber should be logged, as well as general alert of patron mod.' ); > }; > >- > $schema->storage->txn_rollback; > >+subtest 'Test Koha::Patrons::merge' => sub { >+ plan tests => 98; >+ >+ my $schema = Koha::Database->new()->schema(); >+ >+ my $resultsets = $Koha::Patron::RESULTSET_PATRON_ID_MAPPING; >+ >+ $schema->storage->txn_begin; >+ >+ my $keeper = $builder->build_object({ class => 'Koha::Patrons' }); >+ 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->id } }); >+ $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->id } ); >+ 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 = $keeper->merge_with([ $loser_1, $loser_2 ]); >+ >+ while (my ($r, $field) = each(%$resultsets)) { >+ my $keeper_rs = >+ $schema->resultset($r)->search( {$field => $keeper->id } ); >+ 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; >+}; >+ > # TODO Move to t::lib::Mocks and reuse it! > sub set_logged_in_user { > my ($patron) = @_; >diff --git a/t/db_dependent/Patrons.t b/t/db_dependent/Patrons.t >index 53ee4b4e01..f0d116a7bc 100755 >--- a/t/db_dependent/Patrons.t >+++ b/t/db_dependent/Patrons.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 18; >+use Test::More tests => 17; > use Test::Warn; > > use C4::Context; >@@ -104,51 +104,5 @@ 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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=73668">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
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