NAS-114805 / 22.12 / NAS-114805: Refactoring scheduler component #6440
Conversation
bugclerk
changed the title
There was a problem hiding this comment.
This rule causes incorrect preview showing the date in the past. It only does this when you select the current month.
We probably don't want to show anything in the past in the preview or the controls (month drop down selector), since the main goal is to help users understand what the cron tab settings would schedule in the future.
| } | ||
|
|
||
| nextRuns.push(exampleDate); | ||
| i = i + 1; |
There was a problem hiding this comment.
Why not have this in the for loop declaration? for (let i = 0; i < limit; i++)
There was a problem hiding this comment.
Because there is a continue condition that doesn't actually need it.
| let previousDate = toDate(startDate, { timeZone: 'UTC' }); | ||
| const startingMonth = getMonth(previousDate); | ||
|
|
||
| while (getMonth(previousDate) === startingMonth) { |
There was a problem hiding this comment.
This might as well be while(true). Condition for the loop should be the actual loop condition (where break is called maybe?). Could be replaced by do{...} while(condition). Not sure.
|
|
||
| private isWithinTimeConstrains(date: Date): boolean { | ||
| return true; | ||
| // TODO: Will be implemented later. |
There was a problem hiding this comment.
Because I got tired :) It's an edge case feature that I will address in coming PRs soon.
| weekdays: weekdayValues, | ||
| }); | ||
| } catch (error: unknown) { | ||
| console.error(error); |
There was a problem hiding this comment.
console.error seems inconsequential for running into an error while patching values. Typical users obvi won't notice it. Have noticed it else where in this PR as well.
There was a problem hiding this comment.
Here and in other places, I have console.error to log error in Sentry mostly.
This particular try catch wraps around cronParser.parseExpression
� Conflicts: � src/assets/i18n/zh-hans.json
|
Comments got addressed.
While I generally agree that there is no use for being able to go back in time, I decided to keep this functionality for two reasons. It doesn't harm, provides some clues and it would require a bit of ugly code to fix it. |
|
Alright, let's try this again. |
|
Fixed.
|
|
Since we're allowing events in the past, the calendar should also respect this. When I select a specific hour that has already passed in the current day, the calendar view does not highlight the current day.
|
|
That's kinda not what I meant. You can go to past months - yes, mostly to keep code clean, but also to keep calendar UI consistent, but we won't be showing past events. |
So users should be able to navigate the calendar view into the past but no dates should be highlighted. |
|
@undsoft If you use croner to build cron strings, but actually run tasks using system cron. I would recommend using 4.2 and enabling the legacyMode option See Hexagon/croner#53 for details |
Testing: go to Settings -> General -> GUI, observe new "test" input.
Notes: