Move service registration to async_setup in Todoist - #175759
Conversation
|
Hey there @boralyl, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the Todoist integration to register its new_task service from the integration-level async_setup (via a new services.py module) instead of registering it inside the calendar platform's async_setup_entry/async_setup_platform. This makes the service available even when no config entry is loaded, so Home Assistant can validate automations that reference todoist.new_task. It is part of the epic to move service registration to async_setup (home-assistant/epics#65) and addresses issue #170752.
Changes:
- Added
homeassistant/components/todoist/services.pycontaining thenew_taskschema, handler, and a@callback async_setup_serviceshelper that resolves the config entry at call time viaservice.async_get_config_entry. - Registered services once from a new
async_setupin__init__.pyand addedCONFIG_SCHEMA = cv.config_entry_only_config_schema(DOMAIN). - Removed the service schema, handler, and
async_register_services(plus now-unused imports) fromcalendar.py.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| homeassistant/components/todoist/services.py | New module holding the moved new_task service schema/handler and async_setup_services; handler now looks up the config entry dynamically. |
| homeassistant/components/todoist/init.py | Adds async_setup that registers services once and declares CONFIG_SCHEMA. |
| homeassistant/components/todoist/calendar.py | Removes the service schema, handler, async_register_services, and the now-unused imports. |
Holistic assessment: The refactor follows the established pattern (e.g. Cloudflare, Amber Electric) and the code move itself is clean, with imports correctly pruned in calendar.py. However, Todoist still fully supports the legacy YAML calendar platform (async_setup_platform), which never creates a config entry. Because the new handler resolves the config entry via service.async_get_config_entry(..., None), todoist.new_task will fail for YAML-only setups, and the existing service tests (which run through the autouse YAML setup_platform fixture) will break. This needs to be resolved before merging.
The legacy YAML calendar platform creates a coordinator without a config entry, so resolving it through async_get_config_entry fails. Expose the YAML coordinator through hass.data and prefer it in the service handler, falling back to the single loaded config entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Proposed change
Move service registration to async_setup in Todoist.
Todoist still supports the legacy YAML calendar platform (
platform: todoist), which creates a coordinator without a config entry. Because of that, thenew_taskservice cannot rely solely onasync_get_config_entry. The YAML coordinator is now exposed throughhass.dataand preferred by the service handler, which falls back to the single loaded config entry otherwise. This keeps the service working for both YAML and config-entry setups.Part of home-assistant/epics#65
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: