Bug 15653: Remove unused C4::Members::UpdateGuarantees subroutine
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Mon, 25 Jan 2016 12:37:58 +0000 (12:37 +0000)
committerBrendan Gallagher <brendan@bywatersolutions.com>
Sat, 12 Mar 2016 23:39:09 +0000 (23:39 +0000)
Looking at the code, there is some broken with the guarantees code.
It seems that the expected behavior would be to update address, fax,
B_city, mobile, city and phone info of the guarantees when a guarantor
is modified.
But the code in C4::Members::ModMember is broken:

 668         my $borrowercategory= GetBorrowercategory(
$data{'category_type'} );
 669         if ( exists  $borrowercategory->{'category_type'} &&
$borrowercategory->{'category_type'} eq ('A' || 'S') ) {
 670             # is adult check guarantees;
 671             UpdateGuarantees(%data);
 672         }

First, GetBorrowerCategory expects a categorycode, not a category_type.
Then UpdateGuarantees retrieves the param like:

 989 sub UpdateGuarantees {
 990     my %data = shift;

Which means that %data will always be something like ( a_key => undef )
And nothing more.

The updateguarantees subroutine (It has been renamed) has been introduced by

commit 56825e415fc232e38f0a874dc9a81fa2169ef06b
Date:   Mon Aug 30 13:48:58 2004 +0000
    modularizing (with Members.pm) members management
    (beginning of...)

And the `%data = shift` already existed...

This code has never worked and could be removed.

See http://lists.koha-community.org/pipermail/koha-devel/2016-January/042241.html

Test plan:
Confirm the previous assertions.

Note that I have found this bug working on bug 15631, see patch "Bug
15631: Koha::Cities - remove getidcity and GetCities"

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
C4/Members.pm

index 884ee3d..3028b2e 100644 (file)
@@ -654,16 +654,6 @@ sub ModMember {
 
     my $execute_success = $rs->update($new_borrower);
     if ($execute_success ne '0E0') { # only proceed if the update was a success
-
-        # ok if its an adult (type) it may have borrowers that depend on it as a guarantor
-        # so when we update information for an adult we should check for guarantees and update the relevant part
-        # of their records, ie addresses and phone numbers
-        my $borrowercategory= GetBorrowercategory( $data{'category_type'} );
-        if ( exists  $borrowercategory->{'category_type'} && $borrowercategory->{'category_type'} eq ('A' || 'S') ) {
-            # is adult check guarantees;
-            UpdateGuarantees(%data);
-        }
-
         # If the patron changes to a category with enrollment fee, we add a fee
         if ( $data{categorycode} and $data{categorycode} ne $old_categorycode ) {
             if ( C4::Context->preference('FeeOnChangePatronCategory') ) {
@@ -972,31 +962,6 @@ sub GetGuarantees {
     return ( scalar(@$data), $data );
 }
 
-=head2 UpdateGuarantees
-
-  &UpdateGuarantees($parent_borrno);
-  
-
-C<&UpdateGuarantees> borrower data for an adult and updates all the guarantees
-with the modified information
-
-=cut
-
-#'
-sub UpdateGuarantees {
-    my %data = shift;
-    my $dbh = C4::Context->dbh;
-    my ( $count, $guarantees ) = GetGuarantees( $data{'borrowernumber'} );
-    foreach my $guarantee (@$guarantees){
-        my $guaquery = qq|UPDATE borrowers 
-              SET address=?,fax=?,B_city=?,mobile=?,city=?,phone=?
-              WHERE borrowernumber=?
-        |;
-        my $sth = $dbh->prepare($guaquery);
-        $sth->execute($data{'address'},$data{'fax'},$data{'B_city'},$data{'mobile'},$data{'city'},$data{'phone'},$guarantee->{'borrowernumber'});
-    }
-}
-
 =head2 GetPendingIssues
 
   my $issues = &GetPendingIssues(@borrowernumber);