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 <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com

C4/Members.pm

@@ -444,48 +444,6 @@ sub GetMember {
     return;
 }
 
-=head2 GetMemberRelatives
-
- @borrowernumbers = GetMemberRelatives($borrowernumber);
-
- C<GetMemberRelatives> 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 );

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

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 } );

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;

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' );