[FIX] runbot: fix build_error active field changes

When a build_error active field is changed, the onchange leads to a
traceback. Anyway, the onchange was not a good idea as it only reflects
UI changes.

With this commit, the write method is overwritten to change the
child_ids active fields too. Also, the active_test context is used to
correctly compute the childs_ids and children_build_ids.

A test is also added for all that.

runbot/models/build_error.py

@@ -31,7 +31,7 @@ class RunbotBuildError(models.Model):
     tag_ids = fields.Many2many('runbot.build.error.tag', string='Tags')
     build_count = fields.Integer(compute='_compute_build_counts', string='Nb seen', stored=True)
     parent_id = fields.Many2one('runbot.build.error', 'Linked to')
-    child_ids = fields.One2many('runbot.build.error', 'parent_id', string='Child Errors')
+    child_ids = fields.One2many('runbot.build.error', 'parent_id', string='Child Errors', context={'active_test': False})
     children_build_ids = fields.Many2many('runbot.build', compute='_compute_children_build_ids', string='Children builds')
     error_history_ids = fields.One2many('runbot.build.error', compute='_compute_error_history_ids', string='Old errors')
     first_seen_build_id = fields.Many2one('runbot.build', compute='_compute_first_seen_build_id', string='First Seen build')
@@ -49,6 +49,13 @@ class RunbotBuildError(models.Model):
         })
         return super().create(vals)
 
+    @api.multi
+    def write(self, vals):
+        if 'active' in vals:
+            for build_error in self:
+                (build_error.child_ids - self).write({'active': vals['active']})
+        return super(RunbotBuildError, self).write(vals)
+
     @api.depends('build_ids')
     def _compute_build_counts(self):
         for build_error in self:
@@ -91,10 +98,6 @@ class RunbotBuildError(models.Model):
             fingerprints = [error.fingerprint] + [rec.fingerprint for rec in error.child_ids]
             error.error_history_ids = self.search([('fingerprint', 'in', fingerprints), ('active', '=', False), ('id', '!=', error.id)])
 
-    @api.onchange('active')
-    def _onchange_active(self):
-        self.child_ids.write({'active': self.active})
-
     @api.model
     def _digest(self, s):
         """
@@ -138,6 +141,7 @@ class RunbotBuildError(models.Model):
         """
         if len(self) < 2:
             return
+        self = self.with_context(active_test=False)
         build_errors = self.search([('id', 'in', self.ids)], order='responsible asc, random desc, id asc')
         build_errors[1:].write({'parent_id': build_errors[0].id})
 

runbot/tests/test_build_error.py

@@ -91,3 +91,42 @@ class TestBuildError(common.TransactionCase):
         new_build_error = self.BuildError.search([('build_ids','in', [ko_build_new.id])])
         self.assertIn(ko_build_new, new_build_error.build_ids, 'The parsed build with a re-apearing error should generate a new runbot.build.error')
         self.assertIn(build_error, new_build_error.error_history_ids, 'The old error should appear in history')
+
+    @patch('odoo.addons.runbot.models.build.runbot_build._get_params')
+    def test_build_error_links(self, mock_get_params):
+        build_a = self.create_build({'local_result': 'ko'})
+        build_b = self.create_build({'local_result': 'ko'})
+
+        error_a = self.env['runbot.build.error'].create({
+            'content': 'foo',
+            'build_ids': [(6, 0, [build_a.id])],
+            'active': False  # Even a fixed error coul be linked
+        })
+
+        error_b = self.env['runbot.build.error'].create({
+            'content': 'bar',
+            'build_ids': [(6, 0, [build_b.id])],
+            'random': True
+        })
+
+        #  test that the random bug is parent when linking errors
+        all_errors = error_a | error_b
+        all_errors.link_errors()
+
+        self.assertIn(error_b.child_ids, error_a, 'Random error should be the parent')
+
+        #  Test that changing bug resolution is propagated to children
+        error_b.active = True
+        self.assertTrue(error_a.active)
+        error_b.active = False
+        self.assertFalse(error_a.active)
+
+        #  Test build_ids
+        self.assertIn(build_b, error_b.build_ids)
+        self.assertNotIn(build_a, error_b.build_ids)
+
+        #  Test that children builds contains all builds
+        self.assertIn(build_b, error_b.children_build_ids)
+        self.assertIn(build_a, error_b.children_build_ids)
+        self.assertEqual(error_a.build_count, 1)
+        self.assertEqual(error_b.build_count, 2)