From fc9855d419e0cc8e2bcc52055ca120f8401c4418 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Fri, 2 Mar 2018 12:22:57 +0100 Subject: [PATCH] [FIX] sale_timesheet: fix crash in project plan The Project Plan code is quite new, but does some unconventional stuff. It looks like a form view, but it is actually a client action with a custom search view. The problem fixed by this commit is that when we try to open the project plan from the project kanban view, it crashes. It was caused by a bad interaction between the dashboard search view and the action manager. The dashboard search view trigger a search event, which bubbles up to the action manager. Since the project plan is not loaded yet, the action manager assumed that the search event was for the kanban view instead of the project plan. The fix is to prevent the event from bubbling up, since the project plan already handles the search event anyway. Note that we also set the title in this commit to a default value, to make sure that if we reload the client action, its title is set to a more interesting value than 'Undefined'. We also added a few tests, which could have prevented this mess. Now it should be easy to add more tests whenever needed. --- .../static/src/js/timesheet_plan.js | 5 +- .../static/tests/timesheet_plan_tests.js | 94 +++++++++++++++++++ .../views/hr_timesheet_templates.xml | 8 ++ 3 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 addons/sale_timesheet/static/tests/timesheet_plan_tests.js diff --git a/addons/sale_timesheet/static/src/js/timesheet_plan.js b/addons/sale_timesheet/static/src/js/timesheet_plan.js index 3d98d97a6b7..cabcdea8c21 100644 --- a/addons/sale_timesheet/static/src/js/timesheet_plan.js +++ b/addons/sale_timesheet/static/src/js/timesheet_plan.js @@ -1,4 +1,4 @@ -odoo.define('project_timesheet.project_plan', function (require) { +odoo.define('sale_timesheet.ProjectPlan', function (require) { 'use strict'; var ajax = require('web.ajax'); @@ -25,6 +25,7 @@ var PlanAction = Widget.extend(ControlPanelMixin, { this._super.apply(this, arguments); this.action = action; this.action_manager = parent; + this.set('title', action.name || _t('Overview')); }, willStart: function () { var self = this; @@ -213,6 +214,7 @@ var PlanAction = Widget.extend(ControlPanelMixin, { }); }, _onSearch: function (search_event) { + search_event.stopPropagation(); var session = this.getSession(); // group by are disabled, so we don't take care of them var result = pyeval.eval_domains_and_contexts({ @@ -226,4 +228,5 @@ var PlanAction = Widget.extend(ControlPanelMixin, { core.action_registry.add('timesheet.plan', PlanAction); +return PlanAction; }); diff --git a/addons/sale_timesheet/static/tests/timesheet_plan_tests.js b/addons/sale_timesheet/static/tests/timesheet_plan_tests.js new file mode 100644 index 00000000000..d352da61dc9 --- /dev/null +++ b/addons/sale_timesheet/static/tests/timesheet_plan_tests.js @@ -0,0 +1,94 @@ +odoo.define('sale_timesheet.timesheet_plan_tests', function (require) { +"use strict"; + +var ProjectPlan = require('sale_timesheet.ProjectPlan'); +var testUtils = require('web.test_utils'); +var Widget = require('web.Widget'); + +function createPlan(params) { + var Parent = Widget.extend({ + do_push_state: function () {}, + }); + var parent = new Parent(); + testUtils.addMockEnvironment(parent, params); + var plan = new ProjectPlan(parent, params.action); + var selector = params.debug ? 'body' : '#qunit-fixture'; + plan.appendTo($(selector)); + + plan.destroy = function () { + delete plan.destroy; + parent.destroy(); + }; + + return plan; +} + +QUnit.module('Timesheet Plan', { + beforeEach: function () { + this.data = { + 'account.analytic.line': { + fields: { + account_id: {string: "Analytic account", type: "many2one", relation: "account.analytic.account"}, + }, + records: [] + }, + }; + } +}, function () { + + QUnit.test('basic timesheet plan rendering', function (assert) { + assert.expect(5); + + var plan = createPlan({ + archs: { + 'account.analytic.line,false,search': '', + }, + data: this.data, + action: {}, + mockRPC: function (route) { + assert.step(route); + if (route === '/timesheet/plan') { + return $.when({html_content: 'Banach-Tarski'}); + } + return this._super.apply(this, arguments); + }, + intercepts: { + search: function () { + // when the search event get up to the action manager, + // bad things can happen + throw new Error('Search event should not get out of client action'); + }, + }, + }); + assert.verifySteps([ + "/web/dataset/call_kw/account.analytic.line", + "/timesheet/plan" + ]); + assert.strictEqual(plan.$el.text(), 'Banach-Tarski', 'should have rendered html content'); + assert.strictEqual(plan.get('title'), 'Overview', + 'default title should be set'); + plan.destroy(); + }); + + QUnit.test('timesheet action takes action name into account', function (assert) { + assert.expect(1); + + var plan = createPlan({ + archs: { + 'account.analytic.line,false,search': '', + }, + data: this.data, + action: {name: 'Fibonacci'}, + mockRPC: function (route) { + if (route === '/timesheet/plan') { + return $.when({html_content: 'Banach-Tarski'}); + } + return this._super.apply(this, arguments); + }, + }); + assert.strictEqual(plan.get('title'), 'Fibonacci', + 'title should be set from the action name'); + plan.destroy(); + }); +}); +}); diff --git a/addons/sale_timesheet/views/hr_timesheet_templates.xml b/addons/sale_timesheet/views/hr_timesheet_templates.xml index 00f3d8b6858..59327067366 100644 --- a/addons/sale_timesheet/views/hr_timesheet_templates.xml +++ b/addons/sale_timesheet/views/hr_timesheet_templates.xml @@ -8,6 +8,14 @@ + + + +