Attachment 48662 Details for Bug 15653 – Bug 15653: Remove unused C4::Members::UpdateGuarantees subroutine

[patch] Bug 15653: Remove unused C4::Members::UpdateGuarantees subroutine

Bug-15653-Remove-unused-C4MembersUpdateGuarantees-.patch (text/plain), 3.77 KB, created by Jonathan Druart on 2016-03-04 10:07:57 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2016-03-04 10:07:57 UTC

Size: 3.77 KB

patch

obsolete

>From 34b0d65165878e8f429ddd1eb10b835aa1cd2066 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Mon, 25 Jan 2016 12:37:58 +0000
>Subject: [PATCH] Bug 15653: Remove unused C4::Members::UpdateGuarantees
> subroutine
>
>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>
>---
> C4/Members.pm | 34 ----------------------------------
> 1 file changed, 34 deletions(-)
>
>diff --git a/C4/Members.pm b/C4/Members.pm
>index a7f686c..4781b21 100644
>--- a/C4/Members.pm
>+++ b/C4/Members.pm
>@@ -658,16 +658,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') ) {
>@@ -976,30 +966,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);
>-- 
>2.7.0

Actions: View | Diff | Splinter Review

Attachments on bug 15653: 47250 | 47265 | 48662 | 49051