-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Only set timezone when user setting is a valid timezone #57850
Changes from 1 commit
27bd301
4ecefa7
07f5fc4
f3bd5ed
10d3edb
60f76e3
84c655b
c23333b
c7e412d
d8f04bb
4bc4111
4d200d1
ea81cc2
35af6e6
7ad2e8b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test might be a bit more obvious if the tz name was something like |
||
const dow$ = new BehaviorSubject('dow1'); | ||
|
||
const uiSettings = uiSettingsServiceMock.createSetupContract(); | ||
uiSettings.get$.mockReturnValueOnce(tz$).mockReturnValueOnce(dow$); | ||
jbudz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
service.start({ uiSettings }); | ||
await flushPromises(); | ||
expect(momentMock.tz.setDefault).not.toHaveBeenCalledWith('tz4'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add an assertion that it was called with no args? |
||
}); | ||
|
||
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$); | ||
jbudz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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'); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like Moment has moment.tz.zone which checks for existance, returning the zone if loaded or null otherwise. Could something like this work?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I gave it a try - zone ends up returning a
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, Zone is an object. I do think it's better to use |
||
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); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would you mind to update the test suit name?
does not set unknown zone