Skip to content

Conversation

@ChrisMoutsos
Copy link

If there is a Platform.sh Solr relationship, but the project config is not set to use the "solr" search engine (i.e. it's set to "legacy", or not set at all), then the Solr params shouldn't be set here.

Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution. Makes sense in general.
However please create a JIRA ticket and prefix the PR title with it, followed by a comma (:) and a short description of the change (ideally in past simple as it's gonna become history after merge).
If this causes some sort of a visible bug and is reproducible on a clean installation, then some steps to reproduce should be added. I'm assuming that it happens when you configure P.sh to use Solr, but set legacy, right? Does it make the system crash or just index data incorrectly?

As for the change itself:

}

if (isset($relationships['solr'])) {
if (isset($relationships['solr']) && ($_SERVER['SEARCH_ENGINE'] ?? false) === 'solr') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if a defined parameter is a string, then setting bool as a fallback doesn't make any sense, even if it works

Suggested change
if (isset($relationships['solr']) && ($_SERVER['SEARCH_ENGINE'] ?? false) === 'solr') {
if (isset($relationships['solr']) && ($_SERVER['SEARCH_ENGINE'] ?? '') === 'solr') {

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants