Skip to content

Conversation

@roggervalf
Copy link
Collaborator

@roggervalf roggervalf commented May 8, 2025

Why

  1. Why is this change necessary? Keep it up to date and fix [Bug]: Problem with cron intervals #3252

How

Enter the implementation details here.

Additional Notes (Optional)

fixes #3252

/**
* End date when job should stop
*/
endDate?: Date | string | number;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm avoiding to reuse type from cron parser because it does not have descriptions and it also contains an extra custom type in their option attributes that is not compatible with Date class. Justo keeping the primitive types

@roggervalf roggervalf requested review from Copilot and manast May 9, 2025 04:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR upgrades the cron-parser dependency to v5.1.1 and updates related API usages and documentation for consistency. Key changes include adjusting the timezone in job scheduler tests, removing legacy type extensions in RepeatOptions, and switching to the new CronExpressionParser API in repeat and job-scheduler classes.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_job_scheduler.ts Updated timezone from 'Europe/Athens' to 'Etc/UTC' in the job scheduler test.
src/interfaces/repeat-options.ts Removed extension from ParserOptions and added explicit properties for clarity.
src/classes/repeat.ts Replaced parseExpression with CronExpressionParser.parse to align with the update.
src/classes/job-scheduler.ts Updated cron-parser import and parsing usage in line with v5.1.1.
Other files Minor documentation adjustments (doc link formatting) throughout the codebase.
Comments suppressed due to low confidence (4)

tests/test_job_scheduler.ts:1648

  • The timezone was changed from 'Europe/Athens' to 'Etc/UTC'. Please confirm that this change is intentional and that it preserves the intended scheduling behavior in all test scenarios.
tz: 'Etc/UTC',

src/interfaces/repeat-options.ts:8

  • The RepeatOptions interface no longer extends ParserOptions from cron-parser. Please ensure that all previously relied upon properties from ParserOptions are now explicitly defined or handled.
export interface RepeatOptions {

src/classes/repeat.ts:336

  • The switch to CronExpressionParser.parse reflects the new cron-parser API. Verify that this change properly handles the updated API behavior, especially regarding error handling and any differences in options interpretation.
const interval = CronExpressionParser.parse(pattern, {

src/classes/job-scheduler.ts:456

  • Similar to the repeat class, ensure that the CronExpressionParser.parse usage in job-scheduler aligns with cron-parser v5.1.1 API specifications and that any related behavior changes are fully addressed.
const interval = CronExpressionParser.parse(pattern, {

pattern: '0 1 * * *',
endDate: new Date('2017-05-10 13:13:00'),
tz: 'Europe/Athens',
tz: 'Etc/UTC',
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't this be a breaking change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought about it, no sure if aomeone is using utc option as we don't put it in our documentation. But if you prefer, it can be saved for next breaking change

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.

[Bug]: Problem with cron intervals

3 participants