From 27bd301cf38ebb253b7e30d499f055ca431519e1 Mon Sep 17 00:00:00 2001 From: Jonathan Budzenski Date: Tue, 18 Feb 2020 08:49:03 -0600 Subject: [PATCH 1/8] Only set timezone when user setting is a valid timezone Currently we set the global browser timezone based on the user's advanced settings. This setting includes a list of timezones and a non-standard 'Browser' option which can be translated as set the timezone to my current. In order to avoid warnings and possible future errors we only set timezone if it exists in moments list of installed timezones Closes #38515 --- .../moment/moment_service.test.mocks.ts | 1 + .../moment/moment_service.test.ts | 24 +++++++++++++++++++ .../integrations/moment/moment_service.ts | 10 +++++++- 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/core/public/integrations/moment/moment_service.test.mocks.ts b/src/core/public/integrations/moment/moment_service.test.mocks.ts index 2fa013a6bf1321..29684eeea3a847 100644 --- a/src/core/public/integrations/moment/moment_service.test.mocks.ts +++ b/src/core/public/integrations/moment/moment_service.test.mocks.ts @@ -21,6 +21,7 @@ export const momentMock = { locale: jest.fn(() => 'default-locale'), tz: { setDefault: jest.fn(), + names: jest.fn(() => ['tz1', 'tz2', 'tz3']), }, weekdays: jest.fn(() => ['dow1', 'dow2', 'dow3']), updateLocale: jest.fn(), diff --git a/src/core/public/integrations/moment/moment_service.test.ts b/src/core/public/integrations/moment/moment_service.test.ts index c9b4479f2f1636..c051c8f5e45c50 100644 --- a/src/core/public/integrations/moment/moment_service.test.ts +++ b/src/core/public/integrations/moment/moment_service.test.ts @@ -47,6 +47,30 @@ describe('MomentService', () => { expect(momentMock.updateLocale).toHaveBeenCalledWith('default-locale', { week: { dow: 0 } }); }); + it('uses the default timezone when a zone is not defined', async () => { + const tz$ = new BehaviorSubject('tz4'); + const dow$ = new BehaviorSubject('dow1'); + + const uiSettings = uiSettingsServiceMock.createSetupContract(); + uiSettings.get$.mockReturnValueOnce(tz$).mockReturnValueOnce(dow$); + + service.start({ uiSettings }); + await flushPromises(); + expect(momentMock.tz.setDefault).not.toHaveBeenCalledWith('tz4'); + }); + + it('sets timezone when a zone is defined', async () => { + const tz$ = new BehaviorSubject('tz3'); + const dow$ = new BehaviorSubject('dow1'); + + const uiSettings = uiSettingsServiceMock.createSetupContract(); + uiSettings.get$.mockReturnValueOnce(tz$).mockReturnValueOnce(dow$); + + service.start({ uiSettings }); + await flushPromises(); + expect(momentMock.tz.setDefault).toHaveBeenCalledWith('tz3'); + }); + test('updates moment config', async () => { const tz$ = new BehaviorSubject('tz1'); const dow$ = new BehaviorSubject('dow1'); diff --git a/src/core/public/integrations/moment/moment_service.ts b/src/core/public/integrations/moment/moment_service.ts index 65f2bdea02933b..678ce23b456c02 100644 --- a/src/core/public/integrations/moment/moment_service.ts +++ b/src/core/public/integrations/moment/moment_service.ts @@ -35,7 +35,15 @@ export class MomentService implements CoreService { public async setup() {} public async start({ uiSettings }: StartDeps) { - const setDefaultTimezone = (tz: string) => moment.tz.setDefault(tz); + const setDefaultTimezone = (tz: string) => { + const timezones = moment.tz.names(); + const hasTimezone = timezones.includes(tz); + if (hasTimezone) { + moment.tz.setDefault(tz); + } else { + moment.tz.setDefault(); + } + }; const setStartDayOfWeek = (day: string) => { const dow = moment.weekdays().indexOf(day); moment.updateLocale(moment.locale(), { week: { dow } } as any); From 07f5fc47d7a6e226ee79119e8081709921fedb0a Mon Sep 17 00:00:00 2001 From: Jonathan Budzenski Date: Thu, 20 Feb 2020 14:32:45 -0600 Subject: [PATCH 2/8] feedback --- .../integrations/moment/moment_service.test.mocks.ts | 4 +++- .../public/integrations/moment/moment_service.test.ts | 4 ++-- src/core/public/integrations/moment/moment_service.ts | 10 +++------- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/core/public/integrations/moment/moment_service.test.mocks.ts b/src/core/public/integrations/moment/moment_service.test.mocks.ts index 29684eeea3a847..bb13232157b785 100644 --- a/src/core/public/integrations/moment/moment_service.test.mocks.ts +++ b/src/core/public/integrations/moment/moment_service.test.mocks.ts @@ -21,7 +21,9 @@ export const momentMock = { locale: jest.fn(() => 'default-locale'), tz: { setDefault: jest.fn(), - names: jest.fn(() => ['tz1', 'tz2', 'tz3']), + zone: jest.fn( + z => [{ name: 'tz1' }, { name: 'tz2' }, { name: 'tz3' }].find(f => z === f.name) || null + ), }, weekdays: jest.fn(() => ['dow1', 'dow2', 'dow3']), updateLocale: jest.fn(), diff --git a/src/core/public/integrations/moment/moment_service.test.ts b/src/core/public/integrations/moment/moment_service.test.ts index c051c8f5e45c50..e4d1400305f310 100644 --- a/src/core/public/integrations/moment/moment_service.test.ts +++ b/src/core/public/integrations/moment/moment_service.test.ts @@ -48,7 +48,7 @@ describe('MomentService', () => { }); it('uses the default timezone when a zone is not defined', async () => { - const tz$ = new BehaviorSubject('tz4'); + const tz$ = new BehaviorSubject('timezone/undefined'); const dow$ = new BehaviorSubject('dow1'); const uiSettings = uiSettingsServiceMock.createSetupContract(); @@ -56,7 +56,7 @@ describe('MomentService', () => { service.start({ uiSettings }); await flushPromises(); - expect(momentMock.tz.setDefault).not.toHaveBeenCalledWith('tz4'); + expect(momentMock.tz.setDefault).toHaveBeenCalledWith(undefined); }); it('sets timezone when a zone is defined', async () => { diff --git a/src/core/public/integrations/moment/moment_service.ts b/src/core/public/integrations/moment/moment_service.ts index 678ce23b456c02..563748a8b8d415 100644 --- a/src/core/public/integrations/moment/moment_service.ts +++ b/src/core/public/integrations/moment/moment_service.ts @@ -19,6 +19,7 @@ import moment from 'moment-timezone'; import { merge, Subscription } from 'rxjs'; +import { get } from 'lodash'; import { tap } from 'rxjs/operators'; import { IUiSettingsClient } from '../../ui_settings'; @@ -36,13 +37,8 @@ export class MomentService implements CoreService { public async start({ uiSettings }: StartDeps) { const setDefaultTimezone = (tz: string) => { - const timezones = moment.tz.names(); - const hasTimezone = timezones.includes(tz); - if (hasTimezone) { - moment.tz.setDefault(tz); - } else { - moment.tz.setDefault(); - } + const zone: string | undefined = get(moment.tz.zone(tz), 'name'); + moment.tz.setDefault(zone); }; const setStartDayOfWeek = (day: string) => { const dow = moment.weekdays().indexOf(day); From 10d3edb76623fa30b295784f423965f5fb08e985 Mon Sep 17 00:00:00 2001 From: Jonathan Budzenski Date: Mon, 2 Mar 2020 10:38:16 -0600 Subject: [PATCH 3/8] only set timezone if defined --- src/core/public/integrations/moment/moment_service.test.ts | 2 +- src/core/public/integrations/moment/moment_service.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/public/integrations/moment/moment_service.test.ts b/src/core/public/integrations/moment/moment_service.test.ts index e4d1400305f310..0884a59afba9ca 100644 --- a/src/core/public/integrations/moment/moment_service.test.ts +++ b/src/core/public/integrations/moment/moment_service.test.ts @@ -56,7 +56,7 @@ describe('MomentService', () => { service.start({ uiSettings }); await flushPromises(); - expect(momentMock.tz.setDefault).toHaveBeenCalledWith(undefined); + expect(momentMock.tz.setDefault).not.toHaveBeenCalled(); }); it('sets timezone when a zone is defined', async () => { diff --git a/src/core/public/integrations/moment/moment_service.ts b/src/core/public/integrations/moment/moment_service.ts index 563748a8b8d415..e2cbee288c68f9 100644 --- a/src/core/public/integrations/moment/moment_service.ts +++ b/src/core/public/integrations/moment/moment_service.ts @@ -38,7 +38,7 @@ export class MomentService implements CoreService { public async start({ uiSettings }: StartDeps) { const setDefaultTimezone = (tz: string) => { const zone: string | undefined = get(moment.tz.zone(tz), 'name'); - moment.tz.setDefault(zone); + if (zone) moment.tz.setDefault(zone); }; const setStartDayOfWeek = (day: string) => { const dow = moment.weekdays().indexOf(day); From 60f76e3f147e3b4812c9a46899b03f3d475ee94d Mon Sep 17 00:00:00 2001 From: Jonathan Budzenski Date: Tue, 3 Mar 2020 11:43:01 -0600 Subject: [PATCH 4/8] Update src/core/public/integrations/moment/moment_service.ts Co-Authored-By: Mikhail Shustov --- src/core/public/integrations/moment/moment_service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/public/integrations/moment/moment_service.ts b/src/core/public/integrations/moment/moment_service.ts index e2cbee288c68f9..c77b999813254d 100644 --- a/src/core/public/integrations/moment/moment_service.ts +++ b/src/core/public/integrations/moment/moment_service.ts @@ -37,8 +37,8 @@ export class MomentService implements CoreService { public async start({ uiSettings }: StartDeps) { const setDefaultTimezone = (tz: string) => { - const zone: string | undefined = get(moment.tz.zone(tz), 'name'); - if (zone) moment.tz.setDefault(zone); + const zone = moment.tz.zone(tz); + if (zone) moment.tz.setDefault(zone.name); }; const setStartDayOfWeek = (day: string) => { const dow = moment.weekdays().indexOf(day); From 84c655b7d4c7cbe3242cebf8787969cdd0bb7baf Mon Sep 17 00:00:00 2001 From: Jonathan Budzenski Date: Tue, 3 Mar 2020 11:43:15 -0600 Subject: [PATCH 5/8] Update src/core/public/integrations/moment/moment_service.ts Co-Authored-By: Mikhail Shustov --- src/core/public/integrations/moment/moment_service.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/public/integrations/moment/moment_service.ts b/src/core/public/integrations/moment/moment_service.ts index c77b999813254d..69cc29231d6e4b 100644 --- a/src/core/public/integrations/moment/moment_service.ts +++ b/src/core/public/integrations/moment/moment_service.ts @@ -19,7 +19,6 @@ import moment from 'moment-timezone'; import { merge, Subscription } from 'rxjs'; -import { get } from 'lodash'; import { tap } from 'rxjs/operators'; import { IUiSettingsClient } from '../../ui_settings'; From c23333ba00c229cab114bf076caa72dd12c8b35f Mon Sep 17 00:00:00 2001 From: Jonathan Budzenski Date: Tue, 3 Mar 2020 11:43:23 -0600 Subject: [PATCH 6/8] Update src/core/public/integrations/moment/moment_service.test.ts Co-Authored-By: Mikhail Shustov --- src/core/public/integrations/moment/moment_service.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/core/public/integrations/moment/moment_service.test.ts b/src/core/public/integrations/moment/moment_service.test.ts index 0884a59afba9ca..db77c849ad3806 100644 --- a/src/core/public/integrations/moment/moment_service.test.ts +++ b/src/core/public/integrations/moment/moment_service.test.ts @@ -61,10 +61,8 @@ describe('MomentService', () => { it('sets timezone when a zone is defined', async () => { const tz$ = new BehaviorSubject('tz3'); - const dow$ = new BehaviorSubject('dow1'); - const uiSettings = uiSettingsServiceMock.createSetupContract(); - uiSettings.get$.mockReturnValueOnce(tz$).mockReturnValueOnce(dow$); + uiSettings.get$.mockReturnValueOnce(tz$); service.start({ uiSettings }); await flushPromises(); From c7e412d5193aaa4953dba9ebfa88f0c3f9d1c67c Mon Sep 17 00:00:00 2001 From: Jonathan Budzenski Date: Tue, 3 Mar 2020 11:43:40 -0600 Subject: [PATCH 7/8] Update src/core/public/integrations/moment/moment_service.test.ts Co-Authored-By: Mikhail Shustov --- src/core/public/integrations/moment/moment_service.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/core/public/integrations/moment/moment_service.test.ts b/src/core/public/integrations/moment/moment_service.test.ts index db77c849ad3806..95d05bfd580191 100644 --- a/src/core/public/integrations/moment/moment_service.test.ts +++ b/src/core/public/integrations/moment/moment_service.test.ts @@ -49,10 +49,8 @@ describe('MomentService', () => { it('uses the default timezone when a zone is not defined', async () => { const tz$ = new BehaviorSubject('timezone/undefined'); - const dow$ = new BehaviorSubject('dow1'); - const uiSettings = uiSettingsServiceMock.createSetupContract(); - uiSettings.get$.mockReturnValueOnce(tz$).mockReturnValueOnce(dow$); + uiSettings.get$.mockReturnValueOnce(tz$); service.start({ uiSettings }); await flushPromises(); From d8f04bbc5eddce9b4435a2732191fb675abccf86 Mon Sep 17 00:00:00 2001 From: Jonathan Budzenski Date: Tue, 3 Mar 2020 13:27:55 -0600 Subject: [PATCH 8/8] update test name --- src/core/public/integrations/moment/moment_service.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/public/integrations/moment/moment_service.test.ts b/src/core/public/integrations/moment/moment_service.test.ts index 0884a59afba9ca..4423109b8e438a 100644 --- a/src/core/public/integrations/moment/moment_service.test.ts +++ b/src/core/public/integrations/moment/moment_service.test.ts @@ -47,7 +47,7 @@ describe('MomentService', () => { expect(momentMock.updateLocale).toHaveBeenCalledWith('default-locale', { week: { dow: 0 } }); }); - it('uses the default timezone when a zone is not defined', async () => { + it('does not set unknkown zone', async () => { const tz$ = new BehaviorSubject('timezone/undefined'); const dow$ = new BehaviorSubject('dow1');