From 0bd14c7e8e1eb56887dcbc687357dabdf2dae2d5 Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Mon, 14 Jul 2014 11:57:53 +0200
Subject: [PATCH] Bug 12265: [QA Follow-up] Replace DBIx search by find

This patch replaces DBIx search by find at two places in z3950servers.pl.
Furthermore, it adds testing on the results of the find method, passing
a not-found-message to the template whenever needed.
The function ServerSearch is adjusted to use HashRefInflator instead of
manually mapping the columns of result objects.
An additional defined-test in _form_data_hashref prevents shifting of
names and values in the result hash.

Test plan:
List all servers. Edit, copy and delete a server.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
---
 admin/z3950servers.pl                         | 29 ++++++++++++-------
 .../prog/en/modules/admin/z3950servers.tt     |  2 ++
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/admin/z3950servers.pl b/admin/z3950servers.pl
index 113f2f8a47..b4011e6763 100755
--- a/admin/z3950servers.pl
+++ b/admin/z3950servers.pl
@@ -55,10 +55,13 @@ my $schema = Koha::Database->new()->schema();
 # 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;
+    my $server=$schema->resultset('Z3950server')->find( { id => $id } );
+    if( $server ) {
+        $server->delete;
+        $template->param( msg_deleted => 1, msg_add => $server->name );
+    } else {
+        $template->param( msg_notfound => 1, msg_add => $id );
+    }
     $id=0;
 } elsif( $op eq 'add_validated' ) {
     my @fields=qw/host port db userid password rank syntax encoding timeout
@@ -67,9 +70,13 @@ if( $op eq 'delete_confirmed' && $id ) {
     #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} );
+        my $server= $schema->resultset('Z3950server')->find( { id => $id } );
+        if( $server ) {
+            $server->update( $formdata );
+            $template->param( msg_updated => 1, msg_add => $formdata->{name} );
+        } else {
+            $template->param( msg_notfound => 1, msg_add => $id );
+        }
         $id=0;
     } else {
         $schema->resultset('Z3950server')->create( $formdata );
@@ -99,14 +106,14 @@ output_html_with_http_headers $input, $cookie, $template->output;
 
 sub ServerSearch  { #find server(s) by id or name
     my ( $schema, $id, $searchstring )= @_;
-    my @resobjs= $schema->resultset('Z3950server')->search(
+    my $rs = $schema->resultset('Z3950server')->search(
         $id ? { id => $id }: { name => { like => $searchstring.'%' } },
-        { order_by => 'rank,name' },
+        { result_class => 'DBIx::Class::ResultClass::HashRefInflator' }
     );
-    return [ map { {$_->get_columns} } @resobjs ];
+    return [ $rs->all ];
 }
 
 sub _form_data_hashref {
     my ( $input, $fieldref ) = @_;
-    return { map { ( $_ => $input->param($_) ) } @$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 9fcee14197..65e3db2cc4 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/z3950servers.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/z3950servers.tt
@@ -76,6 +76,8 @@
     <div class="dialog alert">Z39.50 server updated ([% msg_add %])</div>
 [% ELSIF msg_added %]
     <div class="dialog alert">Z39.50 server added ([% msg_add %])</div>
+[% ELSIF msg_notfound %]
+    <div class="dialog alert">Error: Server with id [% msg_add %] not found</div>
 [% END %]
 
 [% IF ( add_form ) %]
-- 
2.39.5