From 3809fc891f5b4794493b6ad4ed4bc4000c9a416e Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Wed, 7 Mar 2018 09:23:24 +0100 Subject: [PATCH] [FIX] web,mail: always reload widget mail_activity Before this rev., the field widget 'mail_activity' kept an internal cache of fetched activities (like the 'mail_thread' and 'mail_followers'). However, for activities, this cache may cause issues. For instance, create a new lead in 'My pipeline', schedule a 'Meeting' activity on that lead (it opens the Calendar view so that real meetings can be created), create a meeting 2 weeks from now, go back to the lead form view: the activity still diplays that the meeting is today (when the view is refreshed, the correct date is displayed). This rev. removes the internal cache for this field widget, as the activities can easily be updated in business flows (unlike messages or followers). This rev. also fixes a bug in the BasicModel, where the res_ids of a list datapoint was altered in place to return a list of x2many commands instead of the list of ids when being stringified. This caused a traceback when opening a lead with activities, as soon as the internal cache had been removed. --- addons/mail/static/src/js/activity.js | 147 ++++++++++-------- addons/mail/static/tests/chatter_tests.js | 125 ++++++++++++++- .../static/src/js/views/basic/basic_model.js | 2 +- 3 files changed, 206 insertions(+), 68 deletions(-) diff --git a/addons/mail/static/src/js/activity.js b/addons/mail/static/src/js/activity.js index 59b32f32023..5da6abd0eb1 100644 --- a/addons/mail/static/src/js/activity.js +++ b/addons/mail/static/src/js/activity.js @@ -2,7 +2,7 @@ odoo.define('mail.Activity', function (require) { "use strict"; var AbstractField = require('web.AbstractField'); -var concurrency = require('web.concurrency'); +var BasicModel = require('web.BasicModel'); var core = require('web.core'); var field_registry = require('web.field_registry'); var time = require('web.time'); @@ -11,6 +11,63 @@ var utils = require('mail.utils'); var QWeb = core.qweb; var _t = core._t; +/** + * Fetches activities and postprocesses them. + * + * This standalone function performs an RPC, but to do so, it needs an instance + * of a widget that implements the _rpc() function. + * + * @todo i'm not very proud of the widget instance given in arguments, we should + * probably try to do it a better way in the future. + * + * @param {Widget} self a widget instance that can perform RPCs + * @param {Array} ids the ids of activities to read + * @return {Deferred} resolved with the activities + */ +function _readActivities(self, ids) { + if (!ids.length) { + return $.when([]); + } + return self._rpc({ + model: 'mail.activity', + method: 'read', + args: [ids], + }).then(function (activities) { + // convert create_date and date_deadline to moments + _.each(activities, function (activity) { + activity.create_date = moment(time.auto_str_to_date(activity.create_date)); + activity.date_deadline = moment(time.auto_str_to_date(activity.date_deadline)); + }); + + // sort activities by due date + activities = _.sortBy(activities, 'date_deadline'); + + return activities; + }); +} + +BasicModel.include({ + //-------------------------------------------------------------------------- + // Private + //-------------------------------------------------------------------------- + + /** + * Fetches the activities displayed by the activity field widget in form + * views. + * + * @private + * @param {Object} record - an element from the localData + * @param {string} fieldName + * @return {Deferred} resolved with the activities + */ + _fetchSpecialActivity: function (record, fieldName) { + var localID = (record._changes && fieldName in record._changes) ? + record._changes[fieldName] : + record.data[fieldName]; + return _readActivities(this, this.localData[localID].res_ids); + }, +}); + /** * Set the 'label_delay' entry in activity data according to the deadline date * @param {Array} activities list of activity Object @@ -44,12 +101,6 @@ var setDelayLabel = function(activities){ }; var AbstractActivityField = AbstractField.extend({ - // inherited - init: function () { - this._super.apply(this, arguments); - this.activities = []; - }, - // private _markActivityDone: function (id, feedback) { return this._rpc({ @@ -59,33 +110,6 @@ var AbstractActivityField = AbstractField.extend({ kwargs: {feedback: feedback}, }); }, - _readActivities: function () { - var self = this; - var missing_ids = _.difference(this.value.res_ids, _.pluck(this.activities, 'id')); - var fetch_def; - if (missing_ids.length) { - fetch_def = this._rpc({ - model: 'mail.activity', - method: 'read', - args: [missing_ids], - }); - } - return $.when(fetch_def).then(function (results) { - // convert create_date and date_deadline to moments - _.each(results, function (activity) { - activity.create_date = moment(time.auto_str_to_date(activity.create_date)); - activity.date_deadline = moment(time.auto_str_to_date(activity.date_deadline)); - }); - - // filter out activities that are no longer linked to this record - self.activities = _.filter(self.activities.concat(results || []), function (activity) { - return _.contains(self.value.res_ids, activity.id); - }); - - // sort activities by due date - self.activities = _.sortBy(self.activities, 'date_deadline'); - }); - }, _scheduleActivity: function (id, previous_activity_type_id, callback) { var action = { type: 'ir.actions.act_window', @@ -115,41 +139,37 @@ var Activity = AbstractActivityField.extend({ 'click .o_activity_unlink': '_onUnlinkActivity', 'click .o_activity_done': '_onMarkActivityDone', }, + specialData: '_fetchSpecialActivity', // inherited init: function () { this._super.apply(this, arguments); - this.dp = new concurrency.DropPrevious(); + this.activities = this.record.specialData[this.name]; }, _render: function () { - // note: the rendering of this widget is asynchronous as it needs to - // fetch the details of the linked activities - var self = this; - var fetch_def = this.dp.add(this._readActivities()); - return fetch_def.then(function () { - _.each(self.activities, function (activity) { - if (activity.note) { - activity.note = utils.parse_and_transform(activity.note, utils.add_link); - } - }); - var activities = setDelayLabel(self.activities); - if (activities.length) { - var nbActivities = _.countBy(activities, 'state'); - self.$el.html(QWeb.render('mail.activity_items', { - activities: activities, - nbPlannedActivities: nbActivities.planned, - nbTodayActivities: nbActivities.today, - nbOverdueActivities: nbActivities.overdue, - date_format: time.getLangDateFormat(), - datetime_format: time.getLangDatetimeFormat(), - })); - } else { - self.$el.empty(); + _.each(this.activities, function (activity) { + if (activity.note) { + activity.note = utils.parse_and_transform(activity.note, utils.add_link); } }); + var activities = setDelayLabel(this.activities); + if (activities.length) { + var nbActivities = _.countBy(activities, 'state'); + this.$el.html(QWeb.render('mail.activity_items', { + activities: activities, + nbPlannedActivities: nbActivities.planned, + nbTodayActivities: nbActivities.today, + nbOverdueActivities: nbActivities.overdue, + date_format: time.getLangDateFormat(), + datetime_format: time.getLangDatetimeFormat(), + })); + } else { + this.$el.empty(); + } }, _reset: function (record) { this._super.apply(this, arguments); + this.activities = this.record.specialData[this.name]; // the mail widgets being persistent, one need to update the res_id on reset this.res_id = record.res_id; }, @@ -184,9 +204,6 @@ var Activity = AbstractActivityField.extend({ }); return this.do_action(action, { on_close: function () { - // remove the edited activity from the array of fetched activities to - // force a reload of that activity - self.activities = _.reject(self.activities, {id: activity_id}); self._reload({activity: true, thread: true}); }, }); @@ -300,10 +317,10 @@ var KanbanActivity = AbstractActivityField.extend({ _renderDropdown: function () { var self = this; this.$('.o_activity').html(QWeb.render("mail.KanbanActivityLoading")); - return this._readActivities().then(function () { + return _readActivities(this, this.value.res_ids).then(function (activities) { self.$('.o_activity').html(QWeb.render("mail.KanbanActivityDropdown", { selection: self.selection, - records: _.groupBy(setDelayLabel(self.activities), 'state'), + records: _.groupBy(setDelayLabel(activities), 'state'), uid: self.getSession().uid, })); }); @@ -316,7 +333,9 @@ var KanbanActivity = AbstractActivityField.extend({ // handlers _onButtonClicked: function (event) { event.preventDefault(); - this._renderDropdown(); + if (!this.$el.hasClass('open')) { + this._renderDropdown(); + } }, _onMarkActivityDone: function (event) { event.stopPropagation(); diff --git a/addons/mail/static/tests/chatter_tests.js b/addons/mail/static/tests/chatter_tests.js index 9cf98987778..5c986273ad7 100644 --- a/addons/mail/static/tests/chatter_tests.js +++ b/addons/mail/static/tests/chatter_tests.js @@ -368,12 +368,12 @@ QUnit.test('kanban activity widget with an activity', function (assert) { // click on the activity button to re-open dropdown $record.find('.o_activity_btn').click(); - assert.strictEqual(rpcCount, 2, 'no RPC should be done as the activities are now in cache'); + assert.strictEqual(rpcCount, 3, 'should have reloaded the activities'); // mark activity as done $record.find('.o_mark_as_done').click(); $record = kanban.$('.o_kanban_record').first(); // the record widget has been reset - assert.strictEqual(rpcCount, 4, 'should have done an RPC to mark activity as done, and a read'); + assert.strictEqual(rpcCount, 5, 'should have done an RPC to mark activity as done, and a read'); assert.ok($record.find('.o_mail_activity .o_activity_color_default:not(.o_activity_color_today)').length, "activity widget should have been updated correctly"); assert.strictEqual($record.find('.o_mail_activity.open').length, 1, @@ -732,6 +732,125 @@ QUnit.test('chatter: Attachment viewer', function (assert) { form.destroy(); }); +QUnit.test('form activity widget: read RPCs', function (assert) { + // Records of model 'mail.activity' may be updated in business flows (e.g. + // the date of a 'Meeting' activity is updated when the associated meeting + // is dragged&dropped in the Calendar view). Because of that, the activities + // must be reloaded when the form is reloaded (the widget can't keep an + // internal cache). + assert.expect(6); + this.data.partner.records[0].activity_ids = [1]; + this.data.partner.records[0].activity_state = 'today'; + this.data['mail.activity'].records = [{ + id: 1, + display_name: "An activity", + date_deadline: moment().format("YYYY-MM-DD"), // now + state: "today", + user_id: 2, + activity_type_id: 2, + }]; + + var nbReads = 0; + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '
' + + '
' + + '' + + '
' + + '
', + res_id: 2, + mockRPC: function (route, args) { + if (args.method === 'read' && args.model === 'mail.activity') { + nbReads++; + } + return this._super.apply(this, arguments); + }, + }); + + assert.strictEqual(nbReads, 1, "should have read the activities"); + assert.strictEqual(form.$('.o_mail_activity .o_thread_message').length, 1, + "should display an activity"); + assert.strictEqual(form.$('.o_mail_activity .o_thread_message .o_activity_date').text(), + 'Today', "the activity should be today"); + + form.$buttons.find('.o_form_button_edit').click(); + form.$buttons.find('.o_form_button_save').click(); + + assert.strictEqual(nbReads, 1, "should not have re-read the activities"); + + // simulate a date change, and a reload of the form view + var tomorrow = moment().add(1, 'day').format("YYYY-MM-DD"); + this.data['mail.activity'].records[0].date_deadline = tomorrow; + form.reload(); + + assert.strictEqual(nbReads, 2, "should have re-read the activities"); + assert.strictEqual(form.$('.o_mail_activity .o_thread_message .o_activity_date').text(), + 'Tomorrow', "the activity should be tomorrow"); + + form.destroy(); +}); + +QUnit.test('form activity widget on a new record', function (assert) { + assert.expect(0); + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '
' + + '
' + + '' + + '
' + + '
', + mockRPC: function (route, args) { + if (args.method === 'read' && args.model === 'mail.activity') { + throw new Error("should not do a read on mail.activity"); + } + return this._super.apply(this, arguments); + }, + }); + + form.destroy(); +}); + +QUnit.test('form activity widget with another x2many field in view', function (assert) { + assert.expect(1); + + this.data.partner.fields.m2m = {string: "M2M", type: 'many2many', relation: 'partner'}; + + this.data.partner.records[0].m2m = [2]; + this.data.partner.records[0].activity_ids = [1]; + this.data.partner.records[0].activity_state = 'today'; + this.data['mail.activity'].records = [{ + id: 1, + display_name: "An activity", + date_deadline: moment().format("YYYY-MM-DD"), // now + state: "today", + user_id: 2, + activity_type_id: 2, + }]; + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '
' + + '' + + '
' + + '' + + '
' + + '', + res_id: 2, + }); + + assert.strictEqual(form.$('.o_mail_activity .o_thread_message').length, 1, + "should display an activity"); + + form.destroy(); +}); + QUnit.test('form activity widget: schedule next activity', function (assert) { assert.expect(5); this.data.partner.records[0].activity_ids = [1]; @@ -767,7 +886,7 @@ QUnit.test('form activity widget: schedule next activity', function (assert) { "the feedback should be sent correctly"); return $.when(); } - if (args.method === 'read' && checkReadArgs) { + if (args.method === 'read' && args.model === 'partner' && checkReadArgs) { assert.deepEqual(args.args[1], ['activity_ids', 'message_ids', 'display_name'], "should only read the mail fields"); } diff --git a/addons/web/static/src/js/views/basic/basic_model.js b/addons/web/static/src/js/views/basic/basic_model.js index c7f61edd541..c8bd4ae030f 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -3047,7 +3047,7 @@ var BasicModel = AbstractModel.extend({ if (field.type === 'one2many' || field.type === 'many2many') { var ids; if (!context[fieldName] || _.isArray(context[fieldName])) { // no dataPoint created yet - ids = context[fieldName] || []; + ids = context[fieldName] ? context[fieldName].slice(0) : []; } else { relDataPoint = this._applyX2ManyOperations(this.localData[context[fieldName]]); ids = relDataPoint.res_ids.slice(0);