Problem/Motivation

The SearchApiPageController currently hard-codes the parse mode to "direct". In combination with search_api_solr this behaviour is not always desired: direct mode means no escaping of values leaving the possibility of user input causing errors in solr.

Proposed resolution

Let's make the parse-mode configurable per page.

User interface changes

A new select list is shown in the "page" fieldset, with options: direct, terms and phrase (the default search API options).

API changes

We leave the default to "direct" causing no backwards incompatible changes.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jsst created an issue. See original summary.

legolasbo’s picture

Status: Active » Needs work
+++ b/src/SearchApiPageInterface.php
@@ -106,4 +106,11 @@ interface SearchApiPageInterface extends ConfigEntityInterface {
+  /**
+   * The parse mode to use for query keywords.
+   *
+   * Can be one of: direct, phrase or terms.
+   */
+  public function getParseMode();

This needs an @return string

marcoweijenborg’s picture

Last patch didn't apply anymore. I created a new patch and added the @return to the comment.

marcoweijenborg’s picture

Status: Needs work » Needs review
legolasbo’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me.

borisson_’s picture

Status: Reviewed & tested by the community » Needs work

I was just about to commit this. We did not add a test for this yet. I'd like to have a test that checks that this behavior works, it does not need to be covering everything, as this is tested really well in Search API proper, but at the very least we should test that the admin form works as expected.

marcoweijenborg’s picture

Attached is a patch which adds the test for the admin form.

marcoweijenborg’s picture

Status: Needs work » Needs review
borisson_’s picture

Status: Needs review » Reviewed & tested by the community

Yup! That looks great.

borisson_’s picture

  1. +++ b/src/Form/SearchApiPageForm.php
    @@ -164,6 +164,19 @@ class SearchApiPageForm extends EntityForm {
    +      '#options' => [
    +        'direct' => $this->t('Direct'),
    +        'terms' => $this->t('Terms'),
    +        'phrase' => $this->t('Phrase'),
    +      ],
    

    We should not do this like that, we should get the
    Parse mode plugin manager (plugin.manager.search_api.parse_mode) and list all the plugins here. That way we can use the correct labels for all of them + we don't have to adapt when a new or custom one is added.

    See \Drupal\search_api\Plugin\views\filter\SearchApiFulltext::buildOptionsForm for inspiration.

  2. +++ b/src/SearchApiPageInterface.php
    @@ -106,4 +106,11 @@ interface SearchApiPageInterface extends ConfigEntityInterface {
    +  /**
    +   * The parse mode to use for query keywords.
    +   *
    +   * Can be one of: direct, phrase or terms.
    +   */
    

    The documentation here should be updated to reflect that as well, and we should add @return string + docs.

marcoweijenborg’s picture

I totally agree with you. I created a new patch for this.

marcoweijenborg’s picture

Status: Reviewed & tested by the community » Needs review

Status: Needs review » Needs work

The last submitted patch, 12: 3056363-12.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

borisson_’s picture

+++ b/tests/src/Functional/ParseModeTest.php
@@ -0,0 +1,64 @@
+    foreach ($instances as $name => $instance) {
+      $assert_session->pageTextContains($name);
+    }

pageTextContains looks at the visible html, so we either use responseContains that looks are raw html OR use the label here as well.

The update also introduced some coding standards issues, those should be fixed as well.

marcoweijenborg’s picture

Thanks for your help :) I addressed the issues in the next patch.

marcoweijenborg’s picture

Status: Needs work » Needs review
borisson_’s picture

Status: Needs review » Fixed

  • borisson_ authored 8a14f5b on 8.x-1.x
    Issue #3056363 by marcoweijenborg, borisson_, jsst: Make parse mode...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.