FIX: better handling of same offset timezones (#6680)

FIX: better handling of same offset timezones (#6680)

From 3e116bb14ed788c48bff8bf800bd4f506b8fc908 Mon Sep 17 00:00:00 2001
From: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Date: Tue, 27 Nov 2018 15:17:23 +0100
Subject: [PATCH] FIX: better handling of same offset timezones (#6680)


diff --git a/plugins/discourse-local-dates/assets/javascripts/discourse-local-dates.js.no-module.es6 b/plugins/discourse-local-dates/assets/javascripts/discourse-local-dates.js.no-module.es6
index 6d71eec..f0648ec 100644
--- a/plugins/discourse-local-dates/assets/javascripts/discourse-local-dates.js.no-module.es6
+++ b/plugins/discourse-local-dates/assets/javascripts/discourse-local-dates.js.no-module.es6
@@ -120,10 +120,14 @@
       };
     };
 
+    const compareZones = (timezoneA, timezoneB) => {
+      return (
+        moment.tz(timezoneA).utcOffset() === moment.tz(timezoneB).utcOffset()
+      );
+    };
+
     const _applyFormatting = (dateTime, displayedTimezone, options) => {
-      const sameTimezone =
-        moment.tz(displayedTimezone).utcOffset() ===
-        moment.tz(moment.tz.guess()).utcOffset();
+      const sameTimezone = compareZones(displayedTimezone, moment.tz.guess());
       const inCalendarRange = dateTime.isBetween(
         moment().subtract(2, "days"),
         moment().add(2, "days")
@@ -196,8 +200,11 @@
     const _generatePreviews = (dateTime, displayedTimezone, options) => {
       const previewedTimezones = [];
       const watchingUserTimezone = moment.tz.guess();
+      const timezones = options.timezones.filter(
+        timezone => timezone !== watchingUserTimezone
+      );
 
-      if (displayedTimezone !== watchingUserTimezone) {
+      if (!compareZones(displayedTimezone, watchingUserTimezone)) {
         previewedTimezones.push({
           timezone: watchingUserTimezone,
           current: true,
@@ -207,18 +214,41 @@
         });
       }
 
-      options.timezones
-        .filter(x => x !== watchingUserTimezone)
-        .forEach(timezone => {
-          previewedTimezones.push({
-            timezone,
-            dateTime: options.time
-              ? dateTime.tz(timezone).format("LLL")
-              : createDateTimeRange(dateTime, timezone)
-          });
+      if (
+        displayedTimezone === watchingUserTimezone &&
+        options.timezone !== displayedTimezone &&
+        !compareZones(displayedTimezone, options.timezone)
+      ) {
+        timezones.unshift(options.timezone);
+      }
+
+      timezones.forEach(timezone => {
+        if (compareZones(timezone, displayedTimezone)) {
+          return;
+        }
+
+        if (compareZones(timezone, watchingUserTimezone)) {
+          timezone = watchingUserTimezone;
+        }
+
+        previewedTimezones.push({
+          timezone,
+          dateTime: options.time
+            ? dateTime.tz(timezone).format("LLL")
+            : createDateTimeRange(dateTime, timezone)
         });
+      });
+
+      if (!previewedTimezones.length) {
+        previewedTimezones.push({
+          timezone: "Etc/UTC",
+          dateTime: options.time
+            ? dateTime.tz("Etc/UTC").format("LLL")
+            : createDateTimeRange(dateTime, "Etc/UTC")
+        });
+      }
 
-      return previewedTimezones;
+      return _.uniq(previewedTimezones, "timezone");
     };
 
     const _generateTextPreview = previews => {
diff --git a/plugins/discourse-local-dates/test/javascripts/acceptance/local-dates-test.js.es6 b/plugins/discourse-local-dates/test/javascripts/acceptance/local-dates-test.js.es6
index df2e73d..d265716 100644
--- a/plugins/discourse-local-dates/test/javascripts/acceptance/local-dates-test.js.es6
+++ b/plugins/discourse-local-dates/test/javascripts/acceptance/local-dates-test.js.es6
@@ -4,7 +4,10 @@ const sandbox = sinon.createSandbox();
 
 acceptance("Local Dates", {
   loggedIn: true,
-  settings: { discourse_local_dates_enabled: true },
+  settings: {
+    discourse_local_dates_enabled: true,
+    discourse_local_dates_default_timezones: "Europe/Paris|America/Los_Angeles"
+  },
   beforeEach() {
     freezeDateAndZone();
   },
@@ -56,6 +59,7 @@ function generateHTML(options = {}) {
 
   output += ` data-date="${options.date || DEFAULT_DATE}"`;
   if (options.format) output += ` data-format="${options.format}"`;
+  if (options.timezones) output += ` data-timezones="${options.timezones}"`;
   if (options.time) output += ` data-time="${options.time}"`;
   output += ` data-timezone="${options.timezone || DEFAULT_ZONE}"`;
   if (options.calendar) output += ` data-calendar="${options.calendar}"`;
@@ -319,7 +323,6 @@ test("displayedTimezone", assert => {
 test("tooltip", assert => {
   let html = generateHTML({ timezone: "America/Chicago" });
   let transformed = $(html).applyLocalDates();
-
   let htmlToolip = transformed.attr("data-html-tooltip");
   let currentUserPreview = $(htmlToolip).find(".preview.current");
   let timezone = currentUserPreview.find(".timezone").text();
@@ -348,26 +351,83 @@ test("tooltip", assert => {
     );
   });
 
-  html = generateHTML({ timezone: "America/Chicago", time: "14:00:00" });
+  html = generateHTML({
+    timezones: "Etc/UTC",
+    timezone: "America/Chicago",
+    time: "14:00:00"
+  });
   transformed = $(html).applyLocalDates();
   htmlToolip = transformed.attr("data-html-tooltip");
 
-  const $preview = $(htmlToolip)
-    .find(".preview")
-    .first();
-  dateTime = $preview.find(".date-time").text();
-  timezone = $preview.find(".timezone").text();
-
   assert.ok(
     !exists(".preview.current"),
     "doesn’t create current timezone when displayed timezone equals watching user timezone"
   );
+
+  let $firstPreview = $(htmlToolip).find(".preview:nth-child(1)");
+  dateTime = $firstPreview.find(".date-time").text();
+  timezone = $firstPreview.find(".timezone").text();
+  assert.equal(
+    dateTime,
+    "June 20, 2018 2:00 PM",
+    "it doesn’t create range if time has been set"
+  );
+  assert.equal(timezone, "Chicago", "it adds the timezone of the creator");
+
+  let $secondPreview = $(htmlToolip).find(".preview:nth-child(2)");
+  dateTime = $secondPreview.find(".date-time").text();
+  timezone = $secondPreview.find(".timezone").text();
   assert.equal(
     dateTime,
     "June 20, 2018 7:00 PM",
     "it doesn’t create range if time has been set"
   );
   assert.equal(timezone, "UTC", "Etc/UTC is rewritten to UTC");
+
+  freezeDateAndZone(moment("2018-11-26 21:00:00"), "Europe/Vienna", () => {
+    html = generateHTML({
+      date: "2018-11-22",
+      timezone: "America/Chicago",
+      time: "14:00"
+    });
+    transformed = $(html).applyLocalDates();
+    htmlToolip = transformed.attr("data-html-tooltip");
+
+    $firstPreview = $(htmlToolip)
+      .find(".preview")
+      .first();
+
+    assert.equal(
+      $firstPreview.find(".timezone").text(),
+      "Chicago",
+      "it adds the creator timezone to the previews"
+    );
+    assert.equal(
+      $firstPreview.find(".date-time").text(),
+      "November 22, 2018 2:00 PM",
+      "it adds the creator timezone to the previews"
+    );
+  });
+
+  freezeDateAndZone(DEFAULT_DATE, "Europe/Vienna", () => {
+    html = generateHTML({
+      date: "2018-11-22",
+      timezone: "America/Chicago",
+      timezones: "Europe/Paris"
+    });
+    transformed = $(html).applyLocalDates();
+    htmlToolip = transformed.attr("data-html-tooltip");
+
+    $firstPreview = $(htmlToolip)
+      .find(".preview")
+      .first();
+
+    assert.equal(
+      $firstPreview.find(".timezone").text(),
+      "Vienna",
+      "it rewrites timezone with same offset and different name than watching user"
+    );
+  });
 });
 
 test("test utils", assert => {

GitHub

why not use a regular function here? You could put it at the top of the file. I find it clearer that way to ensure it’s not part of a closure.

1 Like