Add calendar platform to Habitica integration #128248
Conversation
|
Hey there @asmfreak, @leikoilja, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
tr4nt0r
force-pushed
the
habitica_calendar
branch
from
5a5ea6e
to
09c79c1
Compare
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
| """Return calendar events within a datetime range.""" | ||
|
|
||
| today = to_date(self.coordinator.data.user["lastCron"]) | ||
| # returns only todays and future dailies. |
There was a problem hiding this comment.
This also is not meeting the get_event spec. get_events should work for any start/end date range. A calendar can render events from the past as well, and triggers work based on past events.
There was a problem hiding this comment.
In Habitica the start of the day can be set to a different time (in my account it is 4:00 am for example. If at that time all dailies that were due have been completed the cron runs at 4 and resets all dailies, marking the start of the day. If there are incompleted dailies from the past day (called yesterdailies), the user gets a last chance to mark tasks as completed (in case the task was done but forgotten to mark as completed) and then the user presses the button 'start my day', which then marks the start of the day. That's what I want to reflect here.
That's also why it doesn't make sense to render events from the past, dailies are reset every day.
There was a problem hiding this comment.
Also there is no data about past dailies. I could generate events based on the current recurrence settings but that would be a wild guess, because they can be changed anytime and I don't know if they were possibly different in the past
There was a problem hiding this comment.
OK, though completed tasks should probably still be returned here, consistent with how other todos on calendar show up. If needed, we can add more rfc5545 attributes to calendar events and render them different in the UI in the future.
I don't really see that we need to bring in the cron stuff here, rather than ignoring it and letting it show up on the calendar how it shows up on the calendar. (t's very hard to reason about given it seems independent of the recurrence rule and not on a single timeline, so maybe i'm missing it). It feels like its trying to implement todo status tracking and i think we should just add support for todo status tracking if we want that.
I'm worried getting fancy here and not following the spec is going to break triggers/automations since it expects this to be confirming
There was a problem hiding this comment.
Aren't the triggers based on the calendar entities on/off state?
I checked the Todoist integration, it also provides a calendar for tasks with due dates. If a task is completed, it is not shown in the calendar view anymore. That's as far as I can see the only other integration that implements a calendar for tasks. Maybe in the future we could render completed tasks in the calendar in grey, that would be a cool feature, but for now I would prefer to do it like the Todoist integration does it already.
I can't ignore the cron. I need it to determine on what day to render the tasks. That's the only way to know if the task is from yesterday or from today.
|
I realize what you're effectively wanting here is a different behavior from calendar events, given these are todos. This hasn't really been agreed upon, so i'm reviewing this in the currently defined calendar event spec. There is a proposed recurring todo spec here but it hasn't really been decided upon yet. |
Had a quick look and I think that sounds promising. If it gets approved I'm eager to implement it for Habitica 😁 |
tr4nt0r
force-pushed
the
habitica_calendar
branch
from
bd63212
to
e257737
Compare
| events = [ | ||
| CalendarEvent( | ||
| start=next_recurrence.date(), | ||
| end=self.calculate_end_date(next_recurrence), |
There was a problem hiding this comment.
This should always be next_recurrence.date() + timedelta(days=1)
There was a problem hiding this comment.
The intention is, if there still are unresolved tasks from yesterday, they are still the active tasks, so the enddate is the end of today so the state of the entity stays on. Until these aren't resolved by either completing or skipping them and then starting the new day, the tasks for today are still not the active or next upcoming tasks, they are calculated "future" recurrences. When the daily tasks are reset, the state will advance to the next task that is due.
I know it sounds a bit complicated or I may not be the best in explaining. Maybe this is more helpful https://habitica.fandom.com/wiki/Cron
| for task in self.coordinator.data.tasks | ||
| if task["type"] == HabiticaTaskType.DAILY and task["everyX"] | ||
| for recurrence in build_rrule(task).between(start_date, end_date, inc=True) | ||
| if (start := recurrence.date()) > self.today.date() |
There was a problem hiding this comment.
given between above, is this greater than check needed?
There was a problem hiding this comment.
Yes, it is, to differentiate todays active tasks from the future calculated recurrences.
| """Return calendar events within a datetime range.""" | ||
|
|
||
| today = to_date(self.coordinator.data.user["lastCron"]) | ||
| # returns only todays and future dailies. |
There was a problem hiding this comment.
OK, though completed tasks should probably still be returned here, consistent with how other todos on calendar show up. If needed, we can add more rfc5545 attributes to calendar events and render them different in the UI in the future.
I don't really see that we need to bring in the cron stuff here, rather than ignoring it and letting it show up on the calendar how it shows up on the calendar. (t's very hard to reason about given it seems independent of the recurrence rule and not on a single timeline, so maybe i'm missing it). It feels like its trying to implement todo status tracking and i think we should just add support for todo status tracking if we want that.
I'm worried getting fancy here and not following the spec is going to break triggers/automations since it expects this to be confirming
| def extra_state_attributes(self) -> dict[str, bool] | None: | ||
| """Return entity specific state attributes.""" | ||
| if event := self.event: | ||
| return {"yesterdaily": event.start < date.today()} |
There was a problem hiding this comment.
Attributes should always be returned, but may have a None value. See home-assistant/architecture#680
There was a problem hiding this comment.
I'll change that, no problem. But the default state attributes itself aren't returned either if there is no upcoming task. So Home Assistant seems inconsistent here.
Proposed change
This adds the calendar platform to the Habitica integration and adds three calendar entities.
To-do's:
Provides due dates of to-dos as a calendar. Due dates are all-day events.
Dailies
Provides a calendar listing with all dailies for today and all future recurrences of dailies. Dailies for today will only be listed as long as they were not completed. The state of the calendar entity will be on as long as there is still tasks to do for the day and off when all is done.
Reminders
Habitica dailies and to-dos allow setting reminders, these will be listed in this calendar, displaying the name of the event the reminder belongs to. Reminders don't have a duration, but the calendar status will show reminders as active for up to 1hr, unless another reminder goes off.Removed for now, needs some more tweaking, will be added in separate PR
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: