diff --git a/runbot/models/build_error.py b/runbot/models/build_error.py index a7909333..3bec114b 100644 --- a/runbot/models/build_error.py +++ b/runbot/models/build_error.py @@ -298,7 +298,6 @@ class BuildError(models.Model): _logger.debug('Merging errors %s', self) base_error = self[0] base_linked = self[0].parent_id or self[0] - old_error_ids = [] for error in self[1:]: assert base_error.fingerprint == error.fingerprint, f'Errors {base_error.id} and {error.id} have a different fingerprint' if error.test_tags and not base_linked.test_tags: @@ -311,8 +310,12 @@ class BuildError(models.Model): error.message_post(body=f'⚠ trying to merge errors with different test-tags from {base_error._get_form_link()} tag: "{base_error.test_tags}"') continue - base_error.build_ids += error.build_ids - error.build_ids = False + for build_error_link in error.build_error_link_ids: + if build_error_link.build_id not in base_error.build_error_link_ids.build_id: + build_error_link.build_error_id = base_error + else: + # as the relation already exists and was not transferred we can remove the old one + build_error_link.unlink() if error.responsible and not base_linked.responsible: base_error.responsible = error.responsible @@ -326,7 +329,6 @@ class BuildError(models.Model): error.message_post(body=f'Error was merged into {base_linked._get_form_link()}') error.child_ids.parent_id = base_error error.active = False - old_error_ids.append(error.id) #################### # Actions @@ -357,7 +359,7 @@ class BuildError(models.Model): errors_by_fingerprint = self.env['runbot.build.error'].search([('fingerprint', 'in', list(changed_fingerprints))]) for fingerprint in changed_fingerprints: errors_to_merge = errors_by_fingerprint.filtered(lambda r: r.fingerprint == fingerprint) - rec_id = errors_to_merge._merge() + errors_to_merge._merge() def action_assign(self): if not any((not record.responsible and not record.team_id and record.file_path and not record.parent_id) for record in self): diff --git a/runbot/security/ir.model.access.csv b/runbot/security/ir.model.access.csv index d47dd647..c79b6e97 100644 --- a/runbot/security/ir.model.access.csv +++ b/runbot/security/ir.model.access.csv @@ -23,7 +23,7 @@ access_runbot_build_error_user,runbot_build_error_user,runbot.model_runbot_build access_runbot_build_error_admin,runbot_build_error_admin,runbot.model_runbot_build_error,runbot.group_runbot_admin,1,1,1,1 access_runbot_build_error_manager,runbot_build_error_manager,runbot.model_runbot_build_error,runbot.group_runbot_error_manager,1,1,1,1 access_runbot_build_error_link_user,runbot_runbot_build_error_link_user,runbot.model_runbot_build_error_link,group_user,1,0,0,0 -access_runbot_build_error_link_admin,runbot_runbot_build_error_link_admin,runbot.model_runbot_build_error_link,runbot.group_runbot_admin,1,1,1,0 +access_runbot_build_error_link_admin,runbot_runbot_build_error_link_admin,runbot.model_runbot_build_error_link,runbot.group_runbot_admin,1,1,1,1 access_runbot_build_error_link_manager,runbot_runbot_build_error_link_manager,runbot.model_runbot_build_error_link,runbot.group_runbot_error_manager,1,1,1,0 access_runbot_build_error_tag_user,runbot_build_error_tag_user,runbot.model_runbot_build_error_tag,group_user,1,0,0,0 access_runbot_build_error_tag_admin,runbot_build_error_tag_admin,runbot.model_runbot_build_error_tag,runbot.group_runbot_admin,1,1,1,1 diff --git a/runbot/tests/test_build_error.py b/runbot/tests/test_build_error.py index 009462fa..a58bbc48 100644 --- a/runbot/tests/test_build_error.py +++ b/runbot/tests/test_build_error.py @@ -75,8 +75,11 @@ class TestBuildError(RunbotCase): self.assertEqual(len(self.BuildError.search([('fingerprint', '=', error_a.fingerprint)])), 1) self.assertTrue(error_a.active, 'The first merged error should stay active') self.assertFalse(error_b.active, 'The second merged error should have stay deactivated') + self.assertIn(build_a, error_a.build_error_link_ids.build_id) self.assertIn(build_a, error_a.build_ids) + self.assertIn(build_b, error_a.build_error_link_ids.build_id) self.assertIn(build_b, error_a.build_ids) + self.assertFalse(error_b.build_error_link_ids) self.assertFalse(error_b.build_ids) error_c = self.BuildError.create({'content': 'foo foo'}) @@ -85,6 +88,15 @@ class TestBuildError(RunbotCase): with self.assertRaises(AssertionError): (error_a | error_c)._merge() + # merge two build errors while the build <--> build_error relation already exists + error_d = self.BuildError.create({'content': 'foo bar'}) + self.BuildErrorLink.create({'build_id': build_a.id, 'build_error_id': error_d.id}) + (error_a | error_d)._merge() + self.assertIn(build_a, error_a.build_error_link_ids.build_id) + self.assertIn(build_a, error_a.build_ids) + self.assertFalse(error_d.build_error_link_ids) + self.assertFalse(error_d.build_ids) + def test_merge_linked(self): top_error = self.BuildError.create({'content': 'foo foo', 'active': False})