From d6cc18cfdf3a9392a110cdfce539306de9e791a9 Mon Sep 17 00:00:00 2001 From: Noe Antoine Date: Thu, 5 Aug 2021 12:07:43 +0000 Subject: [PATCH 1/2] [IMP] portal: avoid error modal on wizard modal superposition BEFORE THIS COMMIT : When using the action "grant portal access" on contact, it opens a wizard. When trying to granting access, an error is raised if email is not properly formatted or already used by another user, in a modal. The modal of error message is superimposed on the one of the wizard. This looks very unpleasant and not very readable. AFTER THIS COMMIT : Therefore, the error messages due to an invalid email (bad format or already used) are simplified and displayed inline in portal wizard user tree view instead in the form of colored fa-icons buttons, with details as title. In order to have the tooltip service detect mouse events, we make them non-disabled: if clicked, the modal will simply be refreshed. Also, there will be no display of reinvite / grant access buttons as long as an error on email remains. To do so, a new selection field sets the email validity status for each portal_wizard_user : 'ok' / 'ko' / 'exist'. Errors are still thrown if method is called directly, except for the revoke access method. Indeed, we want to be able to revoke access, independantly of the email status. However, the partner's email is only updated if the email is valid. TESTS : Tests are updated accordingly. The last assert of test_portal_wizard_error is repaired. The previous one raised an error as expected but not the good one. As the email had a wrong format, it was detected and error was raised, and not because the user was internal, as supposed to. The portal user is updated as it should be. --- LINKS --- Task Id - 2613780 COM PR - odoo/odoo#74911 --- addons/portal/tests/test_portal_wizard.py | 9 ++- addons/portal/wizard/portal_wizard.py | 66 ++++++++++++-------- addons/portal/wizard/portal_wizard_views.xml | 11 +++- 3 files changed, 54 insertions(+), 32 deletions(-) diff --git a/addons/portal/tests/test_portal_wizard.py b/addons/portal/tests/test_portal_wizard.py index 6ba1ddbc502..f759bc5c841 100644 --- a/addons/portal/tests/test_portal_wizard.py +++ b/addons/portal/tests/test_portal_wizard.py @@ -141,16 +141,19 @@ class TestPortalWizard(MailCommon): portal_user = portal_wizard.user_ids self.internal_user.login = 'test_error@example.com' - portal_user.email = 'test_error@example.com' + portal_user.email = 'test_error@example.com' with self.assertRaises(UserError, msg='Must detect the already used email.'): - portal_user.action_revoke_access() + portal_user._assert_user_email_uniqueness() + self.assertEqual(portal_user.email_state, 'exist', msg='Must detect the already used email.') portal_user.email = 'wrong email format' with self.assertRaises(UserError, msg='Must detect wrong email format.'): - portal_user.action_revoke_access() + portal_user._assert_user_email_uniqueness() + self.assertEqual(portal_user.email_state, 'ko', msg='Must detect wrong email format.') portal_wizard = self.env['portal.wizard'].with_context(active_ids=[self.internal_user.partner_id.id]).create({}) + portal_user = portal_wizard.user_ids with self.assertRaises(UserError, msg='Must not be able to change internal user group.'): portal_user.action_revoke_access() diff --git a/addons/portal/wizard/portal_wizard.py b/addons/portal/wizard/portal_wizard.py index f0b14e46608..fc70dd821ee 100644 --- a/addons/portal/wizard/portal_wizard.py +++ b/addons/portal/wizard/portal_wizard.py @@ -88,6 +88,25 @@ class PortalWizardUser(models.TransientModel): login_date = fields.Datetime(related='user_id.login_date', string='Latest Authentication') is_portal = fields.Boolean('Is Portal', compute='_compute_group_details') is_internal = fields.Boolean('Is Internal', compute='_compute_group_details') + email_state = fields.Selection([ + ('ok', 'Valid'), + ('ko', 'Invalid'), + ('exist', 'Already Registered')], + string='Status', compute='_compute_email_state', default='ok') + + @api.depends('email') + def _compute_email_state(self): + portal_users_with_email = self.filtered(lambda user: email_normalize(user.email)) + (self - portal_users_with_email).email_state = 'ko' + + normalized_emails = [email_normalize(portal_user.email) for portal_user in portal_users_with_email] + existing_users = self.env['res.users'].with_context(active_test=False).sudo().search_read([('login', 'in', normalized_emails)], ['id', 'login']) + + for portal_user in portal_users_with_email: + if next((user for user in existing_users if user['login'] == email_normalize(portal_user.email) and user['id'] != portal_user.user_id.id), None): + portal_user.email_state = 'exist' + else: + portal_user.email_state = 'ok' @api.depends('partner_id') def _compute_user_id(self): @@ -127,10 +146,7 @@ class PortalWizardUser(models.TransientModel): group_portal = self.env.ref('base.group_portal') group_public = self.env.ref('base.group_public') - # update partner email, if a new one was introduced - if self.partner_id.email != self.email: - self.partner_id.write({'email': self.email}) - + self._update_partner_email() user_sudo = self.user_id.sudo() if not user_sudo: @@ -145,7 +161,7 @@ class PortalWizardUser(models.TransientModel): self.with_context(active_test=True)._send_email() - return self.wizard_id._action_open_modal() + return self.action_refresh_modal() def action_revoke_access(self): """Remove the user of the partner from the portal group. @@ -153,17 +169,13 @@ class PortalWizardUser(models.TransientModel): If the user was only in the portal group, we archive it. """ self.ensure_one() - self._assert_user_email_uniqueness() - if not self.is_portal: - raise UserError(_('The partner "%s" has no portal access.', self.partner_id.name)) + raise UserError(_('The partner "%s" has no portal access or is internal.', self.partner_id.name)) group_portal = self.env.ref('base.group_portal') group_public = self.env.ref('base.group_public') - # update partner email, if a new one was introduced - if self.partner_id.email != self.email: - self.partner_id.write({'email': self.email}) + self._update_partner_email() # Remove the sign up token, so it can not be used self.partner_id.sudo().signup_token = False @@ -178,21 +190,24 @@ class PortalWizardUser(models.TransientModel): else: user_sudo.write({'groups_id': [(3, group_portal.id), (4, group_public.id)]}) - return self.wizard_id._action_open_modal() + return self.action_refresh_modal() def action_invite_again(self): """Re-send the invitation email to the partner.""" self.ensure_one() + self._assert_user_email_uniqueness() if not self.is_portal: raise UserError(_('You should first grant the portal access to the partner "%s".', self.partner_id.name)) - # update partner email, if a new one was introduced - if self.partner_id.email != self.email: - self.partner_id.write({'email': self.email}) - + self._update_partner_email() self.with_context(active_test=True)._send_email() + return self.action_refresh_modal() + + def action_refresh_modal(self): + """Refresh the portal wizard modal and keep it open. Used as action of email state icon buttons, + required as they must be non-disabled buttons to fire mouse events to show tooltips on email state.""" return self.wizard_id._action_open_modal() def _create_user(self): @@ -229,16 +244,13 @@ class PortalWizardUser(models.TransientModel): def _assert_user_email_uniqueness(self): """Check that the email can be used to create a new user.""" self.ensure_one() - - email = email_normalize(self.email) - - if not email: + if self.email_state == 'ko': raise UserError(_('The contact "%s" does not have a valid email.', self.partner_id.name)) + if self.email_state == 'exist': + raise UserError(_('The contact "%s" has the same email as an existing user', self.partner_id.name)) - user = self.env['res.users'].sudo().with_context(active_test=False).search([ - ('id', '!=', self.user_id.id), - ('login', '=ilike', email), - ]) - - if user: - raise UserError(_('The contact "%s" has the same email has an existing user (%s).', self.partner_id.name, user.name)) + def _update_partner_email(self): + """Update partner email on portal action, if a new one was introduced and is valid.""" + email_normalized = email_normalize(self.email) + if self.email_state == 'ok' and email_normalize(self.partner_id.email) != email_normalized: + self.partner_id.write({'email': email_normalized}) diff --git a/addons/portal/wizard/portal_wizard_views.xml b/addons/portal/wizard/portal_wizard_views.xml index 8f0f1d15997..2b782e3a96b 100644 --- a/addons/portal/wizard/portal_wizard_views.xml +++ b/addons/portal/wizard/portal_wizard_views.xml @@ -35,15 +35,22 @@ + +