[16.0] account_invoice_supplierinfo_update: fix updating the right supplierinfo #1697
Conversation
|
Hi @legalsylvain, |
sebastienbeau
changed the title
| ( | ||
| 0, | ||
| 0, |
There was a problem hiding this comment.
nitpicking, could be replaced by Command.create
| ( | ||
| 6, | ||
| 0, |
There was a problem hiding this comment.
same here ( Command.set([...]))
| lambda seller: seller.partner_id == self.move_id.supplier_partner_id | ||
| supplierinfos = self.product_id._select_seller( | ||
| partner_id=self.move_id.supplier_partner_id, | ||
| quantity=self.quantity, |
There was a problem hiding this comment.
"Information is a mass noun. It has no plural form"* so infos looks like informations which would be Frenglish and looks TinyERP. Better change that to supplier_info or supplier_data as you want.
- https://preply.com/en/question/plural-of-information
#onestpasamiserieux
There was a problem hiding this comment.
Hi.
@rvalyi. We already talk about that in another PR. summary of the previous comments :
what you mention is not an OCA rule.
- "infos" is used in many OCA repo. (12 occurences in V16, only in the repos I cloned for GRAP project)
- "datas" is used in many OCA repo. (19 occurences in V16, only in the repos I cloned for GRAP project)
So if you want to introduce that new rule, please create a dedicated PR here https://github.com/OCA/odoo-community.org/tree/master/website/Contribution
In the meantime, please avoid to block valid PR.
Thanks !
There was a problem hiding this comment.
I didn't block, only suggest to use better variable names. When native English speakers read that they feel it's cheap while it's easy to improve.
There was a problem hiding this comment.
I didn't block
You set "changes requested". Could you remove it ?
- as said not the place to create new rules. Please create a dedicated issue.
Thanks !
If you have variant of price per qty the module is not going to update the right supplierinfo
Reuse the native _sellect_seller to pick the right supplierinfo + add test
@legalsylvain