[IMP] base : improve messages text in access_error pop-ups

* Improve layout of access error messages from group restriction
 * Improve layout of access error messages from security records rules
 * Use the error title in the js crash manager when possible
 * adapt unit tests

closes odoo/odoo#50369

Taskid: 2206787
Signed-off-by: Yannick Tivisse (yti) <yti@odoo.com>

addons/account/tests/test_portal_attachment.py

@@ -140,7 +140,7 @@ class TestUi(tests.HttpCase):
         post_data['attachment_tokens'] = attachment.access_token
         res = self.url_open(url=post_url, data=post_data)
         self.assertEqual(res.status_code, 403)
-        self.assertIn("Sorry, you are not allowed to access documents of type 'Journal Entry' (account.move).", res.text)
+        self.assertIn("You are not allowed to access 'Journal Entry' (account.move) records.", res.text)
 
         # Test attachment can't be associated if not "pending" state
         post_data['token'] = invoice._portal_ensure_token()

addons/web/static/src/js/services/crash_manager.js

@@ -222,7 +222,12 @@ var CrashManager = AbstractService.extend({
             return;
         }
         var message = error.data ? error.data.message : error.message;
-        var title = _.str.capitalize(error.type) || _t("Something went wrong !");
+        var title = _t("Something went wrong !");
+        if (error.type) {
+            title = _.str.capitalize(error.type);
+        } else if (error.data && error.data.title) {
+            title = _.str.capitalize(error.data.title);
+        }
         return this._displayWarning(message, title, options);
     },
     show_error: function (error) {

odoo/addons/base/models/ir_model.py

@@ -1718,26 +1718,34 @@ class IrModelAccess(models.Model):
 
         if not r and raise_exception:
             groups = '\n'.join('\t- %s' % g for g in self.group_names_with_access(model, mode))
+            document_kind = self.env['ir.model']._get(model).name or model
             msg_heads = {
                 # Messages are declared in extenso so they are properly exported in translation terms
-                'read': _("Sorry, you are not allowed to access documents of type '%(document_kind)s' (%(document_model)s)."),
-                'write':  _("Sorry, you are not allowed to modify documents of type '%(document_kind)s' (%(document_model)s)."),
-                'create': _("Sorry, you are not allowed to create documents of type '%(document_kind)s' (%(document_model)s)."),
-                'unlink': _("Sorry, you are not allowed to delete documents of type '%(document_kind)s' (%(document_model)s)."),
-            }
-            msg_params = {
-                'document_kind': self.env['ir.model']._get(model).name or model,
-                'document_model': model,
+                'read': _("You are not allowed to access '%(document_kind)s' (%(document_model)s) records.", document_kind=document_kind, document_model=model),
+                'write':  _("You are not allowed to modify '%(document_kind)s' (%(document_model)s) records.", document_kind=document_kind, document_model=model),
+                'create': _("You are not allowed to create '%(document_kind)s' (%(document_model)s) records.", document_kind=document_kind, document_model=model),
+                'unlink': _("You are not allowed to delete '%(document_kind)s' (%(document_model)s) records.", document_kind=document_kind, document_model=model),
             }
+            operation_error = msg_heads[mode]
+
             if groups:
-                msg_tail = _("This operation is allowed for the groups:\n%(groups_list)s")
-                msg_params['groups_list'] = groups
+                group_info = _("This operation is allowed for the following groups:\n%(groups_list)s", groups_list=groups)
             else:
-                msg_tail = _("No group currently allows this operation.")
-            msg_tail += u' - ({} {}, {} {})'.format(_('Operation:'), mode, _('User:'), self._uid)
+                group_info = _("No group currently allows this operation.")
+
+            resolution_info = _("Contact your administrator to request access if necessary.")
+
             _logger.info('Access Denied by ACLs for operation: %s, uid: %s, model: %s', mode, self._uid, model)
-            msg = '%s %s' % (msg_heads[mode], msg_tail)
-            raise AccessError(msg % msg_params)
+            msg = """{operation_error}
+
+{group_info}
+
+{resolution_info}""".format(
+                operation_error=operation_error,
+                group_info=group_info,
+                resolution_info=resolution_info)
+
+            raise AccessError(msg)
 
         return bool(r)
 

odoo/addons/base/models/ir_rule.py

@@ -220,36 +220,61 @@ class IrRule(models.Model):
 
         model = records._name
         description = self.env['ir.model']._get(model).name or model
-        if not self.env.user.has_group('base.group_no_one'):
-            return AccessError(_('The requested operation cannot be completed due to security restrictions. Please contact your system administrator.\n\n(Document type: "%(document_kind)s" (%(document_model)s), Operation: %(operation)s)') % {
-                'document_kind': description,
-                'document_model': model,
-                'operation': operation,
-            })
+        msg_heads = {
+            # Messages are declared in extenso so they are properly exported in translation terms
+            'read':   _("Due to security restrictions, you are not allowed to access '%(document_kind)s' (%(document_model)s) records.", document_kind=description, document_model=model),
+            'write':  _("Due to security restrictions, you are not allowed to modify '%(document_kind)s' (%(document_model)s) records.", document_kind=description, document_model=model),
+            'create': _("Due to security restrictions, you are not allowed to create '%(document_kind)s' (%(document_model)s) records.", document_kind=description, document_model=model),
+            'unlink': _("Due to security restrictions, you are not allowed to delete '%(document_kind)s' (%(document_model)s) records.", document_kind=description, document_model=model)
+        }
+        operation_error = msg_heads[operation]
+        resolution_info = _("Contact your administrator to request access if necessary.")
+
+        if not self.env.user.has_group('base.group_no_one') or not self.env.user.has_group('base.group_user'):
+            msg = """{operation_error}
+
+{resolution_info}""".format(
+                operation_error=operation_error,
+                resolution_info=resolution_info)
+            return AccessError(msg)
 
         # This extended AccessError is only displayed in debug mode.
         # Note that by default, public and portal users do not have
         # the group "base.group_no_one", even if debug mode is enabled,
-        # so it is relatively safe here to include the list of rules and
-        # record names.
+        # so it is relatively safe here to include the list of rules and record names.
         rules = self._get_failing(records, mode=operation).sudo()
-        error = AccessError(_("""The requested operation ("%(operation)s" on "%(document_kind)s" (%(document_model)s)) was rejected because of the following rules:
-%(rules_list)s
-%(multi_company_warning)s
-(Records: %(example_records)s, User: %(user_id)s)""") % {
-            'operation': operation,
-            'document_kind': description,
-            'document_model': model,
-            'rules_list': '\n'.join('- %s' % rule.name for rule in rules),
-            'multi_company_warning': ('\n' + _('Note: this might be a multi-company issue.') + '\n') if any(
-                'company_id' in (r.domain_force or []) for r in rules) else '',
-            'example_records': ' - '.join(['%s (id=%s)' % (rec.display_name, rec.id) for rec in records[:6].sudo()]),
-            'user_id': '%s (id=%s)' % (self.env.user.name, self.env.user.id),
-        })
+
+        records_description = ', '.join(['%s (id=%s)' % (rec.display_name, rec.id) for rec in records[:6].sudo()])
+        failing_records = _("Records: %s", records_description)
+
+        user_description = '%s (id=%s)' % (self.env.user.name, self.env.user.id)
+        failing_user = _("User: %s", user_description)
+
+        rules_description = '\n'.join('- %s' % rule.name for rule in rules)
+        failing_rules = _("This restriction is due to the following rules:\n%s", rules_description)
+        if any('company_id' in (r.domain_force or []) for r in rules):
+            failing_rules += "\n\n" + _('Note: this might be a multi-company issue.')
+
+        msg = """{operation_error}
+
+{failing_records}
+{failing_user}
+
+{failing_rules}
+
+{resolution_info}""".format(
+                operation_error=operation_error,
+                failing_records=failing_records,
+                failing_user=failing_user,
+                failing_rules=failing_rules,
+                resolution_info=resolution_info)
+
         # clean up the cache of records prefetched with display_name above
         for record in records[:6]:
             record._cache.clear()
-        return error
+
+        return AccessError(msg)
+
 
 #
 # Hack for field 'global': this field cannot be defined like others, because

odoo/addons/test_access_rights/tests/test_feedback.py

@@ -117,7 +117,11 @@ class TestACLFeedback(Feedback):
             self.record.with_user(self.user).write({'val': 10})
         self.assertEqual(
             ctx.exception.args[0],
-            """Sorry, you are not allowed to modify documents of type 'Object For Test Access Right' (test_access_right.some_obj). No group currently allows this operation. - (Operation: write, User: %d)""" % self.user.id
+            """You are not allowed to modify 'Object For Test Access Right' (test_access_right.some_obj) records.
+
+No group currently allows this operation.
+
+Contact your administrator to request access if necessary."""
         )
 
     def test_one_group(self):
@@ -127,12 +131,20 @@ class TestACLFeedback(Feedback):
             })
         self.assertEqual(
             ctx.exception.args[0],
-            """Sorry, you are not allowed to create documents of type 'Object For Test Access Right' (test_access_right.some_obj). This operation is allowed for the groups:\n\t- Group 0 - (Operation: create, User: %d)""" % self.user.id
+            """You are not allowed to create 'Object For Test Access Right' (test_access_right.some_obj) records.
+
+This operation is allowed for the following groups:\n\t- Group 0
+
+Contact your administrator to request access if necessary."""
         )
 
     def test_two_groups(self):
         r = self.record.with_user(self.user)
