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

[REF] stock,mrp: autoconfirm on move instead of picking #106910

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

amoyaux
Copy link
Contributor

@amoyaux amoyaux commented Nov 30, 2022

Currently there is an issue with the kit. If you create an immediate transfer with a kit, it will have a correct behavior and split it in different moves. However in the code it's not optimal.

It will split the move during the inverse of the field quantity_done It means that during the create when you get the move back with moves = super().create(vals) it will return an id of a move that doens't exists anymore and we can't get easily the new moves created from the action_explode

This commit removes the autoconfirm on the picking level. Instead autoconfirm during the create of the move (the purpose is to return the new sets of moves if an extra processing is required). It continues to confirm on the picking afterward to trigger the scheduler.

It allows to confirm stock.picking with a state different than draft and cancel. It's to be able to remove easier the initial move of the kit. We could made a patch to prevent the reservation and let it draft but the feature of being able to delete every move except done ones is a good idea.

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:


I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@robodoo
Copy link
Contributor

robodoo commented Nov 30, 2022

@C3POdoo C3POdoo added the RD research & development, internal work label Nov 30, 2022
@amoyaux amoyaux marked this pull request as ready for review December 1, 2022 08:12
@C3POdoo C3POdoo requested a review from a team December 1, 2022 08:13
Copy link
Contributor

@ticodoo ticodoo left a comment

Choose a reason for hiding this comment

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

Auto-confirm part looks good to me. Deleting non-draft moves will probably need some tests added/modified to make sure we cover the additional potential flows it will add.

addons/stock/models/stock_move.py Show resolved Hide resolved
addons/stock/models/stock_move.py Show resolved Hide resolved
@amoyaux amoyaux force-pushed the master-remove-autoconfirm-arm branch 3 times, most recently from b215b9d to 8bf8ded Compare December 22, 2022 15:30
Currently there is an issue with the kit. If you create an
immediate transfer with a kit, it will have a correct behavior and
split it in different moves. However in the code it's not optimal.

It will split the move during the inverse of the field `quantity_done`
It means that during the create when you get the move back with
`moves = super().create(vals)` it will return an id of a move that
doens't exists anymore and we can't get easily the new moves created
from the `action_explode`

This commit removes the autoconfirm on the picking level. Instead
autoconfirm during the create of the move (the purpose is to return
the new sets of moves if an extra processing is required).
It continues to confirm on the picking afterward to trigger the
scheduler.

It allows to confirm `stock.picking` with a state different than draft
and cancel. It's to be able to remove easier the initial move of the
kit. We could made a patch to prevent the reservation and let it draft
but the feature of being able to delete every move except done ones is a
good idea.
@amoyaux amoyaux force-pushed the master-remove-autoconfirm-arm branch from 8bf8ded to 704e0ae Compare December 22, 2022 15:35
It's not possible to unlink a move in draft or done state.
You can't cancel a single move neither. The workaround for
it, would be to set the quantity done to zero or not process the
backorder. It would be easier for the user to simply unlink the
unwanted move.

It could be risky in a case with chained moves. But the process already
exist in `_action_cancel` or when a backorder is not process. So we
could reuse it in this case.

Also fix a bug when the move dest state are not recompute after a
cancel.
Add a way to cancel the picking when it's empty.
@amoyaux amoyaux force-pushed the master-remove-autoconfirm-arm branch from 704e0ae to 519ac8d Compare December 22, 2022 16:55
Since the confirmation is on the move now we could also
remove the code from `mrp.production` to keep it simple.
@amoyaux amoyaux force-pushed the master-remove-autoconfirm-arm branch from 83e0d39 to 8b2ba2c Compare December 23, 2022 15:32
@amoyaux
Copy link
Contributor Author

amoyaux commented Dec 27, 2022

robodoo r+ rebase-ff

@robodoo
Copy link
Contributor

robodoo commented Dec 27, 2022

Merge method set to rebase and fast-forward.

robodoo pushed a commit that referenced this pull request Dec 27, 2022
Currently there is an issue with the kit. If you create an
immediate transfer with a kit, it will have a correct behavior and
split it in different moves. However in the code it's not optimal.

It will split the move during the inverse of the field `quantity_done`
It means that during the create when you get the move back with
`moves = super().create(vals)` it will return an id of a move that
doens't exists anymore and we can't get easily the new moves created
from the `action_explode`

This commit removes the autoconfirm on the picking level. Instead
autoconfirm during the create of the move (the purpose is to return
the new sets of moves if an extra processing is required).
It continues to confirm on the picking afterward to trigger the
scheduler.

It allows to confirm `stock.picking` with a state different than draft
and cancel. It's to be able to remove easier the initial move of the
kit. We could made a patch to prevent the reservation and let it draft
but the feature of being able to delete every move except done ones is a
good idea.

Part-of: #106910
robodoo pushed a commit that referenced this pull request Dec 27, 2022
It's not possible to unlink a move in draft or done state.
You can't cancel a single move neither. The workaround for
it, would be to set the quantity done to zero or not process the
backorder. It would be easier for the user to simply unlink the
unwanted move.

It could be risky in a case with chained moves. But the process already
exist in `_action_cancel` or when a backorder is not process. So we
could reuse it in this case.

Also fix a bug when the move dest state are not recompute after a
cancel.
Add a way to cancel the picking when it's empty.

Part-of: #106910
robodoo pushed a commit that referenced this pull request Dec 27, 2022
Since the confirmation is on the move now we could also
remove the code from `mrp.production` to keep it simple.

closes #106910

Signed-off-by: Arnold Moyaux (arm) <arm@odoo.com>
@robodoo
Copy link
Contributor

robodoo commented Dec 27, 2022

@amoyaux staging failed: ci/runbot on 7eaadbec754c422650d1cf02196c744fced9f3ae (view more at https://runbot.odoo.com/runbot/build/22234513)

@rdeodoo
Copy link
Contributor

rdeodoo commented Jan 30, 2023

@amoyaux This one is shown as failed PR on the mergebot dashboard, could you have a look

@amoyaux
Copy link
Contributor Author

amoyaux commented Jan 30, 2023

@rdeodoo I know it's just not a priority

@rdeodoo
Copy link
Contributor

rdeodoo commented Jan 30, 2023

@robodoo r-

I see, let's remove it from the dashboard then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants