From 975e06bd7c3e198048c60d4a0f9968ba89079de7 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Wed, 18 Oct 2017 13:06:22 +0000 Subject: [PATCH] Bug 19482: Add support for defining 'mandatory' mappings To test: 1 - Apply patch 2 - ./installer/data/mysql/updatedatabase.pl 3 - Reset ES mapping: Administration->Search engine configuration , button at bottom of page 4 - 'issues' and 'title' mapping under 'search fields' should be mandatory and not editable 5 - On 'Bibliographic records' tab you should not be able to delete the single entry for issues 6 - You should be able to delete 'title' mappings, however, at the final one you should be stopped by javascript 7 - Bonus: force remove the last mapping from the page using developer tools - attempt to save and should be warned of missing mandatory mapping Signed-off-by: Nicolas Legrand Signed-off-by: Bouzid Fergani Signed-off-by: Katrin Fischer Signed-off-by: Jonathan Druart --- Koha/SearchEngine/Elasticsearch.pm | 2 +- admin/searchengine/elasticsearch/mappings.pl | 9 +- .../searchengine/elasticsearch/mappings.yaml | 2 + .../searchengine/elasticsearch/mappings.tt | 267 ++++++++++-------- 4 files changed, 167 insertions(+), 113 deletions(-) diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm index 182551f4af..7aff9e8394 100644 --- a/Koha/SearchEngine/Elasticsearch.pm +++ b/Koha/SearchEngine/Elasticsearch.pm @@ -353,7 +353,7 @@ sub reset_elasticsearch_mappings { while ( my ( $index_name, $fields ) = each %$indexes ) { while ( my ( $field_name, $data ) = each %$fields ) { - my %sf_params = map { $_ => $data->{$_} } grep { exists $data->{$_} } qw/ type label weight staff_client opac facet_order /; + my %sf_params = map { $_ => $data->{$_} } grep { exists $data->{$_} } qw/ type label weight staff_client opac facet_order mandatory/; # Set default values $sf_params{staff_client} //= 1; diff --git a/admin/searchengine/elasticsearch/mappings.pl b/admin/searchengine/elasticsearch/mappings.pl index 88b8341604..e43c7a8516 100755 --- a/admin/searchengine/elasticsearch/mappings.pl +++ b/admin/searchengine/elasticsearch/mappings.pl @@ -132,6 +132,9 @@ if ( $op eq 'edit' ) { my @facetable_fields = Koha::SearchEngine::Elasticsearch->get_facetable_fields(); my @facetable_field_names = map { $_->name } @facetable_fields; + my $mandatory_before = Koha::SearchFields->search({mandatory=>1})->count; + my $mandatory_after = 0; + my %seen_fields; for my $i ( 0 .. scalar(@index_name) - 1 ) { my $index_name = $index_name[$i]; my $search_field_name = $search_field_name[$i]; @@ -143,6 +146,8 @@ if ( $op eq 'edit' ) { my $mapping_search = $mapping_search[$i]; my $search_field = Koha::SearchFields->find({ name => $search_field_name }, { key => 'name' }); + $mandatory_after++ if $search_field->mandatory && !defined $seen_fields{$search_field_name}; + $seen_fields{$search_field_name} = 1; # TODO Check mapping format my $marc_field = Koha::SearchMarcMaps->find_or_create({ index_name => $index_name, @@ -156,8 +161,9 @@ if ( $op eq 'edit' ) { search => $mapping_search }); } + push @messages, { type => 'error', code => 'missing_mandatory_fields' } if $mandatory_after < $mandatory_before; }; - if ($@) { + if ($@ || @messages) { push @messages, { type => 'error', code => 'error_on_update', message => $@, }; $schema->storage->txn_rollback; } else { @@ -236,6 +242,7 @@ for my $index_name (@index_names) { search_field_name => $name, search_field_label => $s->label, search_field_type => $s->type, + search_field_mandatory => $s->mandatory, marc_field => $s->get_column('marc_field'), sort => $s->get_column('sort') // 'undef', # To avoid warnings "Use of uninitialized value in lc" suggestible => $s->get_column('suggestible'), diff --git a/admin/searchengine/elasticsearch/mappings.yaml b/admin/searchengine/elasticsearch/mappings.yaml index eef51c0f13..0a1cd86454 100644 --- a/admin/searchengine/elasticsearch/mappings.yaml +++ b/admin/searchengine/elasticsearch/mappings.yaml @@ -2209,6 +2209,7 @@ biblios: sort: 1 suggestible: '' type: sum + mandatory: 1 itemnumber: label: itemnumber mappings: @@ -4182,6 +4183,7 @@ biblios: sort: ~ suggestible: '' type: string + mandatory: 1 title-abbreviated: label: title-abbreviated mappings: diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt index af4fc597e3..d7a8a88203 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt @@ -29,7 +29,12 @@ $(document).ready(function() { $("#tabs").tabs(); $('.delete').click(function() { - $(this).parents('tr').remove(); + if( $(this).hasClass('mandatory') && $(".mandatory[data-field_name="+$(this).attr('data-field_name')+"]").length < 2 ){ + alert("This field is mandatory and must have at least one mapping"); + return; + } else { + $(this).parents('tr').remove(); + } }); $("table.mappings").tableDnD( { @@ -87,6 +92,8 @@ a.add, a.delete { [% tx("(search field {field_name} with mapping {marc_field}.)", { field_name = m.values.field_name, marc_field = m.values.marc_field }) | html %] [% CASE 'invalid_field_weight' %] [% tx("Invalid field weight '{weight}', must be a positive decimal number.", { weight = m.weight }) | html %] + [% CASE 'missing_mandatory_fields' %] + [% t("You attempted to delete all mappings for a required index, you must leave at least one mapping") | $raw %] [% CASE 'error_on_update_es_mappings' %] [% tx("An error occurred when updating Elasticsearch index mappings: {message}.", { message = m.message }) | html %] [% CASE 'reindex_required' %] @@ -176,57 +183,66 @@ a.add, a.delete { [% FOREACH search_field IN all_search_fields %] + [% IF search_field.mandatory %] + [% SET is_readonly = "readonly" %] + [% ELSE %] + [% SET is_readonly = "" %] + [% END %] - + - - + - + [% IF is_readonly %] + + + + [% IF search_field.type == "string" %] + + [% ELSE %] + + [% END %] + [% IF search_field.type == "date" %] + + [% ELSE %] + + [% END %] + [% IF search_field.type == "year" %] + + [% ELSE %] + + [% END %] + [% IF search_field.type == "number" %] + + [% ELSE %] + + [% END %] + [% IF search_field.type == "boolean" %] + + [% ELSE %] + + [% END %] + [% IF search_field.type == "sum" %] + + [% ELSE %] + + [% END %] + [% IF search_field.type == "isbn" %] + + [% ELSE %] + + [% END %] + [% IF search_field.type == "stdno" %] + + [% ELSE %] + + [% END %] + + [% END %] - - [% mapping.search_field_label | html %] - - - - - - [% IF mapping.is_facetable %] - + + [% mapping.search_field_label | html %] + + + [% IF mapping.sort == 'undef' %]Undef[% ELSE %][% mapping.sort | html %][% END %] + + + [% IF mapping.facet == 1 %]Yes[% ELSE %]No[% END %] + + + [% IF mapping.suggestible == 1 %]Yes[% ELSE %]No[% END %] + + + [% IF mapping.search == 1 %]Yes[% ELSE %]No[% END %] + + + + + + [% ELSE %] + + + + + [% mapping.search_field_label | html %] + + + + + + [% IF mapping.is_facetable %] + [% ELSE %] - - + + No [% END %] - - [% ELSE %] - - No - [% END %] - - - - - - + [% IF mapping.suggestible %] + + + [% ELSE %] + + + [% END %] + + + + + + + + + [% IF mapping.search_field_mandatory %] + Delete [% ELSE %] - - + Delete [% END %] - - - - - - Delete - + + [% END %] [% END %] -- 2.39.5