-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
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
Add Picnic shopping cart as Todo list #102855
Conversation
Hey there @corneyl, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
def todo_items(self) -> list[TodoItem] | None: | ||
"""Get the current set of items in cart items.""" | ||
if self.coordinator.data is None: | ||
return None |
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.
Please note that none of the To-do have this one covered.
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 good. Just one suggestion of a minor typo i think
Co-authored-by: Allen Porter <[email protected]>
@edenhaus Can you take a quick look at this again? |
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.
Thanks @DCSBL 👍
Would it be possible to update the screenshot in the description? ( I removed it as it is not up to date)
Thanks! Screenshot is updated! -- Follow-up is also updated and ready for review, this allows adding items in the same way as the existing service by Picnic → #102862 |
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.
Please address the comments in a new PR. Thanks!
async_add_entities([PicnicCart(picnic_coordinator, config_entry)]) | ||
|
||
|
||
class PicnicCart(TodoListEntity, CoordinatorEntity): |
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.
Please invert the inheritance order.
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.
Also add a type annotation to the generic CoordinatorEntity
what coordinator type it holds.
TodoItem( | ||
summary=f"{article['name']} ({article['unit_quantity']})", | ||
uid=f"{item['id']}-{article['id']}", | ||
status=TodoItemStatus.NEEDS_ACTION, # We set 'NEEDS_ACTION' so they count as state |
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.
Please move the comment above the line to decrease the line length.
Proposed change
Add new Todo entity for Picnic.
The new todo list is a bit limited for the needs, but you can get a nice overview of what is in your cart.
Limitations
.UPDATE_TODO_ITEM
is not enabledType of change
Additional information
Checklist
black --fast 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: