From aa73c96aed863637531355a548467fd67cd63b83 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 25 Jan 2016 15:41:20 +0000 Subject: [PATCH] Bug 15656: Move guarantor/guarantees code - GetMemberRelatives Note: QA question: Does the Koha::Patron->siblings method should return undef if there is no guarantor? It would avoid the weird != undef, = $borrowernumber conditions. Signed-off-by: Josef Moravec Signed-off-by: Kyle M Hall Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com --- C4/Members.pm | 42 ----------------------------------- Koha/Patron.pm | 25 +++++++++++++++++++++ circ/circulation.pl | 10 ++++++++- members/moremember.pl | 22 +++++++++--------- t/db_dependent/Koha/Patrons.t | 22 +++++++++++++++++- 5 files changed, 67 insertions(+), 54 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index f7147b5e13..ac644778fc 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -444,48 +444,6 @@ sub GetMember { return; } -=head2 GetMemberRelatives - - @borrowernumbers = GetMemberRelatives($borrowernumber); - - C returns a borrowersnumber's list of guarantor/guarantees of the member given in parameter - -=cut - -sub GetMemberRelatives { - my $borrowernumber = shift; - my $dbh = C4::Context->dbh; - my @glist; - - # Getting guarantor - my $query = "SELECT guarantorid FROM borrowers WHERE borrowernumber=?"; - my $sth = $dbh->prepare($query); - $sth->execute($borrowernumber); - my $data = $sth->fetchrow_arrayref(); - push @glist, $data->[0] if $data->[0]; - my $guarantor = $data->[0] ? $data->[0] : undef; - - # Getting guarantees - $query = "SELECT borrowernumber FROM borrowers WHERE guarantorid=?"; - $sth = $dbh->prepare($query); - $sth->execute($borrowernumber); - while ($data = $sth->fetchrow_arrayref()) { - push @glist, $data->[0]; - } - - # Getting sibling guarantees - if ($guarantor) { - $query = "SELECT borrowernumber FROM borrowers WHERE guarantorid=?"; - $sth = $dbh->prepare($query); - $sth->execute($guarantor); - while ($data = $sth->fetchrow_arrayref()) { - push @glist, $data->[0] if ($data->[0] != $borrowernumber); - } - } - - return @glist; -} - =head2 IsMemberBlocked my ($block_status, $count) = IsMemberBlocked( $borrowernumber ); diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 0643ac3780..4464731831 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -66,6 +66,31 @@ sub guarantees { return Koha::Patrons->search({ guarantorid => $self->borrowernumber }); } +=head3 siblings + +Returns the siblings of this patron. + +=cut + +sub siblings { + my ( $self ) = @_; + + my $guarantor = $self->guarantor; + my $guarantorid = $guarantor ? $guarantor->borrowernumber : undef; + + return Koha::Patrons->search( + { + guarantorid => { + '!=' => undef, + '=' => $guarantorid, + }, + borrowernumber => { + '!=' => $self->borrowernumber, + } + } + ); +} + =head3 type =cut diff --git a/circ/circulation.pl b/circ/circulation.pl index e78937019c..924db654b2 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -43,6 +43,7 @@ use Koha::Holds; use C4::Context; use CGI::Session; use C4::Members::Attributes qw(GetBorrowerAttributes); +use Koha::Patron; use Koha::Patron::Debarments qw(GetDebarments IsDebarred); use Koha::DateUtils; use Koha::Database; @@ -582,7 +583,14 @@ my $view = $batch ?'batch_checkout_view' : 'circview'; -my @relatives = GetMemberRelatives( $borrower->{'borrowernumber'} ); +my $patron = Koha::Patrons->find( $borrower->{borrowernumber} ); +my @relatives; +if ( my $guarantor = $patron->guarantor ) { + push @relatives, $guarantor->borrowernumber; + push @relatives, $_->borrowernumber for $patron->siblings; +} else { + push @relatives, $_->borrowernumber for $patron->guarantees; +} my $relatives_issues_count = Koha::Database->new()->schema()->resultset('Issue') ->count( { borrowernumber => \@relatives } ); diff --git a/members/moremember.pl b/members/moremember.pl index 9238438f0c..4a5c06dfd9 100755 --- a/members/moremember.pl +++ b/members/moremember.pl @@ -164,13 +164,20 @@ if ( $category_type eq 'C') { } my $patron = Koha::Patrons->find($data->{borrowernumber}); -my @guarantees = $patron->guarantees; -if ( @guarantees ) { +my @relatives; +if ( my $guarantor = $patron->guarantor ) { + $template->param( guarantor => $guarantor ); + push @relatives, $guarantor->borrowernumber; + push @relatives, $_->borrowernumber for $patron->siblings; +} else { + my @guarantees = $patron->guarantees; $template->param( guarantees => \@guarantees ); + push @relatives, $_->borrowernumber for @guarantees; } -elsif ( $patron->guarantorid ) { - $template->param( guarantor => $patron->guarantor ); -} + +my $relatives_issues_count = + Koha::Database->new()->schema()->resultset('Issue') + ->count( { borrowernumber => \@relatives } ); $template->param( adultborrower => 1 ) if ( $category_type eq 'A' || $category_type eq 'I' ); @@ -219,11 +226,6 @@ if ( C4::Context->preference('OPACPrivacy') ) { $template->param( "privacy".$data->{'privacy'} => 1); } -my @relatives = GetMemberRelatives($borrowernumber); -my $relatives_issues_count = - Koha::Database->new()->schema()->resultset('Issue') - ->count( { borrowernumber => \@relatives } ); - my $today = DateTime->now( time_zone => C4::Context->tz); $today->truncate(to => 'day'); my $overdues_exist = 0; diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 132ec24b77..46eed31cc9 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 => 4; +use Test::More tests => 5; use Koha::Patron; use Koha::Patrons; @@ -77,6 +77,26 @@ subtest 'guarantees' => sub { $_->delete for @guarantees; }; +subtest 'siblings' => sub { + plan tests => 7; + my $siblings = $new_patron_1->siblings; + is( ref($siblings), 'Koha::Patrons', 'Koha::Patron->siblings should not crashed if the patron has not guarantor' ); + my $guarantee_1 = $builder->build( { source => 'Borrower', value => { guarantorid => $new_patron_1->borrowernumber } } ); + my $retrieved_guarantee_1 = Koha::Patrons->find($guarantee_1); + $siblings = $retrieved_guarantee_1->siblings; + is( ref($siblings), 'Koha::Patrons', 'Koha::Patron->siblings should return a Koha::Patrons result set in a scalar context' ); + my @siblings = $retrieved_guarantee_1->siblings; + is( ref( \@siblings ), 'ARRAY', 'Koha::Patron->siblings should return an array in a list context' ); + is( $siblings->count, 0, 'guarantee_1 should not have siblings yet' ); + my $guarantee_2 = $builder->build( { source => 'Borrower', value => { guarantorid => $new_patron_1->borrowernumber } } ); + my $guarantee_3 = $builder->build( { source => 'Borrower', value => { guarantorid => $new_patron_1->borrowernumber } } ); + $siblings = $retrieved_guarantee_1->siblings; + is( $siblings->count, 2, 'guarantee_1 should have 2 siblings' ); + is( $guarantee_2->{borrowernumber}, $siblings->next->borrowernumber, 'guarantee_2 should exist in the guarantees' ); + is( $guarantee_3->{borrowernumber}, $siblings->next->borrowernumber, 'guarantee_3 should exist in the guarantees' ); + $_->delete for $retrieved_guarantee_1->siblings; + $retrieved_guarantee_1->delete; +}; $retrieved_patron_1->delete; is( Koha::Patrons->search->count, $nb_of_patrons + 1, 'Delete should have deleted the patron' ); -- 2.39.5