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

[WIP][WMS][12.0] Available to Promise Release #709

Closed
wants to merge 26 commits into from

Conversation

guewen
Copy link
Member

@guewen guewen commented Sep 10, 2019

Implements: #710

@jgrandguillaume jgrandguillaume mentioned this pull request Sep 13, 2019
32 tasks
@guewen guewen force-pushed the 12.0-wms-virtual-reservation branch 4 times, most recently from b817595 to 2264735 Compare September 27, 2019 14:02
Copy link
Contributor

@florian-dacosta florian-dacosta left a comment

Choose a reason for hiding this comment

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

I know it is a wip no worries, just a small comment.
Also, I try to understand well the use of the module, and I wonder about the make to order process.
If you configure one product with make to order and buy route.
On sale order validation, the delivery order will be created and marked as need_rule_pull.
So the purchase order for the make to order product won't be created.
Then, the delivery order should be releases once the products are virtually reserved, which won't ever happen for the make to order product, right?

def _compute_need_rule_pull(self):
for picking in self:
picking.need_rule_pull = any(
move.need_rule_pull for move in picking
Copy link
Contributor

Choose a reason for hiding this comment

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

should be move.need_rule_pull for move in picking.move_lines

@guewen
Copy link
Member Author

guewen commented Nov 4, 2019

@florian-dacosta spot on! I'll find a correction for the make to order issue, thanks :)

Glue for stock_virtual_reservation and sale
* we want to work with the virtual available as we cannot delivery what
is already outgoing
* call action_assign even if the state is already assigned, because it
may change the quantity
So we can activate the release only on some routes, while leaving
other routes work as normal (eg. MTO). Still have to figure out
if the ordered available to promise quantity is correct in this case.
'partner_id' is not required as the method _get_stock_move_values takes
the 'partner_id' from the 'group'. In fact, it makes the tests fail when
some other non-identified OCA addon is installed.
@guewen guewen changed the title [WIP][WMS][12.0] virtual reservation [WIP][WMS][12.0] Available to Promise Release Nov 5, 2019
@guewen guewen force-pushed the 12.0-wms-virtual-reservation branch from ca7b953 to 836668f Compare November 5, 2019 12:35
@guewen guewen force-pushed the 12.0-wms-virtual-reservation branch from a580542 to 651d86e Compare November 6, 2019 13:29
and not self.location_id.should_bypass_reservation()
)

def _ordered_available_to_promise(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this method not part of _compute_ordered_available_to_promise ? Retrieving the value can be done by reading the field.

Copy link
Member Author

Choose a reason for hiding this comment

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

As we can't have proper depends, if values change in the transaction, the field will not be recomputed.
We could add invalidations everywhere, not sure that's useful. The field is good for the view, but the code better has to recompute the value on every access. I should add a docstring about this.

("product_id", "=", self.product_id.id),
("date_priority", "<=", self.date_priority),
("warehouse_id", "=", self.warehouse_id.id),
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally we should exclude all the moves planned in the future that you can resupply on time. For example, there is no reason a delivery planned 3 months is blocking quantity while you can resupply in 7 days.
Simple solution would be configuring an horizon for considering blocking moves. A more complex and accurate solution would be by looking at the warehouse product orderpoint planned date.
To support sales order commitment dates, something must be done

Copy link
Member

Choose a reason for hiding this comment

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

We go for the horizon solution here

continue
# do not use the computed field, because it will keep
# a value in cache that we cannot invalidate declaratively
available_quantity = move._ordered_available_to_promise()
Copy link
Contributor

Choose a reason for hiding this comment

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

use the field "move._ordered_available_to_promise" instead

Copy link
Member Author

@guewen guewen Nov 8, 2019

Choose a reason for hiding this comment

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

There is the explanation in the comment just above why the field is not used ;)
I can rephrase if not understandable enough.

("product_id", "=", self.product_id.id),
("date_priority", "<=", self.date_priority),
("warehouse_id", "=", self.warehouse_id.id),
]
Copy link
Member

Choose a reason for hiding this comment

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

We go for the horizon solution here

@guewen
Copy link
Member Author

guewen commented Dec 18, 2019

Work continued on 13.0 on #799

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.

4 participants