fix: create repeat tasks if missed in the days before #2694

[ishanarora]

Description

selectTaskRepeatCfgsDueOnDay now selects taskRepeatCfgs that were due on any day after lastTaskCreation until dayDate

Issues Resolved

Fixes #2694

Check List

• New functionality includes testing.
• New functionality has been documented in the README if applicable.

[ishanarora]

@johannesjo this pull request proposes to change the behaviour of selectTaskRepeatCfgsDueOnDay. The existing behaviour is to return only the tasks which are due to repeat on dayDate. The proposed behaviour is to check this for every date after lastTaskCreation date until dayDate. This would make sure to create a repeatable task instance if it was due to be created on any previous day, but was not created for any reason (such as because Super Productivity was not run on that date). Please let me know if you agree with the proposed change.

I see the current behaviour of selectTaskRepeatCfgsDueOnDay is thoroughly tested. With this change the test matrix should be expanded to include different lastTaskCreation states.

[johannesjo]

Thanks for the PR. There are two things we should consider:

1. Ideally the creation date should be set to the original due day so people can spot for which day a task was intended (the value is currently set here:

   super-productivity/src/app/features/task-repeat-cfg/task-repeat-cfg.service.ts

   Lines 211 to 217 in 5d25253

   	addTask({
   	task: {
   	...task,
   	// NOTE otherwise isCreateNew check above would not work as intended and
   	// we use created also for the repeat day label for past tasks
   	created: targetDayDate,
   	},

   ).
2. We should prompt the user in case there are many tasks to be created (e.g. a daily task when the users wasn't online for a couple of days) to avoid that users have manually delete lots of tasks.

[ishanarora]

Thanks for the comments @johannesjo.

1. Ideally the creation date should be set to the original due day so people can spot for which day a task was intended.

That's fair enough.

2. We should prompt the user in case there are many tasks to be created (e.g. a daily task when the users wasn't online for a couple of days) to avoid that users have manually delete lots of tasks.

It seems that in normal daily usage the invariant is that there is only ever one active (not completed) instance of a repeating task config. This is because at midnight all daily repeating tasks of the previous day automatically get marked done. Keeping in line with this invariant, in your example we could either:

1. create just one task for the last missed repeating instance, or
2. we could create all missed instances and mark as completed all task instances except the latest one.

The PR implements the former behaviour.

The latter behaviour is effectively the same as if the app was left running unattended for days. I believe the easiest user experience would be if we stick to this behaviour without prompting the user. Do you agree?

[johannesjo]

Yes you're right! Creating just the last missed instance is probably the best solution :)

Could you have a look at the failing unit tests?

[johannesjo]

ping @ishanarora :)

[ishanarora]

Apologies, I haven't had the time to take this up recently.

It seems the invariant I mentioned above no longer holds since cbc0ff4. In light of that I would be happy to change the behaviour of this PR to create all missed instances with the creation date set to the original due day for each. Agreed?

Since this PR is more of a bugfix, I believe the feature to prompt the user in case there are many tasks to be created can be a separate PR.

[johannesjo]

No worries!

It seems the invariant I mentioned above no longer holds since cbc0ff4. In light of that I would be happy to change the behaviour of this PR to create all missed instances with the creation date set to the original due day for each. Agreed?

Also a possibility, but we should limit this to maybe the last 5(?) instances to avoid a task explosion. Like this we also don't need the prompt. What do you think?

[johannesjo]

After further discussing this I think that the best approach might be to provide a new config to each repeatable tasks that lets the user decide if missed instances should be created, or not. So users can choose themselves. Something like a daily reminder shouldn't be recreated for every missed day, but something like "do your monthly tax form" definitely should.

Are you still interested in working on this?

[ishanarora]

I think that's a good idea. Perhaps we can add configuration options for:

1. An optional limit on the total number of pending task instances.
2. Once the limit is hit then:
   1. should the earlier tasks be marked as done (similar to our earlier behaviour),
   2. or should the new tasks be queued for creation once older tasks are completed by the user,
   3. or should the potential new task instances be lost in the ether, or perhaps they can be created and moved to backlog, or marked as done.

I'm afraid the work ahead is fairly outside my area of expertise. I don't know Angular, and the little TS I know I learned for small PRs like this one. We could close this PR since it will need a rewrite for the changed requirements.

Also I've come across a couple of alternative approches in the wild:

1. In taskwarrior tasks may optionally have until date, after which the task instance would self-destruct. We could also borrow taskwarrior's notion of due date.

2. topydo has task recurrence duration with two modes:

   1. normal mode: periodic start dates (similar to SupProd's periodic creation dates)
   2. strict mode: the next instance is created after the recurrence duration starting from the current task's due date. If the due date for the current task is changed then the creation date of the subsequent task instance also change.

[johannesjo]

@ishanarora Thank you very much for your input! This is very helpful!

I can try to pick this up then, if it is alright with you, though I don't know when I will get to it... :)

[ishanarora]

@johannesjo yes, of course! I that case may I suggest merging this pull request as a hotfix for #2694 in the meantime. I believe #2694 is a critical bug e.g. it caused a missing annual income tax filing task because of the annual recurrence date landed on a Sunday.

The PR still needs updating the unit tests for selectTaskRepeatCfgsDueOnDay, see #2695 (comment). Do you think you could take a jab at it? Thanks!

[github-actions]

This PR has not received any updates in 90 days. Please comment, if this still relevant!