From dd0ba687f1311e78b10d0a8665b27df509b4296d Mon Sep 17 00:00:00 2001 From: genevieve 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