New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Send recurrence data when updating a task in todoist #108269
Conversation
"content": "Soda", | ||
"description": "6-pack", | ||
"due_date": "2024-02-01", | ||
"due_string": "every day", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And verifying that it ends up getting passed as a kwarg when updating the task.
api_data = next((d for d in self.coordinator.data if d.id == uid), None) | ||
if api_data is not None: | ||
if due := api_data.due: | ||
update_data["due_string"] = due.string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this only be set when due_string was already present?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you set a due date/datetime this will always be set in my testing with the api responses.
No due date:
{
"due": null
}
Due date:
{
"due": {
"date": "2024-01-17",
"is_recurring": false,
"lang": "en",
"string": "Jan 17"
}
}
Due date/time:
{
"due": {
"date": "2024-01-17",
"datetime": "2024-01-17T21:30:00",
"is_recurring": false,
"lang": "en",
"string": "Jan 17 9:30 PM"
}
}
Due date recurring:
{
"due": {
"date": "2024-01-17",
"is_recurring": true,
"lang": "en",
"string": "every day"
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, makes sense.
What should happen to the due string when due date is being changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great question. I decided to move the logic inside of _task_api_data
to avoid the situation where previously it would overwrite the due_string
if the user had tried to unset it.
From testing it appears the due_date
and due_datetime
parameters take precedence over due_string
if they don't match in the case where there is no recurrence. For example this data POSTed to the endpoint when changing the due date from 2024-01-18 to 2024-01-31:
{
"content": "test",
"description": "",
"due_date": "2024-01-31",
"due_string": "Jan 18"
}
And the task is updated correctly in todoist with the new due date of 2024-01-31.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Co-authored-by: Allen Porter <allen.porter@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! 👍🏼
Breaking change
Proposed change
This fixes an issue where marking a recurring task as completed using the todo domain would cause the recurrence data to be erased in todoist. See the issue for further information.
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
..coveragerc
.To help with the load of incoming pull requests: