Ensure monthly and yearly recurrences

[schemar]

trafficstars

When a recurrence rule states "every month" and the next month does not have the day of the month that the task is scheduled, that month is skipped and the next valid month is used instead.

For example: When I mark a task that recurs "every month" done on January 31, the next occurrence will be scheduled on March 31, since February does not have 31 days. This is in line with the rrule specification, but not what users expect. This can be triggered by accident, for example when the task is based on the done date and the done date is on the 31st.

@claremacrae architected a fix that I implemented with this change.

To fix the issue, the number of expected skipped months is parsed (defaults to one). While the actualy number of skipped months is higher, the base date is adjusted one day into the past. In practice, this means that the example from above would no schedule the recurring task on the 28th of February (or the 29th for a leap year).

The same applies for yearly recurrences, for example based on the 29th of February.

~~:warning: This is currently still a proof of concept and is still missing:~~

edited to turn the list in to checkboxes..

• [x] Documentation in code
• [x] End user documentation to explain the behavior
• [x] Extensive testing to ensure this change does not introduce regressions

Fixes #614, #1087

[schemar]

Thank you @claremacrae for coming up with this fix and letting me know how it would work! Please check whether the implementation matches your ideas.

[schemar]

One thing that's not ideal is the change to the after instance (using the existing Moment methods). It is not transparent at all, that after changes through the various function invocations.

[claremacrae]

Extensive testing to ensure this change does not introduce regressions

By intentionally breaking the recurrence code, I found that in addition to tests/Recurrence.test.ts, there are existing tests of recurring in:

• tests/Commands/ToggleDone.test.ts- it's actually testing the cursor position after completing recurring tasks

https://github.com/obsidian-tasks-group/obsidian-tasks/blob/11a3712bada058e038e84c852292d28098a747cd/tests/Commands/ToggleDone.test.ts#L96-L110

• tests/Task.test.ts - lots of them:

https://github.com/obsidian-tasks-group/obsidian-tasks/blob/1a889bc1a0d771a6f5342a7e3623581e378a3298/tests/Task.test.ts#L437-L717

[claremacrae]

One thing that's not ideal is the change to the after instance (using the existing Moment methods). It is not transparent at all, that after changes through the various function invocations.

I see what you mean.

Perhaps rename it to something like currentAfter? I thought of nextAfter but that's misleading too...

[claremacrae]

@schemar I grabbed my tests from #614 and added them to Task.test.ts, updating the comments.

They all pass, which is great. Feel free to add them, or I can...

