[16.0] shopinvader_api_address: improve schema data conv #1536
Conversation
Always pass an env or a record. On the same vein of shopinvader#1534
| @@ -55,7 +59,7 @@ class AddressUpdate(StrictExtendableBaseModel, extra="ignore"): | |||
| state_id: int | None = None | |||
| country_id: int | None = None | |||
|
|
|||
| def to_res_partner_vals(self) -> dict: | |||
| def to_res_partner_vals(self, rec: ResPartner, address_id: int) -> dict: | |||
There was a problem hiding this comment.
What is the use of the 'address_id' parameter?
There was a problem hiding this comment.
well, the record you pass is not the real record you deal with. In fact, the method on the model is taking care of filtering the address on the current partner using this ID.
Hence, if you need to do something based on the real record value you need that.
The alternative would be to pass the address directly but yeah.... I've simply chosen to respect the same args that are passed over to _update_shopinvader_$type_address.
There was a problem hiding this comment.
Indeed addresses are filtered in _update_* methods: we pass the address identifier (address_id) to which this specific address is linked.. I still don't see why we should pass this information at this stage since you've the odoo record which is most probably a child of this address_id. Strictly speaking, when it comes to understanding the code, adding this parameter to this method makes no sense at all, and anyone reading the code should know that this parameter has a precise meaning only in a very specific context of use (this meaning is nowhere to be found here). I'd prefer to keep the general meaning of 'from' / 'to' methods and avoid making them polymorphic depending on the presence or absence of a parameter that should be interpreted in a precise sense for a specific use case.
There was a problem hiding this comment.
I agree w/ you, I'm trying to stay consistent w/ what I see 😄
Then... instead of passing the current partner I can browse the address right away.
And once I do this, I wonder why don't we pass the same record all around instead of the 2 args?
There was a problem hiding this comment.
I still prefer to not add a new parameter.
lmignon
changed the title
Always pass an env or a record.
On the same vein of #1534
CC @lmignon