-        expected = """Sorry, you are not allowed to access documents of type 'Object For Test Access Right' (test_access_right.some_obj). This operation is allowed for the groups:\n\t- Group 0\n\t- Group 1 - (Operation: read, User: %d)""" % self.user.id
+        expected = """You are not allowed to access 'Object For Test Access Right' (test_access_right.some_obj) records.
+
+This operation is allowed for the following groups:\n\t- Group 0\n\t- Group 1
+
+Contact your administrator to request access if necessary."""
         with self.assertRaises(AccessError) as ctx:
             # noinspection PyStatementEffect
             r.val
@@ -171,19 +183,26 @@ class TestIRRuleFeedback(Feedback):
             self.record.write({'val': 1})
         self.assertEqual(
             ctx.exception.args[0],
-            'The requested operation cannot be completed due to security restrictions. Please contact your system administrator.\n\n(Document type: "Object For Test Access Right" (test_access_right.some_obj), Operation: write)'
-        )
+            """Due to security restrictions, you are not allowed to modify 'Object For Test Access Right' (test_access_right.some_obj) records.
+
+Contact your administrator to request access if necessary.""")
 
         # debug mode
         self.env.ref('base.group_no_one').write({'users': [(4, self.user.id)]})
+        self.env.ref('base.group_user').write({'users': [(4, self.user.id)]})
         with self.assertRaises(AccessError) as ctx:
             self.record.write({'val': 1})
         self.assertEqual(
             ctx.exception.args[0],
-            """The requested operation ("write" on "Object For Test Access Right" (test_access_right.some_obj)) was rejected because of the following rules:
+            """Due to security restrictions, you are not allowed to modify 'Object For Test Access Right' (test_access_right.some_obj) records.
+
+Records: %s (id=%s)
+User: %s (id=%s)
+
+This restriction is due to the following rules:
 - rule 0
 
-(Records: %s (id=%s), User: %s (id=%s))""" % (self.record.display_name, self.record.id, self.user.name, self.user.id)
+Contact your administrator to request access if necessary.""" % (self.record.display_name, self.record.id, self.user.name, self.user.id)
         )
 
 
@@ -196,58 +215,73 @@ class TestIRRuleFeedback(Feedback):
             p.with_user(self.user).write({'val': 1})
 
     def test_locals(self):
-        self.env.ref('base.group_no_one').write(
-            {'users': [(4, self.user.id)]})
+        self.env.ref('base.group_no_one').write({'users': [(4, self.user.id)]})
+        self.env.ref('base.group_user').write({'users': [(4, self.user.id)]})
         self._make_rule('rule 0', '[("val", "=", 42)]')
         self._make_rule('rule 1', '[("val", "=", 78)]')
         with self.assertRaises(AccessError) as ctx:
             self.record.write({'val': 1})
         self.assertEqual(
             ctx.exception.args[0],
-            """The requested operation ("write" on "Object For Test Access Right" (test_access_right.some_obj)) was rejected because of the following rules:
+            """Due to security restrictions, you are not allowed to modify 'Object For Test Access Right' (test_access_right.some_obj) records.
+
+Records: %s (id=%s)
+User: %s (id=%s)
+
+This restriction is due to the following rules:
 - rule 0
 - rule 1
 
-(Records: %s (id=%s), User: %s (id=%s))""" % (self.record.display_name, self.record.id, self.user.name, self.user.id)
+Contact your administrator to request access if necessary.""" % (self.record.display_name, self.record.id, self.user.name, self.user.id)
         )
 
     def test_globals_all(self):
-        self.env.ref('base.group_no_one').write(
-            {'users': [(4, self.user.id)]})
+        self.env.ref('base.group_no_one').write({'users': [(4, self.user.id)]})
+        self.env.ref('base.group_user').write({'users': [(4, self.user.id)]})
         self._make_rule('rule 0', '[("val", "=", 42)]', global_=True)
         self._make_rule('rule 1', '[("val", "=", 78)]', global_=True)
         with self.assertRaises(AccessError) as ctx:
             self.record.write({'val': 1})
         self.assertEqual(
             ctx.exception.args[0],
-            """The requested operation ("write" on "Object For Test Access Right" (test_access_right.some_obj)) was rejected because of the following rules:
+            """Due to security restrictions, you are not allowed to modify 'Object For Test Access Right' (test_access_right.some_obj) records.
+
+Records: %s (id=%s)
+User: %s (id=%s)
+
+This restriction is due to the following rules:
 - rule 0
 - rule 1
 
-(Records: %s (id=%s), User: %s (id=%s))""" % (self.record.display_name, self.record.id, self.user.name, self.user.id)
+Contact your administrator to request access if necessary.""" % (self.record.display_name, self.record.id, self.user.name, self.user.id)
         )
 
     def test_globals_any(self):
         """ Global rules are AND-eded together, so when an access fails it
         might be just one of the rules, and we want an exact listing
         """
-        self.env.ref('base.group_no_one').write(
-            {'users': [(4, self.user.id)]})
+        self.env.ref('base.group_no_one').write({'users': [(4, self.user.id)]})
+        self.env.ref('base.group_user').write({'users': [(4, self.user.id)]})
         self._make_rule('rule 0', '[("val", "=", 42)]', global_=True)
         self._make_rule('rule 1', '[(1, "=", 1)]', global_=True)
         with self.assertRaises(AccessError) as ctx:
             self.record.write({'val': 1})
         self.assertEqual(
             ctx.exception.args[0],
-            """The requested operation ("write" on "Object For Test Access Right" (test_access_right.some_obj)) was rejected because of the following rules:
+            """Due to security restrictions, you are not allowed to modify 'Object For Test Access Right' (test_access_right.some_obj) records.
+
+Records: %s (id=%s)
+User: %s (id=%s)
+
+This restriction is due to the following rules:
 - rule 0
 
-(Records: %s (id=%s), User: %s (id=%s))""" % (self.record.display_name, self.record.id, self.user.name, self.user.id)
+Contact your administrator to request access if necessary.""" % (self.record.display_name, self.record.id, self.user.name, self.user.id)
         )
 
     def test_combination(self):
-        self.env.ref('base.group_no_one').write(
-            {'users': [(4, self.user.id)]})
+        self.env.ref('base.group_no_one').write({'users': [(4, self.user.id)]})
+        self.env.ref('base.group_user').write({'users': [(4, self.user.id)]})
         self._make_rule('rule 0', '[("val", "=", 42)]', global_=True)
         self._make_rule('rule 1', '[(1, "=", 1)]', global_=True)
         self._make_rule('rule 2', '[(0, "=", 1)]')
@@ -256,53 +290,68 @@ class TestIRRuleFeedback(Feedback):
             self.record.write({'val': 1})
         self.assertEqual(
             ctx.exception.args[0],
-            """The requested operation ("write" on "Object For Test Access Right" (test_access_right.some_obj)) was rejected because of the following rules:
+            """Due to security restrictions, you are not allowed to modify 'Object For Test Access Right' (test_access_right.some_obj) records.
+
+Records: %s (id=%s)
+User: %s (id=%s)
+
+This restriction is due to the following rules:
 - rule 0
 - rule 2
 - rule 3
 
-(Records: %s (id=%s), User: %s (id=%s))""" % (self.record.display_name, self.record.id, self.user.name, self.user.id)
+Contact your administrator to request access if necessary.""" % (self.record.display_name, self.record.id, self.user.name, self.user.id)
         )
 
     def test_warn_company(self):
         """ If one of the failing rules mentions company_id, add a note that
         this might be a multi-company issue.
         """
-        self.env.ref('base.group_no_one').write(
-            {'users': [(4, self.user.id)]})
+        self.env.ref('base.group_no_one').write({'users': [(4, self.user.id)]})
+        self.env.ref('base.group_user').write({'users': [(4, self.user.id)]})
         self._make_rule('rule 0', "[('company_id', '=', user.company_id.id)]")
         self._make_rule('rule 1', '[("val", "=", 0)]', global_=True)
         with self.assertRaises(AccessError) as ctx:
             self.record.write({'val': 1})
         self.assertEqual(
             ctx.exception.args[0],
-            """The requested operation ("write" on "Object For Test Access Right" (test_access_right.some_obj)) was rejected because of the following rules:
+            """Due to security restrictions, you are not allowed to modify 'Object For Test Access Right' (test_access_right.some_obj) records.
+
+Records: %s (id=%s)
+User: %s (id=%s)
+
+This restriction is due to the following rules:
 - rule 0
 
 Note: this might be a multi-company issue.
 
-(Records: %s (id=%s), User: %s (id=%s))""" % (self.record.display_name, self.record.id, self.user.name, self.user.id)
+Contact your administrator to request access if necessary.""" % (self.record.display_name, self.record.id, self.user.name, self.user.id)
         )
 
     def test_read(self):
         """ because of prefetching, read() goes through a different codepath
         to apply rules
         """
-        self.env.ref('base.group_no_one').write(
-            {'users': [(4, self.user.id)]})
+        self.env.ref('base.group_no_one').write({'users': [(4, self.user.id)]})
+        self.env.ref('base.group_user').write({'users': [(4, self.user.id)]})
         self._make_rule('rule 0', "[('company_id', '=', user.company_id.id)]", attr='read')
         self._make_rule('rule 1', '[("val", "=", 1)]', global_=True, attr='read')
         with self.assertRaises(AccessError) as ctx:
             _ = self.record.val
         self.assertEqual(
             ctx.exception.args[0],
-            """The requested operation ("read" on "Object For Test Access Right" (test_access_right.some_obj)) was rejected because of the following rules:
+            """Due to security restrictions, you are not allowed to access 'Object For Test Access Right' (test_access_right.some_obj) records.
+
+Records: %s (id=%s)
+User: %s (id=%s)
+
+This restriction is due to the following rules:
 - rule 0
 - rule 1
 
 Note: this might be a multi-company issue.
 
-(Records: %s (id=%s), User: %s (id=%s))""" % (self.record.display_name, self.record.id, self.user.name, self.user.id)
+Contact your administrator to request access if necessary.""" % (self.record.display_name, self.record.id, self.user.name, self.user.id)
         )
 
         p = self.env['test_access_right.parent'].create({'obj_id': self.record.id})