-
-
Notifications
You must be signed in to change notification settings - Fork 794
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
[FIX] project_timesheet_time_control: Condition always gets evaluated as False #321
[FIX] project_timesheet_time_control: Condition always gets evaluated as False #321
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.
Please change module version number
@@ -32,7 +32,7 @@ def onchange_task_id(self): | |||
self.project_id = self.task_id.project_id.id | |||
|
|||
def eval_date(self, vals): | |||
if 'date_time' in vals and 'date' not in vals: | |||
if vals.get('date_time', False): |
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.
Please change it then to if 'date_time' in vals and vals.get('date_time'):
Done |
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.
@luismontalba @cubells please 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.
Tested on runbot
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 comments
@@ -32,7 +32,7 @@ def onchange_task_id(self): | |||
self.project_id = self.task_id.project_id.id | |||
|
|||
def eval_date(self, vals): | |||
if 'date_time' in vals and 'date' not in vals: | |||
if 'date_time' in vals and vals.get('date_time'): |
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 think that get
is not needed in this case.
if 'date_time' in vals and vals['date_time']:
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.
Actually removing the first part and leaving the get
would be functionally equivalent and cleaner:
if vals.get('date_time'):
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.
Well, I wanted to avoid the None
case, but you're right. It was correct the first time.
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.
Ok, I'm going to change it as @lasley says.
Hello,
We've notice that if a new analytic line is created with a date_time different from today. The field date is never equals to field date_time, because date has a default value defined at analytic module. So the condition in eval_date function is always evaluated as False