-
Notifications
You must be signed in to change notification settings - Fork 2
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
[3758][IMP] stock_owner_restriction, mrp_stock_owner_restriction #83
Conversation
Extend following methods for the scenario where owner restriction should be forced: - _update_available_quantity() (stock.quant) - _update_reserved_quantity() (stock.quant) Split lines to identify the owner_restriction setting into separate method for better extensibility. Fix read_group() (stock.quant) which was incorrectly assigning the owner record in the domain instead of the id.
restricted_owner = self.env.context.get("force_restricted_owner_id", None) | ||
if restricted_owner is not None: | ||
domain = expression.AND( | ||
[domain, [("owner_id", "=", restricted_owner.id)]] | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized this changes is not valid. I think this changes will lead to error when we search the quantity only for no owner_id (https://github.com/OCA/stock-logistics-workflow/blob/4b0301ca409ade61958fd09d50a10209d064b6c5/stock_owner_restriction/models/product.py#L27-L36). The intention of force_restricted_owner_id is ID or False from somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you fix this so that the error does not occur?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can fix but I want to know is there is any reason to change from restricted_owner_id
to restricted_owner
? If not, I will revert it and make adjustments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can fix but I want to know is there is any reason to change from
restricted_owner_id
torestricted_owner
? If not, I will revert it and make adjustments.
Due to the convention. xxx_id
as a variable usually represents an id (integer), which is not the case here.
e7f6af7
to
ac49fe1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functional review
3758
stock_owner_restriction:
[IMP] stock_owner_restriction: increase method coverage
Extend following methods for the scenario where owner restriction should be forced:
Split lines to identify the owner_restriction setting into separate method for
better extensibility.
Fix read_group() (stock.quant) which was incorrectly assigning the owner record
in the domain instead of the id.
mrp_stock_owner_restriction:
[IMP] mrp_stock_owner_restriction: add handling of umbuild orders