Skip to content

Commit 37aff0c

Browse files
greyscaledgreyscaled
authored
fix: FE parsing of schedule with day strings (coder#2006)
Resolves: coder#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!
1 parent 7e89d91 commit 37aff0c

File tree

5 files changed

+28
-105
lines changed

5 files changed

+28
-105
lines changed

‎site/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
"@xstate/react": "3.0.0",
3737
"axios": "0.26.1",
3838
"can-ndjson-stream": "1.0.2",
39+
"cron-parser": "4.4.0",
3940
"cronstrue": "2.5.0",
4041
"dayjs": "1.11.2",
4142
"formik": "2.2.9",

‎site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { useMachine } from "@xstate/react"
2+
import * as cronParser from "cron-parser"
23
import dayjs from "dayjs"
34
import timezone from "dayjs/plugin/timezone"
45
import utc from "dayjs/plugin/utc"
@@ -12,7 +13,7 @@ import {
1213
WorkspaceScheduleFormValues,
1314
} from "../../components/WorkspaceScheduleForm/WorkspaceScheduleForm"
1415
import { firstOrItem } from "../../util/array"
15-
import { dowToWeeklyFlag, extractTimezone, stripTimezone } from "../../util/schedule"
16+
import { extractTimezone, stripTimezone } from "../../util/schedule"
1617
import { workspaceSchedule } from "../../xServices/workspaceSchedule/workspaceScheduleXService"
1718

1819
// REMARK: timezone plugin depends on UTC
@@ -93,7 +94,7 @@ export const formValuesToTTLRequest = (values: WorkspaceScheduleFormValues): Typ
9394

9495
export const workspaceToInitialValues = (workspace: TypesGen.Workspace): WorkspaceScheduleFormValues => {
9596
const schedule = workspace.autostart_schedule
96-
const ttl = workspace.ttl_ms ? workspace.ttl_ms / (1000 * 60 * 60) : 0
97+
const ttlHours = workspace.ttl_ms ? Math.round(workspace.ttl_ms / (1000 * 60 * 60)) : 0
9798

9899
if (!schedule) {
99100
return {
@@ -106,22 +107,22 @@ export const workspaceToInitialValues = (workspace: TypesGen.Workspace): Workspa
106107
saturday: false,
107108
startTime: "",
108109
timezone: "",
109-
ttl,
110+
ttl: ttlHours,
110111
}
111112
}
112113

113114
const timezone = extractTimezone(schedule, dayjs.tz.guess())
114-
const cronString = stripTimezone(schedule)
115115

116-
// parts has the following format: "mm HH * * dow"
117-
const parts = cronString.split(" ")
116+
const expression = cronParser.parseExpression(stripTimezone(schedule))
118117

119-
// -> we skip month and day-of-month
120-
const mm = parts[0]
121-
const HH = parts[1]
122-
const dow = parts[4]
118+
const HH = expression.fields.hour.join("").padStart(2, "0")
119+
const mm = expression.fields.minute.join("").padStart(2, "0")
123120

124-
const weeklyFlags = dowToWeeklyFlag(dow)
121+
const weeklyFlags = [false, false, false, false, false, false, false]
122+
123+
for (const day of expression.fields.dayOfWeek) {
124+
weeklyFlags[day % 7] = true
125+
}
125126

126127
return {
127128
sunday: weeklyFlags[0],
@@ -131,9 +132,9 @@ export const workspaceToInitialValues = (workspace: TypesGen.Workspace): Workspa
131132
thursday: weeklyFlags[4],
132133
friday: weeklyFlags[5],
133134
saturday: weeklyFlags[6],
134-
startTime: `${HH.padStart(2, "0")}:${mm.padStart(2, "0")}`,
135+
startTime: `${HH}:${mm}`,
135136
timezone,
136-
ttl,
137+
ttl: ttlHours,
137138
}
138139
}
139140

‎site/src/util/schedule.test.ts

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { dowToWeeklyFlag, extractTimezone, stripTimezone, WeeklyFlag } from "./schedule"
1+
import { extractTimezone, stripTimezone } from "./schedule"
22

33
describe("util/schedule", () => {
44
describe("stripTimezone", () => {
@@ -20,26 +20,4 @@ describe("util/schedule", () => {
2020
expect(extractTimezone(input)).toBe(expected)
2121
})
2222
})
23-
24-
describe("dowToWeeklyFlag", () => {
25-
it.each<[string, WeeklyFlag]>([
26-
// All days
27-
["*", [true, true, true, true, true, true, true]],
28-
["0-6", [true, true, true, true, true, true, true]],
29-
["1-7", [true, true, true, true, true, true, true]],
30-
31-
// Single number modulo 7
32-
["3", [false, false, false, true, false, false, false]],
33-
["0", [true, false, false, false, false, false, false]],
34-
["7", [true, false, false, false, false, false, false]],
35-
["8", [false, true, false, false, false, false, false]],
36-
37-
// Comma-separated Numbers, Ranges and Mixes
38-
["1,3,5", [false, true, false, true, false, true, false]],
39-
["1-2,4-5", [false, true, true, false, true, true, false]],
40-
["1,3-4,6", [false, true, false, true, true, false, true]],
41-
])(`dowToWeeklyFlag(%p) returns %p`, (dow, weeklyFlag) => {
42-
expect(dowToWeeklyFlag(dow)).toEqual(weeklyFlag)
43-
})
44-
})
4523
})

‎site/src/util/schedule.ts

Lines changed: 0 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -30,72 +30,3 @@ export const extractTimezone = (raw: string, defaultTZ = DEFAULT_TIMEZONE): stri
3030
return defaultTZ
3131
}
3232
}
33-
34-
/**
35-
* WeeklyFlag is an array representing which days of the week are set or flagged
36-
*
37-
* @remarks
38-
*
39-
* A WeeklyFlag has an array size of 7 and should never have its size modified.
40-
* The 0th index is Sunday
41-
* The 6th index is Saturday
42-
*/
43-
export type WeeklyFlag = [boolean, boolean, boolean, boolean, boolean, boolean, boolean]
44-
45-
/**
46-
* dowToWeeklyFlag converts a dow cron string to a WeeklyFlag array.
47-
*
48-
* @example
49-
*
50-
* dowToWeeklyFlag("1") // [false, true, false, false, false, false, false]
51-
* dowToWeeklyFlag("1-5") // [false, true, true, true, true, true, false]
52-
* dowToWeeklyFlag("1,3-4,6") // [false, true, false, true, true, false, true]
53-
*/
54-
export const dowToWeeklyFlag = (dow: string): WeeklyFlag => {
55-
if (dow === "*") {
56-
return [true, true, true, true, true, true, true]
57-
}
58-
59-
const results: WeeklyFlag = [false, false, false, false, false, false, false]
60-
61-
const commaSeparatedRangeOrNum = dow.split(",")
62-
63-
for (const rangeOrNum of commaSeparatedRangeOrNum) {
64-
const flags = processRangeOrNum(rangeOrNum)
65-
66-
flags.forEach((value, idx) => {
67-
if (value) {
68-
results[idx] = true
69-
}
70-
})
71-
}
72-
73-
return results
74-
}
75-
76-
/**
77-
* processRangeOrNum is a helper for dowToWeeklyFlag. It processes a range or
78-
* number (modulo 7) into a Weeklyflag boolean array.
79-
*
80-
* @example
81-
*
82-
* processRangeOrNum("1") // [false, true, false, false, false, false, false]
83-
* processRangeOrNum("1-5") // [false, true, true, true, true, true, false]
84-
*/
85-
const processRangeOrNum = (rangeOrNum: string): WeeklyFlag => {
86-
const result: WeeklyFlag = [false, false, false, false, false, false, false]
87-
88-
const isRange = /^[0-9]-[0-9]$/.test(rangeOrNum)
89-
90-
if (isRange) {
91-
const [first, last] = rangeOrNum.split("-")
92-
93-
for (let i = Number(first); i <= Number(last); i++) {
94-
result[i % 7] = true
95-
}
96-
} else {
97-
result[Number(rangeOrNum) % 7] = true
98-
}
99-
100-
return result
101-
}

