Recurring until parser seems to subtract one day when parsing

TobTobXX commented 1 year ago

Please check that this issue hasn't been reported before.

[X] I searched previous Bug Reports didn't find any similar reports.

Expected Behavior

I have this task:

- [ ] #task Do Stuff 🔁 every day until March 29, 2023 📅 2023-03-24

And I check it, then I should get this:

- [ ] #task Do Stuff 🔁 every day until March 29, 2023 📅 2023-03-25
- [x] #task Do Stuff 🔁 every day until March 29, 2023 📅 2023-03-24 ✅ 2023-03-31

Current behaviour

I have this task:

- [ ] #task Do Stuff 🔁 every day until March 29, 2023 📅 2023-03-24

And I check it and get this:

- [ ] #task Do Stuff 🔁 every day until March 28, 2023 📅 2023-03-25
- [x] #task Do Stuff 🔁 every day until March 29, 2023 📅 2023-03-24 ✅ 2023-03-31

Note that the "until date" has changed.

It's quite funny, because when you open the task in the editor, you can see 2 different dates, because each parsing pass subtracts one day. In the above case it would be "March 29, 2023" in the Markdown file, "March 28, 2023" in the parsed text field in the "Create or edit Task" dialog and just below the (again parsed) text: "March 27, 2023".

Also, as a side note, ISO date format doesn't work there, which is a shame.

Steps to reproduce

see above

Which Operating Systems are you using?

[ ] Android
[ ] iPhone/iPad
[X] Linux
[ ] macOS
[ ] Windows

Obsidian Version

v1.1.16

Tasks Plugin Version

2.0.1

Checks

[ ] I have tried it with all other plugins disabled and the error still occurs

Possible solution

It's quite possible that this is a bug of the NL parser library used?

If you give me a couple of days, I'll dig into the source myself and see if I can come up with a fix.

TobTobXX commented 1 year ago

Welp, it's actually a bug in rrule:

const { RRule } = require('rrule');
let rule = RRule.fromText('every 5 weeks on Monday, Friday until January 31, 2013')

console.log(rule.toText());
// "every 5 weeks on Monday, Friday until January 30, 2013"

TobTobXX commented 1 year ago

Found it: https://github.com/jakubroztocil/rrule/issues/486

The problem apperas to be, that the date is parsed in the local timezone and then converted into UTC wich somehow misses a day. A solution seems to be to add "UTC" to the day, but that won't be output when converted to text again...

EDIT: Just noticed, that the mentioned bug report was made by a maintainer... whoopsies, embarrassing.

TobTobXX commented 1 year ago

Ok, one horrible workaround that could be applied by Tasks (until https://github.com/jakubroztocil/rrule/pull/529 or similar is merged) is this: Whenever the keyword "until" is present in an rrule, just append " UTC". Kinda stupid but should do the trick.

If you have better ideas, I'd love to help, but as it stands now, the until clause is unusable for anyone in a timezone with an offset of >0.

claremacrae commented 1 year ago

Thank you for logging this.

I do believe you have re-discovered the source of point 2 in this section of the Tasks docs:

https://obsidian-tasks-group.github.io/obsidian-tasks/getting-started/recurring-tasks/#known-issues

You can not use rules where recurrence ends on a specific date (until "date"). There is a bug in rrule where until "date" rules are not converted to the correct text. As a consequence, every subsequent task's "until" date will be one day earlier than the one before.

I've added a link, there, to this issue. The change will be published next time a release is done.

If you have better ideas, I'd love to help, but as it stands now, the until clause is unusable for anyone in a timezone with an offset of >0.

I would certainly accept a PR for this, if it was well covered with unit tests, and had been thoroughly tested manually in a variety of time zones.

To get started developing tasks, we now have the Contributing Guide.

If you're interested, I can show you how I figured out to set the timezone when running jest. (I will add it to the Contributing guide at some point, but that's a long way from the top of a very long list.)

claremacrae commented 1 year ago

PS This issue and any analysis of it predates my involvement with this project, hence my not knowing the background - so what you have captured here is very helpful indeed. Thank you.

TobTobXX commented 1 year ago

Ok, I'll look into it at some point.

If you're interested, I can show you how I figured out to set the timezone when running jest.

First I have to figure out how jest works at all :sweat_smile:

claremacrae commented 1 year ago

I’d be happy to pair with you to help set up your dev environment and talk you through the tests.

My email is in my profile here if you would to set a session up.

claremacrae commented 10 months ago

Hi @TobTobXX,

Ok, I'll look into it at some point.

If you're interested, I can show you how I figured out to set the timezone when running jest.

First I have to figure out how jest works at all 😅

If you ever do feel like having a look at this, the following may help...

I've created a Tasks Contributing Guide
There's quite a lot of info on the tests
Someone figured out how to set the timezone in tests. It's currently fixed to UTC so tests run reliably everywhere in the world - but I imagine the value could be changed to simulate different locations...

claremacrae commented 10 months ago

It also occurred to me that if the bug is totally consistent, then a workaround would be:

when a recurring task is completed...
... detect if until is present...
... add 1 day back to the new reference date, as a workaround

pcause commented 2 months ago

how about this as a way to deal with this completely in tasks. find the "until" in the string and remember that string. when the new item is created replace the "until" part of the string with the saved original string.

claremacrae commented 2 months ago

Thanks for the suggestion @pcause!

obsidian-tasks-group / obsidian-tasks