[16.0][ADD] new simple module for bulk product#2259
Conversation
906a9c4 to
bcb7c09
Compare
MDgrap
left a comment
There was a problem hiding this comment.
Tested with runboat, works as intended
legalsylvain
left a comment
There was a problem hiding this comment.
hi. thanks for your contribution !
some remarks inline. a blocking one regarding onchange / computed.
Otherwise, LGTM.
thanks !
| is_bulk = fields.Boolean() | ||
|
|
||
| # Onchange Section | ||
| @api.onchange("uom_id") | ||
| def onchange_uom_id(self): | ||
| self.is_bulk = ( | ||
| True | ||
| if self.uom_id.category_id.measure_type in ["weight", "volume"] | ||
| else False | ||
| ) |
There was a problem hiding this comment.
| is_bulk = fields.Boolean() | |
| # Onchange Section | |
| @api.onchange("uom_id") | |
| def onchange_uom_id(self): | |
| self.is_bulk = ( | |
| True | |
| if self.uom_id.category_id.measure_type in ["weight", "volume"] | |
| else False | |
| ) | |
| is_bulk = fields.Boolean(compute="_compute_is_bulk", readonly=False) | |
| @api.depends("uom_id.category_id.measure_type") | |
| def _compute_is_bulk(self): | |
| for product in self: | |
| product.is_bulk = product.uom_id.category_id.measure_type in ["weight", "volume"] |
Better to replace by a computed function (readonly = False). As a result, the code will be raised in more moment. for exemple,
- if we import product massively, computation will be done. (it is not the case I think if it's onchange).
- when installing the module, the computation will be done for all existing product.
WDYT ?
Note : maybe it is requiring an SQL script to initialize more quickly the database...)
There was a problem hiding this comment.
I chose onchange by simplicity because we also want to handle few cases where product is bulk but is not marked for sale by weight : e.g cookies sold in bulk in a jar, the price being by unit.
But indeed, maybe we can go "compute" and add an inverse function
There was a problem hiding this comment.
Weird, my PR description talks about compute and inverse function, maybe that was my first idea ^^
| # Onchange Section | ||
| @api.onchange("uom_id") | ||
| def onchange_uom_id(self): | ||
| ProductProduct.onchange_uom_id(self) |
There was a problem hiding this comment.
| # Onchange Section | |
| @api.onchange("uom_id") | |
| def onchange_uom_id(self): | |
| ProductProduct.onchange_uom_id(self) |
| def test_01_change_uom(self): | ||
| self.assertEqual(self.product1.is_bulk, False) | ||
| self.product1.uom_id = self.uom_kg | ||
| self.product1.onchange_uom_id() |
There was a problem hiding this comment.
| self.product1.onchange_uom_id() |
| class ProductProduct(models.Model): | ||
| _inherit = "product.product" | ||
|
|
||
| is_bulk = fields.Boolean() |
There was a problem hiding this comment.
I think that an helper could be great for that field. somethink like
help="Check this box if this product is sold without any manufactured packaging. It is usually the case for product whose unit of measure is a unit of weight or volume, but it can be also the case for products priced per unit but sold in bulk (such as cookies, bread, etc.)"
There was a problem hiding this comment.
ok you had the same example :-p
legalsylvain
left a comment
There was a problem hiding this comment.
I think it will work the same, and be more synthetic.
| def _inverse_is_bulk(self): | ||
| pass |
There was a problem hiding this comment.
| def _inverse_is_bulk(self): | |
| pass |
|
|
||
| is_bulk = fields.Boolean( | ||
| compute="_compute_is_bulk", | ||
| inverse="_inverse_is_bulk", |
There was a problem hiding this comment.
| inverse="_inverse_is_bulk", | |
| readonly=False, |
2d8e550 to
b559aa3
Compare
b559aa3 to
e2c8bcb
Compare
|
This PR has the |
|
/ocabot merge nobump |
|
What a great day to merge this nice PR. Let's do it! |
|
Congratulations, your PR was merged at f698b81. Thanks a lot for contributing to OCA. ❤️ |
In a context of organic groceries selling lot of bulk products, we need to quickly identify them so this module adds this simple computed (and inverse) field