shopinvader / odoo-shopinvader

Odoo Modules. Sorry Magento, Shopinvader is coming
GNU Affero General Public License v3.0
119 stars 101 forks source link

Convention converting a pydantic model to odoo vals #1427

Open sebastienbeau opened 11 months ago

sebastienbeau commented 11 months ago

Hi @shopinvader/shopinvader-maintainers

I would like you thoughts, regarding the best and cleanest way to convert a pydantic model into a dict of vals for odoo.

I see that two way have been used.

Converting the pydantic object into a vals into the pydantic class it's self

See here : https://github.com/shopinvader/odoo-shopinvader/blob/10a15b4c369fb3f3c48983230230cbda83f021d6/shopinvader_api_address/schemas.py#L23-L35

Or in the "service"

See here (complex conversion) : https://github.com/shopinvader/odoo-shopinvader/blob/10a15b4c369fb3f3c48983230230cbda83f021d6/shopinvader_api_cart/routers/cart.py#L211 See here : https://github.com/shopinvader/odoo-shopinvader/pull/1421/files#diff-2bc91465c2d564fb6b38ffc94e181249d747a657a72b732b7a535902dab66e5aR240

@lmignon what do you think ?

lmignon commented 11 months ago

@sebastienbeau It depends.

When a Pydantic model is 'linked' to an odoo model, A method to_odoo_model into the pydantic model makes sense. In the case of the Transaction model, this model is not linked to an odoo model. It's a set of information required by an route endpoint to do some logic. In such a case, it makes sens that the conversion occurs into the service layer. (Pydantic model instances should not be propagated outside the service layer)

Regarding, your complex version, IMO it's too complex. You could have a 'convert_to_my_model_write' method defined on your pydantic model. At least, don't use the result of a call to model_dump(exclude_unset=True) as base to build the dictionary you'll use to call the method write on your odoo model. This approach introduce a deep coupling between your pydantic model and your odoo model. If your model is extended to add some specific fields that are not expected by odoo, your code will be broken.

sebastienbeau commented 11 months ago

Thanks for your feedback. I am going to adapt the PR with your recommendation. Thanks

sebastienbeau commented 11 months ago

I have done some change in my PR.

Here is how to convert "input params" into domain : https://github.com/shopinvader/odoo-shopinvader/pull/1421/commits/169b339f7b16873a277c4dd86aff289e5ec752c1#diff-c16e014ac95e1ac4ac046d51df771bcd5b6d4bb2a989646b11ad1a123591bd32R12

Here is how I have converted "input params" to vals for the write https://github.com/shopinvader/odoo-shopinvader/pull/1421/commits/169b339f7b16873a277c4dd86aff289e5ec752c1#diff-58ebe71b902dc742a85164f8e89fb500c247c88eb0aca39d334e565d08923377R33