[IMP] runbot: stop notifying when an error is fixed

When an error is assigned to a user, a notification is sent to the user,
even if the error is not active.

So it happens when we want to mark an error as fixed and add the fixing
informations at the same time (e.g.: the fixing PR, the user that fixed
...).

Author: d-fence

runbot/models/build_error.py

@@ -507,10 +507,10 @@ def write(self, vals):
                     if not vals['active'] and build_error.active and build_error.last_seen_date and build_error.last_seen_date + relativedelta(days=1) > datetime.datetime.now():
                         raise UserError("This error broke less than one day ago can only be deactivated by admin")
 
-        if (responsible_id := vals.get('responsible')):
+        if (responsible_id := vals.get('responsible')) and vals.get('active', True):
             responsible = self.env['res.users'].browse(responsible_id)
             for build_error in self:
-                if responsible != self.env.user:
+                if build_error.active and responsible != self.env.user:
                     _logger.info('Notifying responsible %s of build error %s', responsible.name, build_error.id)
                     build_error.message_notify(
                         body=f'Error {build_error.id} was assigned to you by {self.env.user.name}',

runbot/tests/test_build_error.py

@@ -1,7 +1,10 @@
 import hashlib
+from unittest.mock import patch
 
 from odoo import fields
 from odoo.exceptions import ValidationError
+from odoo.tests import new_test_user
+
 from .common import RunbotCase
 
 RTE_ERROR = """FAIL: TestUiTranslate.test_admin_tour_rte_translator
@@ -684,6 +687,31 @@ def test_dashboard_tile_simple(self):
 
         self.assertEqual(dashboard.build_ids, failed_build)
 
+    def test_build_error_notification(self):
+        self.stop_patcher('isfile')  # prevent user creation
+        responsible = new_test_user(self.env, login='fixman', name='fixman', password='fixpass')
+
+        # check that a message is sent when an error is assigned to someone
+        error = self.BuildError.create({})
+        with patch.object(self.env.registry['runbot.build.error'], 'message_notify') as message_notify:
+            error.responsible = responsible
+            message_notify.assert_called()
+
+        # check that a message is NOT sent when an error disabled AND assigned to someone
+        fixed_error = self.BuildError.create({})
+        with patch.object(self.env.registry['runbot.build.error'], 'message_notify') as message_notify:
+            fixed_error.write({
+                'responsible': responsible.id,
+                'active': False,
+            })
+            message_notify.assert_not_called()
+
+        # Finally check that a message is not sent when adding a responsible on an already fixed error
+        innactive_error = self.BuildError.create({'active': False})
+        with patch.object(self.env.registry['runbot.build.error'], 'message_notify') as message_notify:
+            innactive_error.responsible = responsible
+            message_notify.assert_not_called()
+
 
 class TestErrorMerge(TestBuildErrorCommon):