-
-
Notifications
You must be signed in to change notification settings - Fork 799
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
[11.0] [MIG] purchase_request_department #598
[11.0] [MIG] purchase_request_department #598
Conversation
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.
Improve Code
e27e4f0
to
8c8f2e1
Compare
All comments attended @nikul-serpentcs @jbeficent . |
8c8f2e1
to
515e00d
Compare
515e00d
to
030bccf
Compare
def _get_my_department(self): | ||
employees = self.env.user.employee_ids | ||
return (employees[0].department_id if employees | ||
else self.env['hr.department']) |
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.
does not this return env object and not Id False for the default to digest ?
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.
What should it be done to solve this?? @JayVora-SerpentCS
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.
IMHO, return (employees[0].department_id if employees else False) should do. I could be wrong, but you may check.
030bccf
to
525cd4d
Compare
775286c
to
71a8c91
Compare
oca_dependencies.txt
Outdated
@@ -1,3 +1,5 @@ | |||
server-tools | |||
product-attribute | |||
server-ux | |||
hr | |||
purchase-workflow https://github.com/Eficent/purchase-workflow.git 11.0-mig-purchase_request |
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.
dependency merged, can you remove this and rebase?
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.
done!
71a8c91
to
294fa5d
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.
Thanks 👍
Update 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.
Minor change
self.env.ref('purchase_request.group_purchase_request_user')
use as globel variable,
otherwise LGTM 👍
294fa5d
to
00630e2
Compare
Migration to 11. Minor changes.