Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.x]: Ordering search results breaks external search engine implementation #14293

Closed
ddnetters opened this issue Feb 5, 2024 · 5 comments
Closed

Comments

@ddnetters
Copy link

ddnetters commented Feb 5, 2024

What happened?

Description

A (somewhat) recent change to handling search results breaks our ability to implement ElasticSearch. When ordering search results, the _applySearchParams() function (https://github.com/craftcms/cms/blob/develop/src/elements/db/ElementQuery.php#L2771) will call createDbQuery() if ordering on any field except for score.

We use a custom Search component implementation to use ElasticSearch. As a result there is no database connection or query interface we could provide here. The previous implementation of _applySearchParams() didn't require such a connection:

private function _applySearchParam(Connection $db): void

It'd be great if we can either disable the new business logic, or if the _applySearchParams() could be made public so we can override it.

Commit: 4a52ba8

Steps to reproduce

  1. Install ElasticSearch (the free plugin from the store should yield the same results)
  2. Try to order a field
  3. Search crashes because createDbQuery() isn't implemented and can't be for ElasticSearch

Expected behavior

  1. Install ElasticSearch (the free plugin from the store should yield the same results)
  2. Try to order a field
  3. _applySearchParams() gets overridden by the custom search component and doesn't call the database.

Actual behavior

  1. Install ElasticSearch (the free plugin from the store should yield the same results)
  2. Try to order a field
  3. Search crashes because createDbQuery() isn't implemented and can't be for ElasticSearch

Craft CMS version

4.7.1

PHP version

No response

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

@brandonkelly
Copy link
Member

Can you show me how you are tapping into the search functionality?

@ddnetters
Copy link
Author

For sure, we've basically forked https://github.com/codemonauts/craft-elasticsearch/ and made minor adjustments.

The search component gets registered here: https://github.com/codemonauts/craft-elasticsearch/blob/main/src/Elastic.php#L73

Which makes craft trigger this class when searching: https://github.com/codemonauts/craft-elasticsearch/blob/main/src/services/Search.php#L21. When searching, craft invokes searchElements() on that class. Which in turn leads to ElementQuery::prepare() --> ElementQuery::_applySearchParam --> Craft::$app->getSearch()->createDbQuery()

@ddnetters
Copy link
Author

Do you have some pointer of where I could look to fix this or change my implementation?

brandonkelly added a commit that referenced this issue Feb 14, 2024
@brandonkelly
Copy link
Member

Just added a new shouldCallSearchElements() method to craft\services\Search for Craft 4.8, which you can override to always return true, and force searchElements() to be called instead of createDbQuery(). (cdfdbf4)

To update to the 4.8 branch, change your craftcms/cms requirement to 4.8.x-dev as 4.8.0-alpha in composer.json, and run composer update.

@brandonkelly
Copy link
Member

Craft 4.8.0 is out with that change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants