From 62bc5afc148197c41ae308a582b6cdc8f74ecaa0 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 25 Jan 2016 13:25:42 +0000 Subject: [PATCH] Bug 15656: Move guarantor/guarantees code - GetGuarantees 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 Signed-off-by: Kyle M Hall --- C4/Members.pm | 33 --------- Koha/Patron.pm | 12 +++ .../prog/en/modules/members/moremember.tt | 22 ++++-- members/moremember.pl | 37 ++-------- t/db_dependent/Koha/Patrons.t | 86 ++++++++++++++++++++++ 5 files changed, 118 insertions(+), 72 deletions(-) create mode 100644 t/db_dependent/Koha/Patrons.t diff --git a/C4/Members.pm b/C4/Members.pm index 3028b2e..f7147b5 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -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); diff --git a/Koha/Patron.pm b/Koha/Patron.pm index f2972d6..0643ac3 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -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 diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt index a9ce12a..591dbfa 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt @@ -231,14 +231,20 @@ function validate1(date) { [% IF ( sex ) %]
  • Gender: [% IF ( sex == 'F' ) %]Female[% ELSIF ( sex == 'M' ) %]Male[% ELSE %][% sex %][% END %]
  • [% END %][% END %] - [% IF ( isguarantee ) %] - [% IF ( guaranteeloop ) %] -
  • Guarantees:
  • - [% END %] - [% ELSE %] - [% IF ( guarantorborrowernumber ) %] -
  • Guarantor:[% guarantorsurname %][%IF ( guarantorfirstname ) %], [% guarantorfirstname %] [% END %]
  • - [% END %] + [% IF guarantees %] +
  • + Guarantees: + +
  • + [% ELSIF guarantor %] +
  • + Guarantor: + [% guarantor.firstname %] [% guarantor.surname %] +
  • [% END %] diff --git a/members/moremember.pl b/members/moremember.pl index ba4c0a5..9238438 100755 --- a/members/moremember.pl +++ b/members/moremember.pl @@ -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 index 0000000..132ec24 --- /dev/null +++ b/t/db_dependent/Koha/Patrons.t @@ -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 . + +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; -- 2.1.4