[ui] Use TZ offset to display cron string for user timezone

dagster-io · Nov 5, 2024 · d69c470 · d69c470
1 parent a156e32
commit d69c470
Show file tree

Hide file tree

Showing 12 changed files with 248 additions and 56 deletions.
diff --git a/js_modules/dagster-ui/packages/ui-core/package.json b/js_modules/dagster-ui/packages/ui-core/package.json
@@ -42,7 +42,7 @@
     "chartjs-adapter-date-fns": "^2.0.0",
     "chartjs-plugin-zoom": "^1.1.1",
     "codemirror": "^5.65.2",
-    "cronstrue": "^1.84.0",
+    "cronstrue": "^2.51.0",
     "dagre": "dagster-io/dagre#0.8.5",
     "date-fns": "^2.28.0",
     "dayjs": "^1.11.7",

diff --git a/js_modules/dagster-ui/packages/ui-core/src/assets/OverdueTag.tsx b/js_modules/dagster-ui/packages/ui-core/src/assets/OverdueTag.tsx
@@ -223,7 +223,7 @@ export const freshnessPolicyDescription = (
   const {cronSchedule, maximumLagMinutes, cronScheduleTimezone} = freshnessPolicy;
   const nbsp = '\xa0';
   const cronDesc = cronSchedule
-    ? humanCronString(cronSchedule, cronScheduleTimezone ? cronScheduleTimezone : 'UTC').replace(
+    ? humanCronString(cronSchedule, {longTimezoneName: cronScheduleTimezone || 'UTC'}).replace(
         /^At /,
         '',
       )

diff --git a/js_modules/dagster-ui/packages/ui-core/src/nav/LeftNavItem.tsx b/js_modules/dagster-ui/packages/ui-core/src/nav/LeftNavItem.tsx
@@ -76,7 +76,9 @@ export const LeftNavItem = React.forwardRef(
             return (
               <div>
                 Schedule:{' '}
-                <strong>{humanCronString(cronSchedule, executionTimezone || 'UTC')}</strong>
+                <strong>
+                  {humanCronString(cronSchedule, {longTimezoneName: executionTimezone || 'UTC'})}
+                </strong>
               </div>
             );
           }

diff --git a/js_modules/dagster-ui/packages/ui-core/src/nav/ScheduleAndSensorDialog.tsx b/js_modules/dagster-ui/packages/ui-core/src/nav/ScheduleAndSensorDialog.tsx
@@ -77,7 +77,9 @@ export const ScheduleAndSensorDialog = ({
                       </Link>
                     </td>
                     <td>
-                      {humanCronString(schedule.cronSchedule, schedule.executionTimezone || 'UTC')}
+                      {humanCronString(schedule.cronSchedule, {
+                        longTimezoneName: schedule.executionTimezone || 'UTC',
+                      })}
                     </td>
                   </tr>
                 ))}

diff --git a/js_modules/dagster-ui/packages/ui-core/src/nav/ScheduleOrSensorTag.tsx b/js_modules/dagster-ui/packages/ui-core/src/nav/ScheduleOrSensorTag.tsx
@@ -101,7 +101,7 @@ const MatchingSchedule = ({
           to={workspacePathFromAddress(repoAddress, `/schedules/${schedule.name}`)}
           style={{overflow: 'hidden', textOverflow: 'ellipsis'}}
         >
-          {humanCronString(cronSchedule, executionTimezone || 'UTC')}
+          {humanCronString(cronSchedule, {longTimezoneName: executionTimezone || 'UTC'})}
         </Link>
         {showSwitch ? (
           <ScheduleSwitch size="small" repoAddress={repoAddress} schedule={schedule} />

diff --git a/js_modules/dagster-ui/packages/ui-core/src/schedules/CronTag.tsx b/js_modules/dagster-ui/packages/ui-core/src/schedules/CronTag.tsx
@@ -1,7 +1,11 @@
-import {Tag, Tooltip} from '@dagster-io/ui-components';
+import {CaptionMono, MetadataTable, Tag, Tooltip} from '@dagster-io/ui-components';
+import {useContext} from 'react';
 import styled from 'styled-components';
 
+import {hourOffsetFromUTC} from './hourOffsetFromUTC';
 import {humanCronString} from './humanCronString';
+import {TimeContext} from '../app/time/TimeContext';
+import {browserTimezone} from '../app/time/browserTimezone';
 
 interface Props {
   cronSchedule: string;
@@ -10,12 +14,36 @@ interface Props {
 
 export const CronTag = (props: Props) => {
   const {cronSchedule, executionTimezone} = props;
-  const humanString = humanCronString(cronSchedule, executionTimezone || 'UTC');
+  const {
+    timezone: [storedTimezone],
+  } = useContext(TimeContext);
+
+  const longTimezoneName = executionTimezone || 'UTC';
+  const humanStringWithExecutionTimezone = humanCronString(cronSchedule, {longTimezoneName});
+  const userTimezone = storedTimezone === 'Automatic' ? browserTimezone() : storedTimezone;
+
+  const userTimezoneOffset = hourOffsetFromUTC(userTimezone);
+  const executionTimezoneOffset = hourOffsetFromUTC(longTimezoneName);
+  const tzOffset = userTimezoneOffset - executionTimezoneOffset;
+
+  const humanStringWithUserTimezone = humanCronString(cronSchedule, {
+    longTimezoneName: userTimezone,
+    tzOffset,
+  });
+
+  const tooltipContent = (
+    <MetadataTable
+      rows={[
+        {key: 'Cron value', value: <CaptionMono>{cronSchedule}</CaptionMono>},
+        {key: 'Your time', value: <span>{humanStringWithUserTimezone}</span>},
+      ]}
+    />
+  );
 
   return (
     <Container>
-      <Tooltip content={cronSchedule} placement="top">
-        <Tag icon="schedule">{humanString}</Tag>
+      <Tooltip content={tooltipContent} placement="top">
+        <Tag icon="schedule">{humanStringWithExecutionTimezone}</Tag>
       </Tooltip>
     </Container>
   );

diff --git a/js_modules/dagster-ui/packages/ui-core/src/schedules/ScheduleDetails.tsx b/js_modules/dagster-ui/packages/ui-core/src/schedules/ScheduleDetails.tsx
@@ -165,7 +165,11 @@ export const ScheduleDetails = (props: {
             <td>
               {cronSchedule ? (
                 <Group direction="row" spacing={8}>
-                  <span>{humanCronString(cronSchedule, executionTimezone || 'UTC')}</span>
+                  <span>
+                    {humanCronString(cronSchedule, {
+                      longTimezoneName: executionTimezone || 'UTC',
+                    })}
+                  </span>
                   <Code>({cronSchedule})</Code>
                 </Group>
               ) : (

diff --git a/js_modules/dagster-ui/packages/ui-core/src/schedules/__tests__/hourOffsetFromUTC.test.ts b/js_modules/dagster-ui/packages/ui-core/src/schedules/__tests__/hourOffsetFromUTC.test.ts
@@ -0,0 +1,60 @@
+import {hourOffsetFromUTC} from '../hourOffsetFromUTC';
+
+describe('hourOffsetFromUTC', () => {
+  const JUL_1_2024_9_AM_CDT = new Date('2024-07-01T15:00:00.000Z');
+  const NOV_5_2024_9_AM_CST = new Date('2024-11-05T15:00:00.000Z');
+
+  beforeAll(() => {
+    jest.useFakeTimers();
+  });
+
+  afterAll(() => {
+    jest.useRealTimers();
+  });
+
+  describe('Standard time in Northern Hemisphere', () => {
+    beforeEach(() => {
+      jest.setSystemTime(NOV_5_2024_9_AM_CST);
+    });
+
+    it('returns the offset for a given timezone', () => {
+      // Timezones with DST in Northern Hemisphere:
+      expect(hourOffsetFromUTC('America/New_York')).toBe(-5);
+      expect(hourOffsetFromUTC('America/Los_Angeles')).toBe(-8);
+      expect(hourOffsetFromUTC('America/St_Johns')).toBe(-3.5);
+
+      // Timezones without DST:
+      expect(hourOffsetFromUTC('UTC')).toBe(0);
+      expect(hourOffsetFromUTC('Europe/Berlin')).toBe(1);
+      expect(hourOffsetFromUTC('Asia/Tokyo')).toBe(9);
+      expect(hourOffsetFromUTC('Asia/Shanghai')).toBe(8);
+
+      // Timezones with DST in Southern Hemisphere:
+      expect(hourOffsetFromUTC('America/Santiago')).toBe(-3);
+      expect(hourOffsetFromUTC('Australia/Adelaide')).toBe(10.5);
+    });
+  });
+
+  describe('Daylight savings time in Northern Hemisphere', () => {
+    beforeEach(() => {
+      jest.setSystemTime(JUL_1_2024_9_AM_CDT);
+    });
+
+    it('returns the offset for a given timezone', () => {
+      // Timezones with DST in Northern Hemisphere:
+      expect(hourOffsetFromUTC('America/New_York')).toBe(-4);
+      expect(hourOffsetFromUTC('America/Los_Angeles')).toBe(-7);
+      expect(hourOffsetFromUTC('America/St_Johns')).toBe(-2.5);
+      expect(hourOffsetFromUTC('Europe/Berlin')).toBe(2);
+
+      // Timezones without DST:
+      expect(hourOffsetFromUTC('UTC')).toBe(0);
+      expect(hourOffsetFromUTC('Asia/Tokyo')).toBe(9);
+      expect(hourOffsetFromUTC('Asia/Shanghai')).toBe(8);
+
+      // Timezones with DST in Southern Hemisphere:
+      expect(hourOffsetFromUTC('America/Santiago')).toBe(-4);
+      expect(hourOffsetFromUTC('Australia/Adelaide')).toBe(9.5);
+    });
+  });
+});
diff --git a/js_modules/dagster-ui/packages/ui-core/src/schedules/__tests__/humanCronString.test.ts b/js_modules/dagster-ui/packages/ui-core/src/schedules/__tests__/humanCronString.test.ts
@@ -11,69 +11,127 @@ describe('humanCronString', () => {
   describe('Timezone', () => {
     it('shows timezone if provided, if cron specifies a time', () => {
       const timezone = 'America/Phoenix';
-      expect(humanCronString('@daily', timezone)).toBe('At 12:00 AM MST');
-      expect(humanCronString('@weekly', timezone)).toBe('At 12:00 AM MST, only on Sunday');
-      expect(humanCronString('@monthly', timezone)).toBe('At 12:00 AM MST, on day 1 of the month');
-      expect(humanCronString('0 23 ? * MON-FRI', timezone)).toBe(
+      const timezoneConfig = {longTimezoneName: timezone};
+      expect(humanCronString('@daily', timezoneConfig)).toBe('At 12:00 AM MST');
+      expect(humanCronString('@weekly', timezoneConfig)).toBe('At 12:00 AM MST, only on Sunday');
+      expect(humanCronString('@monthly', timezoneConfig)).toBe(
+        'At 12:00 AM MST, on day 1 of the month',
+      );
+      expect(humanCronString('0 23 ? * MON-FRI', timezoneConfig)).toBe(
         'At 11:00 PM MST, Monday through Friday',
       );
-      expect(humanCronString('0 23 * * *', timezone)).toBe('At 11:00 PM MST');
+      expect(humanCronString('0 23 * * *', timezoneConfig)).toBe('At 11:00 PM MST');
     });
 
     it('does not show timezone even if provided, if cron does not specify a time', () => {
       const timezone = 'America/Phoenix';
-      expect(humanCronString('* * * * *', timezone)).toBe('Every minute');
-      expect(humanCronString('* * * 6-8 *', timezone)).toBe('Every minute, June through August');
-      expect(humanCronString('* * * * MON-FRI', timezone)).toBe(
+      const timezoneConfig = {longTimezoneName: timezone};
+      expect(humanCronString('* * * * *', timezoneConfig)).toBe('Every minute');
+      expect(humanCronString('* * * 6-8 *', timezoneConfig)).toBe(
+        'Every minute, June through August',
+      );
+      expect(humanCronString('* * * * MON-FRI', timezoneConfig)).toBe(
         'Every minute, Monday through Friday',
       );
     });
 
     it('shows timezones on actual times, if cron is showing "X minutes..." or "X seconds..."', () => {
       const timezone = 'America/Phoenix';
-      expect(humanCronString('0 5 0/1 * * ?', timezone)).toBe('At 5 minutes past the hour');
-      expect(humanCronString('30 * * 6-8 *', timezone)).toBe(
+      const timezoneConfig = {longTimezoneName: timezone};
+      expect(humanCronString('0 5 0/1 * * ?', timezoneConfig)).toBe('At 5 minutes past the hour');
+      expect(humanCronString('30 * * 6-8 *', timezoneConfig)).toBe(
         'At 30 minutes past the hour, June through August',
       );
-      expect(humanCronString('30 */5 * * * *', timezone)).toBe(
+      expect(humanCronString('30 */5 * * * *', timezoneConfig)).toBe(
         'At 30 seconds past the minute, every 5 minutes',
       );
-      expect(humanCronString('2,4-5 1 * * *', timezone)).toBe(
+      expect(humanCronString('2,4-5 1 * * *', timezoneConfig)).toBe(
         'At 2 and 4 through 5 minutes past the hour, at 01:00 AM MST',
       );
     });
 
     it('shows timezone in complex time cases', () => {
       const timezone = 'America/Phoenix';
-      expect(humanCronString('2-59/3 1,9,22 11-26 1-6 ?', timezone)).toBe(
+      const timezoneConfig = {longTimezoneName: timezone};
+      expect(humanCronString('2-59/3 1,9,22 11-26 1-6 ?', timezoneConfig)).toBe(
         'Every 3 minutes, minutes 2 through 59 past the hour, at 01:00 AM MST, 09:00 AM MST, and 10:00 PM MST, between day 11 and 26 of the month, January through June',
       );
-      expect(humanCronString('12-50 0-10 6 * * * 2022', timezone)).toBe(
+      expect(humanCronString('12-50 0-10 6 * * * 2022', timezoneConfig)).toBe(
         'Seconds 12 through 50 past the minute, minutes 0 through 10 past the hour, at 06:00 AM MST, only in 2022',
       );
     });
 
     it('shows timezone (UTC) if provided, if cron specifies a time', () => {
       const timezone = 'UTC';
-      expect(humanCronString('@daily', timezone)).toBe('At 12:00 AM UTC');
-      expect(humanCronString('@weekly', timezone)).toBe('At 12:00 AM UTC, only on Sunday');
-      expect(humanCronString('@monthly', timezone)).toBe('At 12:00 AM UTC, on day 1 of the month');
-      expect(humanCronString('0 23 ? * MON-FRI', timezone)).toBe(
+      const timezoneConfig = {longTimezoneName: timezone};
+      expect(humanCronString('@daily', timezoneConfig)).toBe('At 12:00 AM UTC');
+      expect(humanCronString('@weekly', timezoneConfig)).toBe('At 12:00 AM UTC, only on Sunday');
+      expect(humanCronString('@monthly', timezoneConfig)).toBe(
+        'At 12:00 AM UTC, on day 1 of the month',
+      );
+      expect(humanCronString('0 23 ? * MON-FRI', timezoneConfig)).toBe(
         'At 11:00 PM UTC, Monday through Friday',
       );
-      expect(humanCronString('0 23 * * *', timezone)).toBe('At 11:00 PM UTC');
+      expect(humanCronString('0 23 * * *', timezoneConfig)).toBe('At 11:00 PM UTC');
     });
 
     describe('Invalid timezone', () => {
       it('skips showing timezone if invalid', () => {
         const timezone = 'FooBar';
-        expect(humanCronString('@daily', timezone)).toBe('At 12:00 AM');
-        expect(humanCronString('@weekly', timezone)).toBe('At 12:00 AM, only on Sunday');
-        expect(humanCronString('@monthly', timezone)).toBe('At 12:00 AM, on day 1 of the month');
-        expect(humanCronString('0 23 ? * MON-FRI', timezone)).toBe(
+        const timezoneConfig = {longTimezoneName: timezone};
+        expect(humanCronString('@daily', timezoneConfig)).toBe('At 12:00 AM');
+        expect(humanCronString('@weekly', timezoneConfig)).toBe('At 12:00 AM, only on Sunday');
+        expect(humanCronString('@monthly', timezoneConfig)).toBe(
+          'At 12:00 AM, on day 1 of the month',
+        );
+        expect(humanCronString('0 23 ? * MON-FRI', timezoneConfig)).toBe(
           'At 11:00 PM, Monday through Friday',
         );
-        expect(humanCronString('0 23 * * *', timezone)).toBe('At 11:00 PM');
+        expect(humanCronString('0 23 * * *', timezoneConfig)).toBe('At 11:00 PM');
+      });
+    });
+
+    describe('With timezone offset', () => {
+      it('applies timezone offset for westward timezones, if provided', () => {
+        const userTimezone = 'America/Los_Angeles';
+        const timezoneConfig = {
+          longTimezoneName: userTimezone,
+          tzOffset: -1, // Difference between MST and PST
+        };
+        expect(humanCronString('@daily', timezoneConfig)).toBe('At 11:00 PM PST');
+      });
+
+      it('applies timezone offset for eastward timezones, if provided', () => {
+        const userTimezone = 'America/St_Johns';
+        const timezoneConfig = {
+          longTimezoneName: userTimezone,
+          tzOffset: 4.5, // Difference between Newfoundland standard time and PST
+        };
+        expect(humanCronString('@daily', timezoneConfig)).toBe('At 04:30 AM GMT-3:30');
+      });
+
+      it('applies timezone offset for timezones across the dateline, if provided', () => {
+        const userTimezone = 'Asia/Tokyo';
+        const timezoneConfig = {
+          longTimezoneName: userTimezone,
+          tzOffset: 16, // Difference between Tokyo and PST
+        };
+        expect(humanCronString('@daily', timezoneConfig)).toBe('At 04:00 PM GMT+9');
+      });
+
+      it('applies timezone offset for weekly cron schedule for timezones across the dateline', () => {
+        const userTimezone = 'Asia/Tokyo';
+        const timezoneConfig = {
+          longTimezoneName: userTimezone,
+          tzOffset: 16, // Difference between Tokyo and PST
+        };
+
+        // This is wrong! It should be "At 04:00 PM GMT+9, only on Monday". Keeping the test
+        // here so that if we're able to upgrade cronstrue, a failure here tells us that it's fixed.
+        // https://github.com/bradymholt/cRonstrue/issues/313
+        expect(humanCronString('@weekly', timezoneConfig)).toBe(
+          'At 04:00 PM GMT+9, only on Sunday',
+        );
       });
     });
   });
@@ -97,13 +155,16 @@ describe('humanCronString', () => {
     });
 
     it('shows 24h format if locale uses it, and shows timezone if provided, if cron specifies a time', () => {
-      expect(humanCronString('@daily', timezone)).toBe('At 00:00 GMT+7');
-      expect(humanCronString('@weekly', timezone)).toBe('At 00:00 GMT+7, only on Sunday');
-      expect(humanCronString('@monthly', timezone)).toBe('At 00:00 GMT+7, on day 1 of the month');
-      expect(humanCronString('0 23 ? * MON-FRI', timezone)).toBe(
+      const timezoneConfig = {longTimezoneName: timezone};
+      expect(humanCronString('@daily', timezoneConfig)).toBe('At 00:00 GMT+7');
+      expect(humanCronString('@weekly', timezoneConfig)).toBe('At 00:00 GMT+7, only on Sunday');
+      expect(humanCronString('@monthly', timezoneConfig)).toBe(
+        'At 00:00 GMT+7, on day 1 of the month',
+      );
+      expect(humanCronString('0 23 ? * MON-FRI', timezoneConfig)).toBe(
         'At 23:00 GMT+7, Monday through Friday',
       );
-      expect(humanCronString('0 23 * * *', timezone)).toBe('At 23:00 GMT+7');
+      expect(humanCronString('0 23 * * *', timezoneConfig)).toBe('At 23:00 GMT+7');
     });
 
     it('shows 24h format if locale uses it, does not show timezone if not provided', () => {
@@ -115,10 +176,11 @@ describe('humanCronString', () => {
     });
 
     it('shows timezone in complex time cases', () => {
-      expect(humanCronString('2-59/3 1,9,22 11-26 1-6 ?', timezone)).toBe(
+      const timezoneConfig = {longTimezoneName: timezone};
+      expect(humanCronString('2-59/3 1,9,22 11-26 1-6 ?', timezoneConfig)).toBe(
         'Every 3 minutes, minutes 2 through 59 past the hour, at 01:00 GMT+7, 09:00 GMT+7, and 22:00 GMT+7, between day 11 and 26 of the month, January through June',
       );
-      expect(humanCronString('12-50 0-10 22 * * * 2022', timezone)).toBe(
+      expect(humanCronString('12-50 0-10 22 * * * 2022', timezoneConfig)).toBe(
         'Seconds 12 through 50 past the minute, minutes 0 through 10 past the hour, at 22:00 GMT+7, only in 2022',
       );
     });
@@ -127,7 +189,8 @@ describe('humanCronString', () => {
   describe('Cron union', () => {
     it('handles a single-item union of cron strings, with timezone', () => {
       const timezone = 'America/Phoenix';
-      expect(humanCronString("['2,4-5 1 * * *']", timezone)).toBe(
+      const timezoneConfig = {longTimezoneName: timezone};
+      expect(humanCronString("['2,4-5 1 * * *']", timezoneConfig)).toBe(
         'At 2 and 4 through 5 minutes past the hour, at 01:00 AM MST',
       );
     });
@@ -150,7 +213,8 @@ describe('humanCronString', () => {
 
     it('handles a union of cron strings, with timezone', () => {
       const timezone = 'America/Phoenix';
-      expect(humanCronString("['0 5 0/1 * * ?', '0 23 ? * MON-FRI']", timezone)).toBe(
+      const timezoneConfig = {longTimezoneName: timezone};
+      expect(humanCronString("['0 5 0/1 * * ?', '0 23 ? * MON-FRI']", timezoneConfig)).toBe(
         'At 5 minutes past the hour; At 11:00 PM MST, Monday through Friday',
       );
     });