Skip to content

fix: cron.ts#365

Merged
RambokDev merged 1 commit into
mainfrom
fix/cron
Jul 9, 2026
Merged

fix: cron.ts#365
RambokDev merged 1 commit into
mainfrom
fix/cron

Conversation

@RambokDev

@RambokDev RambokDev commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features

    • Added live cron field validation, with the save action disabled until all fields are valid.
    • Improved cron selection controls to better reflect current values and input changes.
  • Bug Fixes

    • Updated cron handling to accept a wider set of valid expressions, including ranges and step values.
    • Standardized default cron values and field options for a smoother editing experience.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Cron validation in a shared utility is rewritten from regex-based checks to explicit numeric-bounds validation supporting wildcards, ranges, and steps. AdvancedCronSelect gains an optional validity callback and controlled-value handling. CronInput tracks per-field validity, uses unpadded numeric options, and disables the Save button when a field is invalid or a save mutation is pending.

Changes

Cron validation and UI wiring

Layer / File(s) Summary
Bounds-driven cron part validation
src/utils/cron.ts
Replaces regex-based validation with explicit min/max bounds per cron field and helpers supporting wildcards, base/step, and start-end ranges.
AdvancedCronSelect validity callback
src/features/database/components/cron-advanced-select.tsx
Adds optional onValidityChange prop, syncs customValue with incoming value/defaultValue, computes validity via useEffect, and controls the dropdown Select with value instead of defaultValue.
CronInput field validity aggregation and Save gating
src/features/database/components/cron-input.tsx
Initializes a default cron expression, tracks per-field validity state, generates unpadded numeric options, wires onValidityChange on each cron part select, and disables the Save button when a field is invalid or the update mutation is pending.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant AdvancedCronSelect
  participant isValidCronPart
  participant CronInput

  User->>AdvancedCronSelect: edit cron field value
  AdvancedCronSelect->>isValidCronPart: validate(type, value)
  isValidCronPart-->>AdvancedCronSelect: valid boolean
  AdvancedCronSelect->>CronInput: onValidityChange(valid)
  CronInput->>CronInput: update fieldValidity, derive hasInvalidField
  CronInput->>CronInput: disable Save button if invalid or pending
Loading

Poem

A hop, a skip, a bounded field,
No more regex to hide and yield.
Each cron part checked with care,
Save button waits, aware, aware.
🐇 Carrots counted, ranges neat—
This bunny's code is validation-sweet!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is related to the main cron validation fix, though it is narrower than the full changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/cron

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/features/database/components/cron-advanced-select.tsx

Oops! Something went wrong! :(

ESLint: 9.39.4

TypeError: Converting circular structure to JSON
--> starting at object with constructor 'Object'
| property 'configs' -> object with constructor 'Object'
| property 'flat' -> object with constructor 'Object'
| ...
| property 'plugins' -> object with constructor 'Object'
--- property 'react' closes the circle
Referenced from: /.eslintrc.json
at JSON.stringify ()
at /node_modules/.pnpm/@eslint+eslintrc@3.3.5/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2255:45
at Array.map ()
at ConfigValidator.formatErrors (/node_modules/.pnpm/@eslint+eslintrc@3.3.5/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2246:23)
at ConfigValidator.validateConfigSchema (/node_modules/.pnpm/@eslint+eslintrc@3.3.5/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2277:84)
at ConfigArrayFactory._normalizeConfigData (/node_modules/.pnpm/@eslint+eslintrc@3.3.5/node_modules/@eslint/

... [truncated 446 characters] ...

c/dist/eslintrc.cjs:3261:25)
at ConfigArrayFactory._normalizeObjectConfigDataBody (/node_modules/.pnpm/@eslint+eslintrc@3.3.5/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3199:25)
(node:2) ESLintRCWarning: You are using an eslintrc configuration file, which is deprecated and support will be removed in v10.0.0. Please migrate to an eslint.config.js file. See https://eslint.org/docs/latest/use/configure/migration-guide for details. An eslintrc configuration file is used because you have the ESLINT_USE_FLAT_CONFIG environment variable set to false. If you want to use an eslint.config.js file, remove the environment variable. If you want to find the location of the eslintrc configuration file, use the --debug flag.
(Use node --trace-warnings ... to show where the warning was created)

src/features/database/components/cron-input.tsx

Oops! Something went wrong! :(

ESLint: 9.39.4

TypeError: Converting circular structure to JSON
--> starting at object with constructor 'Object'
| property 'configs' -> object with constructor 'Object'
| property 'flat' -> object with constructor 'Object'
| ...
| property 'plugins' -> object with constructor 'Object'
--- property 'react' closes the circle
Referenced from: /.eslintrc.json
at JSON.stringify ()
at /node_modules/.pnpm/@eslint+eslintrc@3.3.5/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2255:45
at Array.map ()
at ConfigValidator.formatErrors (/node_modules/.pnpm/@eslint+eslintrc@3.3.5/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2246:23)
at ConfigValidator.validateConfigSchema (/node_modules/.pnpm/@eslint+eslintrc@3.3.5/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2277:84)
at ConfigArrayFactory._normalizeConfigData (/node_modules/.pnpm/@eslint+eslintrc@3.3.5/node_modules/@eslint/

... [truncated 446 characters] ...

c/dist/eslintrc.cjs:3261:25)
at ConfigArrayFactory._normalizeObjectConfigDataBody (/node_modules/.pnpm/@eslint+eslintrc@3.3.5/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3199:25)
(node:2) ESLintRCWarning: You are using an eslintrc configuration file, which is deprecated and support will be removed in v10.0.0. Please migrate to an eslint.config.js file. See https://eslint.org/docs/latest/use/configure/migration-guide for details. An eslintrc configuration file is used because you have the ESLINT_USE_FLAT_CONFIG environment variable set to false. If you want to use an eslint.config.js file, remove the environment variable. If you want to find the location of the eslintrc configuration file, use the --debug flag.
(Use node --trace-warnings ... to show where the warning was created)

src/utils/cron.ts

Oops! Something went wrong! :(

ESLint: 9.39.4

TypeError: Converting circular structure to JSON
--> starting at object with constructor 'Object'
| property 'configs' -> object with constructor 'Object'
| property 'flat' -> object with constructor 'Object'
| ...
| property 'plugins' -> object with constructor 'Object'
--- property 'react' closes the circle
Referenced from: /.eslintrc.json
at JSON.stringify ()
at /node_modules/.pnpm/@eslint+eslintrc@3.3.5/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2255:45
at Array.map ()
at ConfigValidator.formatErrors (/node_modules/.pnpm/@eslint+eslintrc@3.3.5/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2246:23)
at ConfigValidator.validateConfigSchema (/node_modules/.pnpm/@eslint+eslintrc@3.3.5/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2277:84)
at ConfigArrayFactory._normalizeConfigData (/node_modules/.pnpm/@eslint+eslintrc@3.3.5/node_modules/@eslint/

... [truncated 446 characters] ...

c/dist/eslintrc.cjs:3261:25)
at ConfigArrayFactory._normalizeObjectConfigDataBody (/node_modules/.pnpm/@eslint+eslintrc@3.3.5/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3199:25)
(node:2) ESLintRCWarning: You are using an eslintrc configuration file, which is deprecated and support will be removed in v10.0.0. Please migrate to an eslint.config.js file. See https://eslint.org/docs/latest/use/configure/migration-guide for details. An eslintrc configuration file is used because you have the ESLINT_USE_FLAT_CONFIG environment variable set to false. If you want to use an eslint.config.js file, remove the environment variable. If you want to find the location of the eslintrc configuration file, use the --debug flag.
(Use node --trace-warnings ... to show where the warning was created)


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/utils/cron.ts`:
- Around line 25-47: isValidCronPart currently assumes value is always a string,
but CronInput can pass undefined through AdvancedCronSelect when cron parts are
missing. Add a falsy/non-string guard at the start of isValidCronPart before any
split/trim logic, and update AdvancedCronSelect so customValue always falls back
to an empty string instead of undefined; use the identifiers isValidCronPart and
AdvancedCronSelect to locate the fix.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0df42ad0-6d0a-493c-8535-cb53a28cb467

📥 Commits

Reviewing files that changed from the base of the PR and between 5e4a430 and eb0a534.

📒 Files selected for processing (3)
  • src/features/database/components/cron-advanced-select.tsx
  • src/features/database/components/cron-input.tsx
  • src/utils/cron.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: build-image
🔇 Additional comments (3)
src/utils/cron.ts (1)

1-23: LGTM!

src/features/database/components/cron-advanced-select.tsx (1)

1-1: LGTM!

Also applies to: 15-15, 24-37, 60-60

src/features/database/components/cron-input.tsx (1)

5-5: LGTM!

Also applies to: 17-27, 58-103, 128-128

Comment thread src/utils/cron.ts
@RambokDev RambokDev merged commit 3e8a6d5 into main Jul 9, 2026
6 checks passed
@RambokDev RambokDev deleted the fix/cron branch July 9, 2026 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant