From 36b4468d15bf96de097cfc3d7e2d5f99febb984b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Mon, 9 Apr 2018 10:21:27 +0200
Subject: [PATCH] [FIX] web: prevent crash in graph when grouping/aggregating
 on m2o

We recently added the possibility of aggregating the graph view on a
many2one field (with count distinct operator).  This is useful, but then
a rare situation could happen: the view could be grouped by the same
field.

In that case, there is a name clash in the read_group, and the result
will be that the [id, nameget] of a m2o field will be used as an
aggregate.

The readgroup method should be improved (its API is a mess), but
meanwhile, we have a solution for this issue: if we group by a m2o, then
it is guaranteed that each group has an aggregate value of 1 for the
same field.
---
 .../static/src/js/views/graph/graph_model.js  | 13 ++++++++-
 addons/web/static/tests/views/graph_tests.js  | 29 ++++++++++++++++++-
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/addons/web/static/src/js/views/graph/graph_model.js b/addons/web/static/src/js/views/graph/graph_model.js
index f73a40d6b47..2ffbd4c4cf8 100644
--- a/addons/web/static/src/js/views/graph/graph_model.js
+++ b/addons/web/static/src/js/views/graph/graph_model.js
@@ -165,8 +165,19 @@ return AbstractModel.extend({
             labels = _.map(this.chart.groupedBy, function (field) {
                 return self._sanitizeValue(data_pt[field], field);
             });
+            var value = is_count ? data_pt.__count || data_pt[this.chart.groupedBy[0]+'_count'] : data_pt[this.chart.measure];
+            if (value instanceof Array) {
+                // when a many2one field is used as a measure AND as a grouped
+                // field, bad things happen.  The server will only return the
+                // grouped value and will not aggregate it.  Since there is a
+                // nameclash, we are then in the situation where this value is
+                // an array.  Fortunately, if we group by a field, then we can
+                // say for certain that the group contains exactly one distinct
+                // value for that field.
+                value = 1;
+            }
             this.chart.data.push({
-                value: is_count ? data_pt.__count || data_pt[this.chart.groupedBy[0]+'_count'] : data_pt[this.chart.measure],
+                value: value,
                 labels: labels
             });
         }
diff --git a/addons/web/static/tests/views/graph_tests.js b/addons/web/static/tests/views/graph_tests.js
index b3c8dccdd51..d383194dc6f 100644
--- a/addons/web/static/tests/views/graph_tests.js
+++ b/addons/web/static/tests/views/graph_tests.js
@@ -14,7 +14,7 @@ QUnit.module('Views', {
                 fields: {
                     foo: {string: "Foo", type: "integer", store: true},
                     bar: {string: "bar", type: "boolean"},
-                    product_id: {string: "Product", type: "many2one", relation: 'product'},
+                    product_id: {string: "Product", type: "many2one", relation: 'product', store: true},
                     color_id: {string: "Color", type: "many2one", relation: 'color'},
                 },
                 records: [
@@ -529,6 +529,33 @@ QUnit.module('Views', {
             done();
         });
     });
+
+    QUnit.test('use a many2one as a measure and as a groupby should work', function (assert) {
+        assert.expect(2);
+
+        var graph = createView({
+            View: GraphView,
+            model: "foo",
+            data: this.data,
+            arch: '<graph string="Partners">' +
+                        '<field name="product_id" type="row"/>' +
+                '</graph>',
+        });
+        var done = assert.async();
+        return concurrency.delay(0).then(function () {
+            // need to set the measure this way because it cannot be set in the
+            // arch.
+            graph.$buttons.find('li[data-field="product_id"] a').click();
+
+            assert.strictEqual(graph.model.chart.data[0].value, 1,
+                "should have first datapoint with value 1");
+            assert.strictEqual(graph.model.chart.data[1].value, 1,
+                "should have second datapoint with value 1");
+
+            graph.destroy();
+            done();
+        });
+    });
 });
 
 });