From 162f3059d5829050a8bee9a167d8a850c925cfd8 Mon Sep 17 00:00:00 2001 From: Damien Abeloos Date: Wed, 9 Dec 2020 16:51:33 +0000 Subject: [PATCH] [IMP] base: modify some displayed texts for res_partner * Add a new error message upon deleting the partner linked to an active user. * Improve the error message wording upon archiving the partner linked to an active user. Add the linked users names at the end of the message. * If the user has 'write' access on res.users, raise a RedirectWarning instead of a ValidationError, to offer an easy access to the related users : - If there are multiple Users, redirect to a list view. - If there is only one User, redirect to the form view. * Adapt the test_access_deleted_records from test_orm.py to use a more basic model : `res.partner.category` (with less business logic), because the new unlink error message is dependent on a condition that will throw an error in case of multiple unlinks of the same record (MissingError). * Force close the frontend archive confirmation Dialog since _toggleArchiveState may be interrupted by another Warning/Error Dialog. If the new Dialog was the redirectWarning, and the user chose to be redirected, the first Dialog would stay open even thought the context already changed. * Modify the placeholders for a partner Name/Company -> placeholders will change according to an Individual or a Company. * Adapt the main_flow tour to correctly select the partner name field on mobile (tested in enterprise). Task ID : 2410217 PR : https://github.com/odoo/odoo/pull/63370 --- .../src/js/views/list/list_controller.js | 7 ++-- odoo/addons/base/models/res_partner.py | 33 ++++++++++++++++--- odoo/addons/base/models/res_users.py | 23 +++++++++++++ odoo/addons/base/tests/test_base.py | 6 ++-- odoo/addons/base/tests/test_orm.py | 14 ++++---- odoo/addons/base/views/res_partner_views.xml | 10 +++--- .../static/tests/tours/main_flow.js | 4 +-- 7 files changed, 76 insertions(+), 21 deletions(-) diff --git a/addons/web/static/src/js/views/list/list_controller.js b/addons/web/static/src/js/views/list/list_controller.js index c9e97ad15be..9313e6b6332 100644 --- a/addons/web/static/src/js/views/list/list_controller.js +++ b/addons/web/static/src/js/views/list/list_controller.js @@ -438,8 +438,11 @@ var ListController = BasicController.extend({ otherActionItems.push({ description: _t("Archive"), callback: () => { - Dialog.confirm(this, _t("Are you sure that you want to archive all the selected records?"), { - confirm_callback: () => this._toggleArchiveState(true), + const dialog = Dialog.confirm(this, _t("Are you sure that you want to archive all the selected records?"), { + confirm_callback: () => { + this._toggleArchiveState(true); + dialog.close(); + }, }); } }, { diff --git a/odoo/addons/base/models/res_partner.py b/odoo/addons/base/models/res_partner.py index b6d5c03ee04..25908ff5c2b 100644 --- a/odoo/addons/base/models/res_partner.py +++ b/odoo/addons/base/models/res_partner.py @@ -17,7 +17,7 @@ from werkzeug import urls from odoo import api, fields, models, tools, SUPERUSER_ID, _, Command from odoo.modules import get_module_resource from odoo.osv.expression import get_unaccent_wrapper -from odoo.exceptions import UserError, ValidationError +from odoo.exceptions import RedirectWarning, UserError, ValidationError # Global variables used for the warning fields declared on the res.partner # in the following modules : sale, purchase, account, stock @@ -515,9 +515,18 @@ class Partner(models.Model): # This is wrong if the user is not active, as partner.user_ids only returns active users. # Hence this temporary hack until the ORM updates inverse fields correctly. self.invalidate_cache(['user_ids'], self._ids) - for partner in self: - if partner.active and partner.user_ids: - raise ValidationError(_('You cannot archive a contact linked to an internal user.')) + users = self.env['res.users'].sudo().search([('partner_id', 'in', self.ids)]) + if users: + if self.env['res.users'].sudo(False).check_access_rights('write', raise_exception=False): + error_msg = _('You cannot archive contacts linked to an active user.\n' + 'You first need to archive their associated user.\n\n' + 'Linked active users : %(names)s', names=", ".join([u.display_name for u in users])) + action_error = users._action_show() + raise RedirectWarning(error_msg, action_error, _('Go to users')) + else: + raise ValidationError(_('You cannot archive contacts linked to an active user.\n' + 'Ask an administrator to archive their associated user first.\n\n' + 'Linked active users :\n%(names)s', names=", ".join([u.display_name for u in users]))) # res.partner must only allow to set the company_id of a partner if it # is the same as the company of all users that inherit from this partner # (this is to allow the code from res_users to write to the partner!) or @@ -569,6 +578,22 @@ class Partner(models.Model): partner._handle_first_contact_creation() return partners + @api.ondelete(at_uninstall=False) + def _unlink_except_user(self): + users = self.env['res.users'].sudo().search([('partner_id', 'in', self.ids)]) + if not users: + return # no linked user, operation is allowed + if self.env['res.users'].sudo(False).check_access_rights('write', raise_exception=False): + error_msg = _('You cannot delete contacts linked to an active user.\n' + 'You should rather archive them after archiving their associated user.\n\n' + 'Linked active users : %(names)s', names=", ".join([u.display_name for u in users])) + action_error = users._action_show() + raise RedirectWarning(error_msg, action_error, _('Go to users')) + else: + raise ValidationError(_('You cannot delete contacts linked to an active user.\n' + 'Ask an administrator to archive their associated user first.\n\n' + 'Linked active users :\n%(names)s', names=", ".join([u.display_name for u in users]))) + def _load_records_create(self, vals_list): partners = super(Partner, self.with_context(_partners_skip_fields_sync=True))._load_records_create(vals_list) diff --git a/odoo/addons/base/models/res_users.py b/odoo/addons/base/models/res_users.py index fff28a5a618..d53f7d94d1d 100644 --- a/odoo/addons/base/models/res_users.py +++ b/odoo/addons/base/models/res_users.py @@ -835,6 +835,29 @@ class Users(models.Model): # for a few places explicitly clearing the has_group cache has_group.clear_cache = _has_group.clear_cache + def _action_show(self): + """If self is a singleton, directly access the form view. If it is a recordset, open a tree view""" + view_id = self.env.ref('base.view_users_form').id + action = { + 'type': 'ir.actions.act_window', + 'res_model': 'res.users', + 'context': {'create': False}, + } + if len(self) > 1: + action.update({ + 'name': _('Users'), + 'view_mode': 'list,form', + 'views': [[None, 'list'], [view_id, 'form']], + 'domain': [('id', 'in', self.ids)], + }) + else: + action.update({ + 'view_mode': 'form', + 'views': [[view_id, 'form']], + 'res_id': self.id, + }) + return action + def action_show_groups(self): self.ensure_one() return { diff --git a/odoo/addons/base/tests/test_base.py b/odoo/addons/base/tests/test_base.py index c1b7883df1b..15d9108c08f 100644 --- a/odoo/addons/base/tests/test_base.py +++ b/odoo/addons/base/tests/test_base.py @@ -4,7 +4,7 @@ import ast from odoo import SUPERUSER_ID, Command -from odoo.exceptions import UserError, ValidationError +from odoo.exceptions import RedirectWarning, UserError, ValidationError from odoo.tests.common import TransactionCase, BaseCase from odoo.tools import mute_logger from odoo.tools.safe_eval import safe_eval, const_eval, expr_eval @@ -508,8 +508,10 @@ class TestBase(TransactionCase): 'partner_id': test_partner.id, }) # Cannot archive the partner + with self.assertRaises(RedirectWarning): + test_partner.with_user(self.env.ref('base.user_admin')).toggle_active() with self.assertRaises(ValidationError): - test_partner.toggle_active() + test_partner.with_user(self.env.ref('base.user_demo')).toggle_active() # Can archive the user but the partner stays active test_user.toggle_active() diff --git a/odoo/addons/base/tests/test_orm.py b/odoo/addons/base/tests/test_orm.py index 174cd4c1b38..e3d2729234e 100644 --- a/odoo/addons/base/tests/test_orm.py +++ b/odoo/addons/base/tests/test_orm.py @@ -17,9 +17,9 @@ class TestORM(TransactionCase): @mute_logger('odoo.models') def test_access_deleted_records(self): """ Verify that accessing deleted records works as expected """ - p1 = self.env['res.partner'].create({'name': 'W'}) - p2 = self.env['res.partner'].create({'name': 'Y'}) - p1.unlink() + c1 = self.env['res.partner.category'].create({'name': 'W'}) + c2 = self.env['res.partner.category'].create({'name': 'Y'}) + c1.unlink() # read() is expected to skip deleted records because our API is not # transactional for a sequence of search()->read() performed from the @@ -31,12 +31,12 @@ class TestORM(TransactionCase): 'login': 'test2', 'groups_id': [Command.set([self.ref('base.group_user')])], }) - ps = (p1 + p2).with_user(user) - self.assertEqual([{'id': p2.id, 'name': 'Y'}], ps.read(['name']), "read() should skip deleted records") - self.assertEqual([], ps[0].read(['name']), "read() should skip deleted records") + cs = (c1 + c2).with_user(user) + self.assertEqual([{'id': c2.id, 'name': 'Y'}], cs.read(['name']), "read() should skip deleted records") + self.assertEqual([], cs[0].read(['name']), "read() should skip deleted records") # Deleting an already deleted record should be simply ignored - self.assertTrue(p1.unlink(), "Re-deleting should be a no-op") + self.assertTrue(c1.unlink(), "Re-deleting should be a no-op") @mute_logger('odoo.models') def test_access_partial_deletion(self): diff --git a/odoo/addons/base/views/res_partner_views.xml b/odoo/addons/base/views/res_partner_views.xml index a24db7979be..834c899c22c 100644 --- a/odoo/addons/base/views/res_partner_views.xml +++ b/odoo/addons/base/views/res_partner_views.xml @@ -81,11 +81,12 @@

