From ac092e9ccdc71ea6c95ca7c0833337e7a911fcad Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Mon, 15 May 2017 15:04:01 +0200
Subject: [PATCH] [FIX] web: List: force grouped lists to be non editable

Editable grouped list views isn't a supported feature. Before this
rev., grouped lists weren't automatically set as non editable, so
it could cause crashes when the user clicked on rows of grouped
lists with editable="top" or "bottom".

This was for example the case in Timesheets > All timesheets ->
go to list view.
---
 .../js/views/list/list_editable_renderer.js   | 18 ++++++++---
 addons/web/static/tests/views/list_tests.js   | 32 +++++++++++++++++++
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/addons/web/static/src/js/views/list/list_editable_renderer.js b/addons/web/static/src/js/views/list/list_editable_renderer.js
index 49badb22f85..a857fee7103 100644
--- a/addons/web/static/src/js/views/list/list_editable_renderer.js
+++ b/addons/web/static/src/js/views/list/list_editable_renderer.js
@@ -44,8 +44,6 @@ ListRenderer.include({
         // of each line, so the user can delete a record.
         this.addTrashIcon = params.addTrashIcon;
 
-        this.editable = this.arch.attrs.editable;
-
         this.currentRow = null;
         this.currentCol = null;
     },
@@ -223,6 +221,16 @@ ListRenderer.include({
         }
         return n;
     },
+    /**
+     * Returns true iff the list is editable, i.e. if it isn't grouped and if
+     * the editable attribute is set on the root node of its arch.
+     *
+     * @private
+     * @returns {boolean}
+     */
+    _isEditable: function () {
+        return this.mode === 'edit' && !this.state.groupedBy.length && this.arch.attrs.editable;
+    },
     /**
      * Move the cursor on the end of the previous line, if possible.
      * If there is no previous line, then we create a new record.
@@ -474,7 +482,7 @@ ListRenderer.include({
     _onCellClick: function (event) {
         // The special_click property explicitely allow events to bubble all
         // the way up to bootstrap's level rather than being stopped earlier.
-        if (this.mode === 'readonly' || !this.editable || $(event.target).prop('special_click')) {
+        if (!this._isEditable() || $(event.target).prop('special_click')) {
             return;
         }
         var $td = $(event.currentTarget);
@@ -486,7 +494,7 @@ ListRenderer.include({
     /**
      * We need to manually unselect row, because noone else would do it
      */
-    _onEmptyRowClick: function (event) {
+    _onEmptyRowClick: function () {
         this._unselectRow();
     },
     /**
@@ -570,7 +578,7 @@ ListRenderer.include({
      * @private
      */
     _onRowClicked: function () {
-        if (this.mode === 'readonly' || !this.editable) {
+        if (!this._isEditable()) {
             this._super.apply(this, arguments);
         }
     },
diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js
index 460d046ff52..a74192731ec 100644
--- a/addons/web/static/tests/views/list_tests.js
+++ b/addons/web/static/tests/views/list_tests.js
@@ -2134,6 +2134,38 @@ QUnit.module('Views', {
 
         list.destroy();
     });
+
+    QUnit.test('grouped list are not editable', function (assert) {
+        // Editable grouped list views are not supported, so the purpose of this
+        // test is to check that when a list view is grouped, its editable
+        // attribute is ignored
+        assert.expect(4);
+
+        var list = createView({
+            View: ListView,
+            model: 'foo',
+            data: this.data,
+            arch: '<tree editable="top"><field name="foo"/><field name="bar"/></tree>',
+            intercepts: {
+                switch_view: function () {
+                    assert.step('switch view');
+                },
+            },
+        });
+
+        list.$('.o_data_cell:first').click();
+        assert.verifySteps([], 'no switch view should have been requested');
+        assert.strictEqual(list.$('.o_selected_row').length, 1,
+            "a row should be in edition");
+
+        // reload with groupBy
+        list.reload({groupBy: ['bar']});
+        list.$('.o_group_header:first').click();
+        list.$('.o_data_cell:first').click();
+        assert.verifySteps(['switch view'], 'one switch view should have been requested');
+
+        list.destroy();
+    });
 });
 
 });