From 9b1778d9fb24e04e0b766ac2e139f8670e184190 Mon Sep 17 00:00:00 2001 From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Date: Wed, 20 Jan 2016 15:00:33 +0000 Subject: [PATCH] Bug 15631: Koha::Cities - remove getidcity and GetCities C4::Members::getidcity and C4::Members::GetCities simply retrieved cities info from the cities table. The job done in members/memberentry.pl looked really weird and complicated. Either I have missed something, or this patch can simplify it. The expected behavior is: 1. Create a new patron => No city selected 2. Edit an existing patron => The borrowers.city value is selected 3. Add a guarantee => The borrowers.city of the guarantor is selected 4. Edit a guarantee => The borrowers.city of the guarantee is selected Test plan: Confirm that the expected behaviors are the ones before and after this patch. --- C4/Members.pm | 47 ---------------------- .../en/includes/member-main-address-style-de.inc | 19 ++++----- .../en/includes/member-main-address-style-us.inc | 19 ++++----- members/memberentry.pl | 32 +++------------ 4 files changed, 26 insertions(+), 91 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index 4eaea62..78e8994 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -66,13 +66,10 @@ BEGIN { &GetPendingIssues &GetAllIssues - &getidcity - &GetFirstValidEmailAddress &GetNoticeEmailAddress &GetAge - &GetCities &GetSortDetails &GetTitles @@ -1356,21 +1353,6 @@ sub get_cardnumber_length { return ( $min, $max ); } -=head2 getdcity (OUEST-PROVENCE) - -recover cityid with city_name condition - -=cut - -sub getidcity { - my ($city_name) = @_; - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare("select cityid from cities where city_name=? "); - $sth->execute($city_name); - my $data = $sth->fetchrow; - return $data; -} - =head2 GetFirstValidEmailAddress $email = GetFirstValidEmailAddress($borrowernumber); @@ -1677,35 +1659,6 @@ sub SetAge{ return $borrower; } # sub SetAge -=head2 GetCities - - $cityarrayref = GetCities(); - - 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 $dbh = C4::Context->dbh; - my $city_arr = $dbh->selectall_arrayref( - q|SELECT cityid,city_zipcode,city_name,city_state,city_country FROM cities ORDER BY city_name|, - { Slice => {} }); - if ( @{$city_arr} ) { - unshift @{$city_arr}, { - city_zipcode => q{}, - city_name => q{}, - cityid => q{}, - city_state => q{}, - city_country => q{}, - }; - } - - return $city_arr; -} - =head2 GetSortDetails (OUEST-PROVENCE) ($lib) = &GetSortDetails($category,$sortvalue); diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-de.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-de.inc index e2ef537..39a979c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-de.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-de.inc @@ -80,17 +80,18 @@ [% END %] City: </label> <input type="text" id="city" name="city" size="20" value="[% city %]" /> - [% IF ( city_cgipopup ) %]or choose + [% IF cities.count %]or choose <select id="select_city" name="select_city"> - [% FOREACH city_loo IN city_loop %] - [% IF ( city_loo.selected ) %] - <option value="[% city_loo.city_zipcode %]|[% city_loo.city_name %]|[% city_loo.city_state %]|[% city_loo.city_country %]" selected="selected"> - [% ELSE %] - <option value="[% city_loo.city_zipcode %]|[% city_loo.city_name %]|[% city_loo.city_state %]|[% city_loo.city_country %]"> + <option value="|||"></option> + [% FOREACH c IN cities %] + [% IF c.city_name == city %] + <option value="[% c.city_zipcode %]|[% c.city_name %]|[% c.city_state %]|[% c.city_country %]" selected="selected"> + [% ELSE %] + <option value="[% c.city_zipcode %]|[% c.city_name %]|[% c.city_state %]|[% c.city_country %]"> + [% END %] + [% c.city_name %] [% c.city_state %] [% c.city_zipcode %] + </option> [% END %] - [% city_loo.city_name %] [% city_loo.city_state %] [% city_loo.city_zipcode %] - </option> - [% END %] </select> [% END %] [% IF ( mandatorycity ) %]<span class="required">Required</span>[% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-us.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-us.inc index 48844f9..3211684 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-us.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-us.inc @@ -68,17 +68,18 @@ [% END %] City: </label> <input type="text" id="city" name="city" size="20" value="[% city %]" /> - [% IF ( city_cgipopup ) %]or choose + [% IF cities.count %]or choose <select id="select_city" name="select_city"> - [% FOREACH city_loo IN city_loop %] - [% IF ( city_loo.selected ) %] - <option value="[% city_loo.city_zipcode %]|[% city_loo.city_name %]|[% city_loo.city_state %]|[% city_loo.city_country %]" selected="selected"> - [% ELSE %] - <option value="[% city_loo.city_zipcode %]|[% city_loo.city_name %]|[% city_loo.city_state %]|[% city_loo.city_country %]"> + <option value="|||"></option> + [% FOREACH c IN cities %] + [% IF c.city_name == city %] + <option value="[% c.city_zipcode %]|[% c.city_name %]|[% c.city_state %]|[% c.city_country %]" selected="selected"> + [% ELSE %] + <option value="[% c.city_zipcode %]|[% c.city_name %]|[% c.city_state %]|[% c.city_country %]"> + [% END %] + [% c.city_name %] [% c.city_state %] [% c.city_zipcode %] + </option> [% END %] - [% city_loo.city_name %] [% city_loo.city_state %] [% city_loo.city_zipcode %] - </option> - [% END %] </select> [% END %] [% IF ( mandatorycity ) %]<span class="required">Required</span>[% END %] diff --git a/members/memberentry.pl b/members/memberentry.pl index ae54383..3f1e1cf 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -40,6 +40,7 @@ use C4::Letters; use C4::Branch; # GetBranches use C4::Form::MessagingPreferences; use Koha::Borrower::Debarments; +use Koha::Cities; use Koha::DateUtils; use Email::Valid; use Module::Load; @@ -83,12 +84,10 @@ $nodouble = 1 if ($op eq 'modify' or $op eq 'duplicate'); # FIXME hack to rep # modifying an existing patron, it ipso facto # isn't a duplicate. Marking FIXME because this # script needs to be refactored. -my $select_city = $input->param('select_city'); my $nok = $input->param('nok'); my $guarantorinfo = $input->param('guarantorinfo'); my $step = $input->param('step') || 0; my @errors; -my $default_city; my $borrower_data; my $NoUpdateLogin; my $userenv = C4::Context->userenv; @@ -531,31 +530,14 @@ foreach (qw(C A S P I X)) { $template->param('typeloop' => \@typeloop, no_categories => $no_categories); if($no_categories){ $no_add = 1; } -# test in city -if ( $guarantorid ) { - $select_city = getidcity($data{city}); -} -($default_city=$select_city) if ($step eq 0); -if (!defined($select_city) or $select_city eq '' ){ - $default_city = &getidcity($data{'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 $cities = Koha::Cities->search( {}, { order_by => 'city_name' } ); my $roadtypes = C4::Koha::GetAuthorisedValues( 'ROADTYPE', $data{streettype} ); -$template->param( roadtypes => $roadtypes); +$template->param( + roadtypes => $roadtypes, + cities => $cities, +); my $default_borrowertitle = ''; unless ( $op eq 'duplicate' ) { $default_borrowertitle=$data{'title'} } @@ -683,7 +665,6 @@ $template->param( step => $step ) if $step; # associate with step to know wh $template->param( BorrowerMandatoryField => C4::Context->preference("BorrowerMandatoryField"),#field to test with javascript category_type => $category_type,#to know the category type of the borrower - select_city => $select_city, "$category_type" => 1,# associate with step to know where u are destination => $destination,#to know wher u come from and wher u must go in redirect check_member => $check_member,#to know if the borrower already exist(=>1) or not (=>0) @@ -695,7 +676,6 @@ $template->param( borrowernumber => $borrowernumber, #register number guarantorid => ($borrower_data->{'guarantorid'} || $guarantorid), relshiploop => \@relshipdata, - city_loop => $city_arrayref, borrotitlepopup => $borrotitlepopup, guarantorinfo => $guarantorinfo, flagloop => \@flagdata, -- 2.1.0