-
-
Notifications
You must be signed in to change notification settings - Fork 759
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
[16.0][MIG] account_lock_to_date #1732
[16.0][MIG] account_lock_to_date #1732
Conversation
c3486f7
to
bfa46c8
Compare
bfa46c8
to
ce73675
Compare
Looks like this is a duplicate of #1655 |
Right, I'm sorry I probably didn't find that when I opened this one because it wasn't mentioning the migrated module before #1655 (comment), and it is not mentioned in #1472. |
ce73675
to
7a5a4fe
Compare
7a5a4fe
to
6844615
Compare
Rebased. @JordiBForgeFlow @michotm @AaronHForgeFlow @NL66278 @MiquelRForgeFlow you worked on this, would you like to have a look? |
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 👍
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.
👍
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
if move.company_id.period_lock_to_date or move.company_id.fiscalyear_lock_to_date are not set nothing should be done.
Use ValidationError instead of UserError to prevent unfriendly message; Prevent crash when a lock to date not yet set in company.
If setting the Advisor Lock To Date too early, the message instructed the user to do the exact opposite of what he/she should do to correct the problem.
Also use ValidationError instead of UserError in wizard to set lock to dates.
6844615
to
853383c
Compare
@OCA/accounting-maintainers this has enough reviews, can someone please merge? |
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.
Also functional test
/ocabot merge nobump |
Thanks! |
/ocabot merge nobump |
What a great day to merge this nice PR. Let's do it! |
Congratulations, your PR was merged at 5cf0b64. Thanks a lot for contributing to OCA. ❤️ |
Standard migration.