From 4cb1f56aba5899a7dbf1b08daf9c3987d2637189 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 17 Dec 2020 13:02:24 +0000 Subject: [PATCH] [REF] mail, sms, test_mail: remove now unused channel_id field on Py and JS follower models RATIONALE Channel model is a mail.thread enabled model behaving strangely with followers, notifications and discuss. Its code should however be simplified to be more self contained and avoid unwanted side effects on other models. PURPOSE Remove channel ability to follow records as it mainly adds noise without a lot of added value. Simplify channel notification flow by using directly members and not a delegation through a channel self-following trick. Remove followers being channels and posting with added listeners being channels. SPECIFICATIONS As there is no way to add channel-based follower anymore we can remove all fields and code supporting this feature. Notably we can remove ``channel_id`` field on ``mail.follower`` model as well all code using it, notably compute methods. In this commit we also make ``partner_id`` field required as now followers are always partners. Email, name and active fields are now simple related fields on the partner. Code computing data about subscription is also updated and simplified. As we do not have channels anymore but only partners all custom SQL queries are now simplified. JS models for Discuss are also cleaned. Following python change, JS models are simplified to match the backend models. Channel_id is removed, partner_id is now required, and various code is updated according to the simplified model. Side note: we could probably get rid of specific index on ``partner_id`` field. However we have to ensure we never search for followers without being in a model / res_id context. This will be done in another cleaning step to be sure performance are not broken. LINKS Task ID-2070632 (main task) Task ID-2419762 (followup task) COM PR odoo/odoo#62859 ENT PR odoo/enterprise#15172 UPG PR odoo/upgrade#2005 --- addons/mail/controllers/main.py | 1 - addons/mail/models/mail_followers.py | 158 +++++------------- addons/mail/models/mail_thread.py | 24 +-- .../chatter_topbar/chatter_topbar_tests.js | 4 - .../src/components/follower/follower.xml | 2 +- .../src/components/follower/follower_tests.js | 67 +------- .../follower_subtype_tests.js | 9 +- .../static/src/models/follower/follower.js | 91 +--------- .../mail/static/tests/helpers/mock_models.js | 1 - .../mail/static/tests/helpers/mock_server.js | 1 - addons/mail/views/mail_followers_views.xml | 2 - addons/mail/wizard/invite.py | 2 +- addons/mail/wizard/mail_resend_message.py | 2 +- addons/sms/models/mail_followers.py | 10 +- addons/sms/wizard/sms_resend.py | 2 +- addons/test_mail/tests/test_invite.py | 2 - addons/test_mail/tests/test_mail_followers.py | 16 +- .../tests/test_mail_thread_internals.py | 3 - 18 files changed, 78 insertions(+), 319 deletions(-) diff --git a/addons/mail/controllers/main.py b/addons/mail/controllers/main.py index 178baf66ed7..9f8c82e4173 100644 --- a/addons/mail/controllers/main.py +++ b/addons/mail/controllers/main.py @@ -141,7 +141,6 @@ class MailController(http.Controller): followers.append({ 'id': follower.id, 'partner_id': follower.partner_id.id, - 'channel_id': follower.channel_id.id, 'name': follower.name, 'email': follower.email, 'is_active': follower.is_active, diff --git a/addons/mail/models/mail_followers.py b/addons/mail/models/mail_followers.py index 2bfca4e70e1..3055e0b64d6 100644 --- a/addons/mail/models/mail_followers.py +++ b/addons/mail/models/mail_followers.py @@ -29,18 +29,13 @@ class Followers(models.Model): res_id = fields.Many2oneReference( 'Related Document ID', index=True, help='Id of the followed resource', model_field='res_model') partner_id = fields.Many2one( - 'res.partner', string='Related Partner', ondelete='cascade', index=True) - channel_id = fields.Many2one( - 'mail.channel', string='Listener', ondelete='cascade', index=True) + 'res.partner', string='Related Partner', index=True, ondelete='cascade', required=True) subtype_ids = fields.Many2many( 'mail.message.subtype', string='Subtype', help="Message subtypes followed, meaning subtypes that will be pushed onto the user's Wall.") - name = fields.Char('Name', compute='_compute_related_fields', - help="Name of the related partner (if exist) or the related channel") - email = fields.Char('Email', compute='_compute_related_fields', - help="Email of the related partner (if exist) or False") - is_active = fields.Boolean('Is Active', compute='_compute_related_fields', - help="If the related partner is active (if exist) or if related channel exist") + name = fields.Char('Name', related='partner_id.name') + email = fields.Char('Email', related='partner_id.email') + is_active = fields.Boolean('Is Active', related='partner_id.active') def _invalidate_documents(self, vals_list=None): """ Invalidate the cache of the documents followed by ``self``. @@ -73,27 +68,13 @@ class Followers(models.Model): _sql_constraints = [ ('mail_followers_res_partner_res_model_id_uniq', 'unique(res_model,res_id,partner_id)', 'Error, a partner cannot follow twice the same object.'), - ('mail_followers_res_channel_res_model_id_uniq', 'unique(res_model,res_id,channel_id)', 'Error, a channel cannot follow twice the same object.'), - ('partner_xor_channel', 'CHECK((partner_id IS NULL) != (channel_id IS NULL))', 'Error: A follower must be either a partner or a channel (but not both).') ] # -------------------------------------------------- # Private tools methods to fetch followers data # -------------------------------------------------- - @api.depends('partner_id', 'channel_id') - def _compute_related_fields(self): - for follower in self: - if follower.partner_id: - follower.name = follower.partner_id.name - follower.email = follower.partner_id.email - follower.is_active = follower.partner_id.active - else: - follower.name = follower.channel_id.name - follower.is_active = bool(follower.channel_id) - follower.email = False - - def _get_recipient_data(self, records, message_type, subtype_id, pids=None, cids=None): + def _get_recipient_data(self, records, message_type, subtype_id, pids=None): """ Private method allowing to fetch recipients data based on a subtype. Purpose of this method is to fetch all data necessary to notify recipients in a single query. It fetches data from @@ -101,23 +82,20 @@ class Followers(models.Model): * followers (partners and channels) of records that follow the given subtype if records and subtype are set; * partners if pids is given; - * channels if cids is given; :param records: fetch data from followers of records that follow subtype_id; :param message_type: mail.message.message_type in order to allow custom behavior depending on it (SMS for example); :param subtype_id: mail.message.subtype to check against followers; :param pids: additional set of partner IDs from which to fetch recipient data; - :param cids: additional set of channel IDs from which to fetch recipient data; :return: list of recipient data which is a tuple containing - partner ID (void if channel ID), - channel ID (void if partner ID), + partner ID , active value (always True for channels), - share status of partner (void as irrelevant if channel ID), + share status of partner, notification status of partner or channel (email or inbox), - user groups of partner (void as irrelevant if channel ID), + user groups of partner, """ - self.env['mail.followers'].flush(['partner_id', 'channel_id', 'subtype_ids']) + self.env['mail.followers'].flush(['partner_id', 'subtype_ids']) self.env['mail.message.subtype'].flush(['internal']) self.env['res.users'].flush(['notification_type', 'active', 'partner_id', 'groups_id']) self.env['res.partner'].flush(['active', 'partner_share']) @@ -125,9 +103,9 @@ class Followers(models.Model): self.env['mail.channel'].flush(['email_send', 'channel_type']) if records and subtype_id: query = """ -SELECT DISTINCT ON(pid, cid) * FROM ( +SELECT DISTINCT ON (pid) * FROM ( WITH sub_followers AS ( - SELECT fol.id, fol.partner_id, fol.channel_id, subtype.internal + SELECT fol.id, fol.partner_id, subtype.internal FROM mail_followers fol RIGHT JOIN mail_followers_mail_message_subtype_rel subrel ON subrel.mail_followers_id = fol.id @@ -135,8 +113,8 @@ SELECT DISTINCT ON(pid, cid) * FROM ( ON subtype.id = subrel.mail_message_subtype_id WHERE subrel.mail_message_subtype_id = %%s AND fol.res_model = %%s AND fol.res_id IN %%s ) - SELECT partner.id as pid, NULL::int AS cid, - partner.active as active, partner.partner_share as pshare, NULL as ctype, + SELECT partner.id as pid, + partner.active as active, partner.partner_share as pshare, users.notification_type AS notif, array_agg(groups.id) AS groups FROM res_partner partner LEFT JOIN res_users users ON users.partner_id = partner.id AND users.active @@ -144,96 +122,69 @@ SELECT DISTINCT ON(pid, cid) * FROM ( LEFT JOIN res_groups groups ON groups.id = groups_rel.gid WHERE EXISTS ( SELECT partner_id FROM sub_followers - WHERE sub_followers.channel_id IS NULL - AND sub_followers.partner_id = partner.id + WHERE sub_followers.partner_id = partner.id AND (coalesce(sub_followers.internal, false) <> TRUE OR coalesce(partner.partner_share, false) <> TRUE) ) %s GROUP BY partner.id, users.notification_type - UNION - SELECT NULL::int AS pid, channel.id AS cid, - TRUE as active, NULL AS pshare, channel.channel_type AS ctype, - CASE WHEN channel.email_send = TRUE THEN 'email' ELSE 'inbox' END AS notif, NULL AS groups - FROM mail_channel channel - WHERE EXISTS ( - SELECT channel_id FROM sub_followers WHERE partner_id IS NULL AND sub_followers.channel_id = channel.id - ) %s ) AS x -ORDER BY pid, cid, notif -""" % ('OR partner.id IN %s' if pids else '', 'OR channel.id IN %s' if cids else '') +ORDER BY pid, notif +""" % ('OR partner.id IN %s' if pids else '') params = [subtype_id, records._name, tuple(records.ids)] if pids: params.append(tuple(pids)) - if cids: - params.append(tuple(cids)) self.env.cr.execute(query, tuple(params)) res = self.env.cr.fetchall() - elif pids or cids: - params, query_pid, query_cid = [], '', '' - if pids: - query_pid = """ -SELECT partner.id as pid, NULL::int AS cid, - partner.active as active, partner.partner_share as pshare, NULL as ctype, - users.notification_type AS notif, NULL AS groups + elif pids: + params = [] + query_pid = """ +SELECT partner.id as pid, +partner.active as active, partner.partner_share as pshare, +users.notification_type AS notif, NULL AS groups FROM res_partner partner LEFT JOIN res_users users ON users.partner_id = partner.id AND users.active WHERE partner.id IN %s""" - params.append(tuple(pids)) - if cids: - query_cid = """ -SELECT NULL::int AS pid, channel.id AS cid, - TRUE as active, NULL AS pshare, channel.channel_type AS ctype, - CASE when channel.email_send = TRUE then 'email' else 'inbox' end AS notif, NULL AS groups -FROM mail_channel channel WHERE channel.id IN %s """ - params.append(tuple(cids)) - query = ' UNION'.join(x for x in [query_pid, query_cid] if x) - query = 'SELECT DISTINCT ON(pid, cid) * FROM (%s) AS x ORDER BY pid, cid, notif' % query + params.append(tuple(pids)) + query = 'SELECT DISTINCT ON (pid) * FROM (%s) AS x ORDER BY pid, notif' % query_pid self.env.cr.execute(query, tuple(params)) res = self.env.cr.fetchall() else: res = [] return res - def _get_subscription_data(self, doc_data, pids, cids, include_pshare=False, include_active=False): + def _get_subscription_data(self, doc_data, pids, include_pshare=False, include_active=False): """ Private method allowing to fetch follower data from several documents of a given model. Followers can be filtered given partner IDs and channel IDs. :param doc_data: list of pair (res_model, res_ids) that are the documents from which we want to have subscription data; :param pids: optional partner to filter; if None take all, otherwise limitate to pids - :param cids: optional channel to filter; if None take all, otherwise limitate to cids :param include_pshare: optional join in partner to fetch their share status :param include_active: optional join in partner to fetch their active flag :return: list of followers data which is a list of tuples containing follower ID, document ID, - partner ID (void if channel_id), - channel ID (void if partner_id), + partner ID, followed subtype IDs, - share status of partner (void id channel_id, returned only if include_pshare is True) - active flag status of partner (void id channel_id, returned only if include_active is True) + share status of partner (returned only if include_pshare is True) + active flag status of partner (returned only if include_active is True) """ # base query: fetch followers of given documents where_clause = ' OR '.join(['fol.res_model = %s AND fol.res_id IN %s'] * len(doc_data)) where_params = list(itertools.chain.from_iterable((rm, tuple(rids)) for rm, rids in doc_data)) - # additional: filter on optional pids / cids + # additional: filter on optional pids sub_where = [] if pids: sub_where += ["fol.partner_id IN %s"] where_params.append(tuple(pids)) elif pids is not None: sub_where += ["fol.partner_id IS NULL"] - if cids: - sub_where += ["fol.channel_id IN %s"] - where_params.append(tuple(cids)) - elif cids is not None: - sub_where += ["fol.channel_id IS NULL"] if sub_where: where_clause += "AND (%s)" % " OR ".join(sub_where) query = """ -SELECT fol.id, fol.res_id, fol.partner_id, fol.channel_id, array_agg(subtype.id)%s%s +SELECT fol.id, fol.res_id, fol.partner_id, array_agg(subtype.id)%s%s FROM mail_followers fol %s LEFT JOIN mail_followers_mail_message_subtype_rel fol_rel ON fol_rel.mail_followers_id = fol.id @@ -254,23 +205,21 @@ GROUP BY fol.id%s%s""" % ( # Private tools methods to generate new subscription # -------------------------------------------------- - def _insert_followers(self, res_model, res_ids, partner_ids, partner_subtypes, channel_ids, channel_subtypes, + def _insert_followers(self, res_model, res_ids, partner_ids, partner_subtypes, customer_ids=None, check_existing=True, existing_policy='skip'): """ Main internal method allowing to create or update followers for documents, given a res_model and the document res_ids. This method does not handle access rights. This is the role of the caller to ensure there is no security breach. :param partner_subtypes: see ``_add_followers``. If not given, default ones are computed. - :param channel_subtypes: see ``_add_followers``. If not given, default ones are computed. :param customer_ids: see ``_add_default_followers`` :param check_existing: see ``_add_followers``; :param existing_policy: see ``_add_followers``; """ - sudo_self = self.sudo().with_context(default_partner_id=False, default_channel_id=False) - if not partner_subtypes and not channel_subtypes: # no subtypes -> default computation, no force, skip existing + sudo_self = self.sudo().with_context(default_partner_id=False) + if not partner_subtypes: # no subtypes -> default computation, no force, skip existing new, upd = self._add_default_followers( - res_model, res_ids, - partner_ids, channel_ids, + res_model, res_ids, partner_ids, customer_ids=customer_ids, check_existing=check_existing, existing_policy=existing_policy) @@ -278,7 +227,6 @@ GROUP BY fol.id%s%s""" % ( new, upd = self._add_followers( res_model, res_ids, partner_ids, partner_subtypes, - channel_ids, channel_subtypes, check_existing=check_existing, existing_policy=existing_policy) if new: @@ -290,7 +238,7 @@ GROUP BY fol.id%s%s""" % ( for fol_id, values in upd.items(): sudo_self.browse(fol_id).write(values) - def _add_default_followers(self, res_model, res_ids, partner_ids, channel_ids=None, customer_ids=None, + def _add_default_followers(self, res_model, res_ids, partner_ids, customer_ids=None, check_existing=True, existing_policy='skip'): """ Shortcut to ``_add_followers`` that computes default subtypes. Existing followers are skipped as their subscription is considered as more important @@ -304,19 +252,18 @@ GROUP BY fol.id%s%s""" % ( :return: see ``_add_followers`` """ - if not partner_ids and not channel_ids: + if not partner_ids: return dict(), dict() default, _, external = self.env['mail.message.subtype'].default_subtypes(res_model) if partner_ids and customer_ids is None: customer_ids = self.env['res.partner'].sudo().search([('id', 'in', partner_ids), ('partner_share', '=', True)]).ids - c_stypes = dict.fromkeys(channel_ids or [], default.ids) p_stypes = dict((pid, external.ids if pid in customer_ids else default.ids) for pid in partner_ids) - return self._add_followers(res_model, res_ids, partner_ids, p_stypes, channel_ids, c_stypes, check_existing=check_existing, existing_policy=existing_policy) + return self._add_followers(res_model, res_ids, partner_ids, p_stypes, check_existing=check_existing, existing_policy=existing_policy) - def _add_followers(self, res_model, res_ids, partner_ids, partner_subtypes, channel_ids, channel_subtypes, + def _add_followers(self, res_model, res_ids, partner_ids, partner_subtypes, check_existing=False, existing_policy='skip'): """ Internal method that generates values to insert or update followers. Callers have to handle the result, for example by making a valid ORM command, inserting or updating directly @@ -346,16 +293,14 @@ GROUP BY fol.id%s%s""" % ( * update: gives an update dict allowing to add missing subtypes (no subtype removal); """ _res_ids = res_ids or [0] - data_fols, doc_pids, doc_cids = dict(), dict((i, set()) for i in _res_ids), dict((i, set()) for i in _res_ids) + data_fols, doc_pids = dict(), dict((i, set()) for i in _res_ids) if check_existing and res_ids: - for fid, rid, pid, cid, sids in self._get_subscription_data([(res_model, res_ids)], partner_ids or None, channel_ids or None): + for fid, rid, pid, sids in self._get_subscription_data([(res_model, res_ids)], partner_ids or None): if existing_policy != 'force': if pid: doc_pids[rid].add(pid) - elif cid: - doc_cids[rid].add(cid) - data_fols[fid] = (rid, pid, cid, sids) + data_fols[fid] = (rid, pid, sids) if existing_policy == 'force': self.sudo().browse(data_fols.keys()).unlink() @@ -370,7 +315,7 @@ GROUP BY fol.id%s%s""" % ( 'subtype_ids': [Command.set(partner_subtypes[partner_id])], }) elif existing_policy in ('replace', 'update'): - fol_id, sids = next(((key, val[3]) for key, val in data_fols.items() if val[0] == res_id and val[1] == partner_id), (False, [])) + fol_id, sids = next(((key, val[2]) for key, val in data_fols.items() if val[0] == res_id and val[1] == partner_id), (False, [])) new_sids = set(partner_subtypes[partner_id]) - set(sids) old_sids = set(sids) - set(partner_subtypes[partner_id]) update_cmd = [] @@ -381,23 +326,4 @@ GROUP BY fol.id%s%s""" % ( if update_cmd: update[fol_id] = {'subtype_ids': update_cmd} - for channel_id in set(channel_ids or []): - if channel_id not in doc_cids[res_id]: - new.setdefault(res_id, list()).append({ - 'res_model': res_model, - 'channel_id': channel_id, - 'subtype_ids': [Command.set(channel_subtypes[channel_id])], - }) - elif existing_policy in ('replace', 'update'): - fol_id, sids = next(((key, val[3]) for key, val in data_fols.items() if val[0] == res_id and val[2] == channel_id), (False, [])) - new_sids = set(channel_subtypes[channel_id]) - set(sids) - old_sids = set(sids) - set(channel_subtypes[channel_id]) - update_cmd = [] - if fol_id and new_sids: - update_cmd += [Command.link(sid) for sid in new_sids] - if fol_id and old_sids and existing_policy == 'replace': - update_cmd += [Command.unlink(sid) for sid in old_sids] - if update_cmd: - update[fol_id] = {'subtype_ids': update_cmd} - return new, update diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 4a24ed6ed69..4f2909f54dd 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -247,8 +247,7 @@ class MailThread(models.AbstractModel): if not self._context.get('mail_create_nosubscribe'): for thread in threads: self.env['mail.followers']._insert_followers( - thread._name, thread.ids, self.env.user.partner_id.ids, - None, None, None, + thread._name, thread.ids, self.env.user.partner_id.ids, None, customer_ids=[], check_existing=False ) @@ -2384,12 +2383,12 @@ class MailThread(models.AbstractModel): 'partners': [], 'channels': [], } - res = self.env['mail.followers']._get_recipient_data(self, message_type, subtype_id, pids, cids) + res = self.env['mail.followers']._get_recipient_data(self, message_type, subtype_id, pids) if not res: return recipient_data author_id = msg_vals.get('author_id') or message.author_id.id - for pid, cid, active, pshare, ctype, notif, groups in res: + for pid, active, pshare, notif, groups in res: if pid and pid == author_id and not self.env.context.get('mail_notify_author'): # do not notify the author of its own messages continue if pid: @@ -2404,8 +2403,6 @@ class MailThread(models.AbstractModel): recipient_data['partners'].append(dict(pdata, notif=notif, type='portal')) else: # has no user, is therefore customer recipient_data['partners'].append(dict(pdata, notif=notif if notif else 'email', type='customer')) - elif cid: - recipient_data['channels'].append({'id': cid, 'notif': notif, 'type': ctype}) # add partner ids in email channels email_cids = [r['id'] for r in recipient_data['channels'] if r['notif'] == 'email'] @@ -2639,13 +2636,12 @@ class MailThread(models.AbstractModel): if not subtype_ids: self.env['mail.followers']._insert_followers( - self._name, self.ids, partner_ids, None, None, None, + self._name, self.ids, partner_ids, None, customer_ids=customer_ids, check_existing=True, existing_policy='skip') else: self.env['mail.followers']._insert_followers( self._name, self.ids, partner_ids, dict((pid, subtype_ids) for pid in partner_ids), - None, None, customer_ids=customer_ids, check_existing=True, existing_policy='replace') return True @@ -2655,8 +2651,7 @@ class MailThread(models.AbstractModel): # not necessary for computation, but saves an access right check if not partner_ids: return True - user_pid = self.env.user.partner_id.id - if set(partner_ids) == set([user_pid]): + if set(partner_ids) == set([self.env.user.partner_id.id]): self.check_access_rights('read') self.check_access_rule('read') else: @@ -2758,7 +2753,7 @@ class MailThread(models.AbstractModel): if not self: return True - new_partners, new_channels = dict(), dict() + new_partners = dict() # return data related to auto subscription based on subtype matching (aka: # default task subtypes or subtypes from project triggering task subtypes) @@ -2774,8 +2769,8 @@ class MailThread(models.AbstractModel): if udpated_fields: # fetch "parent" subscription data (aka: subtypes on project to propagate on task) doc_data = [(model, [updated_values[fname] for fname in fnames]) for model, fnames in updated_relation.items()] - res = self.env['mail.followers']._get_subscription_data(doc_data, None, None, include_pshare=True, include_active=True) - for fid, rid, pid, cid, subtype_ids, pshare, active in res: + res = self.env['mail.followers']._get_subscription_data(doc_data, None, include_pshare=True, include_active=True) + for fid, rid, pid, subtype_ids, pshare, active in res: # use project.task_new -> task.new link sids = [parent[sid] for sid in subtype_ids if parent.get(sid)] # add checked subtypes matching model_name @@ -2785,8 +2780,6 @@ class MailThread(models.AbstractModel): new_partners[pid] = set(sids) - set(all_int_ids) else: new_partners[pid] = set(sids) - if cid: # never subscribe channels to internal subtypes - new_channels[cid] = set(sids) - set(all_int_ids) notify_data = dict() res = self._message_auto_subscribe_followers(updated_values, def_ids) @@ -2800,7 +2793,6 @@ class MailThread(models.AbstractModel): self.env['mail.followers']._insert_followers( self._name, self.ids, list(new_partners), new_partners, - None, None, check_existing=True, existing_policy=followers_existing_policy) # notify people from auto subscription, for example like assignation diff --git a/addons/mail/static/src/components/chatter_topbar/chatter_topbar_tests.js b/addons/mail/static/src/components/chatter_topbar/chatter_topbar_tests.js index b20dfdfe4c4..ab1334e9acd 100644 --- a/addons/mail/static/src/components/chatter_topbar/chatter_topbar_tests.js +++ b/addons/mail/static/src/components/chatter_topbar/chatter_topbar_tests.js @@ -436,8 +436,6 @@ QUnit.test('rendering with multiple partner followers', async function (assert) this.data['mail.followers'].records.push( { // simulate real return from RPC - // (the presence of the key and the falsy value need to be handled correctly) - channel_id: false, id: 1, name: "Jean Michang", partner_id: 12, @@ -445,8 +443,6 @@ QUnit.test('rendering with multiple partner followers', async function (assert) res_model: 'res.partner', }, { // simulate real return from RPC - // (the presence of the key and the falsy value need to be handled correctly) - channel_id: false, id: 2, name: "Eden Hazard", partner_id: 11, diff --git a/addons/mail/static/src/components/follower/follower.xml b/addons/mail/static/src/components/follower/follower.xml index 26a26ece3f9..c8abebcae95 100644 --- a/addons/mail/static/src/components/follower/follower.xml +++ b/addons/mail/static/src/components/follower/follower.xml @@ -5,7 +5,7 @@
- Avatar + Avatar diff --git a/addons/mail/static/src/components/follower/follower_tests.js b/addons/mail/static/src/components/follower/follower_tests.js index 28058fc9a9f..db041f2d212 100644 --- a/addons/mail/static/src/components/follower/follower_tests.js +++ b/addons/mail/static/src/components/follower/follower_tests.js @@ -52,7 +52,10 @@ QUnit.test('base rendering not editable', async function (assert) { model: 'res.partner', }); const follower = await this.env.models['mail.follower'].create({ - channel: [['insert', { id: 1, model: 'mail.channel', name: "François Perusse" }]], + partner: [['insert', { + id: 1, + name: "François Perusse", + }]], followedThread: [['link', thread]], id: 2, isActive: true, @@ -95,7 +98,10 @@ QUnit.test('base rendering editable', async function (assert) { model: 'res.partner', }); const follower = await this.env.models['mail.follower'].create({ - channel: [['insert', { id: 1, model: 'mail.channel', name: "François Perusse" }]], + partner: [['insert', { + id: 1, + name: "François Perusse", + }]], followedThread: [['link', thread]], id: 2, isActive: true, @@ -134,63 +140,6 @@ QUnit.test('base rendering editable', async function (assert) { ); }); -QUnit.test('click on channel follower details', async function (assert) { - assert.expect(7); - - const bus = new Bus(); - bus.on('do-action', null, payload => { - assert.step('do_action'); - assert.strictEqual( - payload.action.res_id, - 10, - "The redirect action should redirect to the right res id (10)" - ); - assert.strictEqual( - payload.action.res_model, - 'mail.channel', - "The redirect action should redirect to the right res model (mail.channel)" - ); - assert.strictEqual( - payload.action.type, - "ir.actions.act_window", - "The redirect action should be of type 'ir.actions.act_window'" - ); - }); - this.data['res.partner'].records.push({ id: 100 }); - this.data['mail.channel'].records.push({ id: 10 }); - await this.start({ - env: { bus }, - }); - const thread = this.env.models['mail.thread'].create({ - id: 100, - model: 'res.partner', - }); - const follower = await this.env.models['mail.follower'].create({ - channel: [['insert', { id: 10, model: 'mail.channel', name: "channel" }]], - followedThread: [['link', thread]], - id: 2, - isActive: true, - isEditable: true, - }); - await this.createFollowerComponent(follower); - assert.containsOnce( - document.body, - '.o_Follower', - "should have follower component" - ); - assert.containsOnce( - document.body, - '.o_Follower_details', - "should display a details part" - ); - - document.querySelector('.o_Follower_details').click(); - assert.verifySteps( - ['do_action'], - "clicking on channel should redirect to channel form view" - ); -}); - QUnit.test('click on partner follower details', async function (assert) { assert.expect(7); diff --git a/addons/mail/static/src/components/follower_subtype/follower_subtype_tests.js b/addons/mail/static/src/components/follower_subtype/follower_subtype_tests.js index 7c802a7a067..c39f2dc1b77 100644 --- a/addons/mail/static/src/components/follower_subtype/follower_subtype_tests.js +++ b/addons/mail/static/src/components/follower_subtype/follower_subtype_tests.js @@ -53,9 +53,8 @@ QUnit.test('simplest layout of a followed subtype', async function (assert) { model: 'res.partner', }); const follower = this.env.models['mail.follower'].create({ - channel: [['insert', { + partner: [['insert', { id: 1, - model: 'mail.channel', name: "François Perusse", }]], followedThread: [['link', thread]], @@ -114,9 +113,8 @@ QUnit.test('simplest layout of a not followed subtype', async function (assert) model: 'res.partner', }); const follower = this.env.models['mail.follower'].create({ - channel: [['insert', { + partner: [['insert', { id: 1, - model: 'mail.channel', name: "François Perusse", }]], followedThread: [['link', thread]], @@ -172,9 +170,8 @@ QUnit.test('toggle follower subtype checkbox', async function (assert) { model: 'res.partner', }); const follower = this.env.models['mail.follower'].create({ - channel: [['insert', { + partner: [['insert', { id: 1, - model: 'mail.channel', name: "François Perusse", }]], followedThread: [['link', thread]], diff --git a/addons/mail/static/src/models/follower/follower.js b/addons/mail/static/src/models/follower/follower.js index 5ee353fdecc..49236545f08 100644 --- a/addons/mail/static/src/models/follower/follower.js +++ b/addons/mail/static/src/models/follower/follower.js @@ -55,10 +55,7 @@ function factory(dependencies) { * Opens the most appropriate view that is a profile for this follower. */ async openProfile() { - if (this.partner) { - return this.partner.openProfile(); - } - return this.channel.openProfile(); + return this.partner.openProfile(); } /** @@ -158,69 +155,9 @@ function factory(dependencies) { return `${this.modelName}_${data.id}`; } - /** - * @private - * @returns {string} - */ - _computeName() { - if (this.channel) { - return this.channel.name; - } - if (this.partner) { - return this.partner.name; - } - return ''; - } - - /** - * @private - * @returns {integer} - */ - _computeResId() { - if (this.partner) { - return this.partner.id; - } - if (this.channel) { - return this.channel.id; - } - return 0; - } - - /** - * @private - * @returns {string} - */ - _computeResModel() { - if (this.partner) { - return this.partner.model; - } - if (this.channel) { - return this.channel.model; - } - return ''; - } - } Follower.fields = { - resId: attr({ - compute: '_computeResId', - default: 0, - dependencies: [ - 'channelId', - 'partnerId', - ], - }), - channel: many2one('mail.thread'), - channelId: attr({ - related: 'channel.id', - }), - channelModel: attr({ - related: 'channel.model', - }), - channelName: attr({ - related: 'channel.name', - }), followedThread: many2one('mail.thread', { inverse: 'followers', }), @@ -234,29 +171,13 @@ function factory(dependencies) { default: false, }), name: attr({ - compute: '_computeName', - dependencies: [ - 'channelName', - 'partnerName', - ], - }), - partner: many2one('mail.partner'), - partnerId: attr({ - related: 'partner.id', - }), - partnerModel: attr({ - related: 'partner.model', - }), - partnerName: attr({ related: 'partner.name', }), - resModel: attr({ - compute: '_computeResModel', - default: '', - dependencies: [ - 'channelModel', - 'partnerModel', - ], + partner: many2one('mail.partner', { + required: true, + }), + partnerId: attr({ + related: 'partner.id', }), selectedSubtypes: many2many('mail.follower_subtype'), subtypes: many2many('mail.follower_subtype'), diff --git a/addons/mail/static/tests/helpers/mock_models.js b/addons/mail/static/tests/helpers/mock_models.js index 69b600c0a0e..70da8ce0393 100644 --- a/addons/mail/static/tests/helpers/mock_models.js +++ b/addons/mail/static/tests/helpers/mock_models.js @@ -108,7 +108,6 @@ class MockModels { }, 'mail.followers': { fields: { - channel_id: { type: 'integer' }, email: { type: 'char' }, id: { type: 'integer' }, is_active: { type: 'boolean' }, diff --git a/addons/mail/static/tests/helpers/mock_server.js b/addons/mail/static/tests/helpers/mock_server.js index 23fba5ccc93..7bae05d9baa 100644 --- a/addons/mail/static/tests/helpers/mock_server.js +++ b/addons/mail/static/tests/helpers/mock_server.js @@ -1611,7 +1611,6 @@ MockServer.include({ * @param {string} model not in server method but necessary for thread mock * @param {integer[]} ids * @param {integer[]} partner_ids - * @param {integer[]} channel_ids * @param {integer[]} subtype_ids * @returns {boolean} */ diff --git a/addons/mail/views/mail_followers_views.xml b/addons/mail/views/mail_followers_views.xml index d4ba0a0411c..b09d9b50a3b 100644 --- a/addons/mail/views/mail_followers_views.xml +++ b/addons/mail/views/mail_followers_views.xml @@ -12,7 +12,6 @@ - @@ -27,7 +26,6 @@ - diff --git a/addons/mail/wizard/invite.py b/addons/mail/wizard/invite.py index 229fa65d99f..b9659fb5543 100644 --- a/addons/mail/wizard/invite.py +++ b/addons/mail/wizard/invite.py @@ -71,7 +71,7 @@ class Invite(models.TransientModel): }) partners_data = [] recipient_data = self.env['mail.followers']._get_recipient_data(document, 'comment', False, pids=new_partners.ids) - for pid, cid, active, pshare, ctype, notif, groups in recipient_data: + for pid, active, pshare, notif, groups in recipient_data: pdata = {'id': pid, 'share': pshare, 'active': active, 'notif': 'email', 'groups': groups or []} if not pshare and notif: # has an user and is not shared, is therefore user partners_data.append(dict(pdata, type='user')) diff --git a/addons/mail/wizard/mail_resend_message.py b/addons/mail/wizard/mail_resend_message.py index f7cbd78b448..e6568ed184f 100644 --- a/addons/mail/wizard/mail_resend_message.py +++ b/addons/mail/wizard/mail_resend_message.py @@ -63,7 +63,7 @@ class MailResendMessage(models.TransientModel): record = self.env[message.model].browse(message.res_id) if message.is_thread_message() else self.env['mail.thread'] email_partners_data = [] - for pid, cid, active, pshare, ctype, notif, groups in self.env['mail.followers']._get_recipient_data(None, 'comment', False, pids=to_send.ids): + for pid, active, pshare, notif, groups in self.env['mail.followers']._get_recipient_data(None, 'comment', False, pids=to_send.ids): if pid and notif == 'email' or not notif: pdata = {'id': pid, 'share': pshare, 'active': active, 'notif': 'email', 'groups': groups or []} if not pshare and notif: # has an user and is not shared, is therefore user diff --git a/addons/sms/models/mail_followers.py b/addons/sms/models/mail_followers.py index 76be79b677b..d45a9556f74 100644 --- a/addons/sms/models/mail_followers.py +++ b/addons/sms/models/mail_followers.py @@ -7,18 +7,18 @@ from odoo import api, fields, models class Followers(models.Model): _inherit = ['mail.followers'] - def _get_recipient_data(self, records, message_type, subtype_id, pids=None, cids=None): + def _get_recipient_data(self, records, message_type, subtype_id, pids=None): if message_type == 'sms': if pids is None: sms_pids = records._sms_get_default_partners().ids else: sms_pids = pids - res = super(Followers, self)._get_recipient_data(records, message_type, subtype_id, pids=pids, cids=cids) + res = super(Followers, self)._get_recipient_data(records, message_type, subtype_id, pids=pids) new_res = [] - for pid, cid, pactive, pshare, ctype, notif, groups in res: + for pid, active, pshare, notif, groups in res: if pid and pid in sms_pids: notif = 'sms' - new_res.append((pid, cid, pactive, pshare, ctype, notif, groups)) + new_res.append((pid, active, pshare, notif, groups)) return new_res else: - return super(Followers, self)._get_recipient_data(records, message_type, subtype_id, pids=pids, cids=cids) + return super(Followers, self)._get_recipient_data(records, message_type, subtype_id, pids=pids) diff --git a/addons/sms/wizard/sms_resend.py b/addons/sms/wizard/sms_resend.py index 24d59c1a5ea..b7eb6395820 100644 --- a/addons/sms/wizard/sms_resend.py +++ b/addons/sms/wizard/sms_resend.py @@ -86,7 +86,7 @@ class SMSResend(models.TransientModel): numbers = [r.sms_number for r in self.recipient_ids if r.resend and not r.partner_id] rdata = [] - for pid, cid, active, pshare, ctype, notif, groups in self.env['mail.followers']._get_recipient_data(record, 'sms', False, pids=pids): + for pid, active, pshare, notif, groups in self.env['mail.followers']._get_recipient_data(record, 'sms', False, pids=pids): if pid and notif == 'sms': rdata.append({'id': pid, 'share': pshare, 'active': active, 'notif': notif, 'groups': groups or [], 'type': 'customer' if pshare else 'user'}) if rdata or numbers: diff --git a/addons/test_mail/tests/test_invite.py b/addons/test_mail/tests/test_invite.py index c4e91a86173..dc543d1963b 100644 --- a/addons/test_mail/tests/test_invite.py +++ b/addons/test_mail/tests/test_invite.py @@ -26,8 +26,6 @@ class TestInvite(TestMailCommon): # check added followers and that emails were sent self.assertEqual(test_record.message_partner_ids, test_partner | self.user_admin.partner_id) - self.assertEqual(test_record.message_follower_ids.mapped('channel_id'), - self.env['mail.channel']) self.assertSentEmail(self.partner_employee, [test_partner]) self.assertSentEmail(self.partner_employee, [self.partner_admin]) self.assertEqual(len(self._mails), 2) diff --git a/addons/test_mail/tests/test_mail_followers.py b/addons/test_mail/tests/test_mail_followers.py index b2b8f152299..ace318df519 100644 --- a/addons/test_mail/tests/test_mail_followers.py +++ b/addons/test_mail/tests/test_mail_followers.py @@ -126,18 +126,6 @@ class BaseFollowersTest(TestMailCommon): test_record.message_subscribe(partner_ids=[self.user_employee.partner_id.id], subtype_ids=[self.mt_cl_def.id, self.mt_al_def.id]) self.assertEqual(follower.subtype_ids, self.mt_cl_def | self.mt_al_def) - def test_followers_no_DID(self): - """Test that a follower cannot suffer from dissociative identity disorder. - It cannot be both a partner and a channel. - """ - with self.assertRaises(IntegrityError), mute_logger('odoo.sql_db'): - self.env['mail.followers'].create({ - 'res_model': self.test_record._name, - 'res_id': self.test_record.id, - 'partner_id': self.user_employee.partner_id.id, - 'channel_id': self.channel_listen.id, - }) - @users('employee') def test_followers_inactive(self): """ Test standard API does not subscribe inactive partners """ @@ -435,10 +423,10 @@ class DuplicateNotificationTest(TestMailCommon): self.assertEqual(notif.notification_type, 'email') subtype = self.env.ref('mail.mt_comment') - res = self.env['mail.followers']._get_recipient_data(test, 'comment', subtype.id, pids=common_partner.ids) + res = self.env['mail.followers']._get_recipient_data(test, 'comment', subtype.id, pids=common_partner.ids) partner_notif = [r for r in res if r[0] == common_partner.id] self.assertEqual(len(partner_notif), 1) - self.assertEqual(partner_notif[0][5], 'email') + self.assertEqual(partner_notif[0][3], 'email') @tagged('post_install', '-at_install') class UnlinkedNotificationTest(TestMailCommon): diff --git a/addons/test_mail/tests/test_mail_thread_internals.py b/addons/test_mail/tests/test_mail_thread_internals.py index 4e1030dd3f3..90545d5c2de 100644 --- a/addons/test_mail/tests/test_mail_thread_internals.py +++ b/addons/test_mail/tests/test_mail_thread_internals.py @@ -24,7 +24,6 @@ class TestChatterTweaks(TestMailCommon, TestRecipients): self.test_record.with_user(self.user_employee).with_context({'mail_create_nosubscribe': True}).message_post( body='Test Body', message_type='comment', subtype_xmlid='mail.mt_comment') self.assertEqual(self.test_record.message_follower_ids.mapped('partner_id'), original.mapped('partner_id')) - self.assertEqual(self.test_record.message_follower_ids.mapped('channel_id'), original.mapped('channel_id')) @mute_logger('odoo.addons.mail.models.mail_mail') def test_post_no_subscribe_recipients(self): @@ -32,7 +31,6 @@ class TestChatterTweaks(TestMailCommon, TestRecipients): self.test_record.with_user(self.user_employee).with_context({'mail_create_nosubscribe': True}).message_post( body='Test Body', message_type='comment', subtype_xmlid='mail.mt_comment', partner_ids=[self.partner_1.id, self.partner_2.id]) self.assertEqual(self.test_record.message_follower_ids.mapped('partner_id'), original.mapped('partner_id')) - self.assertEqual(self.test_record.message_follower_ids.mapped('channel_id'), original.mapped('channel_id')) @mute_logger('odoo.addons.mail.models.mail_mail') def test_post_subscribe_recipients(self): @@ -40,7 +38,6 @@ class TestChatterTweaks(TestMailCommon, TestRecipients): self.test_record.with_user(self.user_employee).with_context({'mail_create_nosubscribe': True, 'mail_post_autofollow': True}).message_post( body='Test Body', message_type='comment', subtype_xmlid='mail.mt_comment', partner_ids=[self.partner_1.id, self.partner_2.id]) self.assertEqual(self.test_record.message_follower_ids.mapped('partner_id'), original.mapped('partner_id') | self.partner_1 | self.partner_2) - self.assertEqual(self.test_record.message_follower_ids.mapped('channel_id'), original.mapped('channel_id')) def test_chatter_mail_create_nolog(self): """ Test disable of automatic chatter message at create """