Fixes #6374 Make sure top menu URLs are generated properly based on user preferences , refs #3426

3 files changed, 104 insertions, 29 deletions

diff --git a/core/Menu/MenuAbstract.php b/core/Menu/MenuAbstract.php
index 8fcc4786cc..0d77ff21d0 100644
--- a/core/Menu/MenuAbstract.php
+++ b/core/Menu/MenuAbstract.php
@@ -103,7 +103,7 @@ abstract class MenuAbstract extends Singleton
     public function addItem($menuName, $subMenuName, $url, $order = 50, $tooltip = false)
     {
         // make sure the idSite value used is numeric (hack-y fix for #3426)
-        if (!is_numeric(Common::getRequestVar('idSite', false))) {
+        if (isset($url['idSite']) && !is_numeric($url['idSite'])) {
             $idSites = API::getInstance()->getSitesIdWithAtLeastViewAccess();
             $url['idSite'] = reset($idSites);
         }
diff --git a/core/Plugin/Controller.php b/core/Plugin/Controller.php
index 53ceab3f75..b016a019e8 100644
--- a/core/Plugin/Controller.php
+++ b/core/Plugin/Controller.php
@@ -834,32 +834,10 @@ abstract class Controller
     public function redirectToIndex($moduleToRedirect, $actionToRedirect, $websiteId = null, $defaultPeriod = null,
                                     $defaultDate = null, $parameters = array())
     {
-
-        $userPreferences = new UserPreferences();
-
-        if (empty($websiteId)) {
-            $websiteId = $userPreferences->getDefaultWebsiteId();
-        }
-        if (empty($defaultDate)) {
-            $defaultDate = $userPreferences->getDefaultDate();
-        }
-        if (empty($defaultPeriod)) {
-            $defaultPeriod = $userPreferences->getDefaultPeriod();
-        }
-        $parametersString = '';
-        if (!empty($parameters)) {
-            $parametersString = '&' . Url::getQueryStringFromParameters($parameters);
-        }
-
-        if ($websiteId) {
-            $url = "index.php?module=" . $moduleToRedirect
-                . "&action=" . $actionToRedirect
-                . "&idSite=" . $websiteId
-                . "&period=" . $defaultPeriod
-                . "&date=" . $defaultDate
-                . $parametersString;
-            Url::redirectToUrl($url);
-            exit;
+        try {
+            $this->doRedirectToUrl($moduleToRedirect, $actionToRedirect, $websiteId, $defaultPeriod, $defaultDate, $parameters);
+        } catch(Exception $e) {
+            // no website ID to default to, so could not redirect
         }
 
         if (Piwik::hasUserSuperUserAccess()) {
@@ -881,6 +859,7 @@ abstract class Controller
         exit;
     }
 
+
     /**
      * Checks that the token_auth in the URL matches the currently logged-in user's token_auth.
      *
@@ -1000,4 +979,25 @@ abstract class Controller
 				Please check that you are logged in Piwik and have permission to access the specified website.");
         }
     }
+
+    /**
+     * @param $moduleToRedirect
+     * @param $actionToRedirect
+     * @param $websiteId
+     * @param $defaultPeriod
+     * @param $defaultDate
+     * @param $parameters
+     * @throws Exception
+     */
+    private function doRedirectToUrl($moduleToRedirect, $actionToRedirect, $websiteId, $defaultPeriod, $defaultDate, $parameters)
+    {
+        $menu = new Menu();
+        $queryParamsUserPrefs = $menu->urlForDefaultUserParams($websiteId, $defaultPeriod, $defaultDate);
+        $queryParams = !empty($parameters) ? '&' . Url::getQueryStringFromParameters($parameters) : '';
+        $queryParams = $queryParamsUserPrefs . $queryParams;
+        $url = "index.php?module=%s&action=%s";
+        $url = sprintf($url, $moduleToRedirect, $actionToRedirect);
+        $url = $url . $queryParamsUserPrefs . $queryParams;
+        Url::redirectToUrl($url);
+    }
 }
diff --git a/core/Plugin/Menu.php b/core/Plugin/Menu.php
index 6d2bdefccd..0cdc1878df 100644
--- a/core/Plugin/Menu.php
+++ b/core/Plugin/Menu.php
@@ -14,6 +14,7 @@ use Piwik\Menu\MenuReporting;
 use Piwik\Menu\MenuTop;
 use Piwik\Menu\MenuUser;
 use Piwik\Plugin\Manager as PluginManager;
+use Piwik\Plugins\UsersManager\UserPreferences;
 
 /**
  * Base class of all plugin menu providers. Plugins that define their own menu items can extend this class to easily
@@ -105,8 +106,8 @@ class Menu
      * @param  string $module            The name of the module/plugin the action belongs to. The module name is case sensitive.
      * @param  string $controllerAction  The name of the action that should be executed within your controller
      * @param  array  $additionalParams  Optional URL parameters that will be appended to the URL
-     * @return array|null   Returns null if the given module is either not installed or not activated. Returns the URL
-     *                      to the given module action otherwise.
+     * @return array|null   Returns null if the given module is either not installed or not activated. Returns the array
+     *                      of query parameter names and values to the given module action otherwise.
      *
      * @since 2.7.0
      * // not API for now
@@ -130,6 +131,80 @@ class Menu
     }
 
     /**
+     * Generates a URL to the given action of the current module, and it will also append some URL query parameters from the
+     * User preferences: idSite, period, date. If you do not need the parameters idSite, period and date to be generated
+     * use {@link urlForAction()} instead.
+     *
+     * @param  string $controllerAction  The name of the action that should be executed within your controller
+     * @param  array  $additionalParams  Optional URL parameters that will be appended to the URL
+     * @return array   Returns the array of query parameter names and values to the given module action and idSite date and period.
+     *
+     */
+    protected function urlForActionWithDefaultUserParams($controllerAction, $additionalParams = array())
+    {
+        $urlModuleAction = $this->urlForAction($controllerAction);
+        return array_merge(
+            $urlModuleAction,
+            $this->urlForDefaultUserParams(),
+            $additionalParams
+        );
+    }
+
+    /**
+     * Generates a URL to the given action of the given module, and it will also append some URL query parameters from the
+     * User preferences: idSite, period, date. If you do not need the parameters idSite, period and date to be generated
+     * use {@link urlForModuleAction()} instead.
+     *
+     * @param  string $module            The name of the module/plugin the action belongs to. The module name is case sensitive.
+     * @param  string $controllerAction  The name of the action that should be executed within your controller
+     * @param  array  $additionalParams  Optional URL parameters that will be appended to the URL
+     * @return array|null   Returns the array of query parameter names and values to the given module action and idSite date and period.
+     *                      Returns null if the module or action is invalid.
+     *
+     */
+    protected function urlForModuleActionWithDefaultUserParams($module, $controllerAction, $additionalParams = array())
+    {
+        $urlModuleAction = $this->urlForModuleAction($module, $controllerAction);
+        return array_merge(
+            $urlModuleAction,
+            $this->urlForDefaultUserParams(),
+            $additionalParams
+        );
+    }
+
+    /**
+     * Returns the &idSite=X&period=Y&date=Z query string fragment,
+     * fetched from current logged-in user's preferences.
+     *
+     * @param bool $websiteId
+     * @param bool $defaultPeriod
+     * @param bool $defaultDate
+     * @return string eg '&idSite=1&period=week&date=today'
+     * @throws \Exception in case a website was not specified and a default website id could not be found
+     */
+    public function urlForDefaultUserParams($websiteId = false, $defaultPeriod = false, $defaultDate = false)
+    {
+        $userPreferences = new UserPreferences();
+        if (empty($websiteId)) {
+            $websiteId = $userPreferences->getDefaultWebsiteId();
+        }
+        if (empty($websiteId)) {
+            throw new \Exception("A website ID was not specified and a website to default to could not be found.");
+        }
+        if (empty($defaultDate)) {
+            $defaultDate = $userPreferences->getDefaultDate();
+        }
+        if (empty($defaultPeriod)) {
+            $defaultPeriod = $userPreferences->getDefaultPeriod();
+        }
+        return array(
+            'idSite' => $websiteId,
+            'period' => $defaultPeriod,
+            'date'   => $defaultDate,
+        );
+    }
+
+    /**
      * Configures the reporting menu which should only contain links to reports of a specific site such as
      * "Search Engines", "Page Titles" or "Locations & Provider".
      */