-
-
Notifications
You must be signed in to change notification settings - Fork 715
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] stock_orderpoint_automatic_creation: migration to 10.0 #308
[ADD] stock_orderpoint_automatic_creation: migration to 10.0 #308
Conversation
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.
Add your copyright in files
|
||
This module generates stock orderpoints automatically for a product when is | ||
created taking the company of the user who created the product as reference. | ||
Orderpoints creation can be automate from product, product category or company. |
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.
s/automate/automated
"procurement", | ||
"stock", | ||
], | ||
"category": "Inventory, Logistic, Storage", |
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.
Is this category correct?
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.
Yes, It is.
"views/product_views.xml", | ||
], | ||
"installable": True, | ||
"application": 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.
This is not an application
# Copyright 2016 Daniel Campos <danielcampos@avanzosc.es> - Avanzosc S.L. | ||
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). | ||
|
||
from . import product |
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.
product_product
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.
Are you sure?
In oca module, It's defined product.py and product_template.py:
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.
The rule is the model name, and the model name is product.product, not only product
wh_obj = self.env['stock.warehouse'] | ||
create = False | ||
if not result.type == 'product' or result.create_orderpoint == 'no': | ||
return result |
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.
If you make here return, you don't need this if chain
elif (result.categ_id.create_orderpoints == 'yes' or | ||
self.env.user.company_id.create_orderpoints): | ||
create = True | ||
if create: |
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.
Make that if you reach this point, you don't need the if
warehouses = wh_obj.search([('company_id', '=', company.id)]) | ||
for warehouse in warehouses: | ||
orderpoint_obj.create( | ||
{'name': result.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.
Put this in a hook method _prepare_orderpoint_vals
for allowing extensions
return result | ||
|
||
|
||
class ProductCategory(models.Model): |
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.
This should be in a separated file
|
||
orderpoint_product_max_qty = fields.Integer( | ||
'Max. product quantity', | ||
help='Default orderpoint Max. product quantity', default=0) |
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.
Default is already 0, so no need to add it.
create_orderpoints = fields.Boolean( | ||
'Create Orderpoints', help='Check this for automatic orderpoints') | ||
|
||
@api.constrains('orderpoint_product_max_qty', 'orderpoint_product_min_qty') |
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.
This is not @api.multi compliant
class ResCompany(models.Model): | ||
_inherit = 'res.company' | ||
|
||
orderpoint_product_max_qty = fields.Integer( |
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.
@cubells Why not using Product Decimal Precision here (as defined in stock.warehouse.orderpoint)?
string='Max. product quantity', | ||
help='Default orderpoint Max. product quantity', | ||
) | ||
orderpoint_product_min_qty = fields.Integer( |
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.
Same as above ?
@api.constrains('orderpoint_product_max_qty', 'orderpoint_product_min_qty') | ||
def _check_orderpoint_product_qty(self): | ||
for orderpoint in self: | ||
if orderpoint.orderpoint_product_max_qty < 0 \ |
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.
@cubells If you take into account decimal precision, I'd prefer using float_compare
@rousseldenis done! |
@api.constrains('orderpoint_product_max_qty', 'orderpoint_product_min_qty') | ||
def _check_orderpoint_product_qty(self): | ||
for company in self: | ||
if float_compare(company.orderpoint_product_max_qty, 0.0, |
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.
@cubells You cannot set an arbitrary rounding (2) here but you have to take product_id.uom_id.rounding
!
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.
And from where I take the product rounding if I'm in a company @rousseldenis ?
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.
Ok, sorry for that, you can take the Product decimal precision instead I think
@rousseldenis done! |
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.
👍 Code review
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.
some details and a question on usability but overall LGTM
class ProductCategory(models.Model): | ||
_inherit = 'product.category' | ||
|
||
create_orderpoints = fields.Selection( |
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.
A strange choice: why not a simple checkbox? Usability is better: one click instead of 2
Any reason for this choice?
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.
Original author decide that when user select "No" option in product or categories, then orderpoint is not created even if company settings are configured to create them.
if user select not to create orderpoint in product:
https://github.com/OCA/stock-logistics-warehouse/pull/308/files#diff-0743ce7eb081903f7439f511d86f3439R28
or in category:
https://github.com/OCA/stock-logistics-warehouse/pull/308/files#diff-0743ce7eb081903f7439f511d86f3439R30
then orderpoint is not created.
This allows user to avoid create orderpoint always for all products and categories if company are configured to do it.
Stock Orderpoint Automatic Creation | ||
=================================== | ||
|
||
This module generates stock orderpoints automatically for a product when is |
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.
This module automatically generates stock orderpoints for a product when it is
created, based on the information from the user's company.
|
||
To configure this module, you need to: | ||
|
||
#. Go to company form and, in the "Settings" tab, you can customize the default |
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.
Go to company form, tab "settings" and customize the default orderpoints data
Thanks for the merge, Cédric. Please take into account for other merges to see if some commits can be squashed as this case, because they are result of the review process. |
This module generates stock orderpoints automatically for a product when is created taking the company of the user who created the product as reference.
Orderpoints creation can be automate from product, product category or company.
cc @ Tecnativa