Skip to content
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.

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

Daily cron expression with timezone offset displays incorrect day #313

Open
1 task done
khOst opened this issue Jan 15, 2024 · 7 comments
Open
1 task done

Daily cron expression with timezone offset displays incorrect day #313

khOst opened this issue Jan 15, 2024 · 7 comments

Comments

@khOst
Copy link

khOst commented Jan 15, 2024

Cron Expression
0 20 28 * * with a tzOffset set to 5

Expected Output
"At 01:00 AM, on day 29 of the month"

Actual Output
"At 01:00 AM, on day 28 of the month"

Prerequisites

  • The cron expression being passed in is a valid expression. cRonstrue does not validate expressions and assumes the one you pass is already valid. See the FAQ for more details.
@bradymholt
Copy link
Owner

Isn't this correct? With tzOffset set to 5 and hour set to 20, this would cause the time to move to the next day. This was implemented / changed in #296.

@khOst
Copy link
Author

khOst commented Jan 16, 2024

Sorry, expected and actual should be switched in places, I've updated the description. The time is expected to move to next day, but cRonstrue returns the same day, despite the offset.

@khOst
Copy link
Author

khOst commented Jan 16, 2024

Also for a negative tzOffset instead of returning the same day, it should return the previous day. For example, day 27 is expected here https://runkit.com/65a61ad4cca25b00082679bc/65a61ae3fdc4a20008bebfa5.

@bradymholt
Copy link
Owner

@maxtwardowski - Do you think changes in #296 need to be tweaked for this case?

@ZaneGeiser
Copy link

Hi @bradymholt, I just started using this package (tyvm for building it!) this week and noticed this same issue.

I wrote some test that I think would need to pass in order for this issue to be resolved.

    it("5 23 1 * *", function() {
      assert.equal(cronstrue.toString(this.test?.title as string, {
        tzOffset: 2,
      }), "At 01:05 AM, on day 2 of the month")
    });

    it("5 23 31 * *", function() {
      assert.equal(cronstrue.toString(this.test?.title as string, {
        tzOffset: 2,
      }), "At 01:05 AM, on day 1 of the month")
    });

    it("5 23 1 * *", function() {
      assert.equal(cronstrue.toString(this.test?.title as string, {
        tzOffset: -2,
      }), "At 11:05 PM, on day 31 of the month")
    });

I went to try making the code change but noticed there are some special day-of-month cases (like L, WL, and LW), and wasn't sure how it would need to fit in with those cases.

@ZaneGeiser
Copy link

As I think about it more, it probably needs to wrap the L day of the month forward to 1 if the tzOffset pushes over the dateline.

Ie, if L is converted to the day before the last day, then it should remain as the Last day. But if L is converted to the day after the last day then it should write 'day 1 of the month'.

Or as tests:

    it("5 23 L * *", function() {
      assert.equal(cronstrue.toString(this.test?.title as string, {
        tzOffset: -2,
      }), "At 01:05 AM, on the last day of the month")
    });

    it("5 23 L * *", function() {
      assert.equal(cronstrue.toString(this.test?.title as string, {
        tzOffset: 2,
      }), "At 01:05 AM, on day 1 of the month")
    });

@kbazzani
Copy link

kbazzani commented Jul 2, 2024

First, very useful package, keep up the good work!

Second, here are a couple more examples. Along with the day of the month as previously mentioned, the month also doesn't wrap correctly.

construe.toString( "59 23 31 12 3", { locale:"en", tzOffset: 1} )
'At 12:59 AM, on day 31 of the month, and on Thursday, only in December'

Should be Jan 1st

construe.toString( "01 00 01 01 3", { locale:"en", tzOffset: -1} )
'At 11:01 PM, on day 01 of the month, and on Tuesday, only in January'

Should be Dec 31st.

Best!

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

No branches or pull requests

4 participants