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> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Tomas Cohen Arazi
Jonathan Druart
parent
73e7661c49
commit
253dbb758a
|
|
@ -42,6 +42,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
|
||||
|
|
@ -201,6 +228,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;
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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 %]
|
||||
|
|
|
|||
|
|
@ -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 => $_ );
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -19,7 +19,7 @@
|
|||
|
||||
use Modern::Perl;
|
||||
|
||||
use Test::More tests => 29;
|
||||
use Test::More tests => 30;
|
||||
use Test::Warn;
|
||||
use Time::Fake;
|
||||
use DateTime;
|
||||
|
|
@ -1334,9 +1334,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) = @_;
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue