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

[14.0][ADD] sale_stock_mto_as_mts_orderpoint_mrp #1701

Open
wants to merge 2 commits into
base: 14.0
Choose a base branch
from

Conversation

mmequignon
Copy link
Member

No description provided.

Copy link
Contributor

@mt-software-de mt-software-de left a comment

Choose a reason for hiding this comment

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

Could this addon named
sale_stock_mto_as_mts_orderpoint_mrp

@mmequignon
Copy link
Member Author

Could this addon named sale_stock_mto_as_mts_orderpoint_mrp

done

@mmequignon mmequignon force-pushed the 14.0-create_OP_for_components branch 3 times, most recently from 3a93fde to 5c00c61 Compare September 11, 2024 13:54
@jbaudoux jbaudoux changed the title Add sale_stock_mto_as_mts_orderpoint_bom [14.0][ADD] sale_stock_mto_as_mts_orderpoint_bom Sep 11, 2024
@jbaudoux jbaudoux added this to the 14.0 milestone Sep 11, 2024
@mmequignon mmequignon force-pushed the 14.0-create_OP_for_components branch 2 times, most recently from 97c9494 to d996ef7 Compare September 11, 2024 16:12
@mmequignon
Copy link
Member Author

@jbaudoux @mt-software-de
Thanks for your reviews guys, I updated the PR, included a « refactoring » of sale_stock_mto_as_mts_orderpoint, adding a few hooks to ease extension and overrides.
Can you please drop another review please ?

Copy link
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

Algorithm seems now to be properly there.
Needs some final cleanup.
Have a look at how it has been cleaned up in v16. Note that in v16, it's still lacking the support of the mto route on the SO line (but it's also strange to create a permanent orderpoint for this case). The orderpoint is created when the route is set on the product. And orderpoints are triggered automatically in v16.

sale_stock_mto_as_mts_orderpoint/models/stock_move.py Outdated Show resolved Hide resolved
sale_stock_mto_as_mts_orderpoint/models/stock_move.py Outdated Show resolved Hide resolved
sale_stock_mto_as_mts_orderpoint/models/stock_move.py Outdated Show resolved Hide resolved
Comment on lines +20 to +21
@api.model
def _get_mto_orderpoint(self, warehouse, product):
Copy link
Contributor

Choose a reason for hiding this comment

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

Moved it to product.product and try to be close to v16 implementation https://github.com/OCA/stock-logistics-orderpoint/blob/16.0/stock_orderpoint_mto_as_mts/models/product_product.py#L26
You could backport the content of product_product.py from v16.

The main difference in v16 is that the trigger is automatic and you don't need to call the wizard

sale_stock_mto_as_mts_orderpoint/models/stock_move.py Outdated Show resolved Hide resolved
sale_stock_mto_as_mts_orderpoint_mrp/tests/test_bom.py Outdated Show resolved Hide resolved
sale_stock_mto_as_mts_orderpoint_mrp/tests/test_bom.py Outdated Show resolved Hide resolved
sale_stock_mto_as_mts_orderpoint_mrp/tests/test_bom.py Outdated Show resolved Hide resolved
sale_stock_mto_as_mts_orderpoint_mrp/tests/test_bom.py Outdated Show resolved Hide resolved
sale_stock_mto_as_mts_orderpoint_mrp/tests/test_bom.py Outdated Show resolved Hide resolved
@mmequignon mmequignon force-pushed the 14.0-create_OP_for_components branch 3 times, most recently from 2724387 to abc4bf2 Compare September 12, 2024 11:09
@jbaudoux jbaudoux marked this pull request as ready for review September 24, 2024 16:09
Copy link
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

Lgtm

@jbaudoux jbaudoux changed the title [14.0][ADD] sale_stock_mto_as_mts_orderpoint_bom [14.0][ADD] sale_stock_mto_as_mts_orderpoint_mrp Sep 24, 2024
@jbaudoux
Copy link
Contributor

@mmequignon can you rebuild the generated files as you renamed the module. You may also add BCIM as co author in the manifest. Thanks

if bom_lines != move_bom_lines:
continue

is_mto = bom_product._is_mto() or all(bom_moves.is_from_mto_route)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
is_mto = bom_product._is_mto() or all(bom_moves.is_from_mto_route)
is_mto = bom_product._is_mto() or all(bom_move.is_from_mto_route for bom_move in bom_moves)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should point out, that this can only work for simple bom kits. (Where no component is a kit it self)
A more complex bom structure like.
Product 1

  • Component 1.1

  • Component 1.2 (kit)
    -- Component 1.2.1
    -- Component 1.2.2

This can never order all of the necessary products.
Since there will be 1 move (1.1) linked to bom 1
and 2 moves (1.2.1, 1.2.2) linked to bom 1.2.

The move from bom 1 can not be reordered by mto because of this if bom_lines != move_bom_lines:
it would be just possible to reorder Component 1.2 .

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

Successfully merging this pull request may close these issues.

3 participants