- + +

@@ -157,12 +158,13 @@

- + +

diff --git a/odoo/addons/test_main_flows/static/tests/tours/main_flow.js b/odoo/addons/test_main_flows/static/tests/tours/main_flow.js index f407513f4b0..5c2feab32f1 100644 --- a/odoo/addons/test_main_flows/static/tests/tours/main_flow.js +++ b/odoo/addons/test_main_flows/static/tests/tours/main_flow.js @@ -171,7 +171,7 @@ tour.stepUtils.autoExpandMoreButtons('.o_form_readonly'), position: "right", }, { mobile: true, - trigger: ".modal-dialog .o_field_widget[name=name] input", + trigger: "input[name=name]:not(.o_invisible_modifier)", extra_trigger: ".modal:not(.o_inactive_modal) .modal-dialog .o_field_radio.o_field_widget[name=company_type]", content: _t('Select a seller'), position: 'top', @@ -462,7 +462,7 @@ tour.stepUtils.autoExpandMoreButtons('.o_form_readonly'), position: "left", }, { mobile: true, - trigger: '.o_field_widget[name=name] input', + trigger: "input[name=name]:not(.o_invisible_modifier)", extra_trigger: ".modal:not(.o_inactive_modal) .modal-title:contains('Organization / Contact')", content: _t('Let\'s enter the name.'), position: 'left',