Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#10 | 1784804-10.patch | 1.25 KB | Nick_vh |
#9 | 1784804-9.patch | 1.23 KB | Nick_vh |
#8 | 1784804-8.patch | 1.07 KB | Nick_vh |
#1 | default-environment-1784804-1.patch | 692 bytes | cpliakas |
Comments
Comment #1
cpliakas CreditAttribution: cpliakas commentedThe attached patch changes the apachesolr_default_environment() call to a variable_set().
Comment #2
pwolanin CreditAttribution: pwolanin commentedThat'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.
Comment #3
cpliakas CreditAttribution: cpliakas commentedThanks 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.
Comment #4
cpliakas CreditAttribution: cpliakas commentedMarking 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)Comment #5
cpliakas CreditAttribution: cpliakas commentedAnd... changing status.
Comment #6
pwolanin CreditAttribution: pwolanin commentedNo, we shouldn't depend on the other patch being present - we need to set the core search page too.
Comment #7
cpliakas CreditAttribution: cpliakas commentedYes, 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.
Comment #8
Nick_vhThis 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.
Comment #9
Nick_vhThis 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
Comment #10
Nick_vhThinking this through, on install it should not be a problem. People can still change it whenever they want to.
Comment #11
cpliakas CreditAttribution: cpliakas commentedLooks good to me! Works as advertised and passes the unit tests at #1784990-8: Revive unit tests for Acquia Search.
Comment #12
Nick_vhTested the whole flow manually again, works as advertised.
Committed to 7.x-2.x - Moving to Acquia Search for 6.x-3.x version
Comment #13
Nick_vhComment #14
Nick_vhpatch applied cleanly and works as expected after manual test.
Committed!
Comment #15.0
(not verified) CreditAttribution: commentedUpdated issue summary.