[IMP] project: Improve task's partner_id management

Purpose
=======

During the development of the task #2169100, it appeared that the synchronisation
of a task's partner_id according to its project's partner_id and its parent's
partner_id was not very clear.

A first idea was to simplify the partner_id management: as it could be annoying
for the customer to see all the task's partner_id overriden after project's
partner_id modification, the idea was to just use the parent's parnter_id or the
project's partner_id as default value of the task's partner_id. Then, if the
partner_id of the project or of the parent is changed, there is no more impact
on the task partner_id itself.

The main problem of this solution is that the management of the task's partner_id
is also extended in several others modules like sale_project and sale_timesheet.

You can find below what seems to happen with all these task partners.

Start following rules according to which module is installed.
(for the record: sale_timesheet depends on sale_project which depends on project).

Rules are applied in the order they are written.

Rules labelled "O" only apply when changing the project through the task form view.
Rules labelled "C" apply everywhere.

technical: C=compute, O=onchange

project
C.1 If parent's partner changes and the partner is not set, then set the parent's
    partner
C.2 Else if project's partner changes ant the partner is not set, then set the
    project's partner

sale_project [+ project]
C.1 If the project's sale order line's partner changes and the partner is not set,
    then set the project's sale order line's partner
C.2 Else apply (project.C) rules

sale_timesheet + [sale_project + project]
C.1 Apply (sale_project.C) rules
O.1 If the project is billed "At Project rate" or "At Employee Rate" and the
    partner is not set, then set the project's sale order's partner
    the sale order line's partner

Specification
=============

- If the partner changes, we ignore all current tasks and let them be. We set the
  new partner as default value on all new tasks. In detail, we have done
  this:
    - When the user creates a task give the customer of the default project or the
      task parent as default customer for the task.
    - When the user changes the project and no partner in the task, we take the
      partner of the new project (if this project has a customer).
    - When the user changes the customer of the project, the linked tasks are not
      impacted by this changes, even if some tasks have not a customer yet.
    - When the user changes the customerf of the parent task, the child tasks are
      not impacted by this changes, event if these tasks have not a customer yet.
- remove the onchange rule to remain consistent.

TaskID: 2232042

closes #51471

Co-authored-by: Xavier BOL (xbo) <xbo@odoo.com>

addons/project/models/project.py

@@ -531,6 +531,14 @@ class Task(models.Model):
     _order = "priority desc, sequence, id desc"
     _check_company_auto = True
 
+    @api.model
+    def _get_default_partner_id(self, project=None, parent=None):
+        if parent and parent.partner_id:
+            return parent.partner_id.id
+        if project and project.partner_id:
+            return project.partner_id.id
+        return False
+
     def _get_default_stage_id(self):
         """ Gives default stage_id """
         project_id = self.env.context.get('default_project_id')
@@ -1018,6 +1026,18 @@ class Task(models.Model):
         if 'repeat_weekday' in default_fields:
             vals['repeat_weekday'] = self._fields.get('repeat_weekday').selection[week_start][0]
 
+        if 'partner_id' in vals and not vals['partner_id']:
+            # if the default_partner_id=False or no default_partner_id then we search the partner based on the project and parent
+            project_id = vals.get('project_id')
+            parent_id = vals.get('parent_id', self.env.context.get('default_parent_id'))
+            if project_id or parent_id:
+                partner_id = self._get_default_partner_id(
+                    project_id and self.env['project.project'].browse(project_id),
+                    parent_id and self.env['project.task'].browse(parent_id)
+                )
+                if partner_id:
+                    vals['partner_id'] = partner_id
+
         return vals
 
     @api.model_create_multi
@@ -1136,20 +1156,16 @@ class Task(models.Model):
         for task in self:
             task.user_id = task.user_id or task.parent_id.user_id or self.env.uid
 
-    @api.depends('parent_id.partner_id', 'project_id.partner_id')
+    @api.depends('parent_id', 'project_id', 'display_project_id')
     def _compute_partner_id(self):
+        """ Compute the partner_id when the tasks have no partner_id.
+
+            Use the project partner_id if any, or else the parent task partner_id.
         """
