Acquia Search is supposed to be set to the default environment on install, however it is not. The reason is that it calls the apachesolr_default_environment(), which only resets the static variable for the pageload and does not set the system variable storing the default environment.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cpliakas’s picture

Status: Active » Needs review
FileSize
692 bytes

The attached patch changes the apachesolr_default_environment() call to a variable_set().

pwolanin’s picture

Status: Needs review » Needs work

That's sort of funny - it was being set for just the current page load.

We just added a more complete APi function, but maybe we should just copy the code here for now? This patch is missing the code to change the core search page.

cpliakas’s picture

Thanks for reviewing.

Obviously the API functions would be preferable, but I want to make sure we handle the use case where the connector is upgraded and Apache Solr Search Integration is not. I just tested this patch with the one committed to Apache Solr Search Integration, and it actually works as expected. Even without using the API function, the core search page is set to use the Acquia Search environment. Therefore I think we are OK without having to duplicate the code.

cpliakas’s picture

Status: Needs review » Needs work

Marking as needs review again so that the behavior I mentioned in #1784804-3: Acquia Search is not set to the default environment on install can be confirmed. Also see #1784990: Revive unit tests for Acquia Search, which has some tests for this bug. The tests could also be expanded to visit the search page or make sure tat Acquia Search shows up as the default in the UI. (added to patch)

cpliakas’s picture

Status: Needs work » Needs review

And... changing status.

pwolanin’s picture

Status: Needs work » Needs review

No, we shouldn't depend on the other patch being present - we need to set the core search page too.

cpliakas’s picture

Status: Needs review » Needs work

Yes, I see what you are saying now. My assumption was that since the RC3 release is broken then it is boken, but if we can set the default environment explicitly then we effectively "fix" the bug when used in tandem with Acquia Search.

Nick_vh’s picture

FileSize
1.07 KB

This adds some logic to set the default search page to acquia search, I'm not completely satisfied with this yet since this could add some load to the DB on each cron run. Should add some check if the default env is ok or not, and if not - change it.

Nick_vh’s picture

FileSize
1.23 KB

This patch is a little cleaner but still very wrong. What if your default search page is not linked to Acquia Search but to a cloned Acquia Search env or something completely different? I can come up with such cases and even current production cases where this patch would ruin the whole flow.

It would make more sense to indeed wait on the apachesolr patch so that the default search page is always set with the default search environment. This patch should not be committed

Nick_vh’s picture

Status: Needs work » Needs review
FileSize
1.25 KB

Thinking this through, on install it should not be a problem. People can still change it whenever they want to.

cpliakas’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me! Works as advertised and passes the unit tests at #1784990-8: Revive unit tests for Acquia Search.

Nick_vh’s picture

Project: Acquia Connector » Acquia Search
Version: 7.x-2.x-dev » 6.x-3.x-dev

Tested the whole flow manually again, works as advertised.

Committed to 7.x-2.x - Moving to Acquia Search for 6.x-3.x version

Nick_vh’s picture

Status: Reviewed & tested by the community » Patch (to be ported)
Nick_vh’s picture

Status: Patch (to be ported) » Fixed

patch applied cleanly and works as expected after manual test.

Committed!

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

Updated issue summary.