Skip to content
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

[16.0][MIG] stock_orderpoint_mto_as_mts #14

Open
wants to merge 21 commits into
base: 16.0
Choose a base branch
from

Conversation

tuantrantg
Copy link

@tuantrantg tuantrantg commented Apr 16, 2024

Depends on:

Notes:

  • Renamed sale_stock_mto_as_mts_orderpoint to stock_orderpoint_mto_as_mts
  • Removed dependency on stock_orderpoint_manual_procurement as not needed anymore, see [15.0][MIG] stock_orderpoint_manual_procurement: Migration to version 15.0 stock-logistics-warehouse#1641
  • Used is_mto on product (depend on product-attribute/product_route_mto)
  • Used is_mto on route instead of xmlid (provided by stock-logistics-warehouse/stock_route_mto - already pulled by product_route_mto)
  • Performed the bellow in product.product:
    • Processed: the orderpoint must be created only for warehouses where there is no mto rule for that warehouse
    • Moved the orderpoint archiving from product.template and refactored it
  • Made the orderpoint archiving configurable on the warehouse
  • Dropped is_from_mto_route on stock.move. That should not be in this module, not related to orderpoint

@tuantrantg tuantrantg force-pushed the 16.0-mig-stock_orderpoint_mto_as_mts branch 3 times, most recently from c3e741c to fba9cc7 Compare April 16, 2024 08:53
@tuantrantg tuantrantg marked this pull request as ready for review April 16, 2024 08:57
@tuantrantg tuantrantg force-pushed the 16.0-mig-stock_orderpoint_mto_as_mts branch 2 times, most recently from 26327d1 to f405e88 Compare April 17, 2024 04:13
Copy link

@santostelmo santostelmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG good with @jbaudoux changes

test-requirements.txt Outdated Show resolved Hide resolved
@tuantrantg tuantrantg force-pushed the 16.0-mig-stock_orderpoint_mto_as_mts branch from f405e88 to 65bb160 Compare April 25, 2024 08:07
def _get_warehouse_missing_orderpoint_for_mto(self):
res = {}
wh_obj = self.env["stock.warehouse"]
for product in self:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for product in self:
for product in self:
if not product.purchase_ok:
continue

There's no point to create a orderpoints for products that can not be purchased.

That is in conflict with the validation done in purchase_reorder_control
Unpurchaseable Products cannot be reordered
https://github.com/OCA/purchase-workflow/blob/e113f0a3a046ad832f2e324df50301c276932fa4/purchase_reorder_control/models/stock_warehouse_orderpoint.py#L21

@tuantrantg tuantrantg force-pushed the 16.0-mig-stock_orderpoint_mto_as_mts branch from 65bb160 to cbf3127 Compare April 25, 2024 14:55
@tuantrantg tuantrantg force-pushed the 16.0-mig-stock_orderpoint_mto_as_mts branch from cbf3127 to 4c6119b Compare April 25, 2024 15:07
@tuantrantg tuantrantg force-pushed the 16.0-mig-stock_orderpoint_mto_as_mts branch from 4c6119b to fc51a5d Compare April 25, 2024 15:15
self.ensure_one()
res = False
wh_obj = self.env["stock.warehouse"]
for product in self:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop for loop as you have ensure_one()

that have no orderpoint yet.
"""
for product in self:
wh = product._get_warehouse_missing_orderpoint_for_mto()
Copy link

@jbaudoux jbaudoux May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where you check it is a mto product

mto_products = self._filter_mto_products()

return products

def write(self, vals):
# Archive orderpoints when MTO route is removed
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment is misplaced

@rousseldenis
Copy link
Sponsor Contributor

/ocabot migration stock_orderpoint_mto_as_mts

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone May 7, 2024
@OCA-git-bot
Copy link
Contributor

There's no issue in this repo with the title 'Migration to version 16.0' and the milestone 16.0, so not possible to add the comment.

products.sudo()._ensure_default_orderpoint_for_mto()
return products

def write(self, vals):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to look invals for is_mto or company_id. Those are on product.template, so you need to move the hook to write on the right model

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand the method

res = super().write(vals)
if vals.get("is_mto"):
    # when is_mto is set
    self.sudo()._ensure_default_orderpoint_for_mto()
elif not vals.get("is_mto", True):
    # when is_mto is removed
    self.sudo()._archive_orderpoints_on_mto_removal()
elif "company_id" in vals:
    # when company is changed
    self.sudo()._archive_orderpoints_on_mto_removal()
    self.sudo()._ensure_default_orderpoint_for_mto()
return res


_inherit = "stock.warehouse"

mto_as_mts = fields.Boolean(default=False)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you flag this, you could archive from the MTO route the rule for this warehouse

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default=False is useless

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet