From c593e7d5985f2332d3d0534eb46a742d459d4fd1 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Thu, 20 Feb 2020 12:56:51 +0000 Subject: [FIX] website_mail: avoid sending too many RPCs Before this commit, a JS widget would be instanciated for every `.js_follow` on the page. Every widget would then perform a RPC to know if the object was followed. That would be an issue if there was too many `.js_follow` on the DOM. Eg: on the forum tag page (/forum/1/tag), there might be unlimited tags. This is where the biggest issue is ATM, on Odoo.com we have over 5500 tags, which performs 5500 RPC. Your browser will most of the time just crash. This commit improves that behavior by sending only one RPC to get the tags info in once. With 5000 tags, there was 5000 RPCs, now there will only be one for the tags. Also, each RPC were doing 8 requests each, so this will bring requests from 40.000 to 80. Fixes #45576 closes odoo/odoo#46278 Signed-off-by: Quentin Smetz (qsm) --- addons/website_mail/controllers/main.py | 42 ++++++---- addons/website_mail/static/src/js/follow.js | 117 ++++++++++++++++++++-------- 2 files changed, 113 insertions(+), 46 deletions(-) (limited to 'addons') diff --git a/addons/website_mail/controllers/main.py b/addons/website_mail/controllers/main.py index 822373eb4d6..cd0bd09b091 100644 --- a/addons/website_mail/controllers/main.py +++ b/addons/website_mail/controllers/main.py @@ -38,7 +38,21 @@ class WebsiteMail(http.Controller): return True @http.route(['/website_mail/is_follower'], type='json', auth="public", website=True) - def is_follower(self, model, res_id, **post): + def is_follower(self, records, **post): + """ Given a list of `models` containing a list of res_ids, return + the res_ids for which the user is follower and some practical info. + + :param records: dict of models containing record IDS, eg: { + 'res.model': [1, 2, 3..], + 'res.model2': [1, 2, 3..], + .. + } + + :returns: [ + {'is_user': True/False, 'email': 'admin@yourcompany.example.com'}, + {'res.model': [1, 2], 'res.model2': [1]} + ] + """ user = request.env.user partner = None public_user = request.website.user_id @@ -47,17 +61,19 @@ class WebsiteMail(http.Controller): elif request.session.get('partner_id'): partner = request.env['res.partner'].sudo().browse(request.session.get('partner_id')) - values = { + res = {} + if partner: + for model in records: + mail_followers_ids = request.env['mail.followers'].sudo().read_group([ + ('res_model', '=', model), + ('res_id', 'in', records[model]), + ('partner_id', '=', partner.id) + ], ['res_id', 'follow_count:count(id)'], ['res_id']) + # `read_group` will filter out the ones without count result + for m in mail_followers_ids: + res.setdefault(model, []).append(m['res_id']) + + return [{ 'is_user': user != public_user, 'email': partner.email if partner else "", - 'is_follower': False, - } - - record_sudo = request.env[model].sudo().browse(int(res_id)) - if partner and record_sudo.exists(): - values['is_follower'] = bool(request.env['mail.followers'].sudo().search_count([ - ('res_model', '=', model), - ('res_id', '=', record_sudo.id), - ('partner_id', '=', partner.id) - ])) - return values + }, res] diff --git a/addons/website_mail/static/src/js/follow.js b/addons/website_mail/static/src/js/follow.js index a6b4dcc6777..02ec8196b09 100644 --- a/addons/website_mail/static/src/js/follow.js +++ b/addons/website_mail/static/src/js/follow.js @@ -4,25 +4,41 @@ odoo.define('website_mail.follow', function (require) { var publicWidget = require('web.public.widget'); publicWidget.registry.follow = publicWidget.Widget.extend({ - selector: '.js_follow', + selector: '#wrapwrap:has(.js_follow)', disabledInEditableMode: false, + /** + * @override + */ start: function () { var self = this; - this.is_user = false; + this.isUser = false; + var $jsFollowEls = this.$el.find('.js_follow'); var always = function (data) { - self.is_user = data.is_user; - self.email = data.email; - self.toggle_subscription(data.is_follower, data.email); - self.$target.removeClass('d-none'); + self.isUser = data[0].is_user; + const $jsFollowToEnable = $jsFollowEls.filter(function () { + const model = this.dataset.object; + return model in data[1] && data[1][model].includes(parseInt(this.dataset.id)); + }); + self._toggleSubscription(true, data[0].email, $jsFollowToEnable); + self._toggleSubscription(false, data[0].email, $jsFollowEls.not($jsFollowToEnable)); + $jsFollowEls.removeClass('d-none'); }; + const records = {}; + for (const el of $jsFollowEls) { + const model = el.dataset.object; + if (!(model in records)) { + records[model] = []; + } + records[model].push(parseInt(el.dataset.id)); + } + this._rpc({ route: '/website_mail/is_follower', params: { - model: this.$target.data('object'), - res_id: this.$target.data('id'), + records: records, }, }).then(always).guardedCatch(always); @@ -31,50 +47,85 @@ publicWidget.registry.follow = publicWidget.Widget.extend({ $('.js_follow > .input-group-append.d-none').removeClass('d-none'); this.$target.find('.js_follow_btn, .js_unfollow_btn').on('click', function (event) { event.preventDefault(); - self._onClick(); + self._onClick(event); }); } return this._super.apply(this, arguments); }, - _onClick: function () { + + //-------------------------------------------------------------------------- + // Private + //-------------------------------------------------------------------------- + + /** + * Toggles subscription state for every given records. + * + * @private + * @param {boolean} follow + * @param {string} email + * @param {jQuery} $jsFollowEls + */ + _toggleSubscription: function (follow, email, $jsFollowEls) { + if (follow) { + this._updateSubscriptionDOM(follow, email, $jsFollowEls); + } else { + for (const el of $jsFollowEls) { + const follow = !email && el.getAttribute('data-unsubscribe'); + this._updateSubscriptionDOM(follow, email, $(el)); + } + } + }, + /** + * Updates subscription DOM for every given records. + * This should not be called directly, use `_toggleSubscription`. + * + * @private + * @param {boolean} follow + * @param {string} email + * @param {jQuery} $jsFollowEls + */ + _updateSubscriptionDOM: function (follow, email, $jsFollowEls) { + $jsFollowEls.find(".js_follow_btn").toggleClass('d-none', follow); + $jsFollowEls.find(".js_unfollow_btn").toggleClass('d-none', !follow); + $jsFollowEls.find('input.js_follow_email') + .val(email || "") + .attr("disabled", email && (follow || this.isUser) ? "disabled" : false); + $jsFollowEls.attr("data-follow", follow ? 'on' : 'off'); + }, + + //-------------------------------------------------------------------------- + // Handlers + //-------------------------------------------------------------------------- + + /** + * @private + * @param {Event} ev + */ + _onClick: function (ev) { var self = this; - var $email = this.$target.find(".js_follow_email"); + var $jsFollow = $(ev.currentTarget).closest('.js_follow'); + var $email = $jsFollow.find(".js_follow_email"); if ($email.length && !$email.val().match(/.+@.+/)) { - this.$target.addClass('o_has_error').find('.form-control, .custom-select').addClass('is-invalid'); + $jsFollow.addClass('o_has_error').find('.form-control, .custom-select').addClass('is-invalid'); return false; } - this.$target.removeClass('o_has_error').find('.form-control, .custom-select').removeClass('is-invalid'); + $jsFollow.removeClass('o_has_error').find('.form-control, .custom-select').removeClass('is-invalid'); var email = $email.length ? $email.val() : false; - if (email || this.is_user) { + if (email || this.isUser) { this._rpc({ route: '/website_mail/follow', params: { - 'id': +this.$target.data('id'), - 'object': this.$target.data('object'), - 'message_is_follower': this.$target.attr("data-follow") || "off", + 'id': +$jsFollow.data('id'), + 'object': $jsFollow.data('object'), + 'message_is_follower': $jsFollow.attr("data-follow") || "off", 'email': email, }, }).then(function (follow) { - self.toggle_subscription(follow, email); + self._toggleSubscription(follow, email, $jsFollow); }); } }, - toggle_subscription: function (follow, email) { - follow = follow || (!email && this.$target.attr('data-unsubscribe')); - if (follow) { - this.$target.find(".js_follow_btn").addClass('d-none'); - this.$target.find(".js_unfollow_btn").removeClass('d-none'); - } - else { - this.$target.find(".js_follow_btn").removeClass('d-none'); - this.$target.find(".js_unfollow_btn").addClass('d-none'); - } - this.$target.find('input.js_follow_email') - .val(email || "") - .attr("disabled", email && (follow || this.is_user) ? "disabled" : false); - this.$target.attr("data-follow", follow ? 'on' : 'off'); - }, }); }); -- cgit v1.2.3