From afb6d14f253e4bd1828464e0147169597d9c7643 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Mon, 12 May 2014 15:28:41 +0200 Subject: [PATCH] Bug 12265: Improve Z39.50 servers administration This patch makes a few tiny improvements on the form, does some housekeeping/tidying up, and replaces SQL code by DBIC statements. In detail: - Adds an id parameter (more precise than searchfield). - With the move from searchfield to id, you can rename a server now. - A Copy button is added to clone a server. - Confirming a delete is moved to javascript. No additional form anymore. - A message about an insert, update or delete is shown in a div dialog alert above the table instead of a separate form. - Remove offset parameter, Next/Prev button and associated logic. - All SQL statements are replaced by DBIC. - Function StringSearch is renamed to DBICified ServerSearch. - Remove tabs from script and template. Adjust some indentation. Test plan: - Test adding, editing and deleting Z3950 servers. - Test searching for a server with the top search box (not table). - Add a server with quotes in the name. Search for it. Edit or delete. Followed tet plan. Patch behaves as expected. Signed-off-by: Marc Veron Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi --- admin/z3950servers.pl | 303 +++++-------- .../prog/en/modules/admin/z3950servers.tt | 397 ++++++------------ 2 files changed, 237 insertions(+), 463 deletions(-) diff --git a/admin/z3950servers.pl b/admin/z3950servers.pl index 1dbd1189ce..113f2f8a47 100755 --- a/admin/z3950servers.pl +++ b/admin/z3950servers.pl @@ -1,221 +1,112 @@ #!/usr/bin/perl -#script to administer the branches table -#written 20/02/2002 by paul.poulain@free.fr -# This software is placed under the gnu General Public License, v2 (http://www.gnu.org/licenses/gpl.html) - -# ALGO : -# this script use ano $op to know what to do. -# if $op is empty or none of the above values, -# - the default screen is build (with all records, or filtered datas). -# - the user can clic on add, modify or delete record. -# if $op=add_form -# - if primkey exists, this is a modification,so we read the $primkey record -# - builds the add/modify form -# if $op=add_validate -# - the user has just send datas, so we create/modify the record -# if $op=delete_form -# - we show the record having primkey=$primkey and ask for deletion validation form -# if $op=delete_confirm -# - we delete the record having primkey=$primkey - -use strict; -use warnings; +# Copyright 2002 paul.poulain@free.fr +# Copyright 2014 Rijksmuseum +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + + +# This script is used to maintain the Z3950 servers table. +# Parameter $op is operation: list, new, edit, add_validated, delete_confirmed. +# add_validated saves a validated record and goes to list view. +# delete_confirmed deletes a record and goes to list view. + +use Modern::Perl; use CGI; use C4::Context; use C4::Auth; use C4::Output; +use Koha::Database; -sub StringSearch { - my ($searchstring,$type)=@_; - my $dbh = C4::Context->dbh; - my @data = ('%'); - my $count = 1; - if ( defined $searchstring ) { - $searchstring =~ s/\'/\\\'/g; - @data=split(' ',$searchstring); - $count=@data; - } - else { - $searchstring = ''; - } - - my $query = "SELECT host,port,db,userid,password,name,id,checked,rank,syntax,encoding,timeout,recordtype"; - $query .= " FROM z3950servers"; - if ( $searchstring ne '' ) { $query .= " WHERE (name like ?)" } - $query .= " ORDER BY rank,name"; - my $sth=$dbh->prepare($query); - - if ( $searchstring ne '' ) { - $sth->execute("$data[0]\%"); - } - else { - $sth->execute; - } - - my @results; - while (my $data=$sth->fetchrow_hashref) { - push(@results,$data); - } - $sth->finish; - return (scalar(@results),\@results); -} +# Initialize CGI, template, database my $input = new CGI; -my $searchfield=$input->param('searchfield'); -my $offset=$input->param('offset') || 0; -my $script_name="/cgi-bin/koha/admin/z3950servers.pl"; - -my $pagesize=20; -my $op = $input->param('op') || ''; - -my ($template, $loggedinuser, $cookie) - = get_template_and_user({template_name => "admin/z3950servers.tmpl", - query => $input, - type => "intranet", - authnotrequired => 0, - flagsrequired => {parameters => 'parameters_remaining_permissions'}, - debug => 1, - }); - - -$template->param(script_name => $script_name, - searchfield => $searchfield); - - -################## 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 ($searchfield) { - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare( -"select host,port,db,userid,password,name,id,checked,rank,syntax,encoding,timeout,recordtype from z3950servers where (name = ?) order by rank,name" - ); - $sth->execute($searchfield); - $data = $sth->fetchrow_hashref; - $sth->finish; +my $op = $input->param('op') || 'list'; +my $id = $input->param('id') || 0; +my $searchfield = ''; + +my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { + template_name => "admin/z3950servers.tmpl", + query => $input, + type => "intranet", + authnotrequired => 0, + flagsrequired => {parameters => 'parameters_remaining_permissions'}, + debug => 1, +}); +my $script_name = "/cgi-bin/koha/admin/z3950servers.pl"; +$template->param( script_name => $script_name ); + +my $schema = Koha::Database->new()->schema(); + +# Main code +# First process a confirmed delete, or save a validated record + +if( $op eq 'delete_confirmed' && $id ) { + my $rs=$schema->resultset('Z3950server')->search( { id => $id } ); + my $name= $rs->first?$rs->first->name:''; + my $cnt=$rs->delete; + $template->param( msg_deleted => 1, msg_add => $name ) if $cnt==1; + $id=0; +} elsif( $op eq 'add_validated' ) { + my @fields=qw/host port db userid password rank syntax encoding timeout + recordtype checked/; + my $formdata = _form_data_hashref( $input, \@fields ); + #add name from servername (an input with name="name" gave problems) + $formdata->{name} = $input->param('servername'); + if( $id ) { + my @res= $schema->resultset('Z3950server')->search( { id => $id } ); + $res[0]->update( $formdata ); + $template->param( msg_updated => 1, msg_add => $formdata->{name} ); + $id=0; + } else { + $schema->resultset('Z3950server')->create( $formdata ); + $template->param( msg_added => 1, msg_add => $formdata->{name} ); } - $template->param( $_ => $data->{$_} ) - for (qw( host port db userid password checked rank timeout encoding )); - $template->param( $_ . $data->{$_} => 1 ) for (qw( syntax recordtype )); - - # END $OP eq ADD_FORM -################## ADD_VALIDATE ################################## - # called by add_form, used to insert/modify data in DB -} elsif ($op eq 'add_validate') { - my $dbh=C4::Context->dbh; - my $sth=$dbh->prepare("select * from z3950servers where name=?"); - $sth->execute($input->param('searchfield')); - my $checked = $input->param('checked') ? 1 : 0; - if ($sth->rows) { - $template->param(confirm_update => 1); - $sth=$dbh->prepare("update z3950servers set host=?, port=?, db=?, userid=?, password=?, name=?, checked=?, rank=?,syntax=?,encoding=?,timeout=?,recordtype=? where name=?"); - $sth->execute($input->param('host'), - $input->param('port'), - $input->param('db'), - $input->param('userid'), - $input->param('password'), - $input->param('searchfield'), - $checked, - $input->param('rank'), - $input->param('syntax'), - $input->param('encoding'), - $input->param('timeout'), - $input->param('recordtype'), - $input->param('searchfield'), - ); - } - else { - $template->param(confirm_add => 1); - $sth=$dbh->prepare( - "INSERT INTO z3950servers " . - "(host,port,db,userid,password,name,checked,rank,syntax,encoding,timeout,recordtype) " . - "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)" ); - $sth->execute( - $input->param('host'), $input->param('port'), - $input->param('db'), $input->param('userid'), - $input->param('password'), $input->param('searchfield'), - $checked, $input->param('rank'), - $input->param('syntax'), $input->param('encoding'), - $input->param('timeout'), $input->param('recordtype') - ); - } - $sth->finish; +} else { + #use searchfield only in remaining operations + $searchfield = $input->param('searchfield') || ''; +} - # END $OP eq ADD_VALIDATE -################## 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 $dbh = C4::Context->dbh; +# Now list multiple records, or edit one record + +my $data = []; +if ( $op eq 'add' || $op eq 'edit' ) { + $data = ServerSearch( $schema, $id, $searchfield ) if $searchfield || $id; + delete $data->[0]->{id} if @$data && $op eq 'add'; #cloning record + $template->param( add_form => 1, server => @$data? $data->[0]: undef, + op => $op ); +} else { + $data = ServerSearch( $schema, $id, $searchfield ); + $template->param( loop => \@$data, searchfield => $searchfield, id => $id, + op => 'list' ); +} +output_html_with_http_headers $input, $cookie, $template->output; - my $sth2 = $dbh->prepare( -"select host,port,db,userid,password,name,id,checked,rank,syntax,encoding,timeout,recordtype from z3950servers where (name = ?) order by rank,name" - ); - $sth2->execute($searchfield); - my $data = $sth2->fetchrow_hashref; - $sth2->finish; +# End of main code - $template->param( - host => $data->{'host'}, - port => $data->{'port'}, - db => $data->{'db'}, - userid => $data->{'userid'}, - password => $data->{'password'}, - checked => $data->{'checked'}, - rank => $data->{'rank'}, - syntax => $data->{'syntax'}, - timeout => $data->{'timeout'}, - recordtype => $data->{'recordtype'}, - encoding => $data->{'encoding'} +sub ServerSearch { #find server(s) by id or name + my ( $schema, $id, $searchstring )= @_; + my @resobjs= $schema->resultset('Z3950server')->search( + $id ? { id => $id }: { name => { like => $searchstring.'%' } }, + { order_by => 'rank,name' }, ); + return [ map { {$_->get_columns} } @resobjs ]; +} - # END $OP eq DELETE_CONFIRM -################## DELETE_CONFIRMED ################################## -# called by delete_confirm, used to effectively confirm deletion of data in DB -} elsif ($op eq 'delete_confirmed') { - $template->param(delete_confirmed => 1); - my $dbh=C4::Context->dbh; - my $sth=$dbh->prepare("delete from z3950servers where name=?"); - $sth->execute($searchfield); - $sth->finish; - # END $OP eq DELETE_CONFIRMED -################## DEFAULT ################################## -} else { # DEFAULT - $template->param(else => 1); - my ($count,$results)=StringSearch($searchfield,'web'); - my @loop; - - for (my $i=$offset; $i < ($offset+$pagesize<$count?$offset+$pagesize:$count); $i++){ - my $urlsearchfield=$results->[$i]{name}; - $urlsearchfield=~s/ /%20/g; - my %row = ( name => $results->[$i]{'name'}, - host => $results->[$i]{'host'}, - port => $results->[$i]{'port'}, - db => $results->[$i]{'db'}, - userid =>$results->[$i]{'userid'}, - password => ($results->[$i]{'password'}) ? ('#######') : (' '), - checked => $results->[$i]{'checked'}, - rank => $results->[$i]{'rank'}, - syntax => $results->[$i]{'syntax'}, - encoding => $results->[$i]{'encoding'}, - timeout => $results->[$i]{'timeout'}, - recordtype => $results->[$i]{'recordtype'}); - push @loop, \%row; - - } - $template->param(loop => \@loop); - if ($offset>0) { - $template->param(offsetgtzero => 1, - prevpage => $offset-$pagesize); - } - if ($offset+$pagesize<$count) { - $template->param(ltcount => 1, - nextpage => $offset+$pagesize); - } -} #---- END $OP eq DEFAULT -output_html_with_http_headers $input, $cookie, $template->output; +sub _form_data_hashref { + my ( $input, $fieldref ) = @_; + return { map { ( $_ => $input->param($_) ) } @$fieldref }; +} diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/z3950servers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/z3950servers.tt index 1139504089..9fcee14197 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/z3950servers.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/z3950servers.tt @@ -1,22 +1,28 @@ [% INCLUDE 'doc-head-open.inc' %] -Koha › Administration › [% IF ( else ) %]Z39.50 servers[% END %] -[% IF ( add_form ) %] Z39.50 servers › [% IF ( searchfield ) %]Modify Z39.50 server [% searchfield %][% ELSE %]New Z39.50 server[% END %][% END %] -[% IF ( delete_confirm ) %]Z39.50 servers › Confirm deletion[% END %] -[% IF ( confirm_add ) %] Z39.50 servers › Z39.50 server added[% END %] -[% IF ( confirm_update ) %] Z39.50 servers › Z39.50 server updated[% END %] -[% IF ( delete_confirmed ) %]Z39.50 servers › Z39.50 server deleted[% END %] +Koha › Administration › Z39.50 servers +[% IF op == 'edit' %] › Modify Z39.50 server [% server.name %][% END %] +[% IF op == 'add' %] › New Z39.50 server[% END %] + [% INCLUDE 'doc-head-close.inc' %] -[% IF ( else ) %] - -[% INCLUDE 'datatables.inc' %] + +[% IF op == 'list' %] + + [% INCLUDE 'datatables.inc' %] [% END %] -[% IF ( add_form ) %] + -[% END %] -[% IF ( else ) %] - + })); + }); + function ConfirmDelete(name,id) { + if( confirm( _("Choose OK if you really want to delete server ")+ + name+'.')) { + window.location="[% script_name %]?op=delete_confirmed&id="+id; + } + return false; + } [% END %] +//]]> + [% INCLUDE 'header.inc' %] [% INCLUDE 'z3950-admin-search.inc' %] - +
- -
-
-
+
+
+
+ +[% IF msg_deleted %] +
Z39.50 server deleted ([% msg_add %])
+[% ELSIF msg_updated %] +
Z39.50 server updated ([% msg_add %])
+[% ELSIF msg_added %] +
Z39.50 server added ([% msg_add %])
+[% END %] [% IF ( add_form ) %] - -
- -[% IF ( searchfield ) %] -

