Merge lp:~camptocamp/hr-timesheet/7.0-fix-1206843-yvr into lp:~hr-core-editors/hr-timesheet/7.0

[Yannick Vaucher @ Camptocamp]

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

The 'method=True' argument is deprecated in fields.function.

The comment you put in the function does not help me to understand why do you need to do that. I don't understand it. Can you elaborate or reword it please? Thanks!

review: Needs Fixing

[lp:~camptocamp/hr-timesheet/7.0-fix-1206843-yvr]

49. By Yannick Vaucher @ Camptocamp on 2013-08-14

[IMP] timesheet_task - remove deprecated method=True un function fields

50. By Yannick Vaucher @ Camptocamp on 2013-08-14

[IMP] comments

[Vincent Renaville@camptocamp (vrenaville-c2c)]

I have just test this branch and it fix problem when you want to rename a task by exemple

review: Approve

[Stéphane Bidoul (Acsone) (sbi)]

I just made a test on a fresh v7 database with the ocb branches and the latest version of hr-timesheet. I installed timesheet_task, created a project a task and entered some time on the task. Then I renamed the task and everything went fine.

To reproduce the bug, I had to install project_timesheet.

Now, I personally always considered timesheet_task to be incompatible with project_timesheet as they have a fundamentally different approach. Is there a reason to install project_timesheet when one uses timesheet_task, possibly in combination with hr_timesheet_task?

Regarding this MP, it makes me quite uncomfortable to add a dummy field that always return empty value. This can be misleading. That said, I don't immediately see a clean way to make timesheet_task and project_timesheet compatible.

review: Needs Information (code review and test)

[Stéphane Bidoul (Acsone) (sbi)]

I investigated and tested this one a bit further and I conclude it does indeed make timesheet_task compatible with project_timesheet. project_timesheet in turns does bring some useful UI shortcuts related to timesheets so this makes sense. Thanks!

review: Approve (code review and test)

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

Thanks for the review and tests Stéphane!

It is indead not as clean as I would like it to be.

I think the main issue here is that we are changing the relation of a field so it can leads to modules expecting a behavior the new relation doesn't have. I'm just fixing this here by simulating the missing behavior.

An other option would be some cross module monkey patching to disable unwanted actions. I'm not sure it would be much cleaner nor easy to do.

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Thanks for the changes

review: Approve

=== modified file 'timesheet_task/project_task.py'
--- timesheet_task/project_task.py	2013-06-11 08:07:08 +0000
+++ timesheet_task/project_task.py	2013-08-14 15:05:29 +0000
@@ -61,22 +61,22 @@
     _columns = {'work_ids': fields.one2many('hr.analytic.timesheet', 'task_id', 'Work done'),
                 
 
-    'effective_hours': fields.function(_progress_rate, multi="progress", method=True, string='Time Spent',
+    'effective_hours': fields.function(_progress_rate, multi="progress", string='Time Spent',
                                        help="Sum of spent hours of all tasks related to this project and its child projects.",
                                        store={'project.task': (lambda self, cr, uid, ids, c={}: ids, TASK_WATCHERS, 20),
                                                 'account.analytic.line': (_get_analytic_line, TIMESHEET_WATCHERS, 20)}),
 
-    'delay_hours': fields.function(_progress_rate, multi="progress", method=True, string='Deduced Hours',
+    'delay_hours': fields.function(_progress_rate, multi="progress", string='Deduced Hours',
                                     help="Sum of spent hours with invoice factor of all tasks related to this project and its child projects.",
                                     store={'project.task': (lambda self, cr, uid, ids, c={}: ids, TASK_WATCHERS, 20),
                                              'account.analytic.line': (_get_analytic_line, TIMESHEET_WATCHERS, 20)}),
 
-    'total_hours': fields.function(_progress_rate, multi="progress", method=True, string='Total Time',
+    'total_hours': fields.function(_progress_rate, multi="progress", string='Total Time',
                                    help="Sum of total hours of all tasks related to this project and its child projects.",
                                    store={'project.task': (lambda self, cr, uid, ids, c={}: ids, TASK_WATCHERS, 20),
                                             'account.analytic.line': (_get_analytic_line, TIMESHEET_WATCHERS, 20)}),
 
-    'progress': fields.function(_progress_rate, multi="progress", method=True, string='Progress', type='float', group_operator="avg",
+    'progress': fields.function(_progress_rate, multi="progress", string='Progress', type='float', group_operator="avg",
                                      help="Percent of tasks closed according to the total of tasks todo.",
                                      store={'project.task': (lambda self, cr, uid, ids, c={}: ids, TASK_WATCHERS, 20),
                                               'account.analytic.line': (_get_analytic_line, TIMESHEET_WATCHERS, 20)})}
@@ -93,6 +93,32 @@
         return res
 
 class HrAnalyticTimesheet(orm.Model):
+    """
+    Add field:
+    - hr_analytic_timesheet_id:
+    This field is added to make sure a hr.analytic.timesheet can be used
+    instead of a project.task.work.
+
+    This field will always return false as we want to by pass next operations
+    in project.task write method.
+
+    Without this field, it is impossible to write a project.task in which
+    work_ids is empty as a check on it would raise an AttributeError.
+
+    This is because, in project_timesheet module, project.task's write method
+    checks if there is an hr_analytic_timesheet_id on each work_ids.
+
+        (project_timesheet.py, line 250, in write)
+        if not task_work.hr_analytic_timesheet_id:
+            continue
+
+    But as we redefine work_ids to be a relation to hr_analytic_timesheet
+    instead of project.task.work, hr_analytic_timesheet doesn't exists
+    in hr_analytic_timesheet... so it fails.
+
+    An other option would be to monkey patch the project.task's write method...
+    As this method doesn't fit with the change of work_ids relation in model.
+    """
     _inherit = "hr.analytic.timesheet"
     _name = "hr.analytic.timesheet"
 
@@ -116,6 +142,16 @@
                     res['value']['to_invoice'] = p.to_invoice.id
         return res
 
+    def _get_dummy_hr_analytic_timesheet_id(self, cr, uid, ids, names, arg, context=None):
+        """
+        Ensure all hr_analytic_timesheet_id is always False
+        """
+        return dict.fromkeys(ids, False)
+
+    _columns = {
+            'hr_analytic_timesheet_id': fields.function(_get_dummy_hr_analytic_timesheet_id, string='Related Timeline Id', type='boolean')
+            }
+
 class AccountAnalyticLine(orm.Model):
     """We add task_id on AA and manage update of linked task indicators"""
     _inherit = "account.analytic.line"