Custom filter groups on a View (Search API index view) are not working

Created on 21 November 2022, almost 2 years ago
Updated 29 August 2023, about 1 year ago

Problem/Motivation

After adding custom filter groups on a view, the search stops working.

Error: Call to undefined method Drupal\search_api\Query\ConditionGroup::getField() in Drupal\search_api_meilisearch\Plugin\search_api\backend\SearchApiMeiliSearchBackend->extractConditions() (line 557 of modules/contrib/search_api_meilisearch/src/Plugin/search_api/backend/SearchApiMeiliSearchBackend.php).

Steps to reproduce

1. Create a new Search API index view,
2. add some filters and expose them then select "And/or rearrange",
3. 2-3 custom groups and select "Apply",
4. clear cache,
5. try search on a page.

🐛 Bug report
Status

Fixed

Version

1.0

Component

User interface

Created by

🇸🇮Slovenia nmatja

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • First commit to issue fork.
  • Status changed to Needs review over 1 year ago
  • 🇦🇹Austria tgoeg

    Please see my related comment in https://www.drupal.org/project/search_api_meilisearch/issues/3325573#com... 🐛 Call to undefined method Drupal\search_api\Query\Condition::getConditions() Closed: duplicate
    I think my commit fixes all condition group nesting issues in a generalized way.
    Unsure whether this also fixes all issues here at hand (as I don't currently see this), but at least it does so for this very much related aspect.

  • First commit to issue fork.
  • 🇸🇮Slovenia DeaOm

    Updated the code so it works with groups (when adding the groups for testing purposes, keep in mind that current version of meilisearch that is supported on this branch has a limitation of filters depth of 2 levels). This of course needs testing to confirm it's actually working and producing correct search results.

  • 🇸🇮Slovenia DeaOm

    Added test to check if conversion from condition groups/condition queries matches the expected array that is needed for meilisearch filters to work.

  • Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    Waiting for branch to pass
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    2 fail
  • Status changed to Needs work over 1 year ago
  • 🇸🇮Slovenia bcizej

    Two things I would like to point out here:

    1. I'd like to wait until 📌 Providing tests Fixed lands so the tests have the correct configurations and dependencies
    2. This issue has implemented filtering through array expressions. This represents are problem with deeply nested conditions as array expressions have a max depth of 2.
      https://www.meilisearch.com/docs/learn/fine_tuning_results/filtering#cre...

      I did some manual testing via Postman and Meilisearch throws an error when trying to use array expression with more than 2 depths, however string expressions have no such issues and I was able to pass deeply nested conditions with proper results.
      https://www.meilisearch.com/docs/learn/fine_tuning_results/filtering#cre...

      While the views UI allow you to only build conditions with a depth of 1 it is possible to deeply nest the conditions via alter hooks. So the service and tests will need to be rewritten to parse filter conditions as a string expression.
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    35 pass
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    43 pass
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    43 pass
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    87 pass, 2 fail
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    92 pass
  • Status changed to Needs review about 1 year ago
  • 🇸🇮Slovenia bcizej

    Now that 📌 Providing tests Fixed is merged I was able to push this forward:

    1. Filters are now parsed as string expressions. Conditions are traversed recursively and grouped correctly. Automated tests are provided.
    2. Custom plugin @MeilisearchConditionParser is implemented for parsing conditions and operators. This way we avoid bloating the string parser with switch statements and can implement more in the future if needed.
    3. All the common filter operators are now supported. 6 custom plugins are implemented that properly translate search api operators and conditions to Meilisearch v0.28.1 filter syntax. Automated tests are provided for each plugin as well.
    4. Added some tests via search api query execution .
    5. Fixed config schema issues detected during tests.
    6. Fixed backend plugin not waiting for index to be created.

    One thing to note is that using an exposed string filter with "contains" or "does not contain" does not work correctly as Meilisearch does not support filtering by partial matching - only exact matching works. Searching works by partially matching but it is limited as we cannot search one word in specific field and another word in another field.

    For example we cannot perform a search on the phrase "mytitle mybody" and have Meilisearch match "mytitle" in the "title" field and "mybody" in the "body" field. We can only specify the entire search phrase to be searched on multiple fields meaning "mytitle" could be found in "body" title and vice versa.

    What is not included here is language filtering but that can be implemented in a separate issue.

    I have tried to cover as many scenarios as I could think of (read "until I got bored of writing them") in the tests but I'm sure there more to be added.

    Please test and in case of any issues, errors etc. describe the steps to reproduce and we can add automated tests.

  • 🇸🇮Slovenia bcizej

    Upon thinking maybe the plugin system is not the most ideal way to deal with parsing conditions, Using Serialization API would probably be a more appropriate by creating a serializer that converts a query object to a Meilisearch filter string. In the meanwhile this PR can still be tested.

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    92 pass
  • 🇸🇮Slovenia bcizej

    I did some experimenting with the serializer approach but in the end I decided to go with the middle ground with service tags. I've also did some optimizations so that parenthesis are wrapping only the necessary conditions according to the logical operator precedence.

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    90 pass, 2 fail
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    87 pass, 4 fail
  • 🇸🇮Slovenia DeaOm

    When manually testing the search results there was an issue with the numeric fields as they were not providing results (less then, equal to etc). This is due to the casting to string that was removed already in the sort issue, but for tests to pass needed to be removed here as well. The setSettings for filterable attributes was moved to update and insert hooks and then to add/update Index, so it only gets triggered when fields are added/removed and not on search.

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    90 pass, 1 fail
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    90 pass, 1 fail
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    96 pass
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    88 pass, 1 fail
  • 🇸🇮Slovenia bcizej

    Did some code cleanup and added few more tests.

    During tests I found another issue with null filtering. When we are adding documents to the index we are not adding a field to the document if it's value is empty so we have no way to filter on null values. The latest meilisearch version supports IS NULL, IS EMPTY, EXISTS operators but 0.28.1 does not have such operators and filtering on empty strings is also ignored. I've added a todo with description in the code that should test this operation but it's currently not possible. Yet another reason to upgrade support for more recent version...

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    90 pass
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    90 pass
  • 🇸🇮Slovenia bcizej

    Branch rebased

  • Status changed to RTBC about 1 year ago
  • 🇸🇮Slovenia DeaOm

    Manual testing is now providing desired results, tests are added and are also passing, so do not see any reason why this can't be merged, so marking it as RTBC.

  • Status changed to Fixed about 1 year ago
  • 🇸🇮Slovenia bcizej

    Merged, thanks everyone.

  • 🇦🇹Austria tgoeg

    The latest meilisearch version supports IS NULL, IS EMPTY, EXISTS operators but 0.28.1 does not have such operators and filtering on empty strings is also ignored. I've added a todo with description in the code that should test this operation but it's currently not possible. Yet another reason to upgrade support for more recent version...

    Do we need a follow-up issue to keep an eye on this (i.e. Meilisearch dependency update, resolve this todo)?
    In some issue lately I think the consensus was to reduce the current issue queue as much as possible on 0.28.1 (which indeed happened here, so thanks from my side as well!), merge, test, then update the Meilisearch dependency to a current stable version (1.3.2 as of this writing) and see whether everything implemented is forward compatible. If it is, be happy and remove workarounds for older versions.

    The above procedure however is currently not represented in the issue structure.
    I can do that if you like?

  • 🇸🇮Slovenia bcizej

    Sure I have no problems with that, keep in mind though we still have some issues left in the queue I'd like to get done before we go into upgrading the supported version but I'm not against creating new issues for that. We can also create a "roadmap to release" issue that keeps track of all the issues we need to fix before we do a stable release.

  • 🇦🇹Austria tgoeg

    Alright, I think I gathered all of the open issues that are currently being worked on and/or make sense to finish before moving to a new Meilisearch version in #3383952.
    Also created a child issue for the currently commented test you mentioned above.

    Feel free to add anything I might have forgotten!

    The roadmap issue also sounds like a good idea but might be pretty convergent now with #3383952. Except there are more things you'd like to pack into it.
    I think the feature set sounds pretty good however for a v1.1 or similar. Getting out a more stable version than v1.0 quicker and adding features later on might help with adoption. I'd call v1.0 pretty fragile and running it in production is not for the faint of heart (I know for one :-) )

    Thanks!

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

Production build 0.71.5 2024