New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-detect timezone and currency in installer #13092

Merged
merged 3 commits into from Sep 20, 2018

Conversation

Projects
None yet
3 participants
@c960657
Contributor

c960657 commented Jun 21, 2018

To make it easier to install Matomo, the installer should try to auto-detect the timezone and currency.

If PHP or the operating system is configured to use a timezone other than UTC, it is highly likely that this timezone is relevant to use as the default timezone for Matomo. We should preselect this timezone in the timezone selector in the installer, so the user does not have to look for the proper timezone in the huge list of timezones.

Once we have the timezone, we can find the corresponding country and (if the Intl extension is installed) currency. This is likely a better default than the hard-coded default, USD.

@c960657 c960657 changed the title from Auto-detect timezone and currency to Auto-detect timezone and currency in installer Jun 21, 2018

@mattab mattab added this to the 3.7.0 milestone Jun 26, 2018

@mattab mattab added the Needs Review label Jun 26, 2018

// Use server timezone as default. If server timezone is UTC, it is likely
// a default not specified explicitly by the sysadm, so ignore this.
$timezone = PIWIK_DEFAULT_TIMEZONE;
if (in_array($timezone, array('UTC', 'Etc/UTC', 'GMT', 'Etc/GMT'))) {

This comment has been minimized.

@tsteur

tsteur Sep 8, 2018

Member

is it worth it to compare this all Lowercase? Or maybe they cannot be set in different cases?

This comment has been minimized.

@c960657

c960657 Sep 8, 2018

Contributor

Good point. It seems that PHP does indeed treat timezones as case-insensitive. I didn't know that.

$timezone = PIWIK_DEFAULT_TIMEZONE;
if (in_array($timezone, array('UTC', 'Etc/UTC', 'GMT', 'Etc/GMT'))) {
$timezone = null;
}

This comment has been minimized.

@tsteur

tsteur Sep 8, 2018

Member

I wonder if we need to check whether Matomo "knows" the timezone (present in $timezones)?

This comment has been minimized.

@c960657

c960657 Sep 9, 2018

Contributor

The value determines the default selected timezone in the select field. If an invalid timezone is submitted, QuickForm will just ignore it, so there is no need for further validation.

@tsteur

This comment has been minimized.

Member

tsteur commented Sep 16, 2018

Awesome 👍 I've just installed the Intl extension, debugged it step by step and worked 👍 so valuable 👍

Be all good to merge but there is one problem... in some of our scripts (they are not in this repository) we are setting the default timezone and currency before creating the website. By applying this PR it would no longer work and they would be overwritten. We would need to add some checks to skip the detection if for some reason a default currency or timezone is already set. Like this:

diff --git a/plugins/Installation/FormFirstWebsiteSetup.php b/plugins/Installation/FormFirstWebsiteSetup.php
index 13d4dbc4e3..e83a246b8f 100644
--- a/plugins/Installation/FormFirstWebsiteSetup.php
+++ b/plugins/Installation/FormFirstWebsiteSetup.php
@@ -15,6 +15,7 @@ use HTML_QuickForm2_Factory;
 use HTML_QuickForm2_Rule;
 use NumberFormatter;
 use Piwik\Access;
+use Piwik\Option;
 use Piwik\Piwik;
 use Piwik\Plugins\SitesManager\API;
 use Piwik\QuickForm2;
@@ -46,6 +47,11 @@ class FormFirstWebsiteSetup extends QuickForm2
             $timezone = null;
         }
 
+        if (Option::get(API::OPTION_DEFAULT_TIMEZONE)) {
+            // a default timezone was already configured
+            $timezone = null;
+        }
+
         $this->addElement('text', 'siteName')
             ->setLabel(Piwik::translate('Installation_SetupWebSiteName'))
             ->addRule('required', Piwik::translate('General_Required', Piwik::translate('Installation_SetupWebSiteName')));
@@ -99,7 +105,7 @@ class Rule_isValidTimezone extends HTML_QuickForm2_Rule
         }
 
         // If intl extension is installed, get default currency from timezone country.
-        if ($timezone && class_exists('NumberFormatter')) {
+        if (!Option::get(API::OPTION_DEFAULT_CURRENCY) && $timezone && class_exists('NumberFormatter')) {
             try {
                 $zone = new DateTimeZone($timezone);
                 $location = $zone->getLocation();
@c960657

This comment has been minimized.

Contributor

c960657 commented Sep 17, 2018

@tsteur Can you provide an example of such a script? If the default timezone/currency is already stored in the database at this point, I might be able to retrieve it using API::getInstance()->getDefaultTimezone() and ->getDefaultCurrency() and use that to bypass the detection logic?

@tsteur

This comment has been minimized.

Member

tsteur commented Sep 17, 2018

I was thinking of API::getInstance()->getDefaultTimezone() as well initially but then you need to know the default value as it always returns a value like (API::getInstance()->getDefaultTimezone() !== 'UTC') and if this value changes it could break etc. And the default value for currency is USD and you wouldn't know whether it was set manually before or whether it is the default value

The script would do eg

// php timezone might be "Europe/Berlin" for example
$sitesApi->setDefaultTimezone('abcdef...');
$sitesApi->setDefaultCurrency('USD');
$formWebsiteSetup = new FormFirstWebsiteSetup();
...
@c960657

This comment has been minimized.

Contributor

c960657 commented Sep 20, 2018

Done.

@tsteur

This comment has been minimized.

Member

tsteur commented Sep 20, 2018

Cheers 👍

@tsteur tsteur merged commit e5ba290 into matomo-org:3.x-dev Sep 20, 2018

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details

@c960657 c960657 deleted the c960657:detect-timezone-currency branch Sep 24, 2018

InfinityVoid added a commit to InfinityVoid/matomo that referenced this pull request Oct 11, 2018

Auto-detect timezone and currency in installer (matomo-org#13092)
* Auto-detect timezone and currency

* Compare timezone case-insentively

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