[Koha-bugs] [Bug 20388] Elasticsearch - Ability to add search fields from UI

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20388

--- Comment #51 from Julian Maurice <julian.maurice at biblibre.com> ---
Comment on attachment 87201
  --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87201
Bug 20388 - Add/delete search fields from search engine configuration page

Review of attachment 87201:
 --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=20388&attachment=87201)
-----------------------------------------------------------------

::: admin/searchengine/elasticsearch/mappings.pl
@@ +217,5 @@
>      my $search_field_unblessed = $search_field->unblessed;
>      $search_field_unblessed->{mapped_biblios} = 1 if $search_field->is_mapped_biblios;
> +
> +    $search_field_unblessed->{is_mapped} = $schema->resultset('SearchMarcToField')->search(
> +        { search_field_id => $search_field->id })->count;

Can be replaced by

    $search_field_unblessed->{is_mapped} = $search_field->is_mapped;

But why is it unblessed in the first place, and not directly passed to template
?

:::
koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt
@@ +64,5 @@
> +            var name = $(line).find('input[data-id="search_field_name"]').val();
> +            var label = $(line).find('input[data-id="search_field_name"]').val();
> +            if ( name.length > 0 && label.length > 0 ) {
> +                var new_line = clone_line( line );
> +                new_line.appendTo($('table[data-index_name=search_fields]>tbody'));

You should probably use the already existing 'table' variable instead of
searching in the whole DOM

@@ +72,5 @@
> +                clean_line(line);
> +
> +                $(table).tableDnD( {
> +                    onDragClass: "dragClass",
> +                } );

This is weird. Drag and Drop is not enabled on this table until you add a new
field.
And it looks like it is useless to move lines around since they are
automatically ordered alphabetically after a save

@@ +242,5 @@
> +                        [% IF search_field.is_mapped %]
> +                          <a class="btn btn-default btn-xs disabled delete" style="cursor: pointer;"><i class="fa fa-trash"></i> Delete</a>
> +                        [% ELSE %]
> +                          <a class="btn btn-default btn-xs delete" style="cursor: pointer;"><i class="fa fa-trash"></i> Delete</a>
> +                        [% END %]

It might be better to use a <button> here to avoid the inline style, and to use
the disabled attribute instead of the disabled class

@@ +244,5 @@
> +                        [% ELSE %]
> +                          <a class="btn btn-default btn-xs delete" style="cursor: pointer;"><i class="fa fa-trash"></i> Delete</a>
> +                        [% END %]
> +                      </td>
> +                      </td>

Double end tag, should be removed

-- 
You are receiving this mail because:
You are watching all bug changes.