fix: FE parsing of schedule with day strings (#2006)

Resolves: #1901

Summary:

We had a homegrown parser that only understood numbers, not strings like
MON or TUES. We replace the homegrown parser with cron-parser.

Details:

This was nearly a straight drop-in.

Impact:

Much less code/maintenance burden :D

What I learned:

Don't trust the README, sometimes you just gotta read the code or import
it and try it out. The `fields` representation of the parsed expression
was missing from their docs. I might open an issue or PR to update them!

site/package.json

@@ -36,6 +36,7 @@
     "@xstate/react": "3.0.0",
     "axios": "0.26.1",
     "can-ndjson-stream": "1.0.2",
+    "cron-parser": "4.4.0",
     "cronstrue": "2.5.0",
     "dayjs": "1.11.2",
     "formik": "2.2.9",

site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx

@@ -1,4 +1,5 @@
 import { useMachine } from "@xstate/react"
+import * as cronParser from "cron-parser"
 import dayjs from "dayjs"
 import timezone from "dayjs/plugin/timezone"
 import utc from "dayjs/plugin/utc"
@@ -12,7 +13,7 @@ import {
   WorkspaceScheduleFormValues,
 } from "../../components/WorkspaceScheduleForm/WorkspaceScheduleForm"
 import { firstOrItem } from "../../util/array"
-import { dowToWeeklyFlag, extractTimezone, stripTimezone } from "../../util/schedule"
+import { extractTimezone, stripTimezone } from "../../util/schedule"
 import { workspaceSchedule } from "../../xServices/workspaceSchedule/workspaceScheduleXService"
 
 // REMARK: timezone plugin depends on UTC
@@ -93,7 +94,7 @@ export const formValuesToTTLRequest = (values: WorkspaceScheduleFormValues): Typ
 
 export const workspaceToInitialValues = (workspace: TypesGen.Workspace): WorkspaceScheduleFormValues => {
   const schedule = workspace.autostart_schedule
-  const ttl = workspace.ttl_ms ? workspace.ttl_ms / (1000 * 60 * 60) : 0
+  const ttlHours = workspace.ttl_ms ? Math.round(workspace.ttl_ms / (1000 * 60 * 60)) : 0
 
   if (!schedule) {
     return {
@@ -106,22 +107,22 @@ export const workspaceToInitialValues = (workspace: TypesGen.Workspace): Workspa
       saturday: false,
       startTime: "",
       timezone: "",
-      ttl,
+      ttl: ttlHours,
     }
   }
 
   const timezone = extractTimezone(schedule, dayjs.tz.guess())
-  const cronString = stripTimezone(schedule)
 
-  // parts has the following format: "mm HH * * dow"
-  const parts = cronString.split(" ")
+  const expression = cronParser.parseExpression(stripTimezone(schedule))
 
-  // -> we skip month and day-of-month
-  const mm = parts[0]
-  const HH = parts[1]
-  const dow = parts[4]
+  const HH = expression.fields.hour.join("").padStart(2, "0")
+  const mm = expression.fields.minute.join("").padStart(2, "0")
 
-  const weeklyFlags = dowToWeeklyFlag(dow)
+  const weeklyFlags = [false, false, false, false, false, false, false]
+
+  for (const day of expression.fields.dayOfWeek) {
+    weeklyFlags[day % 7] = true
+  }
 
   return {
     sunday: weeklyFlags[0],
@@ -131,9 +132,9 @@ export const workspaceToInitialValues = (workspace: TypesGen.Workspace): Workspa
     thursday: weeklyFlags[4],
     friday: weeklyFlags[5],
     saturday: weeklyFlags[6],
-    startTime: `${HH.padStart(2, "0")}:${mm.padStart(2, "0")}`,
+    startTime: `${HH}:${mm}`,
     timezone,
-    ttl,
+    ttl: ttlHours,
   }
 }
 

site/src/util/schedule.test.ts

@@ -1,4 +1,4 @@
-import { dowToWeeklyFlag, extractTimezone, stripTimezone, WeeklyFlag } from "./schedule"
+import { extractTimezone, stripTimezone } from "./schedule"
 
 describe("util/schedule", () => {
   describe("stripTimezone", () => {
@@ -20,26 +20,4 @@ describe("util/schedule", () => {
       expect(extractTimezone(input)).toBe(expected)
     })
   })
-
-  describe("dowToWeeklyFlag", () => {
-    it.each<[string, WeeklyFlag]>([
-      // All days
-      ["*", [true, true, true, true, true, true, true]],
-      ["0-6", [true, true, true, true, true, true, true]],
-      ["1-7", [true, true, true, true, true, true, true]],
-
-      // Single number modulo 7
-      ["3", [false, false, false, true, false, false, false]],
-      ["0", [true, false, false, false, false, false, false]],
-      ["7", [true, false, false, false, false, false, false]],
-      ["8", [false, true, false, false, false, false, false]],
-
-      // Comma-separated Numbers, Ranges and Mixes
-      ["1,3,5", [false, true, false, true, false, true, false]],
-      ["1-2,4-5", [false, true, true, false, true, true, false]],
-      ["1,3-4,6", [false, true, false, true, true, false, true]],
-    ])(`dowToWeeklyFlag(%p) returns %p`, (dow, weeklyFlag) => {
-      expect(dowToWeeklyFlag(dow)).toEqual(weeklyFlag)
-    })
-  })
 })

site/src/util/schedule.ts

@@ -30,72 +30,3 @@ export const extractTimezone = (raw: string, defaultTZ = DEFAULT_TIMEZONE): stri
     return defaultTZ
   }
 }
