On Thu, 19 Jan 2017 at 04:23:37, Mark Weiman wrote:
Currently, aurweb displays all dates and times in UTC time. This patch adds a capability for each logged in user to set their preferred timezone.
Signed-off-by: Mark Weiman <mark.weiman@markzz.com> --- [...]
Thanks, we're almost there. There are several minor issues left, though. See below.
diff --git a/web/lib/aur.inc.php b/web/lib/aur.inc.php index 9015ae8..8a0ed2b 100644 --- a/web/lib/aur.inc.php +++ b/web/lib/aur.inc.php [...] @@ -356,7 +359,7 @@ function uid_from_sid($sid="") { function html_header($title="", $details=array()) { global $LANG; global $SUPPORTED_LANGS; - +
Unrelated whitespace change?
include('header.php'); return; } [...] diff --git a/web/lib/timezone.inc.php b/web/lib/timezone.inc.php new file mode 100644 index 0000000..f3b4c71 --- /dev/null +++ b/web/lib/timezone.inc.php @@ -0,0 +1,65 @@ [...] +/** + * Set the $TZ global variable.
The comment needs to be updated to reflect the changes in this reroll of the patch series.
+ * + * @return null + */ +function set_tz() { + $timezones = generate_timezone_list(); + $update_cookie = false; + + if (isset($_COOKIE["AURTZ"])) { + $TZ = $_COOKIE["AURTZ"];
It is quite confusing to use an upper case variable name here enough though this no longer is a global variable. Something like $timezone might be more appropriate.
+ } elseif (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); + + if ($result) { + $row = $result->fetchAll(); + $TZ = $row[0][0];
As I mentioned in an earlier review, this can be simplified as $timezone = $result->fetchColumn(0); already assuming that $TZ becomes $timezone.
+ } else { + $TZ = config_get("options", "default_timezone");
This else-branch is not needed. The default case is already handled below.
+ } + + $update_cookie = true; + } + + if (isset($TZ) && array_key_exists($TZ, $timezones)) { + date_default_timezone_set($TZ); + } else { + $TZ = config_get("options", "default_timezone"); + date_default_timezone_set($TZ); + }
As I also mentioned in an earlier review, this can be simplified as follows (also assuming that $TZ is replaced by $timezone): if (!isset($timezone) || !array_key_exists($timezone, $timezones)) { $timezone = config_get("options", "default_timezone"); } date_default_timezone_set($timezone);
+ + if ($update_cookie) { + $timeout = intval(config_get("options", "persistent_cookie_timeout")); + $cookie_time = time() + $timeout; + setcookie("AURTZ", $TZ, $cookie_time, "/"); + } +} diff --git a/web/template/account_edit_form.php b/web/template/account_edit_form.php index 19821a0..8b207e7 100644 --- a/web/template/account_edit_form.php +++ b/web/template/account_edit_form.php @@ -129,6 +129,24 @@ ?> </select> </p> + <p> + <label for="id_timezone"><?= __("Timezone") ?></label> + <select name="TZ" id="id_timezone"> +<?php + $timezones = generate_timezone_list(); + while (list($key, $val) = each($timezones)) { + if ($TZ == "") { + $TZ = config_get("options", "default_timezone"); + }
Broken indentation. I also think this if-block belongs to display_account_form() and should be executed before account_edit_form.php is included.
+ if ($TZ == $key) { + print "<option value=\"".$key."\" selected=\"selected\"> ".$val."</option>\n"; + } else { + print "<option value=\"".$key."\"> ".$val."</option>\n"; + } + } +?> + </select> + </p> </fieldset>
<fieldset> [...]
Looks good otherwise!