From 7cfd364db54e88970ef51c798df3bd4df44222b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 16 Apr 2020 13:35:57 +0000 Subject: [PATCH] [REF] mail, various: clean usage of mail alias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clean the usage of mail aliases and more specifically its associated mixin `mail.alias.mixin` . Stop using context for model of aliases, and correctly give model_id and parent_model_id to the call chain through a cleaned code easier to override. We also merge methods get_alias_model_name and get_alias_values in single one called before record creation (alias first values) and right after (to have values depending on actual record). LINKS Task ID 1919277 Community PR odoo/odoo#41160 Enterprise PR odoo/enterprise#6983 Upgrade PR odoo/upgrade#872 Co-Authored-By: Rémy Voet Co-Authored-By: Thibault Delavallée --- addons/account/models/account_journal.py | 5 +- addons/crm/models/crm_team.py | 36 +++++---------- addons/hr_recruitment/models/hr_job.py | 22 +++++---- addons/mail/models/mail_alias.py | 8 ---- addons/mail/models/mail_alias_mixin.py | 51 ++++++++++++--------- addons/mail/models/mail_channel.py | 18 ++++---- addons/maintenance/models/maintenance.py | 27 ++++------- addons/project/models/project.py | 16 +++---- addons/test_mail/models/test_mail_models.py | 16 +++---- addons/test_mail/tests/test_mail_gateway.py | 27 ++++++++++- 10 files changed, 113 insertions(+), 113 deletions(-) diff --git a/addons/account/models/account_journal.py b/addons/account/models/account_journal.py index 3e918b05d64..04b68f6134a 100644 --- a/addons/account/models/account_journal.py +++ b/addons/account/models/account_journal.py @@ -325,8 +325,9 @@ class AccountJournal(models.Model): if self.alias_id: self.alias_id.write(alias_values) else: - self.alias_id = self.env['mail.alias'].with_context(alias_model_name='account.move', - alias_parent_model_name='account.journal').create(alias_values) + alias_values['alias_model_id'] = self.env['ir.model']._get('account.move').id + alias_values['alias_parent_model_id'] = self.env['ir.model']._get('account.journal').id + self.alias_id = self.env['mail.alias'].create(alias_values) if vals.get('alias_name'): # remove alias_name to avoid useless write on alias diff --git a/addons/crm/models/crm_team.py b/addons/crm/models/crm_team.py index 9161c69116c..94062248003 100644 --- a/addons/crm/models/crm_team.py +++ b/addons/crm/models/crm_team.py @@ -97,43 +97,29 @@ class Team(models.Model): # ORM # ------------------------------------------------------------ - @api.model - def create(self, vals): - alias_values = self._synchronize_alias(vals) - if alias_values: - vals.update(alias_values) - return super(Team, self).create(vals) - def write(self, vals): result = super(Team, self).write(vals) if 'use_leads' in vals or 'alias_defaults' in vals: for team in self: team.alias_id.write(team.get_alias_values()) if 'use_leads' in vals or 'use_opportunities' in vals: - for team in self: - team.write(team._synchronize_alias(vals)) + self.filtered(lambda team: not team.use_leads and not team.use_opportunities).alias_name = False return result - def _synchronize_alias(self, values): - use_leads = self.use_leads if self else values.get('use_leads', False) - use_opportunities = self.use_opportunities if self else values.get('use_opportunities', True) - if not use_leads and not use_opportunities: - return {'alias_name': False} - return {} - # ------------------------------------------------------------ # MESSAGING # ------------------------------------------------------------ - def get_alias_model_name(self, vals): - return 'crm.lead' - - def get_alias_values(self): - has_group_use_lead = self.env.user.has_group('crm.group_use_lead') - values = super(Team, self).get_alias_values() - values['alias_defaults'] = defaults = ast.literal_eval(self.alias_defaults or "{}") - defaults['type'] = 'lead' if has_group_use_lead and self.use_leads else 'opportunity' - defaults['team_id'] = self.id + def _alias_get_creation_values(self): + values = super(Team, self)._alias_get_creation_values() + values['alias_model_id'] = self.env['ir.model']._get('crm.lead').id + if self.id: + if not self.use_leads and not self.use_opportunities: + values['alias_name'] = False + values['alias_defaults'] = defaults = ast.literal_eval(self.alias_defaults or "{}") + has_group_use_lead = self.env.user.has_group('crm.group_use_lead') + defaults['type'] = 'lead' if has_group_use_lead and self.use_leads else 'opportunity' + defaults['team_id'] = self.id return values # ------------------------------------------------------------ diff --git a/addons/hr_recruitment/models/hr_job.py b/addons/hr_recruitment/models/hr_job.py index 7fb74433099..5b7efdc78c8 100644 --- a/addons/hr_recruitment/models/hr_job.py +++ b/addons/hr_recruitment/models/hr_job.py @@ -1,5 +1,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. +import ast + from odoo import api, fields, models, _ @@ -91,16 +93,16 @@ class Job(models.Model): [("job_id", "=", job.id), ("stage_id", "=", job._get_first_stage().id)] ) - def get_alias_model_name(self, vals): - return 'hr.applicant' - - def get_alias_values(self): - values = super(Job, self).get_alias_values() - values['alias_defaults'] = { - 'job_id': self.id, - 'department_id': self.department_id.id, - 'company_id': self.department_id.company_id.id if self.department_id else self.company_id.id - } + def _alias_get_creation_values(self): + values = super(Job, self)._alias_get_creation_values() + values['alias_model_id'] = self.env['ir.model']._get('hr.applicant').id + if self.id: + values['alias_defaults'] = defaults = ast.literal_eval(self.alias_defaults or "{}") + defaults.update({ + 'job_id': self.id, + 'department_id': self.department_id.id, + 'company_id': self.department_id.company_id.id if self.department_id else self.company_id.id, + }) return values @api.model diff --git a/addons/mail/models/mail_alias.py b/addons/mail/models/mail_alias.py index 1c781edff7a..ed36d8a3a63 100644 --- a/addons/mail/models/mail_alias.py +++ b/addons/mail/models/mail_alias.py @@ -94,16 +94,8 @@ class Alias(models.Model): model ID of the ``model_name`` context value, if provided. Also, it raises UserError if given alias name is already assigned. """ - model_name = self._context.get('alias_model_name') - parent_model_name = self._context.get('alias_parent_model_name') if vals.get('alias_name'): vals['alias_name'] = self._clean_and_check_unique(vals.get('alias_name')) - if model_name: - model = self.env['ir.model']._get(model_name) - vals['alias_model_id'] = model.id - if parent_model_name: - model = self.env['ir.model']._get(parent_model_name) - vals['alias_parent_model_id'] = model.id return super(Alias, self).create(vals) def write(self, vals): diff --git a/addons/mail/models/mail_alias_mixin.py b/addons/mail/models/mail_alias_mixin.py index 58583b00e5b..f60acdc677f 100644 --- a/addons/mail/models/mail_alias_mixin.py +++ b/addons/mail/models/mail_alias_mixin.py @@ -26,11 +26,16 @@ class AliasMixin(models.AbstractModel): @api.model def create(self, vals): """ Create a record with ``vals``, and create a corresponding alias. """ - record = super(AliasMixin, self.with_context( - alias_model_name=self.get_alias_model_name(vals), - alias_parent_model_name=self._name, - )).create(vals) - record.alias_id.sudo().write(record.get_alias_values()) + new_alias = not 'alias_id' in vals + if new_alias: + alias_vals, record_vals = self._alias_filter_fields(vals) + alias_vals.update(self._alias_get_creation_values()) + alias = self.env['mail.alias'].create(alias_vals) + record_vals['alias_id'] = alias.id + + record = super(AliasMixin, self).create(record_vals) + alias.sudo().write(record._alias_get_creation_values()) + return record def unlink(self): @@ -50,12 +55,6 @@ class AliasMixin(models.AbstractModel): def _init_column_alias_id(self): # both self and the alias model must be present in 'ir.model' - alias_ctx = { - 'alias_model_name': self.get_alias_model_name({}), - 'alias_parent_model_name': self._name, - } - alias_model = self.env['mail.alias'].sudo().with_context(alias_ctx) - child_ctx = { 'active_test': False, # retrieve all records 'prefetch_fields': False, # do not prefetch fields on records @@ -64,7 +63,7 @@ class AliasMixin(models.AbstractModel): for record in child_model.search([('alias_id', '=', False)]): # create the alias, and link it to the current record - alias = alias_model.create(record.get_alias_values()) + alias = self.env['mail.alias'].sudo().create(record._alias_get_creation_values()) record.with_context(mail_notrack=True).alias_id = alias _logger.info('Mail alias created for %s %s (id %s)', record._name, record.display_name, record.id) @@ -73,19 +72,27 @@ class AliasMixin(models.AbstractModel): # MIXIN TOOL OVERRIDE METHODS # -------------------------------------------------- - def get_alias_model_name(self, vals): - """ Return the model name for the alias. Incoming emails that are not - replies to existing records will cause the creation of a new record - of this alias model. The value may depend on ``vals``, the dict of - values passed to ``create`` when a record of this model is created. - """ - return None - - def get_alias_values(self): + def _alias_get_creation_values(self): """ Return values to create an alias, or to write on the alias after its creation. """ - return {'alias_parent_thread_id': self.id} + return { + 'alias_parent_thread_id': self.id if self.id else False, + 'alias_parent_model_id': self.env['ir.model']._get(self._name).id, + } + + def _alias_filter_fields(self, values, filters=False): + """ Split the vals dict into two dictionnary of vals, one for alias + field and the other for other fields """ + if not filters: + filters = self.env['mail.alias']._fields.keys() + alias_values, record_values = {}, {} + for fname in values.keys(): + if fname in filters: + alias_values[fname] = values.get(fname) + else: + record_values[fname] = values.get(fname) + return alias_values, record_values # -------------------------------------------------- # GATEWAY diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py index 04642b70126..1bab01aca8e 100644 --- a/addons/mail/models/mail_channel.py +++ b/addons/mail/models/mail_channel.py @@ -208,9 +208,8 @@ class Channel(models.Model): # Create channel and alias channel = super(Channel, self.with_context( - alias_model_name=self._name, alias_parent_model_name=self._name, mail_create_nolog=True, mail_create_nosubscribe=True) + mail_create_nolog=True, mail_create_nosubscribe=True) ).create(vals) - channel.alias_id.write({"alias_force_thread_id": channel.id, 'alias_parent_thread_id': channel.id}) if vals.get('group_ids'): channel._subscribe_users() @@ -222,8 +221,6 @@ class Channel(models.Model): return channel def unlink(self): - aliases = self.mapped('alias_id') - # Delete mail.channel try: all_emp_group = self.env.ref('mail.channel_all_employees') @@ -231,10 +228,7 @@ class Channel(models.Model): all_emp_group = None if all_emp_group and all_emp_group in self and not self._context.get(MODULE_UNINSTALL_FLAG): raise UserError(_('You cannot delete those groups, as the Whole Company group is required by other modules.')) - res = super(Channel, self).unlink() - # Cascade-delete mail aliases as well, as they should not exist without the mail.channel. - aliases.sudo().unlink() - return res + return super(Channel, self).unlink() def write(self, vals): # First checks if user tries to modify moderation fields and has not the right to do it. @@ -257,8 +251,12 @@ class Channel(models.Model): return result - def get_alias_model_name(self, vals): - return vals.get('alias_model', 'mail.channel') + def _alias_get_creation_values(self): + values = super(Channel, self)._alias_get_creation_values() + values['alias_model_id'] = self.env['ir.model']._get('mail.channel').id + if self.id: + values['alias_force_thread_id'] = self.id + return values def _subscribe_users(self): for mail_channel in self: diff --git a/addons/maintenance/models/maintenance.py b/addons/maintenance/models/maintenance.py index 4b8598796f4..ae96a3e6cf6 100644 --- a/addons/maintenance/models/maintenance.py +++ b/addons/maintenance/models/maintenance.py @@ -1,5 +1,7 @@ # -*- coding: utf-8 -*- +import ast + from datetime import date, datetime, timedelta from odoo import api, fields, models, SUPERUSER_ID, _ @@ -61,29 +63,18 @@ class MaintenanceEquipmentCategory(models.Model): for category in self: category.maintenance_count = mapped_data.get(category.id, 0) - @api.model - def create(self, vals): - self = self.with_context(alias_model_name='maintenance.request', alias_parent_model_name=self._name) - category_id = super(MaintenanceEquipmentCategory, self).create(vals) - category_id.alias_id.write({'alias_parent_thread_id': category_id.id, 'alias_defaults': {'category_id': category_id.id}}) - return category_id - def unlink(self): - MailAlias = self.env['mail.alias'] for category in self: if category.equipment_ids or category.maintenance_ids: raise UserError(_("You cannot delete an equipment category containing equipments or maintenance requests.")) - MailAlias += category.alias_id - res = super(MaintenanceEquipmentCategory, self).unlink() - MailAlias.unlink() - return res + return super(MaintenanceEquipmentCategory, self).unlink() - def get_alias_model_name(self, vals): - return vals.get('alias_model', 'maintenance.request') - - def get_alias_values(self): - values = super(MaintenanceEquipmentCategory, self).get_alias_values() - values['alias_defaults'] = {'category_id': self.id} + def _alias_get_creation_values(self): + values = super(MaintenanceEquipmentCategory, self)._alias_get_creation_values() + values['alias_model_id'] = self.env['ir.model']._get('maintenance.request').id + if self.id: + values['alias_defaults'] = defaults = ast.literal_eval(self.alias_defaults or "{}") + defaults['category_id'] = self.id return values diff --git a/addons/project/models/project.py b/addons/project/models/project.py index 64a25144a69..ed79097320d 100644 --- a/addons/project/models/project.py +++ b/addons/project/models/project.py @@ -69,14 +69,6 @@ class Project(models.Model): _rating_satisfaction_days = False # takes all existing ratings _check_company_auto = True - def get_alias_model_name(self, vals): - return vals.get('alias_model', 'project.task') - - def get_alias_values(self): - values = super(Project, self).get_alias_values() - values['alias_defaults'] = {'project_id': self.id} - return values - def _compute_attached_docs_count(self): Attachment = self.env['ir.attachment'] for project in self: @@ -401,6 +393,14 @@ class Project(models.Model): self.mapped('tasks').message_unsubscribe(partner_ids=partner_ids, channel_ids=channel_ids) return super(Project, self).message_unsubscribe(partner_ids=partner_ids, channel_ids=channel_ids) + def _alias_get_creation_values(self): + values = super(Project, self)._alias_get_creation_values() + values['alias_model_id'] = self.env['ir.model']._get('project.task').id + if self.id: + values['alias_defaults'] = defaults = ast.literal_eval(self.alias_defaults or "{}") + defaults['project_id'] = self.id + return values + # --------------------------------------------------- # Actions # --------------------------------------------------- diff --git a/addons/test_mail/models/test_mail_models.py b/addons/test_mail/models/test_mail_models.py index 340bcbcb9c5..0e420b83eba 100644 --- a/addons/test_mail/models/test_mail_models.py +++ b/addons/test_mail/models/test_mail_models.py @@ -115,15 +115,13 @@ class MailTestAlias(models.Model): 'mail.alias', 'Alias', delegate=True) - def get_alias_model_name(self, vals): - return vals.get('alias_model', 'mail.test') - - def get_alias_values(self): - self.ensure_one() - res = super(MailTestAlias, self).get_alias_values() - res['alias_force_thread_id'] = self.id - res['alias_parent_thread_id'] = self.id - return res + def _alias_get_creation_values(self): + values = super(MailTestAlias, self)._alias_get_creation_values() + values['alias_model_id'] = self.env['ir.model']._get('mail.test').id + if self.id: + values['alias_force_thread_id'] = self.id + values['alias_parent_thread_id'] = self.id + return values class MailModel(models.Model): diff --git a/addons/test_mail/tests/test_mail_gateway.py b/addons/test_mail/tests/test_mail_gateway.py index d536d0236ae..41db5b25f2e 100644 --- a/addons/test_mail/tests/test_mail_gateway.py +++ b/addons/test_mail/tests/test_mail_gateway.py @@ -13,6 +13,7 @@ from odoo.addons.test_mail.data.test_mail_data import MAIL_TEMPLATE from odoo.addons.test_mail.models.test_mail_models import MailTestGateway from odoo.addons.test_mail.tests.common import TestMailCommon from odoo.tests import tagged +from odoo.tests.common import users from odoo.tools import email_split_and_format, formataddr, mute_logger @@ -73,8 +74,32 @@ class TestEmailParsing(TestMailCommon): @tagged('mail_gateway') class TestMailAlias(TestMailCommon): + @users('employee') + def test_alias_creation(self): + record = self.env['mail.test'].create({ + 'name': 'Test Record', + 'alias_name': 'alias.test', + 'alias_contact': 'followers', + }) + self.assertEqual(record.alias_id.alias_model_id, self.env['ir.model']._get('mail.test')) + self.assertEqual(record.alias_id.alias_force_thread_id, record.id) + self.assertEqual(record.alias_id.alias_parent_model_id, self.env['ir.model']._get('mail.test')) + self.assertEqual(record.alias_id.alias_parent_thread_id, record.id) + self.assertEqual(record.alias_id.alias_name, 'alias.test') + self.assertEqual(record.alias_id.alias_contact, 'followers') + + record.write({ + 'alias_force_thread_id': False, + 'alias_defaults': "{'default_name': 'defaults'}" + }) + self.assertFalse(record.alias_id.alias_force_thread_id) + self.assertEqual(record.alias_id.alias_defaults, "{'default_name': 'defaults'}") + def test_alias_setup(self): - alias = self.env['mail.alias'].with_context(alias_model_name='mail.test').create({'alias_name': 'b4r+_#_R3wl$$'}) + alias = self.env['mail.alias'].create({ + 'alias_model_id': self.env['ir.model']._get('mail.test').id, + 'alias_name': 'b4r+_#_R3wl$$', + }) self.assertEqual(alias.alias_name, 'b4r+_-_r3wl-', 'Disallowed chars should be replaced by hyphens') def test_alias_name_unique(self):