Skip to content

fix: FE parsing of schedule with day strings #2006

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 3, 2022
Merged

fix: FE parsing of schedule with day strings #2006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6fbece8
fix: FE parsing of schedule with day strings
greyscaled Jun 2, 2022
ddd6d5c
Merge branch 'main' into vapurrmaid/gh-1901/cron-parser
greyscaled Jun 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions site/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
27 changes: 14 additions & 13 deletions site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REVIEW: This is technically a separate change/uncaught previously.

Basically if you set your ttl via CLI to something like 1.5 hours, we'd show the 1.5 initially in the form, but you were forced to change it to an integer.

This feels safer until we have a form that allows minutes in addition to hours.


if (!schedule) {
return {
Expand All @@ -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],
Expand All @@ -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,
}
}

Expand Down
24 changes: 1 addition & 23 deletions site/src/util/schedule.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { dowToWeeklyFlag, extractTimezone, stripTimezone, WeeklyFlag } from "./schedule"
import { extractTimezone, stripTimezone } from "./schedule"

describe("util/schedule", () => {
describe("stripTimezone", () => {
Expand All @@ -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)
})
})
})
69 changes: 0 additions & 69 deletions site/src/util/schedule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
12 changes: 12 additions & 0 deletions site/yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down