Fixed Installed but not enabled sets default ordering anyway

[Rasmus Vind]

Hey guys,

I just purchased the ES addon. I am excited to use it. I am taking things slowly, so I installed this addon, and had a look at my search dialog before enabling ES. I found that the default order was not set, most likely because the ES addon sets the default order to "relevance", even if it is not enabled yet.

I made the following change:

--- a/library/XenES/Proxy/ControllerSearch.php
+++ b/library/XenES/Proxy/ControllerSearch.php
@@ -12,6 +12,7 @@ class XenES_Proxy_ControllerSearch extends XFCP_XenES_Proxy_ControllerSearch
                if ($result instanceof XenForo_ControllerResponse_View
                        && !empty($result->params['search'])
                        && empty($result->params['search']['existing'])
+                       && XenForo_Application::getOptions()->enableElasticsearch
                )
                {
                        $result->params['search']['order'] = XenForo_Application::getOptions()->esDefaultSearchOrder;
@@ -22,7 +23,7 @@ class XenES_Proxy_ControllerSearch extends XFCP_XenES_Proxy_ControllerSearch

        public function actionSearch()
        {
-               if (!$this->_request->getParam('order'))
+               if (!$this->_request->getParam('order') && XenForo_Application::getOptions()->enableElasticsearch)
                {
                        $this->_request->setParam('order', XenForo_Application::getOptions()->esDefaultSearchOrder);
                }

Which makes it refrain from changing the default unless the ES addon is enabled.

Before:

After:


I hope you will accept this patch.

 

[Chris D]

Thanks we will look into it.

 

[Rasmus Vind]

One more note. I think my fix was not completely in line with your way of coding. If you have a look at the file XenES_Listener you see this:

if (XenForo_Application::get('options')->enableElasticsearch)
        {
            $class = 'XenES_Search_SourceHandler_ElasticSearch';
            return false; // stop running other listeners
        }

So I would probably alter the listener similarly instead of adding the condition to check if it is enabled before extending it.

 

[Mike]

Just checking that it's been activated in the options is basically the correct fix, so I've done that now, thanks.