From 2f4358b921b619257879ac3a60daeeae7876a1e4 Mon Sep 17 00:00:00 2001
From: Vincent Schippefilt <vsc@odoo.com>
Date: Mon, 20 Aug 2018 10:12:44 +0200
Subject: [PATCH] [FIX] web: view list grouped renders nicely

Before this fix, the list grouped did not render correctly when
the list was not selectable and there was 1 or 2 fields (the sum in the
footer was not aligned on the correct column)

Also fixed the fact that the group pager was not displayed if the list
had only 1 field and it was grouped
---
 .../static/src/js/views/list/list_renderer.js | 22 +++---
 addons/web/static/tests/helpers/test_utils.js |  3 +
 addons/web/static/tests/views/list_tests.js   | 71 ++++++++++++++++++-
 3 files changed, 84 insertions(+), 12 deletions(-)

diff --git a/addons/web/static/src/js/views/list/list_renderer.js b/addons/web/static/src/js/views/list/list_renderer.js
index 2070fc7cf18..942b769e23d 100644
--- a/addons/web/static/src/js/views/list/list_renderer.js
+++ b/addons/web/static/src/js/views/list/list_renderer.js
@@ -66,6 +66,7 @@ var ListRenderer = BasicRenderer.extend({
         this.selection = params.selectedRecords || [];
         this.pagers = []; // instantiated pagers (only for grouped lists)
         this.editable = params.editable;
+        this.isGrouped = this.state.groupedBy.length > 0;
     },
 
     //--------------------------------------------------------------------------
@@ -83,6 +84,7 @@ var ListRenderer = BasicRenderer.extend({
      * @override
      */
     updateState: function (state, params) {
+        this.isGrouped = state.groupedBy.length > 0;
         this._processColumns(params.columnInvisibleFields || {});
         if (params.selectedRecords) {
             this.selection = params.selectedRecords;
@@ -216,11 +218,13 @@ var ListRenderer = BasicRenderer.extend({
         var self = this;
 
         return _.map(this.columns, function (column, index) {
-            if (isHeader && index < HEADING_COLUMNS_TO_SKIP_IN_GROUPS - 1 &&
-                self.columns.length > HEADING_COLUMNS_TO_SKIP_IN_GROUPS) {
+            if (isHeader  && index === 0) {
                 return;
             }
             var $cell = $('<td>');
+            if (config.debug) {
+                $cell.addClass(column.attrs.name);
+            }
             if (column.attrs.name in aggregateValues) {
                 var field = self.state.fields[column.attrs.name];
                 var value = aggregateValues[column.attrs.name].value;
@@ -324,6 +328,7 @@ var ListRenderer = BasicRenderer.extend({
      * @returns {jQuery} a <button> element
      */
     _renderButton: function (record, node) {
+        var self = this;
         var $button = this._renderButtonFromNode(node, {
             extraClass: node.attrs.icon ? 'o_icon_button' : undefined,
             textAsTitle: !!node.attrs.icon,
@@ -333,7 +338,6 @@ var ListRenderer = BasicRenderer.extend({
 
         if (record.res_id) {
             // TODO this should be moved to a handler
-            var self = this;
             $button.on("click", function (e) {
                 e.stopPropagation();
                 self.trigger_up('button_clicked', {
@@ -343,7 +347,6 @@ var ListRenderer = BasicRenderer.extend({
             });
         } else {
             if (node.attrs.options.warn) {
-                var self = this;
                 $button.on("click", function (e) {
                     e.stopPropagation();
                     self.do_warn(_t("Warning"), _t('Please click on the "save" button first.'));
@@ -437,7 +440,7 @@ var ListRenderer = BasicRenderer.extend({
         var $th = $('<th>')
             .addClass('o_group_name')
             .text(name + ' (' + group.count + ')');
-        if (this.columns.length >= HEADING_COLUMNS_TO_SKIP_IN_GROUPS) {
+        if (this.hasSelectors) {
             $th.attr('colspan', HEADING_COLUMNS_TO_SKIP_IN_GROUPS);
         }
         var $arrow = $('<span>')
@@ -451,7 +454,7 @@ var ListRenderer = BasicRenderer.extend({
         $th.prepend($arrow);
         if (group.isOpen && !group.groupedBy.length && (group.count > group.data.length)) {
             var $pager = this._renderGroupPager(group);
-            var $lastCell = $cells[$cells.length - 1];
+            var $lastCell = $cells[$cells.length - 1] || $th;
             $lastCell.addClass('o_group_pager').append($pager);
         }
         return $('<tr>')
@@ -657,11 +660,10 @@ var ListRenderer = BasicRenderer.extend({
         var $table = $('<table>').addClass('o_list_view table table-sm table-hover table-striped');
         this.$el.addClass('table-responsive')
             .append($table);
-        var is_grouped = !!this.state.groupedBy.length;
         this._computeAggregates();
-        $table.toggleClass('o_list_view_grouped', is_grouped);
-        $table.toggleClass('o_list_view_ungrouped', !is_grouped);
-        if (is_grouped) {
+        $table.toggleClass('o_list_view_grouped', this.isGrouped);
+        $table.toggleClass('o_list_view_ungrouped', !this.isGrouped);
+        if (this.isGrouped) {
             $table
                 .append(this._renderHeader(true))
                 .append(this._renderGroups(this.state.data))
diff --git a/addons/web/static/tests/helpers/test_utils.js b/addons/web/static/tests/helpers/test_utils.js
index 1b9e1ccf57d..803c671ff56 100644
--- a/addons/web/static/tests/helpers/test_utils.js
+++ b/addons/web/static/tests/helpers/test_utils.js
@@ -207,6 +207,9 @@ function createAsyncView(params) {
         context: params.context || {},
         groupBy: params.groupBy || [],
     };
+    if (params.hasSelectors) {
+        viewOptions.hasSelectors = params.hasSelectors;
+    }
 
     _.extend(viewOptions, params.viewOptions);
 
diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js
index 5b0e23157a2..d389e4ef34c 100644
--- a/addons/web/static/tests/views/list_tests.js
+++ b/addons/web/static/tests/views/list_tests.js
@@ -307,6 +307,73 @@ QUnit.module('Views', {
         assert.strictEqual(list.$('th:contains(Bar)').length, 1, "should contain Bar");
         assert.strictEqual(list.$('tr.o_group_header').length, 2, "should have 2 .o_group_header");
         assert.strictEqual(list.$('th.o_group_name').length, 2, "should have 2 .o_group_name");
+    });
+
+    QUnit.test('basic grouped list rendering 1 col without selector', function (assert) {
+        assert.expect(1);
+
+        var list = createView({
+            View: ListView,
+            model: 'foo',
+            data: this.data,
+            arch: '<tree ><field name="foo"/></tree>',
+            groupBy: ['bar'],
+            hasSelectors: false,
+        });
+
+        assert.strictEqual(list.$('.o_group_header:first').children().length, 1,
+        "group header should have exactly 1 column");
+        list.destroy();
+    });
+
+    QUnit.test('basic grouped list rendering 1 col with selector', function (assert) {
+        assert.expect(1);
+
+        var list = createView({
+            View: ListView,
+            model: 'foo',
+            data: this.data,
+            arch: '<tree ><field name="foo"/></tree>',
+            groupBy: ['bar'],
+            hasSelectors: true,
+        });
+
+        assert.strictEqual(list.$('.o_group_header:first').children().length, 1,
+            "group header should have exactly 1 column");
+        list.destroy();
+    });
+
+    QUnit.test('basic grouped list rendering 2 col without selector', function (assert) {
+        assert.expect(1);
+
+        var list = createView({
+            View: ListView,
+            model: 'foo',
+            data: this.data,
+            arch: '<tree ><field name="foo"/><field name="bar"/></tree>',
+            groupBy: ['bar'],
+            hasSelectors: false,
+        });
+
+        assert.strictEqual(list.$('.o_group_header:first').children().length, 2,
+            "group header should have exactly 2 column");
+        list.destroy();
+    });
+
+    QUnit.test('basic grouped list rendering 2 col with selector', function (assert) {
+        assert.expect(1);
+
+        var list = createView({
+            View: ListView,
+            model: 'foo',
+            data: this.data,
+            arch: '<tree ><field name="foo"/><field name="bar"/></tree>',
+            groupBy: ['bar'],
+            hasSelectors: true,
+        });
+
+        assert.strictEqual(list.$('.o_group_header:first').children().length, 2,
+        "group header should have exactly 2 column");
         list.destroy();
     });
 
@@ -740,7 +807,7 @@ QUnit.module('Views', {
             model: 'foo',
             data: this.data,
             groupBy: ['m2o'],
-            arch: '<tree editable="bottom"><field name="int_field" sum="Sum"/></tree>',
+            arch: '<tree editable="bottom"><field name="foo" /><field name="int_field" sum="Sum"/></tree>',
         });
 
         var $groupHeader1 = list.$('.o_group_header').filter(function (index, el) {
@@ -807,7 +874,7 @@ QUnit.module('Views', {
             model: 'foo',
             data: this.data,
             groupBy: ['foo'],
-            arch: '<tree editable="bottom"><field name="int_field" sum="Sum"/></tree>',
+            arch: '<tree editable="bottom"><field name="foo" /><field name="int_field" sum="Sum"/></tree>',
             mockRPC: function (route, args) {
                 if (args.method === 'read_group') {
                     assert.step(args.kwargs.orderby || 'default order');