Allow use of date_string in service call#13256
Allow use of date_string in service call#13256fabaff merged 2 commits intohome-assistant:devfrom bjw-s:todoist
Conversation
| item.update(date_lang=call.data[DATE_LANG]) | ||
|
|
||
| if DUE_DATE in call.data: | ||
| # Make sure only DUE_DATE is set |
There was a problem hiding this comment.
I think this should at least issue a warning here, if it's not already caught by service validation (see above).
| vol.Optional(DUE_DATE): cv.string, | ||
| vol.Optional(DATE_STRING): cv.string, | ||
| vol.Optional(DATE_LANG): vol.All(cv.string, vol.Length(min=2, max=2)), | ||
| vol.Optional(DUE_DATE): cv.string |
There was a problem hiding this comment.
Since specifying both a date string and a due date is not possible (see below), this should probably already be caught in validation?
| description: The day this task is due, in natural language (Optional). | ||
| example: "tomorrow" | ||
| date_lang: | ||
| description: The langurage of date_string (Optional). |
| priority: | ||
| description: The priority of this task, from 1 (normal) to 4 (urgent) (Optional). | ||
| example: 2 | ||
| date_string: |
There was a problem hiding this comment.
The other due date parameter is called due_date - so one might wonder whether this is another type of date? Perhaps a name like due_date_string would make things clearer?
|
@tinloaf Thanks for the feedback, think I've processed it all in the latest commit 👍 |
tinloaf
left a comment
tinloaf
left a comment
There was a problem hiding this comment.
Looking good, Thanks! 👍 You've missed to update the naming in the documentation PR, I mentioned it there.
|
Doh... Missed that one indeed. Fixed now, thanks 👍 |
|
Do I need to do anything else to have this PR (and the accompanying docs PR) merged? |
* Allow use of date_string in service call * Add stricter validation, fix descriptions
ghost
added
4 participants
Description:
In the original implementation of the Todoist calendar component it was not possible to allow Todoist natural language for due dates. This PR adds this capability.
Also fixes the
services.yamlfor the service callPull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#4923
Checklist:
tox. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed: