[15.0][MIG] stock_reserve_rule: Migration to 15.0. #1753
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Before the change, the implementation of the fallback goes like this: If I reserve a move of 3000 and it finds 600 units, it splits the move to create a new move of 2400 and pretend to the caller that 3000 was reserved so the initial move is changed to 'assigned'. Now, if we have a move of 2400 and finds zero, it still splits the move, and pretend to the caller that 2400 was reserved → the initial move has no move line but is assigned. In this case, we should not split the move but only update the source location of the move.
Example of configuration: Rule location: Stock Removal rule 1: Stock/Zone1 Removal rule 2: Stock/Zone2 Reservation of a stock move with Stock/Zone2 as source location. Previously, it would reserve in Stock/Zone1. Now, it will never be allowed to reserve in Stock/Zone1. A warning message was added previously to warn the user about potential issues, which is now obsolete so I removed it.
Searching all rules then filtering in python the parent path is more efficient than finding all the parent locations and finding the matching rules.
It could not work properly here as we need the "fallback" to apply even if there is no quantity at all in the stock. As we hook the reservation rules in StockMove._update_reserved_quantity(), and this method is called only if we have at least 1 product in qty, the fallback was not applied with zero qty. A new module will handle this concept: OCA/wms#28
As the logger outputs an error log during tests, travis counts it as a failure of a test.
This reverts commit 768f186. Which is not more optimized, the optimization based on parent_path doesn't make sense here as the ORM will read parent_path in the location and get the parent ids by splitting the ids, it doesn't need more than one query on stock_location which is done based on its id and can reuse the cache, there is no lookup on parent path for parent_of. >>> env["stock.reserve.rule"].search([("location_id", "parent_of", 3125)]) 2020-05-27 05:36:59,938 1 DEBUG log_p odoo.sql_db: query: SELECT "stock_location"."id" as "id","stock_location"."name" as "name","stock_location"."complete_name" as "complete_name","stock_location"."active" as "active","stock_location"."usage" as "usage","stock_location"."location_id" as "location_id","stock_location"."comment" as "comment","stock_location"."parent_path" as "parent_path", <stripped>,"stock_location"."create_uid" as "create_uid","stock_location"."create_date" as "create_date","stock_location"."write_uid" as "write_uid","stock_location"."write_date" as "write_date" FROM "stock_location" WHERE "stock_location".id IN (3125) 2020-05-27 05:36:59,942 1 DEBUG log_p odoo.sql_db: query: SELECT "stock_reserve_rule".id FROM "stock_reserve_rule" WHERE (("stock_reserve_rule"."active" = true) AND ("stock_reserve_rule"."location_id" in (1,7,8,133,134,135,144,207,3125))) ORDER BY "stock_reserve_rule"."sequence" ,"stock_reserve_rule"."id"
When rules are configured and have been applied, we should not have an implicit fallback on the base location, as it would kind of cancel the benefits of the rules (as it would then take whatever it wants anywhere in all the locations).
The rules created in demo data of stock_reserve_rule make the tests of stock_vertical_lift (and possibly other modules) fail because the transfers can't be made available. Deactivate the rule in stock_reserve_rule and activate it only in its tests. Users can still activate the rule manually to test.
The former implementation was sorting the quants per location and trying to take as much quantities as possible from the same locations, to limit the number of operations to do. Then, only, removal order (fifo, ...) was applied. It is more important to respect removal order than limiting the operations, so remove this "optimization".
The former implementation was to take as much as possible of the largest packaging, to the smallest packacking, to have less to move. Then, only, removal order (fifo, ...) was applied for equal quantities. It is more important to respect removal order than limiting the operations, so remove this "optimization".
To remove the duplicate implementation of StockLocation.is_sublocation_of()
Ricardoalso
force-pushed
the
15.0-mig-stock_reserve_rule
branch
from
246f774 to
4524ecf
Compare
Ricardoalso
force-pushed
the
15.0-mig-stock_reserve_rule
branch
from
c197168 to
13a4bc2
Compare
|
/ocabot migration stock_reserve_rule |
…elds available in stock module V15
Ricardoalso
force-pushed
the
15.0-mig-stock_reserve_rule
branch
from
13a4bc2 to
8c3fb8f
Compare
leemannd
approved these changes
Jun 27, 2023
|
This PR has the |
|
/ocabot merge nobump |
|
What a great day to merge this nice PR. Let's do it! |
OCA-git-bot
added a commit
that referenced
this pull request
Jul 4, 2023
|
@dreispt your merge command was aborted due to failed check(s), which you can inspect on this commit of 15.0-ocabot-merge-pr-1753-by-dreispt-bump-nobump. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
|
It seems the pre-commit test has been killed during the merge -> https://github.com/OCA/stock-logistics-warehouse/actions/runs/5455628789/jobs/9927342186 |
|
/ocabot merge nobump |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
Congratulations, your PR was merged at c488165. 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.