-
-
Notifications
You must be signed in to change notification settings - Fork 623
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
[14.0][MIG] stock_move_backdating #1494
Conversation
use sudo: false to enable container build use cache: pip to cache pip packages
… method _create_account_move_line [FIX] upgrade quant_ids date in a single call
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 ok!
This PR has the |
@rousseldenis good to go? thanks! |
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.
Code review
@@ -21,7 +21,7 @@ def onchange_date_backdating(self): | |||
check_date(self.date_backdating) | |||
|
|||
def post_inventory(self): | |||
no_backdate_inventories = self.env["stock.inventory"].browse() | |||
no_backdate_inventories = self.env["stock.inventory"] |
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.
@toita86 The former writing was the good one.
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.
Why is it? We get the same result
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.
Yes, but formally this is better as instance is not really the same as a browse record set. Nevertheless, this one is not blocking
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.
Thank you for the review, I implemented almost all the changes you suggested.
@@ -0,0 +1,2 @@ | |||
id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink | |||
access_fill_date_backdating,access_fill_date_backdating,model_fill_date_backdating,base.group_user,1,0,0,0 |
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.
Shouldn't it be stock.group_stock_user instead ?
e5731fc
to
8a49d4e
Compare
# Disable pylint to match the exact signature in super | ||
# pylint: disable=dangerous-default-value |
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.
chore: these two lines can be removed, it's no longer necessary to disable pylint
8a49d4e
to
705ce77
Compare
@newtratip could you take a look? thanks! |
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.
Code review, LGTM
@rousseldenis ready for your review, thanks! |
@rousseldenis good for merge? |
/ocabot merge patch |
On my way to merge this fine PR! |
This PR has the |
@rousseldenis The merge process could not be finalized, because command
|
No description provided.