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.

Sign up for GitHub

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

Jump to bottom

[14.0][IMP] stock_move_auto_assign_auto_release: Release release_ready moves together #2122

Merged
merged 1 commit into from
Aug 7, 2024
Merged

[14.0][IMP] stock_move_auto_assign_auto_release: Release release_ready moves together #2122

merged 1 commit into from
Aug 7, 2024

Conversation

mt-software-de
Copy link
Contributor

A a transfer is waiting for more than product to release and the release policy is "one".
The whole transfer should be released, instead of only the last received one.

Within this PR
Also releaseable siblings will be released.

@mt-software-de
Copy link
Contributor Author

cc @lmignon @TDu @jbaudoux

lmignon
lmignon reviewed Jul 30, 2024
View reviewed changes
Copy link
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

LGTM (Code review only)

@mt-software-de mt-software-de force-pushed the 14-fix-stock_move_auto_assign_auto_release branch from c8a037c to d1465d8 Compare July 30, 2024 12:58
jbaudoux
jbaudoux reviewed Jul 30, 2024
View reviewed changes
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.

What about dropping this moves_auto_release method and in _enqueue_auto_assign, check that the move.picking_id is is_auto_release_allowed, and then use as delayable _delay_auto_release_available_to_promise (which is better placed in stock_move_auto_assign_auto_release than in stock_release_channel_auto_release)

@jbaudoux jbaudoux changed the title [IMP] stock_move_auto_assign_auto_release: Release release_ready moves together [14.0][IMP] stock_move_auto_assign_auto_release: Release release_ready moves together Jul 30, 2024
@jbaudoux
Copy link
Contributor

A a transfer is waiting for more than product to release and the release policy is "one". The whole transfer should be released, instead of only the last received one.

It would be worth adding a fix on stock_available_to_promise_release so that when release_available_to_promise is called on stock.move it raises an error. And on stock.picking release_available_to_promise, call moves...._run_stock_rule instead of moves....release_available_to_promise

@mt-software-de
Copy link
Contributor Author

A a transfer is waiting for more than product to release and the release policy is "one". The whole transfer should be released, instead of only the last received one.

It would be worth adding a fix on stock_available_to_promise_release so that when release_available_to_promise is called on stock.move it raises an error. And on stock.picking release_available_to_promise, call moves...._run_stock_rule instead of moves....release_available_to_promise

So you mean a check if all moves of a picking are in the recordset if the policy is one?

@mt-software-de
Copy link
Contributor Author

What about dropping this moves_auto_release method and in _enqueue_auto_assign, check that the move.picking_id is is_auto_release_allowed, and then use as delayable _delay_auto_release_available_to_promise (which is better placed in stock_move_auto_assign_auto_release than in stock_release_channel_auto_release)

Ok you mean moving _delay_auto_release_available_to_promise into stock_move_auto_assign_auto_release - yes this sounds good.
Needs to be merged before OCA/wms#566

@mt-software-de mt-software-de force-pushed the 14-fix-stock_move_auto_assign_auto_release branch from d1465d8 to e703def Compare August 1, 2024 09:43
This was referenced Aug 1, 2024
Closed
Merged
jbaudoux
jbaudoux reviewed Aug 5, 2024
View reviewed changes
stock_move_auto_assign_auto_release/tests/test_assign_auto_release.py
job = self._get_job_for_method(
trap.enqueued_jobs, shipping.auto_release_available_to_promise
)
job.perform()
Copy link
Contributor

Choose a reason for hiding this comment

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

This is failing
https://github.com/OCA/stock-logistics-warehouse/actions/runs/10195951314/job/28205684041?pr=2122#step:8:1118

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know why yet, i will have a look.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Locally it works. :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Somehow the trap jobs wasn't working as expected with the current queue.job version. I fixed it

@mt-software-de mt-software-de force-pushed the 14-fix-stock_move_auto_assign_auto_release branch from e703def to d7f63f4 Compare August 7, 2024 07:48
TDu
TDu approved these changes Aug 7, 2024
View reviewed changes
Copy link
Member

@TDu TDu left a comment

Choose a reason for hiding this comment

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

Maybe the readme could be updated also ?
LG otherwise.

stock_move_auto_assign_auto_release/models/product_product.py Outdated Show resolved Hide resolved
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@mt-software-de
[IMP] stock_move_auto_assign_auto_release: Release release_ready pick…
6eb9c8f 
…ings instead of moves

Instead of just releasing the release ready moves of a give product
it now releases the whole transfer
This ensures that a transfer with a release_policy=one gets not split
@mt-software-de mt-software-de force-pushed the 14-fix-stock_move_auto_assign_auto_release branch from d7f63f4 to 6eb9c8f Compare August 7, 2024 11:53
@jbaudoux
Copy link
Contributor

jbaudoux commented Aug 7, 2024

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 14.0-ocabot-merge-pr-2122-by-jbaudoux-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit f566242 into OCA:14.0 Aug 7, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at cc8e67d. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbaudoux jbaudoux jbaudoux approved these changes

@lmignon lmignon lmignon left review comments

@TDu TDu TDu approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mt-software-de @jbaudoux @OCA-git-bot @lmignon @TDu