From a54457e04dc6937ce16a87c97ca30e6fea9568ba Mon Sep 17 00:00:00 2001 From: Mitali Patel Date: Fri, 15 Mar 2019 07:11:16 +0000 Subject: [PATCH] [IMP] mail: better exception activity handling in kanban and tree The purpose of this task is to add a visual indicator on kanban and list views when there is an exception activity on a record. To do so, two fields have been added on the activity mixin that determine if the record has an exception activity and its corresponding icon. When there's an exception activity, the next activity icon is different in the kanban and in the list view, the icon is displayed at the end of the column with no label. Prefetching all activity_type_ids in compute will allow to lessen queries (3 query for this compute) and cache may benefit to other mail.activity.mixin computed field, like activity_state in kanban view Task 1889379 --- addons/mail/models/mail_activity.py | 29 ++++++ addons/mail/static/src/js/activity.js | 67 +++++++++++-- .../static/src/xml/web_kanban_activity.xml | 5 +- addons/mail/static/tests/chatter_tests.js | 99 ++++++++++++++++++- 4 files changed, 185 insertions(+), 15 deletions(-) diff --git a/addons/mail/models/mail_activity.py b/addons/mail/models/mail_activity.py index e1be4f3f250..bc17d710f79 100644 --- a/addons/mail/models/mail_activity.py +++ b/addons/mail/models/mail_activity.py @@ -590,6 +590,35 @@ class MailActivityMixin(models.AbstractModel): related='activity_ids.summary', readonly=False, search='_search_activity_summary', groups="base.group_user",) + activity_exception_decoration = fields.Selection([ + ('warning', 'Alert'), + ('danger', 'Error')], + compute='_compute_activity_exception_type', + search='_search_activity_exception_decoration', + help="Type of the exception activity on record.") + activity_exception_icon = fields.Char('Icon', help="Icon to indicate an exception activity.", + compute='_compute_activity_exception_type') + + @api.depends('activity_ids.activity_type_id.decoration_type', 'activity_ids.activity_type_id.icon') + def _compute_activity_exception_type(self): + # prefetch all activity types for all activities, this will avoid any query in loops + self.mapped('activity_ids.activity_type_id.decoration_type') + + for record in self: + activity_type_ids = record.activity_ids.mapped('activity_type_id') + exception_activity_type_id = False + for activity_type_id in activity_type_ids: + if activity_type_id.decoration_type == 'danger': + exception_activity_type_id = activity_type_id + break + if activity_type_id.decoration_type == 'warning': + exception_activity_type_id = activity_type_id + if exception_activity_type_id: + record.activity_exception_decoration = exception_activity_type_id.decoration_type + record.activity_exception_icon = exception_activity_type_id.icon + + def _search_activity_exception_decoration(self, operator, operand): + return [('activity_ids.activity_type_id.decoration_type', operator, operand)] @api.depends('activity_ids.state') def _compute_activity_state(self): diff --git a/addons/mail/static/src/js/activity.js b/addons/mail/static/src/js/activity.js index 79bb48819d5..5596e407c09 100644 --- a/addons/mail/static/src/js/activity.js +++ b/addons/mail/static/src/js/activity.js @@ -650,6 +650,10 @@ var KanbanActivity = BasicActivity.extend({ events:_.extend({}, BasicActivity.prototype.events, { 'show.bs.dropdown': '_onDropdownShow', }), + fieldDependencies: _.extend({}, BasicActivity.prototype.fieldDependencies, { + activity_exception_decoration: {type: 'selection'}, + activity_exception_icon: {type: 'char'} + }), /** * @override @@ -685,13 +689,18 @@ var KanbanActivity = BasicActivity.extend({ * @private */ _render: function () { - var $span = this.$('.o_activity_btn > span'); - $span.removeClass(function (index, classNames) { - return classNames.split(/\s+/).filter(function (className) { - return _.str.startsWith(className, 'o_activity_color_'); - }).join(' '); - }); - $span.addClass('o_activity_color_' + (this.activityState || 'default')); + // span classes need to be updated manually because the template cannot + // be re-rendered eaasily (because of the dropdown state) + const spanClasses = ['fa', 'fa-lg', 'fa-fw']; + spanClasses.push('o_activity_color_' + (this.activityState || 'default')); + if (this.recordData.activity_exception_decoration) { + spanClasses.push('text-' + this.recordData.activity_exception_decoration); + spanClasses.push(this.recordData.activity_exception_icon); + } else { + spanClasses.push('fa-clock-o'); + } + this.$('.o_activity_btn > span').removeClass().addClass(spanClasses.join(' ')); + if (this.$el.hasClass('show')) { // note: this part of the rendering might be asynchronous this._renderDropdown(); @@ -744,9 +753,51 @@ var KanbanActivity = BasicActivity.extend({ }, }); +// ----------------------------------------------------------------------------- +// Activity Exception Widget to display Exception icon ('activity_exception' widget) +// ----------------------------------------------------------------------------- + +var ActivityException = AbstractField.extend({ + noLabel: true, + fieldDependencies: _.extend({}, AbstractField.prototype.fieldDependencies, { + activity_exception_icon: {type: 'char'} + }), + + //------------------------------------------------------------ + // Private + //------------------------------------------------------------ + + /** + * There is no edit mode for this widget, the icon is always readonly. + * + * @override + * @private + */ + _renderEdit: function () { + return this._renderReadonly(); + }, + + /** + * Displays the exception icon if there is one. + * + * @override + * @private + */ + _renderReadonly: function () { + this.$el.empty(); + if (this.value) { + this.$el.attr({ + 'title': _t('This record has an exception activity.'), + 'class': "pull-right mt-1 text-" + this.value + " fa " + this.recordData.activity_exception_icon + }); + } + } +}); + field_registry .add('mail_activity', Activity) - .add('kanban_activity', KanbanActivity); + .add('kanban_activity', KanbanActivity) + .add('activity_exception', ActivityException); return Activity; diff --git a/addons/mail/static/src/xml/web_kanban_activity.xml b/addons/mail/static/src/xml/web_kanban_activity.xml index 4d6c290f11f..2c5574c826c 100644 --- a/addons/mail/static/src/xml/web_kanban_activity.xml +++ b/addons/mail/static/src/xml/web_kanban_activity.xml @@ -4,8 +4,9 @@
- - + + + diff --git a/addons/mail/static/tests/chatter_tests.js b/addons/mail/static/tests/chatter_tests.js index bb569645c4d..1c57150e28c 100644 --- a/addons/mail/static/tests/chatter_tests.js +++ b/addons/mail/static/tests/chatter_tests.js @@ -6,6 +6,7 @@ var mailTestUtils = require('mail.testUtils'); var core = require('web.core'); var FormView = require('web.FormView'); var KanbanView = require('web.KanbanView'); +var ListView = require('web.ListView'); var testUtils = require('web.test_utils'); var concurrency = require('web.concurrency'); @@ -60,6 +61,15 @@ QUnit.module('Chatter', { relation: 'mail.activity', relation_field: 'res_id', }, + activity_exception_decoration: { + string: 'Decoration', + type: 'selection', + selection: [['warning', 'Alert'], ['danger', 'Error']], + }, + activity_exception_icon: { + string: 'icon', + type: 'char', + }, activity_state: { string: 'State', type: 'selection', @@ -109,11 +119,14 @@ QUnit.module('Chatter', { type: 'selection', selection: [['default', 'Other'], ['upload_file', 'Upload File']], }, + decoration_type: { string: "Decoration Type", type: "selection", selection: [['warning', 'Alert'], ['danger', 'Error']]}, + icon: {string: 'icon', type:"char"}, }, records: [ { id: 1, name: "Type 1" }, { id: 2, name: "Type 2" }, { id: 3, name: "Type 3", category: 'upload_file' }, + { id: 4, name: "Exception", decoration_type: "warning", icon: "fa-warning"} ], }, 'mail.message': { @@ -660,10 +673,12 @@ QUnit.test('kanban activity widget with no activity', async function (assert) { }); QUnit.test('kanban activity widget with an activity', async function (assert) { - assert.expect(12); + assert.expect(14); - this.data.partner.records[0].activity_ids = [1]; + this.data.partner.records[0].activity_ids = [1, 2]; this.data.partner.records[0].activity_state = 'today'; + this.data.partner.records[0].activity_exception_decoration = 'warning'; + this.data.partner.records[0].activity_exception_icon = 'fa-warning'; this.data['mail.activity'].records = [{ id: 1, display_name: "An activity", @@ -673,6 +688,15 @@ QUnit.test('kanban activity widget with an activity', async function (assert) { user_id: 2, create_uid: 2, activity_type_id: 1, + },{ + id: 2, + display_name: "An exception activity", + date_deadline: moment().format("YYYY-MM-DD"), // now + can_write: true, + state: "today", + user_id: 2, + create_uid: 2, + activity_type_id: 4, }]; var rpcCount = 0; var kanban = await createView({ @@ -691,6 +715,7 @@ QUnit.test('kanban activity widget with an activity', async function (assert) { var current_ids = this.data.partner.records[0].activity_ids; var done_ids = args.args[0]; this.data.partner.records[0].activity_ids = _.difference(current_ids, done_ids); + this.data.partner.records[0].activity_exception_decoration = false; this.data.partner.records[0].activity_state = false; return Promise.resolve(); } @@ -704,13 +729,15 @@ QUnit.test('kanban activity widget with an activity', async function (assert) { "activity widget should have been rendered correctly"); assert.strictEqual(rpcCount, 1, '1 RPC (search_read) should have been done'); + // Check exception activity icon fa-warning + assert.hasClass($record.find('.o_activity_btn span'), "fa-warning", "should display correct exception activity icon"); // click on the activity button await testUtils.dom.click($record.find('.o_activity_btn')); assert.strictEqual(rpcCount, 2, 'a read should have been done to fetch the activity details'); - assert.strictEqual($record.find('.o_activity_title').length, 1, "should have an activity scheduled"); + assert.strictEqual($record.find('.o_activity_title').length, 2, "should have two activity scheduled"); var label = $record.find('.o_activity_log .o_activity_color_today'); assert.strictEqual(label.find('strong').text(), "Today", "should display the correct label"); - assert.strictEqual(label.find('.badge-warning').text(), "1", "should display the correct count"); + assert.strictEqual(label.find('.badge-warning').text(), "2", "should display the correct count"); // click on the activity button to close the dropdown await testUtils.dom.click($record.find('.o_activity_btn')); @@ -720,11 +747,19 @@ QUnit.test('kanban activity widget with an activity', async function (assert) { await testUtils.dom.click($record.find('.o_activity_btn')); assert.strictEqual(rpcCount, 3, 'should have reloaded the activities'); + // mark exception activity as done + await testUtils.dom.click($record.find('.o_mark_as_done:eq(1)')); + await testUtils.dom.click($record.find('.o_activity_popover_done')); + $record = kanban.$('.o_kanban_record').first(); // the record widget has been reset + + // check activity icon replaced with fa-clock + assert.hasClass($record.find('.o_activity_btn span'), "fa-clock-o", "should display correct activity icon"); + // mark activity as done await testUtils.dom.click($record.find('.o_mark_as_done')); await testUtils.dom.click($record.find('.o_activity_popover_done')); $record = kanban.$('.o_kanban_record').first(); // the record widget has been reset - assert.strictEqual(rpcCount, 5, 'should have done an RPC to mark activity as done, and a read'); + assert.strictEqual(rpcCount, 8, '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.show').length, 1, @@ -792,6 +827,60 @@ QUnit.test('kanban activity widget popover test', async function (assert) { kanban.destroy(); }); +QUnit.test('list activity exception widget with activity', async function (assert) { + assert.expect(3); + + this.data.partner.records[0].activity_ids = [1]; + this.data.partner.records.push({ + id: 3, + message_attachment_count: 3, + display_name: "second partner", + foo: "Tommy", + message_follower_ids: [], + message_ids: [], + activity_ids: [2], + activity_exception_decoration: 'warning', + activity_exception_icon: 'fa-warning', + }); + this.data['mail.activity'].records = [{ + id: 1, + display_name: "An activity", + date_deadline: moment().format("YYYY-MM-DD"), // now + can_write: true, + state: "today", + user_id: 2, + create_uid: 2, + activity_type_id: 1, + },{ + id: 2, + display_name: "An exception activity", + date_deadline: moment().format("YYYY-MM-DD"), // now + can_write: true, + state: "today", + user_id: 2, + create_uid: 2, + activity_type_id: 4, + }]; + + var list = await createView({ + View: ListView, + model: 'partner', + data: this.data, + arch: '' + + ''+ + ' ' + + '', + }); + + assert.containsN(list, '.o_data_row', 2, "should have two records"); + assert.doesNotHaveClass(list.$('.o_data_row:eq(0) .o_activity_exception_cell div'), 'fa-warning', + "there is no any exception activity on record"); + assert.hasClass(list.$('.o_data_row:eq(1) .o_activity_exception_cell div'), 'fa-warning', + "there is an exception on a record"); + + list.destroy(); +}); + QUnit.test('chatter: post, receive and star messages', async function (assert) { assert.expect(26);