Bug 15656: Move guarantor/guarantees code - GetGuarantees
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Mon, 25 Jan 2016 13:25:42 +0000 (13:25 +0000)
committerBrendan Gallagher <brendan@bywatersolutions.com>
Sat, 12 Mar 2016 23:40:10 +0000 (23:40 +0000)
2 subroutines of C4::Members deal with guarantor/guarantees:
GetGuarantees and GetMemberRelatives.
Since we already have a Koha::Patron->guarantor method, it makes sense
to move these 2 subroutines to this module.

This first patch deals with GetGuarantees.

Test plan for the entire patch set:
1/ Create 5 patrons A (adult), B (child), C (child), D (child), E
(child), F (adult)
2/ Add relation between them: A is father of B, C and D.
E does not have a guarantor
F does not have guarantees
3/ Check some items out for all of these patrons
4/ On the "Check out" and "Details" tabs, you should not see any
differences with these patch applied : The "Relatives' checkouts" tabs
should list all of the guarantor/guarantee/siblings checkouts

Note:
$template->param('C' => 1);
I have not found any reference of this 'C' in the template.
It seems it's an old c/p from members/memberentrygen.tt

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
Koha/Patron.pm
koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
members/moremember.pl
t/db_dependent/Koha/Patrons.t [new file with mode: 0644]

index 3028b2e..f7147b5 100644 (file)
@@ -61,8 +61,6 @@ BEGIN {
         &GetMemberRelatives
         &GetMember
 
-        &GetGuarantees
-
         &GetMemberIssuesAndFines
         &GetPendingIssues
         &GetAllIssues
@@ -931,37 +929,6 @@ sub fixup_cardnumber {
     return $cardnumber;     # just here as a fallback/reminder 
 }
 
-=head2 GetGuarantees
-
-  ($num_children, $children_arrayref) = &GetGuarantees($parent_borrno);
-  $child0_cardno = $children_arrayref->[0]{"cardnumber"};
-  $child0_borrno = $children_arrayref->[0]{"borrowernumber"};
-
-C<&GetGuarantees> takes a borrower number (e.g., that of a patron
-with children) and looks up the borrowers who are guaranteed by that
-borrower (i.e., the patron's children).
-
-C<&GetGuarantees> returns two values: an integer giving the number of
-borrowers guaranteed by C<$parent_borrno>, and a reference to an array
-of references to hash, which gives the actual results.
-
-=cut
-
-#'
-sub GetGuarantees {
-    my ($borrowernumber) = @_;
-    my $dbh              = C4::Context->dbh;
-    my $sth              =
-      $dbh->prepare(
-"select cardnumber,borrowernumber, firstname, surname from borrowers where guarantorid=?"
-      );
-    $sth->execute($borrowernumber);
-
-    my @dat;
-    my $data = $sth->fetchall_arrayref({}); 
-    return ( scalar(@$data), $data );
-}
-
 =head2 GetPendingIssues
 
   my $issues = &GetPendingIssues(@borrowernumber);
index f2972d6..0643ac3 100644 (file)
@@ -54,6 +54,18 @@ sub image {
     return Koha::Patron::Images->find( $self->borrowernumber )
 }
 
+=head3 guarantees
+
+Returns the guarantees (list of Koha::Patron) of this patron
+
+=cut
+
+sub guarantees {
+    my ( $self ) = @_;
+
+    return Koha::Patrons->search({ guarantorid => $self->borrowernumber });
+}
+
 =head3 type
 
 =cut
index a9ce12a..591dbfa 100644 (file)
@@ -231,14 +231,20 @@ function validate1(date) {
     [% IF ( sex ) %]<li><span class="label">Gender:</span>
     [% IF ( sex == 'F' ) %]Female[% ELSIF ( sex == 'M' ) %]Male[% ELSE %][% sex %][% END %]
     </li>[% END %][% END %]
-    [% IF ( isguarantee ) %]
-        [% IF ( guaranteeloop ) %]
-            <li><span class="label">Guarantees:</span><ul>[% FOREACH guaranteeloo IN guaranteeloop %]<li><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% guaranteeloo.borrowernumber %]">[% guaranteeloo.name %]  </a></li>[% END %]</ul></li>
-        [% END %]
-    [% ELSE %]
-        [% IF ( guarantorborrowernumber ) %]
-            <li><span class="label">Guarantor:</span><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% guarantorborrowernumber %]">[% guarantorsurname %][%IF ( guarantorfirstname ) %], [% guarantorfirstname %] [% END %]</a></li>
-        [% END %]
+    [% IF guarantees %]
+        <li>
+            <span class="label">Guarantees:</span>
+            <ul>
+                [% FOREACH guarantee IN guarantees %]
+                    <li><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% guarantee.borrowernumber %]">[% guarantee.firstname %] [% guarantee.surname %]</a></li>
+                [% END %]
+            </ul>
+        </li>
+    [% ELSIF guarantor %]
+        <li>
+            <span class="label">Guarantor:</span>
+            <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% guarantor.borrowernumber %]">[% guarantor.firstname %] [% guarantor.surname %]</a>
+        </li>
     [% END %]
 </ol>
 </div>