-        If a task has no partner_id, use the project partner_id if any, or else the parent task partner_id.
-        Once the task partner_id has been set:
-            1) if the project partner_id changes, the task partner_id is automatically changed also.
-            2) if the parent task partner_id changes, the task partner_id remains the same.
-        """
-        for task in self:
-            if task.partner_id:
-                if task.project_id.partner_id:
-                    task.partner_id = task.project_id.partner_id
-            else:
-                task.partner_id = task.project_id.partner_id or task.parent_id.partner_id
+        for task in self.filtered(lambda task: not task.partner_id):
+            # When the task has a parent task, the display_project_id can be False or the project choose by the user for this task.
+            project = task.display_project_id if task.parent_id and task.display_project_id else task.project_id
+            task.partner_id = self._get_default_partner_id(project, task.parent_id)
 
     @api.depends('partner_id.email', 'parent_id.email_from')
     def _compute_email_from(self):

addons/project/tests/test_project_flow.py

@@ -134,11 +134,8 @@ class TestProjectFlow(TestProjectCommon):
         })
 
         # the child task 1 is linked to a project without partner_id (goats project)
-        child_task_1 = Task.create({
+        child_task_1 = Task.with_context(default_project_id=self.project_goats.id, default_parent_id=parent_task.id).create({
             'name': 'Task Child with project',
-            'parent_id': parent_task.id,
-            'project_id': self.project_goats.id,
-            'display_project_id': self.project_goats.id,
             'planned_hours': 3,
         })
 
@@ -156,8 +153,8 @@ class TestProjectFlow(TestProjectCommon):
             "When no project partner_id has been set, a subtask should have the same partner as its parent")
 
         self.assertEqual(
-            child_task_2.partner_id, child_task_2.project_id.partner_id,
-            "When a project partner_id has been set, a subtask should have the same partner as its project")
+            child_task_2.partner_id, child_task_2.parent_id.partner_id,
+            "When a project partner_id has been set, a subtask should have the same partner as its parent")
 
         self.assertEqual(
             parent_task.subtask_count, 2,
@@ -182,17 +179,17 @@ class TestProjectFlow(TestProjectCommon):
         })
 
         self.assertEqual(
-            child_task_2.partner_id, child_task_2.project_id.partner_id,
+            child_task_2.partner_id, parent_task.partner_id,
             "When changing the parent task of a subtask with a project, the partner_id should remain the same.")
 
         # set a project with partner_id to a subtask without project partner_id
         child_task_1.write({
-            'project_id': self.project_pigs.id
+            'display_project_id': self.project_pigs.id
         })
 
-        self.assertEqual(
+        self.assertNotEqual(
             child_task_1.partner_id, self.project_pigs.partner_id,
-            "When the project changes, the subtask should have the same partner id as the new project.")
+            "When the project changes, the subtask should keep its partner id as its partner id is set.")
 
         # restore the partner_id of the 'goats' project
         self.project_goats.write({
@@ -201,12 +198,12 @@ class TestProjectFlow(TestProjectCommon):
 
         # set a project with partner_id to a subtask with a project partner_id
         child_task_2.write({
-            'project_id': self.project_goats.id
+            'display_project_id': self.project_goats.id
         })
 
         self.assertEqual(
-            child_task_2.partner_id, self.project_goats.partner_id,
-            "When the project changes, the subtask should have the same partner id as the new project.")
+            child_task_2.partner_id, parent_task.partner_id,
+            "When the project changes, the subtask should keep the same partner id even it has a new project.")
 
     def test_rating(self):
         """Check if rating works correctly even when task is changed from project A to project B"""

addons/project/views/project_views.xml

@@ -720,7 +720,7 @@
                             <field name="description" type="html"/>
                         </page>
                         <page name="sub_tasks_page" string="Sub-tasks" attrs="{'invisible': [('allow_subtasks', '=', False)]}">
-                            <field name="child_ids" context="{'default_project_id': project_id, 'default_user_id': user_id}">
+                            <field name="child_ids" context="{'default_project_id': project_id, 'default_user_id': user_id, 'default_parent_id': id, 'default_partner_id': partner_id}">
                                 <tree editable="bottom">
                                     <field name="project_id" invisible="1"/>
                                     <field name="is_closed" invisible="1"/>

addons/sale_project/models/project.py

@@ -54,18 +54,12 @@ class ProjectTask(models.Model):
     invoice_count = fields.Integer("Number of invoices", related='sale_order_id.invoice_count')
     task_to_invoice = fields.Boolean("To invoice", compute='_compute_task_to_invoice', search='_search_task_to_invoice', groups='sales_team.group_sale_salesman_all_leads')
 
-    @api.depends('project_id.sale_line_id.order_partner_id')
-    def _compute_partner_id(self):
-        for task in self:
-            if not task.partner_id:
-                task.partner_id = task.project_id.sale_line_id.order_partner_id
-        super()._compute_partner_id()
-
     @api.depends('commercial_partner_id', 'sale_line_id.order_partner_id.commercial_partner_id', 'parent_id.sale_line_id', 'project_id.sale_line_id')
     def _compute_sale_line(self):
         for task in self:
             if not task.sale_line_id:
-                task.sale_line_id = task.parent_id.sale_line_id or task.project_id.sale_line_id
+                # if the display_project_id is set then it means the task is classic task or a subtask with another project than its parent.
+                task.sale_line_id = task.display_project_id.sale_line_id or task.parent_id.sale_line_id or task.project_id.sale_line_id
             # check sale_line_id and customer are coherent
             if task.sale_line_id.order_partner_id.commercial_partner_id != task.partner_id.commercial_partner_id:
                 task.sale_line_id = False

addons/sale_project/tests/test_child_tasks.py

@@ -41,6 +41,11 @@ class TestNestedTaskUpdate(TransactionCase):
     def test_creating_subtask_partner_id_on_parent_goes_on_child(self):
         parent = self.env['project.task'].create({'name': 'parent', 'partner_id': self.user.partner_id.id})
         child = self.env['project.task'].create({'name': 'child', 'parent_id': parent.id})
+        child._compute_partner_id()  # the compute will be triggered since the user set the parent_id.
+        self.assertEqual(child.partner_id, self.user.partner_id)
+
+        # Another case, it is the parent as a default value
+        child = self.env['project.task'].with_context(default_parent_id=parent.id).create({'name': 'child'})
         self.assertEqual(child.partner_id, self.user.partner_id)
 
     def test_creating_subtask_email_from_on_parent_goes_on_child(self):
@@ -96,9 +101,9 @@ class TestNestedTaskUpdate(TransactionCase):
         child = self.env['project.task'].create({'name': 'child', 'partner_id': False, 'parent_id': parent.id})
         self.assertFalse(child.partner_id)
         parent.write({'partner_id': self.user.partner_id.id})
-        self.assertEqual(child.partner_id, parent.partner_id)
+        self.assertNotEqual(child.partner_id, parent.partner_id)
         parent.write({'partner_id': False})
-        self.assertEqual(child.partner_id, self.user.partner_id)
+        self.assertNotEqual(child.partner_id, self.user.partner_id)
 
     def test_write_email_from_on_parent_write_on_child(self):
         parent = self.env['project.task'].create({'name': 'parent'})
@@ -121,6 +126,7 @@ class TestNestedTaskUpdate(TransactionCase):
     def test_write_sale_line_id_on_parent_write_on_child_with_partner_if_not_set(self):
         parent = self.env['project.task'].create({'name': 'parent', 'partner_id': self.partner.id})
         child = self.env['project.task'].create({'name': 'child', 'parent_id': parent.id})
+        child._compute_partner_id()
         self.assertFalse(child.sale_line_id)
         parent.write({'sale_line_id': self.order_line.id})
         self.assertEqual(child.sale_line_id, parent.sale_line_id)
@@ -192,6 +198,7 @@ class TestNestedTaskUpdate(TransactionCase):
         parent = self.env['project.task'].create({'name': 'parent', 'user_id': False, 'partner_id': self.partner.id})
         children_values = [{'name': 'child%s' % i, 'user_id': False, 'parent_id': parent.id} for i in range(5)]
         children = self.env['project.task'].create(children_values)
+        children._compute_partner_id()
         # test writing sale_line_id
         for child in children:
             self.assertFalse(child.sale_line_id)

addons/sale_timesheet/models/project.py

@@ -255,6 +255,13 @@ class Project(models.Model):
 class ProjectTask(models.Model):
     _inherit = "project.task"
 
+    def _get_default_partner_id(self, project, parent):
+        res = super()._get_default_partner_id(project, parent)
+        if not res and project:
+            if project.pricing_type == 'employee_rate':
+                return project.sale_line_employee_ids.sale_line_id.order_partner_id[:1]
+        return res
+
     # override sale_order_id and make it computed stored field instead of regular field.
     sale_order_id = fields.Many2one(compute='_compute_sale_order_id', store=True, readonly=False,
     domain="['|', '|', ('partner_id', '=', partner_id), ('partner_id', 'child_of', commercial_partner_id), ('partner_id', 'parent_of', partner_id)]")
@@ -326,14 +333,6 @@ class ProjectTask(models.Model):
         for task in self:
             task.has_multi_sol = task.timesheet_ids and task.timesheet_ids.so_line != task.sale_line_id
 
-    @api.onchange('project_id')
-    def _onchange_project(self):
-        if self.project_id and self.project_id.pricing_type != 'task_rate':
-            if not self.partner_id:
-                self.partner_id = self.project_id.partner_id
-            if not self.sale_line_id:
-                self.sale_line_id = self.project_id.sale_line_id
-
     def _get_last_sol_of_customer(self):
         # Get the last SOL made for the customer in the current task where we need to compute
         self.ensure_one()

addons/sale_timesheet/tests/test_project_billing.py

@@ -78,8 +78,14 @@ class TestProjectBilling(TestCommonSaleTimesheet):
 
         cls.project_project_rate = cls.project_task_rate.copy({
             'name': 'Project with pricing_type="project_rate"',
+            'sale_order_id': cls.sale_order_1.id,
             'sale_line_id': cls.so1_line_order_no_task.id,
         })
+
+        # FIXME: [XBO] since the both projects have a SOL than the pricing_type should not be task_rate !
+        cls.project_task_rate = cls.env['project.project'].search([('sale_line_id', '=', cls.so2_line_deliver_project_task.id)], limit=1)
+        cls.project_task_rate2 = cls.env['project.project'].search([('sale_line_id', '=', cls.so2_line_deliver_project_template.id)], limit=1)
+
         cls.project_employee_rate = cls.project_task_rate.copy({
             'name': 'Project with pricing_type="employee_rate"',
             'partner_id': cls.sale_order_1.partner_id.id,
@@ -297,15 +303,14 @@ class TestProjectBilling(TestCommonSaleTimesheet):
         task.write({
             'project_id': self.project_task_rate.id,
         })
-        task._onchange_project()
 
         self.assertTrue(task.allow_billable, "Task in project 'task rate' should be billed at task rate")
-        self.assertEqual(task.sale_line_id, self.so2_line_deliver_project_task, "The SOL in the task should be the one that we selected")
-        self.assertEqual(task.partner_id, task.project_id.partner_id, "Task created in a project billed on 'employee rate' should have the same customer as the one from the project")
+        self.assertEqual(task.sale_line_id, self.so1_line_deliver_no_task, "The task should keep the same SOL since the partner_id has not changed when the project of the task has changed.")
+        self.assertEqual(task.partner_id, self.partner_a, "Task created in a project billed on 'employee rate' should have the same customer when it has been created.")
 
         # move subtask into task rate project
         subtask.write({
-            'project_id': self.project_task_rate2.id,
+            'display_project_id': self.project_task_rate2.id,
         })
 
         self.assertTrue(subtask.allow_billable, "Subtask should keep the billable type from its parent, even when they are moved into another project")
@@ -352,7 +357,6 @@ class TestProjectBilling(TestCommonSaleTimesheet):
         task = Task.with_context(default_project_id=self.project_task_rate.id).create({
             'name': 'first task',
         })
-        task._onchange_project()
 
         self.assertEqual(task.sale_line_id, self.so2_line_deliver_project_task, "Task created in a project billed on 'task rate' should be linked to a SOL containing a prepaid service product and the remaining hours of this SOL should be greater than 0.")
         self.assertEqual(task.partner_id, task.project_id.partner_id, "Task created in a project billed on 'task rate' should have the same customer as the one from the project")
@@ -369,9 +373,10 @@ class TestProjectBilling(TestCommonSaleTimesheet):
         self.assertEqual(task.sale_line_id, timesheet1.so_line, "The timesheet should be linked to the SOL associated to the task since the pricing type of the project is task rate.")
 
         # create a subtask
-        subtask = Task.with_context(default_project_id=self.project_subtask.id).create({
+        subtask = Task.with_context(default_project_id=self.project_task_rate.id).create({
             'name': 'first subtask task',
             'parent_id': task.id,
+            'display_project_id': self.project_subtask.id,
         })
 
         self.assertEqual(subtask.partner_id, subtask.parent_id.partner_id, "Subtask should have the same customer as the one from their mother")
@@ -379,30 +384,27 @@ class TestProjectBilling(TestCommonSaleTimesheet):
         # log timesheet on subtask
         timesheet2 = Timesheet.create({
             'name': 'Test Line on subtask',
-            'project_id': subtask.project_id.id,
+            'project_id': subtask.display_project_id.id,
             'task_id': subtask.id,
             'unit_amount': 50,
             'employee_id': self.employee_user.id,
         })
-
-        self.assertEqual(subtask.project_id, timesheet2.project_id, "The timesheet is in the subtask project")
+        self.assertEqual(subtask.display_project_id, timesheet2.project_id, "The timesheet is in the subtask project")
         self.assertFalse(timesheet2.so_line, "The timesheet should not be linked to SOL as it's a non billable project")
 
         # move task and subtask into task rate project
         task.write({
             'project_id': self.project_employee_rate.id,
         })
-        task._onchange_project()
         subtask.write({
-            'project_id': self.project_employee_rate.id,
+            'display_project_id': self.project_employee_rate.id,
         })
-        subtask._onchange_project()
 
-        self.assertEqual(task.sale_line_id, task.project_id.sale_line_id, "Task moved in a employee rate billable project have the SOL of the project")
-        self.assertEqual(task.partner_id, task.project_id.partner_id, "Task created in a project billed on 'employee rate' should have the same customer as the one from the project")
+        self.assertEqual(task.sale_line_id, self.project_task_rate.sale_line_id, "Task moved in a employee rate billable project should keep its SOL because the partner_id has not changed too.")
+        self.assertEqual(task.partner_id, self.project_task_rate.partner_id, "Task created in a project billed on 'employee rate' should have the same customer as the one from its initial project.")
 
-        self.assertEqual(subtask.sale_line_id, task.project_id.sale_line_id, "Subask moved in a employee rate billable project have the SOL of the project")
-        self.assertEqual(subtask.partner_id, task.project_id.partner_id, "Subask created in a project billed on 'employee rate' should have the same customer as the one from the project")
+        self.assertEqual(subtask.sale_line_id, subtask.parent_id.sale_line_id, "Subtask moved in a employee rate billable project should have the SOL of its parent since it keep its partner_id and this partner is different than the one in the destination project.")
+        self.assertEqual(subtask.partner_id, subtask.parent_id.partner_id, "Subtask moved in a project billed on 'employee rate' should keep its initial customer, that is the one of its parent.")
 
     def test_customer_change_in_project(self):
         """ Test when the user change the customer in a project

addons/sale_timesheet/tests/test_sale_service.py

@@ -536,7 +536,7 @@ class TestSaleService(TestCommonSaleTimesheet):
         self.assertEqual(task2.partner_id, so_line_deliver_new_task_project.order_partner_id, "A new task in a billable project should have the same SO line as its project")
 
         # moving subtask in another project
-        subtask.write({'project_id': self.project_global.id})
+        subtask.write({'display_project_id': self.project_global.id})
 
         self.assertEqual(subtask.sale_line_id, task.sale_line_id, "A child task should always have the same SO line as its mother, even when changing project")
         self.assertEqual(subtask.sale_line_id, so_line_deliver_new_task_project)