Bugzilla – Attachment 97227 Details for
Bug 19482
Elasticsearch - prevent removal / editing of required indexes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 19482 - Add support for defining 'mandatory' mappings
Bug-19482---Add-support-for-defining-mandatory-map.patch (text/plain), 22.77 KB, created by
Nick Clemens (kidclamp)
on 2020-01-10 14:50:14 UTC
(
hide
)
Description:
Bug 19482 - Add support for defining 'mandatory' mappings
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2020-01-10 14:50:14 UTC
Size:
22.77 KB
patch
obsolete
>From 1122f17250ab4a4b06f0df97ad112808f805ed82 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >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 <nicolas.legrand@bulac.fr> >--- > Koha/SearchEngine/Elasticsearch.pm | 2 +- > admin/searchengine/elasticsearch/mappings.pl | 9 +- > admin/searchengine/elasticsearch/mappings.yaml | 2 + > .../admin/searchengine/elasticsearch/mappings.tt | 257 ++++++++++++--------- > 4 files changed, 162 insertions(+), 108 deletions(-) > >diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm >index 3d932b6dce..c06fafd9da 100644 >--- a/Koha/SearchEngine/Elasticsearch.pm >+++ b/Koha/SearchEngine/Elasticsearch.pm >@@ -299,7 +299,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 28802f6586..3ded6474e8 100755 >--- a/admin/searchengine/elasticsearch/mappings.pl >+++ b/admin/searchengine/elasticsearch/mappings.pl >@@ -130,6 +130,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]; >@@ -141,6 +144,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, >@@ -154,8 +159,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 { >@@ -235,6 +241,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 e2b17b25f8..10d57d2a40 100644 >--- a/admin/searchengine/elasticsearch/mappings.yaml >+++ b/admin/searchengine/elasticsearch/mappings.yaml >@@ -2215,6 +2215,7 @@ biblios: > sort: 1 > suggestible: '' > type: sum >+ mandatory: 1 > itemnumber: > label: itemnumber > mappings: >@@ -4197,6 +4198,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 e3a207db96..7c8f467bc7 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( { >@@ -85,6 +90,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' %] >@@ -172,52 +179,61 @@ a.add, a.delete { > </thead> > <tbody> > [% FOREACH search_field IN all_search_fields %] >+ [% IF search_field.mandatory %] >+ [% SET is_readonly = "readonly" %] >+ [% ELSE %] >+ [% SET is_readonly = "" %] >+ [% END %] > <tr> > <td> >- <input type="text" name="search_field_name" value="[% search_field.name | html %]" /> >+ <input type="text" name="search_field_name" value="[% search_field.name | html %]" [% is_readonly %]/> > </td> > <td> >- <input type="text" name="search_field_label" value="[% search_field.label | html %]" /> >- </td> >+ <input type="text" name="search_field_label" value="[% search_field.label | html %]" [% is_readonly %]/> > <td> >- <select name="search_field_type"> >- <option value=""></option> >- [% IF search_field.type == "string" %] >- <option value="string" selected="selected">String</option> >- [% ELSE %] >- <option value="string">String</option> >- [% END %] >- [% IF search_field.type == "date" %] >- <option value="date" selected="selected">Date</option> >- [% ELSE %] >- <option value="date">Date</option> >- [% END %] >- [% IF search_field.type == "number" %] >- <option value="number" selected="selected">Number</option> >- [% ELSE %] >- <option value="number">Number</option> >- [% END %] >- [% IF search_field.type == "boolean" %] >- <option value="boolean" selected="selected">Boolean</option> >- [% ELSE %] >- <option value="boolean">Boolean</option> >- [% END %] >- [% IF search_field.type == "sum" %] >- <option value="sum" selected="selected">Sum</option> >- [% ELSE %] >- <option value="sum">Sum</option> >- [% END %] >- [% IF search_field.type == "isbn" %] >- <option value="isbn" selected="selected">ISBN</option> >- [% ELSE %] >- <option value="isbn">ISBN</option> >- [% END %] >- [% IF search_field.type == "stdno" %] >- <option value="stdno" selected="selected">Std. Number</option> >- [% ELSE %] >- <option value="stdno">Std. Number</option> >- [% END %] >- </select> >+ [% IF is_readonly %] >+ <input type="hidden" name="search_field_type" value="[% search_field.type | html %]" /> >+ <select name="search_field_type" disabled> >+ [% ELSE %] >+ <select name="search_field_type"> >+ <option value=""></option> >+ [% IF search_field.type == "string" %] >+ <option value="string" selected="selected">String</option> >+ [% ELSE %] >+ <option value="string">String</option> >+ [% END %] >+ [% IF search_field.type == "date" %] >+ <option value="date" selected="selected">Date</option> >+ [% ELSE %] >+ <option value="date">Date</option> >+ [% END %] >+ [% IF search_field.type == "number" %] >+ <option value="number" selected="selected">Number</option> >+ [% ELSE %] >+ <option value="number">Number</option> >+ [% END %] >+ [% IF search_field.type == "boolean" %] >+ <option value="boolean" selected="selected">Boolean</option> >+ [% ELSE %] >+ <option value="boolean">Boolean</option> >+ [% END %] >+ [% IF search_field.type == "sum" %] >+ <option value="sum" selected="selected">Sum</option> >+ [% ELSE %] >+ <option value="sum">Sum</option> >+ [% END %] >+ [% IF search_field.type == "isbn" %] >+ <option value="isbn" selected="selected">ISBN</option> >+ [% ELSE %] >+ <option value="isbn">ISBN</option> >+ [% END %] >+ [% IF search_field.type == "stdno" %] >+ <option value="stdno" selected="selected">Std. Number</option> >+ [% ELSE %] >+ <option value="stdno">Std. Number</option> >+ [% END %] >+ </select> >+ [% END %] > </td> > <td> > <select name="search_field_staff_client"> >@@ -269,74 +285,103 @@ a.add, a.delete { > </thead> > <tbody> > [% FOREACH mapping IN index.mappings %] >- <tr> >- <td> >- <input type="hidden" name="mapping_index_name" value="[% index.index_name | html %]" /> >- <input type="hidden" name="mapping_search_field_name" value="[% mapping.search_field_name | html %]"> >- [% mapping.search_field_label | html %] >- </td> >- <td> >- <select name="mapping_sort"> >- [% IF mapping.sort == 'undef' %] >- <option value="undef" selected="selected">Undef</option> >- [% ELSE %] >- <option value="undef">Undef</option> >- [% END %] >- [% IF mapping.sort == 0 %] >- <option value="0" selected="selected">0</option> >- [% ELSE %] >- <option value="0">0</option> >- [% END %] >- [% IF mapping.sort == 1 %] >- <option value="1" selected="selected">1</option> >- [% ELSE %] >- <option value="1">1</option> >- [% END %] >- </select> >- </td> >- <td> >- [% IF mapping.is_facetable %] >- <select name="mapping_facet"> >- [% IF mapping.facet %] >- <option value="0">No</option> >- <option value="1" selected="selected">Yes</option> >+ [% IF mapping.search_field_mandatory && mapping.search_field_label != loop.next.search_field_label && mapping.search_field_label != loop.prev.search_field_label %] >+ <tr> >+ <td> >+ <input type="hidden" name="mapping_index_name" value="[% index.index_name | html %]" /> >+ <input type="hidden" name="mapping_search_field_name" value="[% mapping.search_field_name | html %]"> >+ [% mapping.search_field_label | html %] >+ </td> >+ <td> >+ <input type="hidden" name="mapping_sort" value="[% mapping.sort | html %]" readonly />[% IF mapping.sort == 'undef' %]Undef[% ELSE %][% mapping.sort | html %][% END %] >+ </td> >+ <td> >+ <input type="hidden" name="mapping_facet" value="[% mapping.facet | html %]" readonly />[% IF mapping.facet == 1 %]Yes[% ELSE %]No[% END %] >+ </td> >+ <td> >+ <input type="hidden" name="mapping_suggestible" value="[% mapping.suggestible | html %]" readonly />[% IF mapping.suggestible == 1 %]Yes[% ELSE %]No[% END %] >+ </td> >+ <td> >+ <input type="hidden" name="mapping_search" value="[% mapping.search | html %]" readonly />[% IF mapping.search == 1 %]Yes[% ELSE %]No[% END %] >+ </td> >+ <td> >+ <input name="mapping_marc_field" type="text" value="[% mapping.marc_field | html %]" /> >+ </td> >+ </tr> >+ [% ELSE %] >+ <tr> >+ <td> >+ <input type="hidden" name="mapping_index_name" value="[% index.index_name | html %]" /> >+ <input type="hidden" name="mapping_search_field_name" value="[% mapping.search_field_name | html %]"> >+ [% mapping.search_field_label | html %] >+ </td> >+ <td> >+ <select name="mapping_sort"> >+ [% IF mapping.sort == 'undef' %] >+ <option value="undef" selected="selected">Undef</option> >+ [% ELSE %] >+ <option value="undef">Undef</option> >+ [% END %] >+ [% IF mapping.sort == 0 %] >+ <option value="0" selected="selected">0</option> >+ [% ELSE %] >+ <option value="0">0</option> >+ [% END %] >+ [% IF mapping.sort == 1 %] >+ <option value="1" selected="selected">1</option> >+ [% ELSE %] >+ <option value="1">1</option> >+ [% END %] >+ </select> >+ </td> >+ <td> >+ [% IF mapping.is_facetable %] >+ <select name="mapping_facet"> >+ [% IF mapping.facet %] >+ <option value="0">No</option> >+ <option value="1" selected="selected">Yes</option> >+ [% ELSE %] >+ <option value="0" selected="selected">No</option> >+ <option value="1">Yes</option> >+ [% END %] >+ </select> > [% ELSE %] >- <option value="0" selected="selected">No</option> >- <option value="1">Yes</option> >+ <input type="hidden" name="mapping_facet" value="0" /> >+ No > [% END %] >- </select> >- [% ELSE %] >- <input type="hidden" name="mapping_facet" value="0" /> >- No >- [% END %] >- </td> >- <td> >- <select name="mapping_suggestible"> >- [% IF mapping.suggestible %] >- <option value="0">No</option> >- <option value="1" selected="selected">Yes</option> >- [% ELSE %] >- <option value="0" selected="selected">No</option> >- <option value="1">Yes</option> >- [% END %] >- </select> >- </td> >- <td> >- <select name="mapping_search"> >- [% IF mapping.search %] >- <option value="0">No</option> >- <option value="1" selected="selected">Yes</option> >+ </td> >+ <td> >+ <select name="mapping_suggestible"> >+ [% IF mapping.suggestible %] >+ <option value="0">No</option> >+ <option value="1" selected="selected">Yes</option> >+ [% ELSE %] >+ <option value="0" selected="selected">No</option> >+ <option value="1">Yes</option> >+ [% END %] >+ </select> >+ </td> >+ <td> >+ <select name="mapping_search"> >+ [% IF mapping.search %] >+ <option value="0">No</option> >+ <option value="1" selected="selected">Yes</option> >+ [% ELSE %] >+ <option value="0" selected="selected">No</option> >+ <option value="1">Yes</option> >+ [% END %] >+ </select> >+ </td> >+ <td> >+ <input name="mapping_marc_field" type="text" value="[% mapping.marc_field | html %]" /> >+ </td> >+ [% IF mapping.search_field_mandatory %] >+ <td><a class="btn btn-default btn-xs delete mandatory" data-field_name="[% mapping.search_field_name %]" style="cursor: pointer;"><i class="fa fa-trash"></i> Delete</a></td> > [% ELSE %] >- <option value="0" selected="selected">No</option> >- <option value="1">Yes</option> >+ <td><a class="btn btn-default btn-xs delete" style="cursor: pointer;"><i class="fa fa-trash"></i> Delete</a></td> > [% END %] >- </select> >- </td> >- <td> >- <input name="mapping_marc_field" type="text" value="[% mapping.marc_field | html %]" /> >- </td> >- <td><a class="btn btn-default btn-xs delete" style="cursor: pointer;"><i class="fa fa-trash"></i> Delete</a></td> >- </tr> >+ </tr> >+ [% END %] > [% END %] > </tbody> > <tfoot> >-- >2.11.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=97227">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 19482
:
68246
|
68247
|
68248
|
69250
|
72846
|
72847
|
72848
|
72878
|
72879
|
72880
|
77007
|
77008
|
77009
|
77010
|
80817
|
80818
|
80819
|
80820
|
93366
|
93367
|
93368
|
93371
|
93372
|
93373
|
97226
|
97227
|
97228
|
101312
|
101313
|
101314
|
108467
|
108468
|
108469
|
108470
|
108471
|
111923
|
111924
|
111925
|
111926
|
111927
|
111929
|
112456
|
112459
|
112460
|
112461
|
112462
|
112463
|
112464
|
112465