‎site/yarn.lock

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5372,6 +5372,13 @@ create-require@^1.1.0:
53725372
resolved "https://registry.yarnpkg.com/create-require/-/create-require-1.1.1.tgz#c1d7e8f1e5f6cfc9ff65f9cd352d37348756c333"
53735373
integrity sha512-dcKFX3jn0MpIaXjisoRvexIJVEKzaq7z2rZKxf+MSr9TkdmHmsU4m2lcLojrj/FHl8mk5VxMmYA+ftRkP/3oKQ==
53745374

5375+
cron-parser@4.4.0:
5376+
version "4.4.0"
5377+
resolved "https://registry.yarnpkg.com/cron-parser/-/cron-parser-4.4.0.tgz#829d67f9e68eb52fa051e62de0418909f05db983"
5378+
integrity sha512-TrE5Un4rtJaKgmzPewh67yrER5uKM0qI9hGLDBfWb8GGRe9pn/SDkhVrdHa4z7h0SeyeNxnQnogws/H+AQANQA==
5379+
dependencies:
5380+
luxon "^1.28.0"
5381+
53755382
cronstrue@2.5.0:
53765383
version "2.5.0"
53775384
resolved "https://registry.yarnpkg.com/cronstrue/-/cronstrue-2.5.0.tgz#1d69bd53520ce536789fb666d9fd562065b491c6"
@@ -9280,6 +9287,11 @@ lru-cache@^6.0.0:
92809287
dependencies:
92819288
yallist "^4.0.0"
92829289

9290+
luxon@^1.28.0:
9291+
version "1.28.0"
9292+
resolved "https://registry.yarnpkg.com/luxon/-/luxon-1.28.0.tgz#e7f96daad3938c06a62de0fb027115d251251fbf"
9293+
integrity sha512-TfTiyvZhwBYM/7QdAVDh+7dBTBA29v4ik0Ce9zda3Mnf8on1S5KJI8P2jKFZ8+5C0jhmr0KwJEO/Wdpm0VeWJQ==
9294+
92839295
lz-string@^1.4.4:
92849296
version "1.4.4"
92859297
resolved "https://registry.yarnpkg.com/lz-string/-/lz-string-1.4.4.tgz#c0d8eaf36059f705796e1e344811cf4c498d3a26"

0 commit comments

Comments
 (0)