openerp-community-reviewer team mailing list archive
-
openerp-community-reviewer team
-
Mailing list archive
-
Message #01802
[Merge] lp:~camptocamp/ocb-addons/7.0-bugfix-1189480-with-perf-mdh into lp:ocb-addons
Matthieu Dietrich @ camptocamp has proposed merging lp:~camptocamp/ocb-addons/7.0-bugfix-1189480-with-perf-mdh into lp:ocb-addons.
Requested reviews:
Nicolas Bessi - Camptocamp (nbessi-c2c)
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c): code review, no test
Related bugs:
Bug #1189480 in OpenERP Community Backports (Addons): "planned_hours, total_hours, effective_hours are wrongly calculated in parent project"
https://bugs.launchpad.net/ocb-addons/+bug/1189480
For more details, see:
https://code.launchpad.net/~camptocamp/ocb-addons/7.0-bugfix-1189480-with-perf-mdh/+merge/197325
The quick fix proposed in lp:~camptocamp/openobject-addons/7.0-fixes-bug-1189480 works, but the multiple read() severely impact performance. This solution uses a WITH RECURSIVE SQL statement in order to 1) avoid any extraneous read() 2) get all the information in one request.
I also added a YAML test to test both this new method and a hierarchical computation, since all YAML tests in addons are for a single level of projects.
--
https://code.launchpad.net/~camptocamp/ocb-addons/7.0-bugfix-1189480-with-perf-mdh/+merge/197325
Your team OpenERP Community Backports Team is subscribed to branch lp:ocb-addons.
=== modified file 'project/__openerp__.py'
--- project/__openerp__.py 2012-10-23 16:05:04 +0000
+++ project/__openerp__.py 2013-12-02 08:29:42 +0000
@@ -82,6 +82,7 @@
'test/project_demo.yml',
'test/project_process.yml',
'test/task_process.yml',
+ 'test/hours_process.yml',
],
'installable': True,
'auto_install': False,
=== modified file 'project/project.py'
--- project/project.py 2013-11-22 16:55:04 +0000
+++ project/project.py 2013-12-02 08:29:42 +0000
@@ -135,6 +135,7 @@
res.update(ids)
return list(res)
+ # Deprecated; the _progress_rate method does not use this anymore
def _get_project_and_children(self, cr, uid, ids, context=None):
""" retrieve all children projects of project ids;
return a dictionary mapping each project to its parent project (or None)
@@ -154,28 +155,75 @@
return res
def _progress_rate(self, cr, uid, ids, names, arg, context=None):
- child_parent = self._get_project_and_children(cr, uid, ids, context)
# compute planned_hours, total_hours, effective_hours specific to each project
+ # How this works: the WITH RECURSIVE statement will create an union line
+ # for each parent project with the hours of each child project; the final
+ # SUM(...) ensures we get a total of hours by project.
cr.execute("""
- SELECT project_id, COALESCE(SUM(planned_hours), 0.0),
- COALESCE(SUM(total_hours), 0.0), COALESCE(SUM(effective_hours), 0.0)
- FROM project_task WHERE project_id IN %s AND state <> 'cancelled'
- GROUP BY project_id
- """, (tuple(child_parent.keys()),))
+ WITH RECURSIVE recur_table(project_id,
+ parent_id,
+ planned_hours,
+ total_hours,
+ effective_hours) AS (
+ SELECT project.id,
+ parent.id,
+ COALESCE(task.planned, 0.0),
+ COALESCE(task.total, 0.0),
+ COALESCE(task.effective, 0.0)
+ FROM project_project project
+ LEFT JOIN account_analytic_account account
+ ON project.analytic_account_id = account.id
+ LEFT JOIN project_project parent
+ ON parent.analytic_account_id = account.parent_id
+ LEFT JOIN (SELECT project_id,
+ SUM(planned_hours) as planned,
+ SUM(total_hours) as total,
+ SUM(effective_hours) as effective
+ FROM project_task
+ WHERE state <> 'cancelled'
+ GROUP BY project_id) AS task
+ ON project.id = task.project_id
+ UNION ALL
+ SELECT project.id,
+ parent.id,
+ recur_table.planned_hours,
+ recur_table.total_hours,
+ recur_table.effective_hours
+ FROM project_project project
+ LEFT JOIN account_analytic_account account
+ ON project.analytic_account_id = account.id
+ LEFT JOIN project_project parent
+ ON parent.analytic_account_id = account.parent_id
+ LEFT JOIN (SELECT project_id,
+ SUM(planned_hours) as planned,
+ SUM(total_hours) as total,
+ SUM(effective_hours) as effective
+ FROM project_task
+ WHERE state <> 'cancelled'
+ GROUP BY project_id) AS task
+ ON project.id = task.project_id
+ JOIN recur_table ON project.id = recur_table.parent_id
+ )
+ SELECT project_id,
+ SUM(planned_hours),
+ SUM(total_hours),
+ SUM(effective_hours)
+ FROM recur_table
+ WHERE project_id IN %s
+ GROUP BY project_id
+ """, (tuple(ids),))
# aggregate results into res
- res = dict([(id, {'planned_hours':0.0,'total_hours':0.0,'effective_hours':0.0}) for id in ids])
- for id, planned, total, effective in cr.fetchall():
- # add the values specific to id to all parent projects of id in the result
- while id:
- if id in ids:
- res[id]['planned_hours'] += planned
- res[id]['total_hours'] += total
- res[id]['effective_hours'] += effective
- id = child_parent[id]
+ res = dict([(result_line[0], {'planned_hours': result_line[1],
+ 'total_hours': result_line[2],
+ 'effective_hours': result_line[3]})
+ for result_line in cr.fetchall()])
# compute progress rates
for id in ids:
if res[id]['total_hours']:
- res[id]['progress_rate'] = round(100.0 * res[id]['effective_hours'] / res[id]['total_hours'], 2)
+ res[id]['progress_rate'] = round(100.0 *
+ res[id]['effective_hours'] /
+ res[id]['total_hours'],
+ 2)
else:
res[id]['progress_rate'] = 0.0
return res
@@ -192,7 +240,7 @@
res = super(project, self).unlink(cr, uid, ids, context=context)
mail_alias.unlink(cr, uid, alias_ids, context=context)
return res
-
+
def _get_attached_docs(self, cr, uid, ids, field_name, arg, context):
res = {}
attachment = self.pool.get('ir.attachment')
@@ -203,7 +251,7 @@
task_attachments = attachment.search(cr, uid, [('res_model', '=', 'project.task'), ('res_id', 'in', task_ids)], context=context, count=True)
res[id] = (project_attachments or 0) + (task_attachments or 0)
return res
-
+
def _task_count(self, cr, uid, ids, field_name, arg, context=None):
if context is None:
context = {}
@@ -228,7 +276,7 @@
def attachment_tree_view(self, cr, uid, ids, context):
task_ids = self.pool.get('project.task').search(cr, uid, [('project_id', 'in', ids)])
domain = [
- '|',
+ '|',
'&', ('res_model', '=', 'project.project'), ('res_id', 'in', ids),
'&', ('res_model', '=', 'project.task'), ('res_id', 'in', task_ids)
]
@@ -736,7 +784,7 @@
new_name = _("%s (copy)") % (default.get('name', ''))
default.update({'name':new_name})
return super(task, self).copy_data(cr, uid, id, default, context)
-
+
def copy(self, cr, uid, id, default=None, context=None):
if context is None:
context = {}
=== added file 'project/test/hours_process.yml'
--- project/test/hours_process.yml 1970-01-01 00:00:00 +0000
+++ project/test/hours_process.yml 2013-12-02 08:29:42 +0000
@@ -0,0 +1,174 @@
+-
+ First, create the analytic accounts
+-
+ !record {model: account.analytic.account, id: parent_project_account}:
+ name: Parent Account
+ code: PAR
+ type: view
+-
+ !record {model: account.analytic.account, id: child1_project_account}:
+ name: Child Account 1
+ code: C1
+ type: view
+-
+ I create a main project
+-
+ !record {model: project.project, id: project_project_parent}:
+ company_id: base.main_company
+ name: Parent Project
+ user_id: base.user_demo
+ parent_id: all_projects_account
+ analytic_account_id: parent_project_account
+ alias_model: project.task
+-
+ A first child project
+-
+ !record {model: project.project, id: project_project_child_1}:
+ company_id: base.main_company
+ name: Child Project 1
+ user_id: base.user_demo
+ parent_id: parent_project_account
+ analytic_account_id: child1_project_account
+ alias_model: project.task
+-
+ A second child project
+-
+ !record {model: project.project, id: project_project_child_2}:
+ company_id: base.main_company
+ name: Child Project 2
+ user_id: base.user_demo
+ parent_id: parent_project_account
+ alias_model: project.task
+-
+ A child of the first child project
+-
+ !record {model: project.project, id: project_project_child_1_1}:
+ company_id: base.main_company
+ name: Child Project 1 1
+ user_id: base.user_demo
+ parent_id: child1_project_account
+ alias_model: project.task
+-
+ A task for the main project, with 20 hours planned and 20 hours remaining
+-
+ !record {model: project.task, id: project_task_parent_project}:
+ date_start: !eval time.strftime('%Y-05-%d %H:%M:%S')
+ name: Task parent project
+ planned_hours: 20.0
+ remaining_hours: 20.0
+ project_id: project_project_parent
+ user_id: base.user_demo
+-
+ I test the hours
+-
+ !python {model: project.project}: |
+ project = self.browse(cr, uid, ref("project_project_parent"))
+ assert abs(project.planned_hours - 20.0) < 10**-4, "Planned hours are not correct! 20.0 != %s" % project.planned_hours
+ assert abs(project.total_hours - 20.0) < 10**-4, "Total hours are not correct! 20.0 != %s" % project.total_hours
+ assert abs(project.effective_hours - 0.0) < 10**-4, "Effective hours are not correct! 0.0 != %s" % project.effective_hours
+ assert abs(project.progress_rate - 0.0) < 10**-4, "Progress rate is not correct! 0.0 != %s" % project.progress_rate
+-
+ A task for the first child project, with 30 hours planned and 30 hours remaining
+-
+ !record {model: project.task, id: project_task_child_project_1}:
+ date_start: !eval time.strftime('%Y-05-%d %H:%M:%S')
+ name: Task child project 1
+ planned_hours: 30.0
+ remaining_hours: 30.0
+ project_id: project_project_child_1
+ user_id: base.user_demo
+-
+ I test the hours
+-
+ !python {model: project.project}: |
+ project = self.browse(cr, uid, ref("project_project_parent"))
+ assert abs(project.planned_hours - 50.0) < 10**-4, "Planned hours are not correct! 50.0 != %s" % project.planned_hours
+ assert abs(project.total_hours - 50.0) < 10**-4, "Total hours are not correct! 50.0 != %s" % project.total_hours
+ assert abs(project.effective_hours - 0.0) < 10**-4, "Effective hours are not correct! 0.0 != %s" % project.effective_hours
+ assert abs(project.progress_rate - 0.0) < 10**-4, "Progress rate is not correct! 0.0 != %s" % project.progress_rate
+ project = self.browse(cr, uid, ref("project_project_child_1"))
+ assert abs(project.planned_hours - 30.0) < 10**-4, "Planned hours are not correct! 30.0 != %s" % project.planned_hours
+ assert abs(project.total_hours - 30.0) < 10**-4, "Total hours are not correct! 30.0 != %s" % project.total_hours
+ assert abs(project.effective_hours - 0.0) < 10**-4, "Effective hours are not correct! 0.0 != %s" % project.effective_hours
+ assert abs(project.progress_rate - 0.0) < 10**-4, "Progress rate is not correct! 0.0 != %s" % project.progress_rate
+-
+ A task for the second child project, with 40 hours planned and 10 hours remaining
+-
+ !record {model: project.task, id: project_task_child_project_2}:
+ date_start: !eval time.strftime('%Y-05-%d %H:%M:%S')
+ name: Task child project 2
+ planned_hours: 40.0
+ remaining_hours: 10.0
+ project_id: project_project_child_2
+ user_id: base.user_demo
+-
+ I test the hours
+-
+ !python {model: project.project}: |
+ project = self.browse(cr, uid, ref("project_project_parent"))
+ assert abs(project.planned_hours - 90.0) < 10**-4, "Planned hours are not correct! 90.0 != %s" % project.planned_hours
+ assert abs(project.total_hours - 60.0) < 10**-4, "Total hours are not correct! 60.0 != %s" % project.total_hours
+ assert abs(project.effective_hours - 0.0) < 10**-4, "Effective hours are not correct! 0.0 != %s" % project.effective_hours
+ assert abs(project.progress_rate - 0.0) < 10**-4, "Progress rate is not correct! 0.0 != %s" % project.progress_rate
+ project = self.browse(cr, uid, ref("project_project_child_2"))
+ assert abs(project.planned_hours - 40.0) < 10**-4, "Planned hours are not correct! 40.0 != %s" % project.planned_hours
+ assert abs(project.total_hours - 10.0) < 10**-4, "Total hours are not correct! 10.0 != %s" % project.total_hours
+ assert abs(project.effective_hours - 0.0) < 10**-4, "Effective hours are not correct! 0.0 != %s" % project.effective_hours
+ assert abs(project.progress_rate - 0.0) < 10**-4, "Progress rate is not correct! 0.0 != %s" % project.progress_rat
+-
+ A task for the child child project, with 50 hours planned and 50 hours remaining
+-
+ !record {model: project.task, id: project_task_child_project_1_1}:
+ date_start: !eval time.strftime('%Y-05-%d %H:%M:%S')
+ name: Task child project 1 1
+ planned_hours: 50.0
+ remaining_hours: 50.0
+ project_id: project_project_child_1_1
+ user_id: base.user_demo
+-
+ I test the hours
+-
+ !python {model: project.project}: |
+ project = self.browse(cr, uid, ref("project_project_parent"))
+ assert abs(project.planned_hours - 140.0) < 10**-4, "Planned hours are not correct! 140.0 != %s" % project.planned_hours
+ assert abs(project.total_hours - 110.0) < 10**-4, "Total hours are not correct! 110.0 != %s" % project.total_hours
+ assert abs(project.effective_hours - 0.0) < 10**-4, "Effective hours are not correct! 0.0 != %s" % project.effective_hours
+ assert abs(project.progress_rate - 0.0) < 10**-4, "Progress rate is not correct! 0.0 != %s" % project.progress_rate
+ project = self.browse(cr, uid, ref("project_project_child_1"))
+ assert abs(project.planned_hours - 80.0) < 10**-4, "Planned hours are not correct! 80.0 != %s" % project.planned_hours
+ assert abs(project.total_hours - 80.0) < 10**-4, "Total hours are not correct! 80.0 != %s" % project.total_hours
+ assert abs(project.effective_hours - 0.0) < 10**-4, "Effective hours are not correct! 0.0 != %s" % project.effective_hours
+ assert abs(project.progress_rate - 0.0) < 10**-4, "Progress rate is not correct! 0.0 != %s" % project.progress_rate
+ project = self.browse(cr, uid, ref("project_project_child_1_1"))
+ assert abs(project.planned_hours - 50.0) < 10**-4, "Planned hours are not correct! 50.0 != %s" % project.planned_hours
+ assert abs(project.total_hours - 50.0) < 10**-4, "Total hours are not correct! 50.0 != %s" % project.total_hours
+ assert abs(project.effective_hours - 0.0) < 10**-4, "Effective hours are not correct! 0.0 != %s" % project.effective_hours
+ assert abs(project.progress_rate - 0.0) < 10**-4, "Progress rate is not correct! 0.0 != %s" % project.progress_rate
+-
+ I create a task work on child 1 task
+-
+ !record {model: project.task.work, id: project_child_1_work}:
+ name: test work
+ hours: 10.0
+ task_id: project_task_child_project_1
+ user_id: base.user_demo
+-
+ and refresh the task to account for it
+-
+ !record {model: project.task, id: project_task_child_project_1}:
+ planned_hours: 20.0
+-
+ I test the hours
+-
+ !python {model: project.project}: |
+ project = self.browse(cr, uid, ref("project_project_parent"))
+ assert abs(project.planned_hours - 130.0) < 10**-4, "Planned hours are not correct! 130.0 != %s" % project.planned_hours
+ assert abs(project.total_hours - 110.0) < 10**-4, "Total hours are not correct! 110.0 != %s" % project.total_hours
+ assert abs(project.effective_hours - 10.0) < 10**-4, "Effective hours are not correct! 10.0 != %s" % project.effective_hours
+ assert abs(project.progress_rate - 9.09) < 10**-4, "Progress rate is not correct! 9.09 != %s" % project.progress_rate
+ project = self.browse(cr, uid, ref("project_project_child_1"))
+ assert abs(project.planned_hours - 70.0) < 10**-4, "Planned hours are not correct! 70.0 != %s" % project.planned_hours
+ assert abs(project.total_hours - 80.0) < 10**-4, "Total hours are not correct! 80.0 != %s" % project.total_hours
+ assert abs(project.effective_hours - 10.0) < 10**-4, "Effective hours are not correct! 10.0 != %s" % project.effective_hours
+ assert abs(project.progress_rate - 12.5) < 10**-4, "Progress rate is not correct! 12.5 != %s" % project.progress_rate
+
Follow ups