Modify Z39.50 server

+ + + [% IF op == 'edit' %] +

Modify Z39.50 server

+ [% ELSE %] -

New Z39.50 server

+

New Z39.50 server

[% END %]
-
    [% IF ( searchfield ) %] -
  1. Z39.50 server: [% searchfield %]
    2. +
      +
    1. Required
      2. + +
    2. Required +
      3. +
    3. Required +
      4. +
    4. Required +
      5. +
    5. +
      6. +
    6. +
      7. +
    7. + [% IF ( server.checked ) %] + [% ELSE %] -
    8. Required
      9. + [% END %] + +
    9. +
      10. + +
    10. + +
      11. -
    11. Required -
      12. -
    12. Required -
      13. -
    13. Required -
      14. -
    14. -
      15. -
    15. -
      16. -
    16. - [% IF ( checked ) %] - - [% ELSE %] - - [% END %] -
      17. -
    17. -
      18. -
    18. - -
      19. - -
    19. - -
      20. -
    20. - seconds -
      21. -
    21. - + [% FOREACH enc IN [ 'utf8' 'EUC-KR' 'ISO_5426' 'ISO_6937' 'ISO_8859-1' 'MARC-8' ] %] + [% END %] -
      22. -
    + + +
  2. + seconds +
    3. +
  3. + +
    4. +
