From dd0ba687f1311e78b10d0a8665b27df509b4296d Mon Sep 17 00:00:00 2001
From: genevieve <genevieve.plantin@inlibro.com>
Date: Wed, 9 Sep 2015 15:47:43 -0400
Subject: [PATCH] Bug 12446 - Clean code according to Comment 12

For the first recommendation, I tried to be more explicit, I don't know if you would prefer it because it is slower.

For the second point, I've got rid of the UPDATE line so we don't activate a new functionnality.

For the third point, I was told that the coma was there in case of new addition.

Fourth, I corrected the mistake.

For the fifth recommendation, I tried to separate the hardcode categories, but it is simple for the display (it only changes a the choice in the select for yes, but the user can still change it aftewards). However, I modified it so, if someone wants to add a new category to be displayed as guarantee by default, he only has to add the category code in the array instead of writting another select == 'new category'.

For the sixth point, I replaced the space.

Lastly, I deleted these lines because, as you said it, it wasn't used.
---
 C4/Utils/DataTables/Members.pm                     |   14 ++++++++++----
 .../atomicupdate/bug_12446-EnableAdultGarantee.sql |    3 ---
 members/memberentry.pl                             |    2 +-
 members/moremember.pl                              |    5 -----
 4 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/C4/Utils/DataTables/Members.pm b/C4/Utils/DataTables/Members.pm
index 72f9e59..6625833 100644
--- a/C4/Utils/DataTables/Members.pm
+++ b/C4/Utils/DataTables/Members.pm
@@ -34,12 +34,18 @@ sub search {
     my $dbh = C4::Context->dbh;
     my @columns = qw( borrowernumber surname firstname streetnumber streettype address address2 city state zipcode country cardnumber dateexpiry borrowernotes branchcode email userid dateofbirth categorycode );
     if( my @guarantor_attributes = @{ get_guarantor_shared_attributes() }){
-        my %seen = ();
-        @columns = grep { ! $seen{ $_ }++ } ( @columns, @guarantor_attributes );
+        foreach my $item (@guarantor_attributes) {
+             if (! grep {$_ eq $item} @columns) {
+                 push @columns, $item;
+	     }
+	}
     };
-    $_ = "borrowers.".$dbh->quote_identifier($_) for (@columns);
+    my $borrowers_columns = "";
+    foreach my $item (@columns) {
+        $borrowers_columns .= "borrowers." . $item . ", ";
+    }
 
-    my $select = "SELECT ${ \join(',', @columns) },
+    my $select = "SELECT " . $borrowers_columns . "
         categories.description AS category_description, categories.category_type,
         branches.branchname";
     my $from = "FROM borrowers
diff --git a/installer/data/mysql/atomicupdate/bug_12446-EnableAdultGarantee.sql b/installer/data/mysql/atomicupdate/bug_12446-EnableAdultGarantee.sql
index cb3266e..267e19f 100644
--- a/installer/data/mysql/atomicupdate/bug_12446-EnableAdultGarantee.sql
+++ b/installer/data/mysql/atomicupdate/bug_12446-EnableAdultGarantee.sql
@@ -9,6 +9,3 @@ VALUES ('AdditionalGuarantorField','',NULL,'Additional fields name to be transfe
 -- ********* --
 ALTER TABLE categories ADD COLUMN `canbeguarantee` tinyint(1) NOT NULL default '0';
 
--- Set defaults for new column
-UPDATE categories set canbeguarantee = 1 WHERE category_type IN ('C', 'P');
-
diff --git a/members/memberentry.pl b/members/memberentry.pl
index 7b6196a..b7c1402 100755
--- a/members/memberentry.pl
+++ b/members/memberentry.pl
@@ -251,7 +251,7 @@ if ( $guarantorid and ( $category_type eq 'C' || $category_type eq 'P')) {
 	        foreach (qw(streetnumber address streettype address2
                         zipcode country city state phone phonepro mobile fax email emailpro branchcode
                         B_streetnumber B_streettype B_address B_address2
-                        B_city B_state B_zipcode B_country B_email B_phone)) {		        
+                        B_city B_state B_zipcode B_country B_email B_phone)) {
                         $newdata{$_} = $guarantordata->{$_};
 	        }
             my $additionalGuarantorField=C4::Context->preference("AdditionalGuarantorField");
diff --git a/members/moremember.pl b/members/moremember.pl
index 6f0d788..be8f9af 100755
--- a/members/moremember.pl
+++ b/members/moremember.pl
@@ -201,11 +201,6 @@ if ( $count ) {
     ( $template->param( adultborrower => 1 ) ) if ( $category_type eq 'A' || $category_type eq 'I' );
 
 }
-else {
-	#if ($category_type eq 'C'){
-	#	$template->param('C' => 1);
-	#}
-}
 if ($data->{'guarantorid'}){
     my ($guarantor) = GetMember( 'borrowernumber' =>$data->{'guarantorid'});
     $template->param(guarantor => 1);
-- 
1.7.9.5