[aur-dev] [PATCH v4] Add user set timezones

Lukas Fleischer lfleischer at archlinux.org
Sat Jan 7 16:44:04 UTC 2017


On Thu, 29 Dec 2016 at 04:10:00, Mark Weiman wrote:
> [...]
> @@ -64,7 +66,7 @@ if (in_request("Action") == "NewAccount") {
>         }
>  } else {
>         print '<p>' . __("Use this form to create an account.") . '</p>';
> -       display_account_form("NewAccount", "", "", "", "", "", "", "", "", $LANG);
> +       display_account_form("NewAccount", "", "", "", "", "", "", "", "", $LANG, config_get("options", "default_timezone"));

I know I explicitly asked you to add this when reviewing a previous
version of this patch. Looking at it again, it seems better to leave
this line untouched and handle the default case in the function itself,
though.

> [...]
> diff --git a/web/lib/timezone.inc.php b/web/lib/timezone.inc.php
> new file mode 100644
> index 0000000..363d6a6
> --- /dev/null
> +++ b/web/lib/timezone.inc.php
> @@ -0,0 +1,61 @@
> +<?php
> +set_include_path(get_include_path() . PATH_SEPARATOR . '../lib');
> +
> +/**
> + * Generate an associative of the PHP timezones and display text.
> + *
> + * @return array PHP Timezone => Displayed Description
> + */
> +function generate_timezone_list() {
> +       $php_timezones = DateTimeZone::listIdentifiers(DateTimeZone::ALL);
> +
> +       $offsets = array();
> +       foreach ($php_timezones as $timezone) {
> +               $tz = new DateTimeZone($timezone);
> +               $offset = $tz->getOffset(new DateTime());
> +               $offsets[$timezone] = "(UTC" . ($offset < 0 ? "-" : "+") . gmdate("H:i", abs($offset)) .
> +                                                               ") " . $timezone;
> +       }
> +
> +       asort($offsets);
> +       return $offsets;
> +}
> +
> +/**
> + * Set the $TZ global variable.
> + *
> + * @global string $TZ Timezone set for session.
> + *
> + * @return null
> + */
> +function set_tz() {
> +       global $TZ;
> +
> +       $timezones = generate_timezone_list();
> +
> +       if (isset($_COOKIE['AURSID'])) {
> +               $dbh = DB::connect();
> +               $q = "SELECT Timezone FROM Users, Sessions ";
> +               $q .= "WHERE Users.ID = Sessions.UsersID ";
> +               $q .= "AND Sessions.SessionID = ";
> +               $q .= $dbh->quote($_COOKIE['AURSID']);
> +               $result = $dbh->query($q);

I wonder whether it is better to store the timezone in a cookie.

The set_tz() function could query the database the first time a user
logs in only, set the cookie and become a no-op for any subsequent
calls. By doing that, we could also get rid of the global variable and
the patch could be extended easily such that unregistered users can
specify a timezone.

> +
> +               if ($result) {
> +                       $row = $result->fetchAll();
> +                       $TZ = $row[0][0];
> +               } else {
> +                       $TZ = config_get("options", "default_timezone");
> +               }
> +
> +               if (!array_key_exists($TZ, $timezones)) {
> +                       $TZ = config_get("options", "default_timezone");
> +               }
> +       }
> +
> +       if (isset($TZ) && $TZ != "") {
> +               date_default_timezone_set($TZ);
> +       } else {
> +               date_default_timezone_set(config_get("options", "default_timezone"));
> +       }
> +}

This part can be simplified as follows (untested and assuming that we
replace the global variable by a local variable as suggested above):

    unset($timezone);
    if ($result) {
        $timezone = $result->fetchColumn(0);
    }

    if (!isset($timezone) || !array_key_exists($timezone, $timezones)) {
        $timezone = config_get("options", "default_timezone");
    }

    date_default_timezone_set($timezone);

The rest looks fine to me, thanks!


More information about the aur-dev mailing list