Cancel
-
-[% END %] - -[% IF ( confirm_add ) %] -

Z39.50 server added

-
- -
-[% END %] - -[% IF ( confirm_update ) %] -

Z39.50 server updated

-
- -
-[% END %] - -[% IF ( delete_confirm ) %] - [% reqsel %] -

Confirm deletion of server [% searchfield %]

-
    -
  • Target: [% searchfield %]
    • -
  • Hostname: [% host %]
    • -
  • Port: [% port %]
    • -
  • Database: [% db %]
    • -
  • Userid: [% userid %]
    • -
  • Password: [% password %]
    • -
  • Checked: [% checked %]
    • -
  • Rank: [% rank %]
    • -
  • Syntax: [% syntax %]
    • -
  • Encoding: [% encoding %]
    • -
  • Timeout: [% timeout %]
    • -
  • Record type: [% recordtype %]
    • -
- - - -[% END %] - -[% IF ( delete_confirmed ) %] -

Z39.50 server deleted

-
- -
-[% END %] - -[% IF ( else ) %] - - - -

Z39.50 servers administration

- - [% IF ( searchfield ) %] - You searched for [% searchfield %] + +[% END %] + +[% IF op == 'list' %] + +

Z39.50 servers administration

+ [% IF id %] + You searched for record [% id %] + [% ELSIF searchfield %] + You searched for [% searchfield %] + [% END %] + + + + + [% FOREACH loo IN loop %] + [% UNLESS ( loop.odd ) %] + + [% ELSE %] + + [% END %] + + + + [% END %] -
TargetHostname/PortDatabaseUseridPasswordCheckedRankSyntaxEncodingTimeoutRecord typeOptions
[% loo.name %][% loo.host %]:[% loo.port %][% loo.db %][% loo.userid %][% IF loo.password %]########[% END %][% IF ( loo.checked ) %]Yes[% ELSE %]No[% END %][% loo.rank %][% loo.syntax %][% loo.encoding %][% loo.timeout %][% loo.recordtype %]Edit Copy Delete
- - - [% FOREACH loo IN loop %] - [% UNLESS ( loop.odd ) %] - - [% ELSE %] - - [% END %] - - - - [% END %] -
TargetHostname/PortDatabaseUseridPasswordCheckedRankSyntaxEncodingTimeoutRecord type  
[% loo.name %][% loo.host %]:[% loo.port %][% loo.db %][% loo.userid %][% loo.password %][% IF ( loo.checked ) %]Yes[% ELSE %]No[% END %][% loo.rank %][% loo.syntax %][% loo.encoding %][% loo.timeout %][% loo.recordtype %]EditDelete
- -[% IF ( offsetgtzero ) %]
- - -
[% END %] - -[% IF ( ltcount ) %]
- - -
[% END %] - - + + [% END %]
-- 2.39.5