- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 2
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, Update and Delete shopping items #168
Add, Update and Delete shopping items #168
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #168 +/- ##
==========================================
+ Coverage 96.80% 99.56% +2.75%
==========================================
Files 4 4
Lines 188 230 +42
Branches 21 23 +2
==========================================
+ Hits 182 229 +47
+ Misses 4 1 -3
+ Partials 2 0 -2 ☔ View full report in Codecov by Sentry. |
…em class
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…ions # Conflicts: # poetry.lock
src/aiomealie/models.py
Outdated
class ShoppingItem(DataClassORJSONMixin, DataClassDictMixin): | ||
"""ShoppingItem model.""" | ||
|
||
item_id: str = field(metadata=field_options(alias="id")) | ||
list_id: str = field(metadata=field_options(alias="shoppingListId")) | ||
note: str | ||
display: str | ||
checked: bool | ||
position: int | ||
is_food: bool = field(metadata=field_options(alias="isFood")) | ||
disable_amount: bool = field(metadata=field_options(alias="disableAmount")) | ||
quantity: float | ||
label_id: str = field(metadata=field_options(alias="labelId")) | ||
food_id: str = field(metadata=field_options(alias="foodId")) | ||
unit_id: str = field(metadata=field_options(alias="unitId")) | ||
item_id: Optional[str] = field(default=None, metadata=field_options(alias="id")) | ||
list_id: Optional[str] = field( | ||
default=None, metadata=field_options(alias="shoppingListId") | ||
) | ||
note: Optional[str] = field(default=None) | ||
display: Optional[str] = field(default=None) | ||
checked: Optional[bool] = field(default=None) | ||
position: Optional[int] = field(default=None) | ||
is_food: Optional[bool] = field( | ||
default=None, metadata=field_options(alias="isFood") | ||
) | ||
disable_amount: Optional[bool] = field( | ||
default=None, metadata=field_options(alias="disableAmount") | ||
) | ||
quantity: Optional[float] = field(default=None) | ||
label_id: Optional[str] = field( | ||
default=None, metadata=field_options(alias="labelId") | ||
) | ||
food_id: Optional[str] = field(default=None, metadata=field_options(alias="foodId")) | ||
unit_id: Optional[str] = field(default=None, metadata=field_options(alias="unitId")) | ||
|
||
class Config(BaseConfig): # pylint: disable=too-few-public-methods | ||
"""Mashumaro Config.""" | ||
|
||
serialize_by_alias = True | ||
code_generation_options = ["TO_DICT_ADD_OMIT_NONE_FLAG"] |
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.
Hmm, I think it would be better if we split the objects and have a CreateShoppingItem
object. This would save us a lot of assert item.something
in the code because everything is now nullable.
WDYT?
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 did start off with two but thought it was duplicating it. The CreateShoppingItem
would still need all the optional/default none.
Where are you thinking the asserts will be need, on the get list?
Also CreateShoppingItem
would also be used for Updates, so any suggestion on a better name?
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.
MutateShoppingItem
It's more that if we want to use it in Home assistant, that if we want to use the value where they need a str
, the value we have currently is typed str | None
, so we have to assert something is not None
before mypy sees it as str
.
Also, please use the union type over the Optional str | None
> Optional[str]
src/aiomealie/models.py
Outdated
note: str | None = field(default=None) | ||
display: str | None = field(default=None) | ||
checked: bool | None = field(default=None) | ||
position: int | None = field(default=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.
note: str | None = field(default=None) | |
display: str | None = field(default=None) | |
checked: bool | None = field(default=None) | |
position: int | None = field(default=None) | |
note: str | None = None | |
display: str | None = None | |
checked: bool | None = None | |
position: int | None = None |
I know this works to set it as None, but not sure if this works the same with code_generation_options = ["TO_DICT_ADD_OMIT_NONE_FLAG"]
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'll test it out, I have a bodged version of the lib in my own integration to try this stuff.
tests/test_mealie.py
Outdated
METH_POST, | ||
headers=HEADERS, | ||
params=None, | ||
json=item.to_dict(omit_none=True), |
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.
Let's assert the raw dict here so we are directly testing what mealie is receiving. This also helps testing the changes mashumaro can make.
…ions # Conflicts: # src/aiomealie/models.py
Proposed Changes
Implemented the shopping item mutation methods and basic tests.
Related Issues
N/A