From e7bdbd0583eaa3fbc5e4fcc58833d2345ec42137 Mon Sep 17 00:00:00 2001
From: Colin Campbell <colin.campbell@ptfs-europe.com>
Date: Wed, 24 Feb 2010 16:59:46 +0000
Subject: [PATCH] Bug 4248 Rework Cities dropdown for members entry

Garry Collum's patch correctly identified the core flaw in operation
looking closer revealed some other flaws (2 "blank" options, some vars
and documentation belonging to a superceded version, etc.
Changed GetCities to return a structure rather than the messy two returns
Let the template do the concatenation and removed the CGI::Popup

Signed-off-by: Galen Charlton <gmcharlt@gmail.com>
---
 C4/Members.pm                                 | 51 +++++++------------
 .../en/modules/members/memberentrygen.tmpl    | 11 +++-
 members/memberentry.pl                        | 26 ++++++----
 3 files changed, 42 insertions(+), 46 deletions(-)

diff --git a/C4/Members.pm b/C4/Members.pm
index 4f79e3e16a..60a3164e64 100644
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -1508,48 +1508,31 @@ sub add_member_orgs {
 
 }    # sub add_member_orgs
 
-=head2 GetCities (OUEST-PROVENCE)
+=head2 GetCities
 
-  ($id_cityarrayref, $city_hashref) = &GetCities();
+  $cityarrayref = GetCities();
 
-Looks up the different city and zip in the database. Returns two
-elements: a reference-to-array, which lists the zip city
-codes, and a reference-to-hash, which maps the name of the city.
-WHERE =>OUEST PROVENCE OR EXTERIEUR
+  Returns an array_ref of the entries in the cities table
+  If there are entries in the table an empty row is returned
+  This is currently only used to populate a popup in memberentry
 
 =cut
 
 sub GetCities {
 
-    #my ($type_city) = @_;
     my $dbh   = C4::Context->dbh;
-    my $query = qq|SELECT cityid,city_zipcode,city_name 
-        FROM cities 
-        ORDER BY city_name|;
-    my $sth = $dbh->prepare($query);
-
-    #$sth->execute($type_city);
-    $sth->execute();
-    my %city;
-    my @id;
-    #    insert empty value to create a empty choice in cgi popup
-    push @id, " ";
-    $city{""} = "";
-    while ( my $data = $sth->fetchrow_hashref ) {
-        push @id, $data->{'city_zipcode'}."|".$data->{'city_name'};
-        $city{ $data->{'city_zipcode'}."|".$data->{'city_name'} } = $data->{'city_name'};
-    }
-
-#test to know if the table contain some records if no the function return nothing
-    my $id = @id;
-    if ( $id == 1 ) {
-        # all we have is the one blank row
-        return ();
-    }
-    else {
-        unshift( @id, "" );
-        return ( \@id, \%city );
-    }
+    my $city_arr = $dbh->selectall_arrayref(
+        q|SELECT cityid,city_zipcode,city_name FROM cities ORDER BY city_name|,
+        { Slice => {} });
+    if ( @{$city_arr} ) {
+        unshift @{$city_arr}, {
+            city_zipcode => q{},
+            city_name    => q{},
+            cityid       => q{},
+        };
+    }
+
+    return  $city_arr;
 }
 
 =head2 GetSortDetails (OUEST-PROVENCE)
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tmpl
index 503fe549cc..811b6223b9 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tmpl
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tmpl
@@ -403,7 +403,16 @@
       City, State: </label>
         
         <input type="text" id="city" name="city" size="20" value="<!-- TMPL_VAR NAME="city" -->" />
-        <!-- TMPL_IF NAME="city_cgipopup" -->or <strong>choose</strong> <!-- TMPL_VAR NAME="citypopup" --><!-- /TMPL_IF -->
+        <!-- TMPL_IF NAME="city_cgipopup" -->or <strong>choose</strong>
+        <select id="select_city" name "select_city">
+        <!-- TMPL_LOOP NAME="city_loop" -->
+            <option value="<!-- TMPL_VAR NAME='city_zipcode'-->|<!-- TMPL_VAR NAME='city_name' -->"
+                <!-- TMPL_IF NAME="selected" --> selected="1" <!-- /TMPL_IF --> >
+                <!-- TMPL_VAR NAME="city_name" --> <!-- TMPL_VAR NAME="city_zipcode" -->
+            </option>
+        <!-- /TMPL_LOOP -->
+        </select>
+        <!-- /TMPL_IF -->
 	  <!-- TMPL_IF NAME="mandatorycity" --><span class="required">Required</span><!-- /TMPL_IF -->
     </li>
     <li> 
diff --git a/members/memberentry.pl b/members/memberentry.pl
index c99dcb99b2..c15828575d 100755
--- a/members/memberentry.pl
+++ b/members/memberentry.pl
@@ -69,7 +69,6 @@ my $op             = $input->param('op');
 my $destination    = $input->param('destination');
 my $cardnumber     = $input->param('cardnumber');
 my $check_member   = $input->param('check_member');
-my $name_city      = $input->param('name_city');
 my $nodouble       = $input->param('nodouble');
 $nodouble = 1 if $op eq 'modify'; # FIXME hack to represent fact that if we're
                                   # modifying an existing patron, it ipso facto
@@ -434,15 +433,20 @@ $select_city=getidcity($data{'city'}) if defined $guarantorid and ($guarantorid
 if (!defined($select_city) or $select_city eq '' ){
 	$default_city = &getidcity($data{'city'});
 }
-my($cityid);
-($cityid,$name_city)=GetCities();
-$template->param( city_cgipopup => 1) if ($cityid );
-my $citypopup = CGI::popup_menu(-name=>'select_city',
-        -id => 'select_city',
-        '-values' =>$cityid,
-        -labels=>$name_city,
-        -default=>$default_city,
-        );  
+
+my $city_arrayref = GetCities();
+if (@{$city_arrayref} ) {
+    $template->param( city_cgipopup => 1);
+
+    if ($default_city) { # flag the current or default val
+        for my $city ( @{$city_arrayref} ) {
+            if ($default_city == $city->{cityid}) {
+                $city->{selected} = 1;
+                last;
+            }
+        }
+    }
+}
   
 my $default_roadtype;
 $default_roadtype=$data{'streettype'} ;
@@ -628,7 +632,7 @@ $template->param(
   guarantorid => (defined($borrower_data->{'guarantorid'})) ? $borrower_data->{'guarantorid'} : $guarantorid,
   ethcatpopup => $ethcatpopup,
   relshiploop => \@relshipdata,
-  citypopup => $citypopup,
+  city_loop => $city_arrayref,
   roadpopup => $roadpopup,  
   borrotitlepopup => $borrotitlepopup,
   guarantorinfo   => $guarantorinfo,
-- 
2.39.5