From 6fbece8423906f6825caec3c00d0c7f4acf268f8 Mon Sep 17 00:00:00 2001 From: G r e y Date: Thu, 2 Jun 2022 21:09:50 +0000 Subject: [PATCH] fix: FE parsing of schedule with day strings 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 | 1 + .../WorkspaceSchedulePage.tsx | 27 ++++---- site/src/util/schedule.test.ts | 24 +------ site/src/util/schedule.ts | 69 ------------------- site/yarn.lock | 12 ++++ 5 files changed, 28 insertions(+), 105 deletions(-) diff --git a/site/package.json b/site/package.json index 8e9fdfe5c5f32..509d3843aefa4 100644 --- a/site/package.json +++ b/site/package.json @@ -35,6 +35,7 @@ "@xstate/inspect": "0.6.5", "@xstate/react": "3.0.0", "axios": "0.26.1", + "cron-parser": "4.4.0", "cronstrue": "2.5.0", "dayjs": "1.11.2", "formik": "2.2.9", diff --git a/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx b/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx index d777c8249b400..4cc2593fd662c 100644 --- a/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx +++ b/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, } } diff --git a/site/src/util/schedule.test.ts b/site/src/util/schedule.test.ts index 95bc27433e667..d7ed65299cd67 100644 --- a/site/src/util/schedule.test.ts +++ b/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) - }) - }) }) diff --git a/site/src/util/schedule.ts b/site/src/util/schedule.ts index 81dd10694f4bb..67e25fd8ba0e1 100644 --- a/site/src/util/schedule.ts +++ b/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 -} diff --git a/site/yarn.lock b/site/yarn.lock index abc20e7ca7004..c39b2fd73fd4e 100644 --- a/site/yarn.lock +++ b/site/yarn.lock @@ -5360,6 +5360,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" @@ -9268,6 +9275,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"