From 939435e42b96dde8a1f73144ca737d7744fc4d4c Mon Sep 17 00:00:00 2001 From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Date: Thu, 24 Sep 2015 15:01:31 +0100 Subject: [PATCH] [PASSED QA] Bug 14888: use Koha::Cit[y|ies] in admin/itemtypes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test plan: Add/edit/remove cities from the administration module (admin/cities.pl). You should get message feedback after each action. Tested both patches together, works as expected. Signed-off-by: Marc VĂ©ron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> --- admin/cities.pl | 164 ++++++++------- .../intranet-tmpl/prog/en/modules/admin/cities.tt | 233 ++++++++++++--------- 2 files changed, 215 insertions(+), 182 deletions(-) diff --git a/admin/cities.pl b/admin/cities.pl index 5fe3847..306927f 100755 --- a/admin/cities.pl +++ b/admin/cities.pl @@ -17,97 +17,99 @@ # You should have received a copy of the GNU General Public License # along with Koha; if not, see <http://www.gnu.org/licenses>. -use strict; -use warnings; +use Modern::Perl; use CGI qw ( -utf8 ); use C4::Context; use C4::Auth; use C4::Output; -sub StringSearch { - my $sth = C4::Context->dbh->prepare("SELECT * FROM cities WHERE (city_name LIKE ?)"); - $sth->execute("%" . (shift || '') . "%"); - return $sth->fetchall_arrayref({}); -} +use Koha::Cities; -my $input = new CGI; -my $script_name = "/cgi-bin/koha/admin/cities.pl"; -my $searchfield = $input->param('city_name'); +my $input = new CGI; +my $searchfield = $input->param('city_name') // q||; my $cityid = $input->param('cityid'); -my $op = $input->param('op') || ''; - -my ($template, $loggedinuser, $cookie) - = get_template_and_user({template_name => "admin/cities.tt", - query => $input, - type => "intranet", - authnotrequired => 0, - flagsrequired => {parameters => 'parameters_remaining_permissions'}, - debug => 1, - }); +my $op = $input->param('op') || 'list'; +my @messages; -$template->param( script_name => $script_name, - cityid => $cityid , - searchfield => $searchfield); +my ( $template, $loggedinuser, $cookie ) = get_template_and_user( + { template_name => "admin/cities.tt", + query => $input, + type => "intranet", + authnotrequired => 0, + flagsrequired => { parameters => 'parameters_remaining_permissions' }, + debug => 1, + } +); my $dbh = C4::Context->dbh; -################## ADD_FORM ################################## -# called by default. Used to create form to add or modify a record -if ($op eq 'add_form') { - $template->param(add_form => 1); - - #---- if primkey exists, it's a modify action, so read values to modify... - my $data; - if ($cityid) { - my $sth=$dbh->prepare("select cityid,city_name,city_state,city_zipcode,city_country from cities where cityid=?"); - $sth->execute($cityid); - $data=$sth->fetchrow_hashref; - } +if ( $op eq 'add_form' ) { + my $city; + if ($cityid) { + $city = Koha::Cities->find($cityid); + } + + $template->param( city => $city, ); +} elsif ( $op eq 'add_validate' ) { + my $cityid = $input->param('cityid'); + my $city_name = $input->param('city_name'); + my $city_state = $input->param('city_state'); + my $city_zipcode = $input->param('city_zipcode'); + my $city_country = $input->param('city_country'); + + if ($cityid) { + my $city = Koha::Cities->find($cityid); + $city->city_name($city_name); + $city->city_state($city_state); + $city->city_zipcode($city_zipcode); + $city->city_country($city_country); + eval { $city->store; }; + if ($@) { + push @messages, { type => 'error', code => 'error_on_update' }; + } else { + push @messages, { type => 'message', code => 'success_on_update' }; + } + } else { + my $city = Koha::City->new( + { city_name => $city_name, + city_state => $city_state, + city_zipcode => $city_zipcode, + city_country => $city_country, + } + ); + eval { $city->store; }; + if ($@) { + push @messages, { type => 'error', code => 'error_on_insert' }; + } else { + push @messages, { type => 'message', code => 'success_on_insert' }; + } + } + $searchfield = q||; + $op = 'list'; +} elsif ( $op eq 'delete_confirm' ) { + my $city = Koha::Cities->find($cityid); + $template->param( city => $city, ); +} elsif ( $op eq 'delete_confirmed' ) { + my $city = Koha::Cities->find($cityid); + my $deleted = eval { $city->delete; }; + + if ( $@ or not $deleted ) { + push @messages, { type => 'error', code => 'error_on_delete' }; + } else { + push @messages, { type => 'message', code => 'success_on_delete' }; + } + $op = 'list'; +} + +if ( $op eq 'list' ) { + my $cities = Koha::Cities->search( { city_name => { -like => "%$searchfield%" } } ); + $template->param( cities => $cities, ); +} - $template->param( - city_name => $data->{'city_name'}, - city_state => $data->{'city_state'}, - city_zipcode => $data->{'city_zipcode'}, - city_country => $data->{'city_country'}); -# END $OP eq ADD_FORM -################## ADD_VALIDATE ################################## -# called by add_form, used to insert/modify data in DB -} elsif ($op eq 'add_validate') { - my $sth; - - if ($input->param('cityid') ){ - $sth=$dbh->prepare("UPDATE cities SET city_name=?,city_state=?,city_zipcode=?,city_country=? WHERE cityid=?"); - $sth->execute($input->param('city_name'),$input->param('city_state'),$input->param('city_zipcode'),$input->param('city_country'),$input->param('cityid')); - } - else{ - $sth=$dbh->prepare("INSERT INTO cities (city_name,city_state,city_zipcode,city_country) values (?,?,?,?)"); - $sth->execute($input->param('city_name'),$input->param('city_state'),$input->param('city_zipcode'),$input->param('city_country')); - } - print $input->redirect($script_name); - exit; -################## DELETE_CONFIRM ################################## -# called by default form, used to confirm deletion of data in DB -} elsif ($op eq 'delete_confirm') { - $template->param(delete_confirm => 1); - my $sth=$dbh->prepare("select cityid,city_name,city_state,city_zipcode,city_country from cities where cityid=?"); - $sth->execute($cityid); - my $data=$sth->fetchrow_hashref; - $template->param( - city_name => $data->{'city_name'}, - city_state => $data->{'city_state'}, - city_zipcode => $data->{'city_zipcode'}, - city_country => $data->{'city_country'}, - ); -################## DELETE_CONFIRMED ################################## -# called by delete_confirm, used to effectively confirm deletion of data in DB -} elsif ($op eq 'delete_confirmed') { - my $sth=$dbh->prepare("delete from cities where cityid=?"); - $sth->execute($cityid); - print $input->redirect($script_name); - exit; # FIXME: what's the point of redirecting to this same page? - # END $OP eq DELETE_CONFIRMED -} else { # DEFAULT - $template->param(else => 1); - $template->param(loop => StringSearch($searchfield)); +$template->param( + cityid => $cityid, + searchfield => $searchfield, + messages => \@messages, + op => $op, +); -} #---- END $OP eq DEFAULT output_html_with_http_headers $input, $cookie, $template->output; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/cities.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/cities.tt index 3404483..a526209 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/cities.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/cities.tt @@ -1,5 +1,5 @@ [% INCLUDE 'doc-head-open.inc' %] -<title>Koha › Administration › [% IF ( add_form ) %]Cities › [% IF ( cityid ) %] Modify city[% ELSE %] New city[% END %][% ELSE %][% IF ( delete_confirm ) %]Cities › Confirm deletion of city[% ELSE %] Cities[% END %][% END %]</title> +<title>Koha › Administration › [% IF op =='add_form' %]Cities › [% IF city.cityid %] Modify city[% ELSE %] New city[% END %][% ELSE %][% IF op == 'delete_confirm' %]Cities › Confirm deletion of city[% ELSE %] Cities[% END %][% END %]</title> [% INCLUDE 'doc-head-close.inc' %] <link rel="stylesheet" type="text/css" href="[% themelang %]/css/datatables.css" /> [% INCLUDE 'datatables.inc' %] @@ -26,123 +26,154 @@ <a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a href="/cgi-bin/koha/admin/admin-home.pl">Administration</a> › <a href="/cgi-bin/koha/admin/cities.pl">Cities</a> - [% IF ( add_form ) %] - › [% IF ( cityid ) %]Modify[% ELSE %]New[% END %] City - [% ELSIF ( delete_confirm ) %] + [% IF op == 'add_form' %] + › [% IF city.cityid %]Modify[% ELSE %]New[% END %] City + [% ELSIF op == 'delete_confirm' %] › Confirm deletion of city [% END %] </div> <div id="doc3" class="yui-t2"> - + <div id="bd"> - <div id="yui-main"> - <div class="yui-b"> + <div id="yui-main"> + <div class="yui-b"> + +[% FOR m IN messages %] + <div class="dialog [% m.type %]"> + [% SWITCH m.code %] + [% CASE 'error_on_update' %] + An error occurred when updating this city. Perhaps it already exists. + [% CASE 'error_on_insert' %] + An error occurred when inserting this city. The city id might already exist. + [% CASE 'error_on_delete' %] + An error occurred when deleting this city. Check the logs. + [% CASE 'success_on_update' %] + City updated successfully. + [% CASE 'success_on_insert' %] + City inserted successfully. + [% CASE 'success_on_delete' %] + City deleted successfully. + [% CASE 'already_exists' %] + This city already exists. + [% CASE %] + [% m.code %] + [% END %] + </div> +[% END %] -[% IF ( add_form ) %] - [% IF ( cityid ) %] - <h1>Modify a city</h1> - [% ELSE %] - <h1>New city</h1> - [% END %] +[% IF op == 'add_form' %] + [% IF city %] + <h1>Modify a city</h1> + [% ELSE %] + <h1>New city</h1> + [% END %] - <form action="[% script_name %]" name="Aform" method="post" class="validated"> - <input type="hidden" name="op" value="add_validate" /> - <input type="hidden" name="checked" value="0" /> - <input type="hidden" name="cityid" value="[% cityid %]" /> + <form action="/cgi-bin/koha/admin/cities.pl" name="Aform" method="post" class="validated"> + <input type="hidden" name="op" value="add_validate" /> + <input type="hidden" name="cityid" value="[% city.cityid %]" /> -<fieldset class="rows"><ol> [% IF ( cityid ) %] - <li> - <span class="label">City ID: </span>[% cityid %]</li> - [% END %] - <li> - <label for="city_name" class="required">City: </label> - <input type="text" name="city_name" id="city_name" size="80" maxlength="100" value="[% city_name |html %]" required="required" class="required" /> <span class="required">Required</span> - </li> - <li> - <label for="city_state">State: </label> - <input type="text" name="city_state" id="city_state" size="80" maxlength="100" value="[% city_state |html %]" /> - </li> - <li> - <label for="city_zipcode" class="required">Zip/Postal code: </label> - <input type="text" name="city_zipcode" id="city_zipcode" size="20" maxlength="20" value="[% city_zipcode %]" required="required" class="required" /> <span class="required">Required</span> - </li> - <li> - <label for="city_country">Country: </label> - <input type="text" name="city_country" id="city_country" size="80" maxlength="100" value="[% city_country |html %]" /> - </li></ol></fieldset> - - <fieldset class="action"> - <input type="submit" value="Submit" /> <a class="cancel" href="/cgi-bin/koha/admin/cities.pl">Cancel</a> - </fieldset> - </form> + <fieldset class="rows"> + <ol> + [% IF city %] + <li><span class="label">City ID: </span>[% city.cityid %]</li> + [% END %] + <li> + <label for="city_name" class="required">City: </label> + <input type="text" name="city_name" id="city_name" size="80" maxlength="100" value="[% city.city_name |html %]" required="required" class="required" /> <span class="required">Required</span> + </li> + <li> + <label for="city_state">State: </label> + <input type="text" name="city_state" id="city_state" size="80" maxlength="100" value="[% city.city_state |html %]" /> + </li> + <li> + <label for="city_zipcode" class="required">Zip/Postal code: </label> + <input type="text" name="city_zipcode" id="city_zipcode" size="20" maxlength="20" value="[% city.city_zipcode %]" required="required" class="required" /> <span class="required">Required</span> + </li> + <li> + <label for="city_country">Country: </label> + <input type="text" name="city_country" id="city_country" size="80" maxlength="100" value="[% city.city_country |html %]" /> + </li> + </ol> + </fieldset> + <fieldset class="action"> + <input type="submit" value="Submit" /> + <a class="cancel" href="/cgi-bin/koha/admin/cities.pl">Cancel</a> + </fieldset> + </form> [% END %] -[% IF ( delete_confirm ) %] + +[% IF op == 'delete_confirm' %] <div class="dialog alert"> - <h3>Delete City "[% city_name %]?"</h3> - <table> - <tr><th>City id</th> - <td>[% cityid %]</td> - </tr> - <tr><th>City</th> - <td>[% city_name %]</td> - </tr> - <tr><th>State</th> - <td>[% city_state %]</td> - </tr> - <tr><th>Zip/Postal code</th> - <td>[% city_zipcode %]</td> - </tr> - <tr><th>Country</th> - <td>[% city_country %]</td> - </tr> - </table> - <form action="[% script_name %]" method="post"> - <input type="hidden" name="op" value="delete_confirmed" /> - <input type="hidden" name="cityid" value="[% cityid %]" /> - <input type="submit" class="approve" value="Yes, delete" /> - </form> - <form action="[% script_name %]" method="get"> - <input type="submit" class="deny" value="No, do not Delete" /> - </form> -</div> + <h3>Delete City "[% city.city_name %]?"</h3> + <table> + <tr><th>City id</th> + <td>[% city.cityid %]</td> + </tr> + <tr><th>City</th> + <td>[% city.city_name %]</td> + </tr> + <tr><th>State</th> + <td>[% city.city_state %]</td> + </tr> + <tr><th>Zip/Postal code</th> + <td>[% city.city_zipcode %]</td> + </tr> + <tr><th>Country</th> + <td>[% city.city_country %]</td> + </tr> + </table> + <form action="/cgi-bin/koha/admin/cities.pl" method="post"> + <input type="hidden" name="op" value="delete_confirmed" /> + <input type="hidden" name="cityid" value="[% city.cityid %]" /> + <input type="submit" class="approve" value="Yes, delete" /> + </form> + <form action="/cgi-bin/koha/admin/cities.pl" method="get"> + <input type="submit" class="deny" value="No, do not Delete" /> + </form> + </div> [% END %] -[% IF ( else ) %] +[% IF op == 'list' %] -<div id="toolbar" class="btn-toolbar"> - <a class="btn btn-small" id="newcity" href="[% script_name %]?op=add_form"><i class="icon-plus"></i> New city</a> -</div> + <div id="toolbar" class="btn-toolbar"> + <a class="btn btn-small" id="newcity" href="/cgi-bin/koha/admin/cities.pl?op=add_form"><i class="icon-plus"></i> New city</a> + </div> - <h2>Cities</h2> - [% IF ( searchfield ) %] - Searching: [% searchfield %] - [% END %] + <h2>Cities</h2> + [% IF searchfield %] + Searching: [% searchfield %] + [% END %] -[% IF ( loop ) %] -<table id="table_cities"> - <thead> - <th>City ID</th> - <th>City</th> - <th>State</th> - <th>Zip/Postal code</th> - <th>Country</th> - <th> </th> - <th> </th> - </thead> - [% FOREACH loo IN loop %] - <tr> - <td>[% loo.cityid %]</td> - <td>[% loo.city_name %]</td> - <td>[% loo.city_state %]</td> - <td>[% loo.city_zipcode %]</td> - <td>[% loo.city_country %]</td> - <td><a href="[% loo.script_name %]?op=add_form&cityid=[% loo.cityid %]">Edit</a></td> - <td><a href="[% loo.script_name %]?op=delete_confirm&cityid=[% loo.cityid %]">Delete</a></td> - </tr> - [% END %] - </table>[% END %] + [% IF cities %] + <table id="table_cities"> + <thead> + <th>City ID</th> + <th>City</th> + <th>State</th> + <th>Zip/Postal code</th> + <th>Country</th> + <th> </th> + <th> </th> + </thead> + <tbody> + [% FOREACH city IN cities %] + <tr> + <td>[% city.cityid %]</td> + <td>[% city.city_name %]</td> + <td>[% city.city_state %]</td> + <td>[% city.city_zipcode %]</td> + <td>[% city.city_country %]</td> + <td><a href="/cgi-bin/koha/admin/cities.pl?op=add_form&cityid=[% city.cityid %]">Edit</a></td> + <td><a href="/cgi-bin/koha/admin/cities.pl?op=delete_confirm&cityid=[% city.cityid %]">Delete</a></td> + </tr> + [% END %] + </tbody> + </table> + [% ELSE %] + There is no city defined. <a href="/cgi-bin/koha/admin/cities.pl?op=add_form">Create a new city</a>. + [% END %] [% END %] </div> -- 1.9.1