Bug 12748: (QA followup) fix several tiny QA tools warnings

* admin/searchengine/elasticsearch/mappings.yaml:
    * Fix indentation so QA tools believe it's valid YAML
  * C4::AuthoritiesMarc: Remove tab character
  * C4::Search: Fix POD
  * Koha::BiblioUtils: remove "return undef"
  * Koha::Schema::Result::Search*: rename type to _type
  * Koha::SE::ES::Search:
    * Fix spelling/@_[0]
    * Add my to loop variable
    * Remove unused var $error
    * Add use Modern::Perl
    * Fix undefined variables
  * Koha::ES::Indexer: fix POD
  * Koha::SE::ES::QueryBuilder:
    * Don't define variables in conditionals
    * Fix POD
  * Koha::SE::Zebra::Search:
    * Fix debug warns/POD/barewords
    * Koha::SE::Zebra::Search: add use Modern::Perl
  * kohastructure.sql: remove old, unused elasticsearch_mappings table,
    add new tables
  * labels/label-item-search.pl: remove C4::Dates
  * opac/elasticsearch.pl: remove old file

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

C4/AuthoritiesMarc.pm

@@ -355,7 +355,7 @@ sub CountUsage {
         # Should really be replaced with a real count call, this is a
         # bad way.
         my $searcher = Koha::SearchEngine::Search->new({index => $Koha::SearchEngine::BIBLIOS_INDEX});
-		my ($err,$res,$result) = $searcher->simple_search_compat($query,0,1);
+        my ($err,$res,$result) = $searcher->simple_search_compat($query,0,1);
         if ($err) {
             warn "Error: $err from search $query";
             $result = 0;

C4/Search.pm

@@ -1330,6 +1330,7 @@ Shim function to ease the transition from buildQuery to a new QueryParser.
 This function is called at the beginning of buildQuery, and modifies
 buildQuery's input. If it can handle the input, it returns a query that
 buildQuery will not try to parse.
+
 =cut
 
 sub parseQuery {

Koha/BiblioUtils.pm

@@ -120,7 +120,7 @@ sub get_all_biblios_iterator {
         # Warn and skip bad records, otherwise we break the loop
         while (1) {
             my $row = $rs->next();
-            return undef if !$row;
+            return if !$row;
             my $marc = C4::Biblio::GetMarcBiblio( $row->biblionumber, 1 );
             my $next = eval {
                 __PACKAGE__->new($marc, $row->biblionumber);

Koha/ElasticSearch/Indexer.pm

@@ -41,8 +41,6 @@ Koha::ElasticSearch::Indexer - handles adding new records to the index
 
 =head1 FUNCTIONS
 
-=cut
-
 =head2 $indexer->update_index($biblionums, $records);
 
 C<$biblionums> is an arrayref containing the biblionumbers for the records.
@@ -138,6 +136,7 @@ sub delete_index_background {
     my $self = shift;
     $self->delete_index(@_);
 }
+
 =head2 $indexer->drop_index();
 
 Drops the index from the elasticsearch server. Calling C<update_index>
@@ -206,5 +205,3 @@ __END__
 =item Robin Sheat C<< <robin@catalyst.net.nz> >>
 
 =back
-
-=cut

Koha/SearchEngine/Elasticsearch/QueryBuilder.pm

@@ -134,6 +134,7 @@ to be searched must have been indexed with an appropriate mapping as a
 "phrase" subfield, which pretty much everything has.
 
 =cut
+
 # XXX this isn't really a browse query like we want in the end
 sub build_browse_query {
     my ( $self, $field, $query ) = @_;
@@ -224,14 +225,17 @@ sub build_query_compat {
 
     #die Dumper($query);
     # We roughly emulate the CGI parameters of the zebra query builder
-    my $query_cgi = 'idx=kw&q=' . uri_escape_utf8( $operands->[0] ) if @$operands;
-    my $simple_query = $operands->[0] if @$operands == 1;
+    my $query_cgi;
+    $query_cgi = 'idx=kw&q=' . uri_escape_utf8( $operands->[0] ) if @$operands;
+    my $simple_query;
+    $simple_query = $operands->[0] if @$operands == 1;
     my $query_desc   = $simple_query;
     my $limit        = $self->_join_queries( $self->_convert_index_strings(@$limits));
     my $limit_cgi = ( $orig_limits and @$orig_limits )
       ? '&limit=' . join( '&limit=', map { uri_escape_utf8($_) } @$orig_limits )
       : '';
-    my $limit_desc = "$limit" if $limit;
+    my $limit_desc;
+    $limit_desc = "$limit" if $limit;
     return (
         undef,  $query,     $simple_query, $query_cgi, $query_desc,
         $limit, $limit_cgi, $limit_desc,   undef,      undef
@@ -311,8 +315,8 @@ sub build_authorities_query {
 
     # We need to add '.phrase' to all the sort headings otherwise it'll sort
     # based on the tokenised form.
+    my %s;
     if ( exists $search->{sort} ) {
-        my %s;
         foreach my $k ( keys %{ $search->{sort} } ) {
             my $f = $self->_sort_field($k);
             $s{"$f.phrase"} = $search->{sort}{$k};
@@ -321,7 +325,8 @@ sub build_authorities_query {
     }
 
     # extract the sort stuff
-    my %sort = ( sort => [ $search->{sort} ] ) if exists $search->{sort};
+    my %sort;
+    %sort = ( sort => [ $search->{sort} ] ) if exists $search->{sort};
     my $query;
     if (@filter_parts) {
         $query =
@@ -374,11 +379,11 @@ truncation). If left blank, then word list, right truncted, anywhere is used.
 
 The actual user-provided string value to search for.
 
-=authtypecode
+=item authtypecode
 
 The authority type code to search within. If blank, then all will be searched.
 
-=orderby
+=item orderby
 
 The order to sort the results by. Options are Relevance, HeadingAsc,
 HeadingDsc, AuthidAsc, AuthidDsc.

Koha/SearchEngine/Elasticsearch/Search.pm

@@ -38,6 +38,8 @@ Koha::SearchEngine::ElasticSearch::Search - search functions for Elasticsearch
 
 =cut
 
+use Modern::Perl;
+
 use base qw(Koha::ElasticSearch);
 use C4::Context;
 use Koha::ItemTypes;
@@ -93,7 +95,6 @@ sub search {
             %$params,
         )
     ) unless $self->store;
-    my $error;
     my $results = eval {
         $self->store->bag->search( %$query, %paging );
     };
@@ -108,7 +109,7 @@ sub search {
     my $count = $searcher->count($query);
 
 This mimics a search request, but just gets the result count instead. That's
-faster than pulling all the data in, ususally.
+faster than pulling all the data in, usually.
 
 =cut
 
@@ -157,7 +158,7 @@ sub search_compat {
         # right place in the array, according to $offset
     $results->each(sub {
             # The results come in an array for some reason
-            my $marc_json = @_[0]->{record};
+            my $marc_json = $_[0]->{record};
             my $marc = $self->json2marc($marc_json);
             $records[$index++] = $marc;
         });
@@ -191,7 +192,7 @@ sub search_auth_compat {
     $res->each(
         sub {
             my %result;
-            my $record    = @_[0];
+            my $record    = $_[0];
             my $marc_json = $record->{record};
 
             # I wonder if these should be real values defined in the mapping
@@ -333,7 +334,7 @@ sub simple_search_compat {
     my @records;
     $results->each(sub {
             # The results come in an array for some reason
-            my $marc_json = @_[0]->{record};
+            my $marc_json = $_[0]->{record};
             my $marc = $self->json2marc($marc_json);
             push @records, $marc;
         });
@@ -358,7 +359,7 @@ sub json2marc {
     # fields are like:
     # [ '245', '1', '2', 'a' => 'Title', 'b' => 'Subtitle' ]
     # conveniently, this is the form that MARC::Field->new() likes
-    foreach $field (@$marcjson) {
+    foreach my $field (@$marcjson) {
         next if @$field < 5;    # Shouldn't be possible, but...
         if ( $field->[0] eq 'LDR' ) {
             $marc->leader( $field->[4] );
@@ -387,7 +388,7 @@ than just 5 like normal.
 sub _convert_facets {
     my ( $self, $es, $exp_facet ) = @_;
 
-    return undef if !$es;
+    return if !$es;
 
     # These should correspond to the ES field names, as opposed to the CCL
     # things that zebra uses.
@@ -412,7 +413,7 @@ sub _convert_facets {
     );
     my @facets;
     $exp_facet //= '';
-    while ( ( $type, $data ) = each %$es ) {
+    while ( my ( $type, $data ) = each %$es ) {
         next if !exists( $type_to_label{$type} );
 
         # We restrict to the most popular $limit !results
@@ -430,6 +431,7 @@ sub _convert_facets {
         foreach my $term ( @{ $data->{terms} }[ 0 .. $limit - 1 ] ) {
             my $t = $term->{term};
             my $c = $term->{count};
+            my $label;
             if ( exists( $special{$type} ) ) {
                 $label = $special{$type}->{$t} // $t;
             }

Koha/SearchEngine/Zebra/Search.pm

@@ -21,11 +21,23 @@ package Koha::SearchEngine::Zebra::Search;
 #use Moose::Role;
 #with 'Koha::SearchEngine::SearchRole';
 
+use Modern::Perl;
+
 use base qw(Class::Accessor);
 
 use C4::Search; # :(
 use C4::AuthoritiesMarc;
 
+=head1 NAME
+
+Koha::SearchEngine::Zebra::Search - Search implementation for Zebra
+
+=head1 METHODS
+
+=head2 search
+
+=cut
+
 sub search {
     my ($self,$query_string) = @_;
 
@@ -35,15 +47,12 @@ sub search {
        query => $query_string,
      );
 
-    warn "search for $query_string";
-
     my $results = $self->searchengine->search($query);
 
     foreach my $item (@{ $results->items }) {
         my $title = $item->get_value('ste_title');
         #utf8::encode($title);
         print "$title\n";
-                warn dump $title;
     }
 }
 
@@ -71,7 +80,7 @@ sub simple_search_compat {
     return C4::Search::SimpleSearch(@_);
 }
 
-=head search_auth_compat
+=head2 search_auth_compat
 
 This passes the search query on to C4::AuthoritiesMarc::SearchAuthorities
 
@@ -81,9 +90,9 @@ sub search_auth_compat {
     my ( $self, $q, $startfrom, $resperpage ) = @_;
 
     my @params = (
-        @{$q}{ marclist, and_or, excluding, operator, value },
+        @{$q}{ 'marclist', 'and_or', 'excluding', 'operator', 'value' },
         $startfrom - 1,
-        $resperpage, @{$q}{ authtypecode, orderby }
+        $resperpage, @{$q}{ 'authtypecode', 'orderby' }
     );
     C4::AuthoritiesMarc::SearchAuthorities(@params);
 }

Koha/SearchField.pm

@@ -42,7 +42,7 @@ sub add_to_search_marc_maps {
 
 =cut
 
-sub type {
+sub _type {
     return 'SearchField';
 }
 

Koha/SearchFields.pm

@@ -39,7 +39,7 @@ Koha::SearchFields - Koha SearchField Object set class
 
 =cut
 
-sub type {
+sub _type {
     return 'SearchField';
 }
 

Koha/SearchMarcMap.pm

@@ -43,7 +43,7 @@ sub add_to_search_fields {
 
 =cut
 
-sub type {
+sub _type {
     return 'SearchMarcMap';
 }
 

Koha/SearchMarcMaps.pm

@@ -39,7 +39,7 @@ Koha::SearchMarcMaps - Koha SearchMarcMap Object set class
 
 =cut
 
-sub type {
+sub _type {
     return 'SearchMarcMap';
 }
 

installer/data/mysql/kohastructure.sql

@@ -981,21 +981,6 @@ CREATE TABLE `deleteditems` (
   KEY `itype_idx` (`itype`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
---
--- Table structure for table `elasticsearch_mapping`
---
-
-DROP TABLE IF EXISTS `elasticsearch_mapping`;
-CREATE TABLE `elasticsearch_mapping` (
-  `id` int(11) NOT NULL AUTO_INCREMENT,
-  `mapping` varchar(255) DEFAULT NULL,
-  `type` varchar(255) DEFAULT NULL,
-  `marc21` varchar(255) DEFAULT NULL,
-  `unimarc` varchar(255) DEFAULT NULL,
-  `normarc` varchar(255) DEFAULT NULL,
-  PRIMARY KEY (`id`)
-) ENGINE=InnoDB AUTO_INCREMENT=24 DEFAULT CHARSET=utf8;
-
 --
 -- Table structure for table `export_format`
 --
@@ -1961,6 +1946,19 @@ CREATE TABLE saved_reports (
    PRIMARY KEY  (`id`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
+--
+-- Table structure for table 'search_field'
+--
+
+DROP TABLE IF EXISTS search_field;
+CREATE TABLE `search_field` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  `name` varchar(255) NOT NULL COMMENT 'the name of the field as it will be stored in the search engine',
+  `label` varchar(255) NOT NULL COMMENT 'the human readable name of the field, for display',
+  `type` ENUM('string', 'date', 'number', 'boolean', 'sum') NOT NULL COMMENT 'what type of data this holds, relevant when storing it in the search engine',
+  PRIMARY KEY (`id`),
+  UNIQUE KEY (`name`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 --
 -- Table structure for table `search_history`
@@ -1981,6 +1979,36 @@ CREATE TABLE IF NOT EXISTS `search_history` ( -- patron's opac search history
   PRIMARY KEY  (`id`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci COMMENT='Opac search history results';
 
+--
+-- Table structure for table 'search_marc_map'
+--
+
+DROP TABLE IF EXISTS search_marc_map;
+CREATE TABLE `search_marc_map` (
+  id int(11) NOT NULL AUTO_INCREMENT,
+  index_name ENUM('biblios','authorities') NOT NULL COMMENT 'what storage index this map is for',
+  marc_type ENUM('marc21', 'unimarc', 'normarc') NOT NULL COMMENT 'what MARC type this map is for',
+  marc_field VARCHAR(255) NOT NULL COMMENT 'the MARC specifier for this field',
+  PRIMARY KEY(`id`),
+  UNIQUE key(index_name, marc_field, marc_type),
+  INDEX (`index_name`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+
+--
+-- Table structure for table 'search_marc_to_field'
+--
+
+DROP TABLE IF EXISTS search_marc_to_field;
+CREATE TABLE `search_marc_to_field` (
+  search_marc_map_id int(11) NOT NULL,
+  search_field_id int(11) NOT NULL,
+  facet boolean DEFAULT FALSE COMMENT 'true if a facet field should be generated for this',
+  suggestible boolean DEFAULT FALSE COMMENT 'true if this field can be used to generate suggestions for browse',
+  sort boolean DEFAULT NULL COMMENT 'true/false creates special sort handling, null doesn''t',
+  PRIMARY KEY(search_marc_map_id, search_field_id),
+  FOREIGN KEY(search_marc_map_id) REFERENCES search_marc_map(id) ON DELETE CASCADE ON UPDATE CASCADE,
+  FOREIGN KEY(search_field_id) REFERENCES search_field(id) ON DELETE CASCADE ON UPDATE CASCADE
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 --
 -- Table structure for table `serial`

labels/label-item-search.pl

@@ -29,7 +29,6 @@ use C4::Auth qw(get_template_and_user);
 use C4::Output qw(output_html_with_http_headers);
 use C4::Context;
 use C4::Search qw(SimpleSearch);
-use C4::Dates;
 use C4::Biblio qw(TransformMarcToKoha);
 use C4::Items qw(GetItemInfosOf get_itemnumbers_of);
 use C4::Koha qw(GetItemTypes);

opac/elasticsearch.pl

@@ -1,102 +0,0 @@
-#!/usr/bin/perl
-
-# Copyright 2013 Catalyst
-#
-# 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, write to the Free Software Foundation, Inc.,
-# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
-
-use Modern::Perl;
-
-use C4::Context;
-use CGI;
-use C4::Auth;
-use C4::Koha;
-use C4::Output;
-
-# TODO this should use the moose thing that auto-picks.
-use Koha::SearchEngine::Elasticsearch::QueryBuilder;
-use Koha::ElasticSearch::Search;
-
-my $cgi = new CGI;
-
-my $template_name;
-my $template_type = "basic";
-if ( $cgi->param("idx") or $cgi->param("q") ) {
-    $template_name = 'search/results.tt';
-}
-else {
-    $template_name = 'search/advsearch.tt';
-    $template_type = 'advsearch';
-}
-
-# load the template
-my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
-    {
-        template_name   => $template_name,
-        query           => $cgi,
-        type            => "opac",
-        authnotrequired => 1,
-    }
-);
-my %template_params;
-my $format = $cgi->param("format") || 'html';
-
-# load the Type stuff
-my $itemtypes = GetItemTypes;
-
-my $page = $cgi->param("page") || 1;
-my $count =
-     $cgi->param('count')
-  || C4::Context->preference('OPACnumSearchResults')
-  || 20;
-my $q = $cgi->param("q");
-
-my $searcher = Koha::ElasticSearch::Search->new();
-my $builder = Koha::SearchEngine::Elasticsearch::QueryBuilder->new();
-my $query;
-if ($cgi->param('type') eq 'browse') {
-    $query = $builder->build_browse_query($cgi->param('browse_field') || undef, $q );
-    $template_params{browse} = 1;
-} else {
-    $query = $builder->build_query($q);
-}
-my $results = $searcher->search( $query, $page, $count );
-#my $results = $searcher->search( { "match_phrase_prefix" => { "title" => "the" } } );
-
-# This is temporary, but will do the job for now.
-my @hits;
-$results->each(sub {
-        push @hits, { _source => @_[0] };
-    });
-# Make a list of the page numbers
-my @pages = map { { page => $_, current => ($_ == ( $page || 1)) } } 1 .. int($results->total / $count);
-my $max_page = int($results->total / $count);
-# Pager template params
-$template->param(
-    SEARCH_RESULTS  => \@hits,
-    PAGE_NUMBERS    => \@pages,
-    total           => $results->total,
-    previous_page   => ( $page > 1 ? $page - 1 : undef ),
-    next_page       => ( $page < $max_page ? $page + 1 : undef ),
-    follower_params => [
-        { var => 'type',  val => $cgi->param('type') },
-        { var => 'q',     val => $q },
-        { var => 'count', val => $count },
-    ],
-    %template_params,
-);
-
-my $content_type = ( $format eq 'rss' or $format eq 'atom' ) ? $format : 'html';
-output_with_http_headers $cgi, $cookie, $template->output, $content_type;