diff --git a/tests/Task.test.ts b/tests/Task.test.ts
index 9566b09..822260f 100644
--- a/tests/Task.test.ts
+++ b/tests/Task.test.ts
@@ -650,6 +650,26 @@ describe('toggle done', () => {
             nextScheduled: '2021-10-18',
             nextDue: '2021-10-20',
         },
+        {
+            // every month - due 31 March, and so 31 April would not exist: it used to skip forward 2 months
+            interval: 'every month',
+            due: '2021-03-31',
+            nextDue: '2021-04-30',
+        },
+        {
+            // every month - due 29 January, and so 29 February would not exist: it used to skip forward 2 months
+            interval: 'every month',
+            due: '2021-01-29',
+            nextDue: '2021-02-28',
+        },
+
+        // Testing yearly repetition around leap days
+        {
+            // yearly, due on leap day 29th February: it used to skip forward 4 years
+            interval: 'every year',
+            due: '2020-02-29',
+            nextDue: '2021-02-28',
+        },
     ];
 
     test.concurrent.each<RecurrenceCase>(recurrenceCases)(

[claremacrae]

@schemar Without comments I'm making slow progress on reading the code, so thought I would see if I could break it!

These added to Task.test.ts fail. Have I got my nextDue values correct?

        // Some boundary cases
        {
            interval: 'every 11 months', // should skip forward to February next year
            due: '2020-03-31',
            nextDue: '2021-02-28', // gives 2022-01-31
        },
        {
            interval: 'every 13 months', // should skip forward to February next year
            due: '2020-01-31',
            nextDue: '2021-02-28', // Gives 2022-03-31
        },

[schemar]

One thing that's not ideal is the change to the after instance (using the existing Moment methods). It is not transparent at all, that after changes through the various function invocations.

I see what you mean.

Perhaps rename it to something like currentAfter? I thought of nextAfter but that's misleading too...

For now I will add documentation to the code.

@schemar I grabbed my tests from #614 and added them to Task.test.ts, updating the comments.

They all pass, which is great. Feel free to add them, or I can...

I will add these three tests.

@schemar Without comments I'm making slow progress on reading the code, so thought I would see if I could break it!

These added to Task.test.ts fail. Have I got my nextDue values correct?

        // Some boundary cases
        {
            interval: 'every 11 months', // should skip forward to February next year
            due: '2020-03-31',
            nextDue: '2021-02-28', // gives 2022-01-31
        },
        {
            interval: 'every 13 months', // should skip forward to February next year
            due: '2020-01-31',
            nextDue: '2021-02-28', // Gives 2022-03-31
        },

Looks correct to me. Let me investigate.

[schemar]

@claremacrae there are comments in the code now, hope it helps.

[schemar]

@schemar Without comments I'm making slow progress on reading the code, so thought I would see if I could break it!

These added to Task.test.ts fail. Have I got my nextDue values correct?

        // Some boundary cases
        {
            interval: 'every 11 months', // should skip forward to February next year
            due: '2020-03-31',
            nextDue: '2021-02-28', // gives 2022-01-31
        },
        {
            interval: 'every 13 months', // should skip forward to February next year
            due: '2020-01-31',
            nextDue: '2021-02-28', // Gives 2022-03-31
        },

Thank you! I added the test cases to the Recurrence test file and fixed the bug.

[claremacrae]

Thank you! I added the test cases to the Recurrence test file and fixed the bug.

The fix is really clear.

I have to go out for an errand - will try to think of extra corner cases for tests on my walk... Other than that, I think the code in this is good to go.

[claremacrae]

Is it worth jotting down the outline of what content might go in the user docs? Did you have any ideas?

[claremacrae]

I've converted the list of 3 outstanding tasks in the description to checklists, and marked 2 as done - in the believe that the current tests in Task.test.ts are good enough.

[schemar]

Thank you for all the very valid comments @claremacrae! Unfortunately I won't have more time to work on this until at least Thursday. Would be lovely if you could pick it up from here. If not, I will pick it up next week :+1:

[schemar]

Is it worth jotting down the outline of what content might go in the user docs? Did you have any ideas?

I added a short warning. Feel free to change as desired!

[claremacrae]

Thank you for all the very valid comments @claremacrae! Unfortunately I won't have more time to work on this until at least Thursday. Would be lovely if you could pick it up from here. If not, I will pick it up next week 👍

Huge thanks @schemar!

[schemar]

I wonder if there are cases where the while loop is endless…

[claremacrae]

I wonder if there are cases where the while loop is endless…

I thought that. I will try to come up with a test to make it happen….

[claremacrae]

Also, how does it work with invalid dates in the recurrence rule and the task. Like the user writing 31 Sep, which I did recently.

[schemar]

Also, how does it work with invalid dates in the recurrence rule and the task. Like the user writing 31 Sep, which I did recently.

No idea.

[claremacrae]

Also, how does it work with invalid dates in the recurrence rule and the task. Like the user writing 31 Sep, which I did recently.

No idea.

Hahah - that was a rhetorical reminder to myself to write tests

[claremacrae]

I've done a few more hours of testing, and some documentation. I've pushed the changes. I will add more info later, but there is a show-stopper issue that every month on the last gets stuck on the same date, if the new date is invalid. More info later...

[claremacrae]

I've done a few more hours of testing, and some documentation. I've pushed the changes. I will add more info later, but there is a show-stopper issue that every month on the last gets stuck on the same date, if the new date is invalid. More info later...

@schemar, Hi, below is the behaviour mentioned above.

Explanation of what's going on:

• The new due date works fine on every occurrence until 2022-01-31.
• The next occurrence should be 2022-02-28, but is 2022-01-31
• At which point, it get stuck on that date forever more.

- [ ] #task do stuff 🔁 every month on the 31st 📅 2022-01-31
- [x] #task do stuff 🔁 every month on the 31st 📅 2022-01-31 ✅ 2022-10-03
- [x] #task do stuff 🔁 every month on the 31st 📅 2022-01-31 ✅ 2022-10-03
- [x] #task do stuff 🔁 every month on the 31st 📅 2022-01-31 ✅ 2022-10-03
- [x] #task do stuff 🔁 every month on the 31st 📅 2021-12-31 ✅ 2022-10-03
- [x] #task do stuff 🔁 every month on the 31st 📅 2021-11-30 ✅ 2022-10-03

[aubreyz]

This may be an ignorant comment, but in the tasks dialog box the dates work fine. So what library is that using and could it use the same (and why does it differ?). For example if you type "one month after 2022-01-31" in the start field it yields 2022-02-28.

[claremacrae]

This may be an ignorant comment, but in the tasks dialog box the dates work fine. So what library is that using and could it use the same (and why does it differ?). For example if you type "one month after 2022-01-31" in the start field it yields 2022-02-28.

Oh wow, that's a really useful and interesting insight - thank you! Here's a bit more info...

• The date-related libraries used by Tasks:
  • chrono: for the Add/edit dialog, and auto-complete's converting of plain text date descriptions to actual dates
  • rrule: for calculating new dates for recurring tasks
• There have been Tasks bugs caused by behaviours in both libraries, prompting this suggestion: Improve date parsing - maybe replace chrono and/or rrule

Your comment has prompted a couple of ideas:

• I was wanting to find a way to test many recurrence examples, and writing the tests by hand is tedious and time-consuming. Perhaps I could generate a large number of recurrence tests that do the calculations in both libraries, and report any differences
• Better still, if we can map some recurrence rules written in rrules syntax to chrono syntax instead, perhaps we could rewrite some of the recurrence code based on chrono and get better results.

[claremacrae]

Things to consider when fixing the on the 31st case:

The following are all treated as the same thing:

every month on the 31st
every month on the 31th
every month on 31st
every month on 31th

(I may edit this comment with more cases)

[claremacrae]

There is also this case to consider (taken from the rule tests):

every week on the 3rd, 10th, 17th and 24th

[schemar]

Would ignoring all on the be on option? I considered it when I did the original implementation, but wasn't sure if it was needed. If the user explicitly wants recurrence on a certain day, then that should be honored in my opinion. What do you think?

[claremacrae]

Would ignoring all on the be on option? I considered it when I did the original implementation, but wasn't sure if it was needed. If the user explicitly wants recurrence on a certain day, then that should be honored in my opinion. What do you think?

Ooh, I like that.

I think if we ignored on and documented the limitation, that is probably the best we can do at this point.

(See about 2 or 3 comments back for the the being optional)

[dgreen]

I just tried Apple Calendar and BusyCal on the iPhone and they skip November when I schedule October 31st and monthly repeat. Monthly repeat in BusyCal is shown as Monthly on 31st after I save the entry. I found this discussion involving differing behavior of Outlook and Google (9 years ago post).

A trio of thoughts:

1. I wonder if there is covered by standard like iCalendar.

2. I generally want either last day of month or last business day of month. I wonder if this could be provide some day in addition to any solution chosen.

3. it seems like another challenge present in this implementation is that while an original desire of say October 31st and monthly is chosen by the user, any implementation that respects monthly will modify the date and then executing the monthly reschedule won't know the original request.

[claremacrae]

I found this discussion involving differing behavior of Outlook and Google (9 years ago post).

Thanks @dgreen. Did you mean to add a link in the above text?