index ba4c0a5..9238438 100755 (executable)
@@ -163,38 +163,13 @@ if ( $category_type eq 'C') {
    $template->param( 'catcode' =>    $catcodes->[0])  if $cnt == 1;
 }
 
-my ( $count, $guarantees ) = GetGuarantees( $data->{'borrowernumber'} );
-if ( $count ) {
-    $template->param( isguarantee => 1 );
-
-    # FIXME
-    # It looks like the $i is only being returned to handle walking through
-    # the array, which is probably better done as a foreach loop.
-    #
-    my @guaranteedata;
-    for ( my $i = 0 ; $i < $count ; $i++ ) {
-        push(@guaranteedata,
-            {
-                borrowernumber => $guarantees->[$i]->{'borrowernumber'},
-                cardnumber     => $guarantees->[$i]->{'cardnumber'},
-                name           => $guarantees->[$i]->{'firstname'} . " "
-                                . $guarantees->[$i]->{'surname'}
-            }
-        );
-    }
-    $template->param( guaranteeloop => \@guaranteedata );
+my $patron = Koha::Patrons->find($data->{borrowernumber});
+my @guarantees = $patron->guarantees;
+if ( @guarantees ) {
+    $template->param( guarantees => \@guarantees );
 }
-else {
-    if ($data->{'guarantorid'}){
-           my ($guarantor) = GetMember( 'borrowernumber' =>$data->{'guarantorid'});
-               $template->param(guarantor => 1);
-               foreach (qw(borrowernumber cardnumber firstname surname)) {        
-                         $template->param("guarantor$_" => $guarantor->{$_});
-        }
-    }
-       if ($category_type eq 'C'){
-               $template->param('C' => 1);
-       }
+elsif ( $patron->guarantorid ) {
+    $template->param( guarantor => $patron->guarantor );
 }
 
 $template->param( adultborrower => 1 ) if ( $category_type eq 'A' || $category_type eq 'I' );
diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t
new file mode 100644 (file)
index 0000000..132ec24
--- /dev/null
@@ -0,0 +1,86 @@
+#!/usr/bin/perl
+
+# Copyright 2015 Koha Development team
+#
+# 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 Test::More tests => 4;
+
+use Koha::Patron;
+use Koha::Patrons;
+use Koha::Database;
+
+use t::lib::TestBuilder;
+
+my $schema = Koha::Database->new->schema;
+$schema->storage->txn_begin;
+
+my $builder       = t::lib::TestBuilder->new;
+my $library = $builder->build({source => 'Branch' });
+my $category = $builder->build({source => 'Category' });
+my $nb_of_patrons = Koha::Patrons->search->count;
+my $new_patron_1  = Koha::Patron->new(
+    {   cardnumber => 'test_cn_1',
+        branchcode => $library->{branchcode},
+        categorycode => $category->{categorycode},
+        surname => 'surname for patron1',
+        firstname => 'firstname for patron1',
+    }
+)->store;
+my $new_patron_2  = Koha::Patron->new(
+    {   cardnumber => 'test_cn_2',
+        branchcode => $library->{branchcode},
+        categorycode => $category->{categorycode},
+        surname => 'surname for patron2',
+        firstname => 'firstname for patron2',
+    }
+)->store;
+
+is( Koha::Patrons->search->count, $nb_of_patrons + 2, 'The 2 patrons should have been added' );
+
+my $retrieved_patron_1 = Koha::Patrons->find( $new_patron_1->borrowernumber );
+is( $retrieved_patron_1->cardnumber, $new_patron_1->cardnumber, 'Find a patron by borrowernumber should return the correct patron' );
+
+subtest 'guarantees' => sub {
+    plan tests => 8;
+    my $guarantees = $new_patron_1->guarantees;
+    is( ref($guarantees), 'Koha::Patrons', 'Koha::Patron->guarantees should return a Koha::Patrons result set in a scalar context' );
+    is( $guarantees->count, 0, 'new_patron_1 should have 0 guarantee' );
+    my @guarantees = $new_patron_1->guarantees;
+    is( ref(\@guarantees), 'ARRAY', 'Koha::Patron->guarantees should return an array in a list context' );
+    is( scalar(@guarantees), 0, 'new_patron_1 should have 0 guarantee' );
+
+    my $guarantee_1 = $builder->build({ source => 'Borrower', value => { guarantorid => $new_patron_1->borrowernumber }});
+    my $guarantee_2 = $builder->build({ source => 'Borrower', value => { guarantorid => $new_patron_1->borrowernumber }});
+
+    $guarantees = $new_patron_1->guarantees;
+    is( ref($guarantees), 'Koha::Patrons', 'Koha::Patron->guarantees should return a Koha::Patrons result set in a scalar context' );
+    is( $guarantees->count, 2, 'new_patron_1 should have 2 guarantees' );
+    @guarantees = $new_patron_1->guarantees;
+    is( ref(\@guarantees), 'ARRAY', 'Koha::Patron->guarantees should return an array in a list context' );
+    is( scalar(@guarantees), 2, 'new_patron_1 should have 2 guarantees' );
+    $_->delete for @guarantees;
+};
+
+
+$retrieved_patron_1->delete;
+is( Koha::Patrons->search->count, $nb_of_patrons + 1, 'Delete should have deleted the patron' );
+
+$schema->storage->txn_rollback;
+
+1;