From 00a1dec7a6dc5394ce64dbba80d872b820c9c0fc Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 10 Apr 2017 07:44:58 -0700 Subject: [PATCH] Render timezones in event reminder mail, and render them more nicely Summary: Fixes T12356. - In this mail, we currently render "6:00 AM". Instead, render "6:00 AM (PDT)" or similar. This is consistent with times in other modern Transaction mail. - Previously, we would render "UTC-7". Render "PDT" instead. For obscure zones with no known timezone abbreviation, fall back to "UTC-7". Test Plan: - Used `bin/calendar notify --minutes X` to trigger notifications, read email bodies. - Used this script to list all `T` values and checked them for sanity: ```lang=php setTimeZone($zone); printf( "%s (%s)\n", $locale, $now->format('T')); } ``` Reviewers: chad Reviewed By: chad Maniphest Tasks: T12356 Differential Revision: https://secure.phabricator.com/D17646 --- src/__phutil_library_map__.php | 1 + ...habricatorCalendarEventNotificationView.php | 8 ++++++++ .../PhabricatorCalendarNotificationEngine.php | 2 +- .../PhabricatorModularTransactionType.php | 15 ++++----------- src/view/viewutils.php | 18 ++++++++++++++++++ 5 files changed, 32 insertions(+), 12 deletions(-) diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 279e9d6c44..60bbb25b86 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -4692,6 +4692,7 @@ phutil_register_library_map(array( 'javelin_tag' => 'infrastructure/javelin/markup.php', 'phabricator_date' => 'view/viewutils.php', 'phabricator_datetime' => 'view/viewutils.php', + 'phabricator_datetimezone' => 'view/viewutils.php', 'phabricator_form' => 'infrastructure/javelin/markup.php', 'phabricator_format_local_time' => 'view/viewutils.php', 'phabricator_relative_date' => 'view/viewutils.php', diff --git a/src/applications/calendar/notifications/PhabricatorCalendarEventNotificationView.php b/src/applications/calendar/notifications/PhabricatorCalendarEventNotificationView.php index 7568f7d8dc..8c619c7fb2 100644 --- a/src/applications/calendar/notifications/PhabricatorCalendarEventNotificationView.php +++ b/src/applications/calendar/notifications/PhabricatorCalendarEventNotificationView.php @@ -58,4 +58,12 @@ final class PhabricatorCalendarEventNotificationView return phabricator_datetime($epoch, $viewer); } + public function getDisplayTimeWithTimezone() { + $viewer = $this->getViewer(); + + $epoch = $this->getEpoch(); + return phabricator_datetimezone($epoch, $viewer); + } + + } diff --git a/src/applications/calendar/notifications/PhabricatorCalendarNotificationEngine.php b/src/applications/calendar/notifications/PhabricatorCalendarNotificationEngine.php index b0a762b577..4ae007fc0c 100644 --- a/src/applications/calendar/notifications/PhabricatorCalendarNotificationEngine.php +++ b/src/applications/calendar/notifications/PhabricatorCalendarNotificationEngine.php @@ -268,7 +268,7 @@ final class PhabricatorCalendarNotificationEngine '%s is starting in %s minute(s), at %s.', $event->getEvent()->getName(), $event->getDisplayMinutes(), - $event->getDisplayTime())); + $event->getDisplayTimeWithTimezone())); $body->addLinkSection( pht('EVENT DETAIL'), diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php index 55ad678539..a315a5b9cf 100644 --- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php +++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php @@ -234,22 +234,15 @@ abstract class PhabricatorModularTransactionType if ($all_day) { $display = phabricator_date($epoch, $viewer); - } else { - $display = phabricator_datetime($epoch, $viewer); - + } else if ($this->isRenderingTargetExternal()) { // When rendering to text, we explicitly render the offset from UTC to // provide context to the date: the mail may be generating with the // server's settings, or the user may later refer back to it after // changing timezones. - if ($this->isRenderingTargetExternal()) { - $offset = $viewer->getTimeZoneOffsetInHours(); - if ($offset >= 0) { - $display = pht('%s (UTC+%d)', $display, $offset); - } else { - $display = pht('%s (UTC-%d)', $display, abs($offset)); - } - } + $display = phabricator_datetimezone($epoch, $viewer); + } else { + $display = phabricator_datetime($epoch, $viewer); } return $this->renderValue($display); diff --git a/src/view/viewutils.php b/src/view/viewutils.php index e0629800c7..7eee3e03fb 100644 --- a/src/view/viewutils.php +++ b/src/view/viewutils.php @@ -48,6 +48,24 @@ function phabricator_datetime($epoch, $user) { $user->getUserSetting($time_key))); } +function phabricator_datetimezone($epoch, $user) { + $datetime = phabricator_datetime($epoch, $user); + $timezone = phabricator_format_local_time($epoch, $user, 'T'); + + // Some obscure timezones just render as "+03" or "-09". Make these render + // as "UTC+3" instead. + if (preg_match('/^[+-]/', $timezone)) { + $timezone = (int)trim($timezone, '+'); + if ($timezone < 0) { + $timezone = pht('UTC-%s', $timezone); + } else { + $timezone = pht('UTC+%s', $timezone); + } + } + + return pht('%s (%s)', $datetime, $timezone); +} + /** * This function does not usually need to be called directly. Instead, call * @{function:phabricator_date}, @{function:phabricator_time}, or