-
-/**
- * WeeklyFlag is an array representing which days of the week are set or flagged
- *
- * @remarks
- *
- * A WeeklyFlag has an array size of 7 and should never have its size modified.
- * The 0th index is Sunday
- * The 6th index is Saturday
- */
-export type WeeklyFlag = [boolean, boolean, boolean, boolean, boolean, boolean, boolean]
-
-/**
- * dowToWeeklyFlag converts a dow cron string to a WeeklyFlag array.
- *
- * @example
- *
- * dowToWeeklyFlag("1") // [false, true, false, false, false, false, false]
- * dowToWeeklyFlag("1-5") // [false, true, true, true, true, true, false]
- * dowToWeeklyFlag("1,3-4,6") // [false, true, false, true, true, false, true]
- */
-export const dowToWeeklyFlag = (dow: string): WeeklyFlag => {
-  if (dow === "*") {
-    return [true, true, true, true, true, true, true]
-  }
-
-  const results: WeeklyFlag = [false, false, false, false, false, false, false]
-
-  const commaSeparatedRangeOrNum = dow.split(",")
-
-  for (const rangeOrNum of commaSeparatedRangeOrNum) {
-    const flags = processRangeOrNum(rangeOrNum)
-
-    flags.forEach((value, idx) => {
-      if (value) {
-        results[idx] = true
-      }
-    })
-  }
-
-  return results
-}
-
-/**
- * processRangeOrNum is a helper for dowToWeeklyFlag. It processes a range or
- * number (modulo 7) into a Weeklyflag boolean array.
- *
- * @example
- *
- * processRangeOrNum("1") // [false, true, false, false, false, false, false]
- * processRangeOrNum("1-5") // [false, true, true, true, true, true, false]
- */
-const processRangeOrNum = (rangeOrNum: string): WeeklyFlag => {
-  const result: WeeklyFlag = [false, false, false, false, false, false, false]
-
-  const isRange = /^[0-9]-[0-9]$/.test(rangeOrNum)
-
-  if (isRange) {
-    const [first, last] = rangeOrNum.split("-")
-
-    for (let i = Number(first); i <= Number(last); i++) {
-      result[i % 7] = true
-    }
-  } else {
-    result[Number(rangeOrNum) % 7] = true
-  }
-
-  return result
-}

site/yarn.lock

@@ -5372,6 +5372,13 @@ create-require@^1.1.0:
   resolved "https://registry.yarnpkg.com/create-require/-/create-require-1.1.1.tgz#c1d7e8f1e5f6cfc9ff65f9cd352d37348756c333"
   integrity sha512-dcKFX3jn0MpIaXjisoRvexIJVEKzaq7z2rZKxf+MSr9TkdmHmsU4m2lcLojrj/FHl8mk5VxMmYA+ftRkP/3oKQ==
 
+cron-parser@4.4.0:
+  version "4.4.0"
+  resolved "https://registry.yarnpkg.com/cron-parser/-/cron-parser-4.4.0.tgz#829d67f9e68eb52fa051e62de0418909f05db983"
+  integrity sha512-TrE5Un4rtJaKgmzPewh67yrER5uKM0qI9hGLDBfWb8GGRe9pn/SDkhVrdHa4z7h0SeyeNxnQnogws/H+AQANQA==
+  dependencies:
+    luxon "^1.28.0"
+
 cronstrue@2.5.0:
   version "2.5.0"
   resolved "https://registry.yarnpkg.com/cronstrue/-/cronstrue-2.5.0.tgz#1d69bd53520ce536789fb666d9fd562065b491c6"
@@ -9280,6 +9287,11 @@ lru-cache@^6.0.0:
   dependencies:
     yallist "^4.0.0"
 
+luxon@^1.28.0:
+  version "1.28.0"
+  resolved "https://registry.yarnpkg.com/luxon/-/luxon-1.28.0.tgz#e7f96daad3938c06a62de0fb027115d251251fbf"
+  integrity sha512-TfTiyvZhwBYM/7QdAVDh+7dBTBA29v4ik0Ce9zda3Mnf8on1S5KJI8P2jKFZ8+5C0jhmr0KwJEO/Wdpm0VeWJQ==
+
 lz-string@^1.4.4:
   version "1.4.4"
   resolved "https://registry.yarnpkg.com/lz-string/-/lz-string-1.4.4.tgz#c0d8eaf36059f705796e1e344811cf4c498d3a26"