pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] web: pivot model accept no __count key or null values

Browse Source 
The _mergeData function used in pivot view comparison mode
was not designed to accept the absence of the __count key or
null values for measures in the objects returned by a 'read_group'.

The absence of the __count key would result in a traceback and null
values for measures into wrong calculations (infinities).

One could ask why null values did not cause problem before.
The simple reason is that the formatters used to format cell values
render a 'null' value like they render a zero.
This fix makes explicit that null values have to be considered
as zeros and avoid in that way Infinity values when computing
variations.
This commit is contained in:
Mathieu Duckerts-Antoine
2018-09-04 17:37:07 +02:00
parent 004b6e8b1c
commit d4b26c3372
2 changed files with 123 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web/static/src/js/views/pivot/pivot_model.js
+52 -16
View File
@@ -791,10 +791,25 @@ var PivotModel = AbstractModel.extend({
return header;
},
/**
* @param {Object} data
* @param {Object} comparisonData
* @param {(string[])[]} groupBys
* @returns {Object}
* Here data and comparisonData are arrays of arrays of objects.
* Each one of those objects is called a dataPoint and represents a group of records
* (determined by some groupbys values) together with measure values aggregated.
* An exception: if a many2one is selected as a measure and a groupby simultaneously,
* we have only the corresponding groupby value which is the of the form ['id', 'name_get']
* but the measure value can be inferred, it is indeed 1.
* In case 'this.data.compare' is true (a comparison is required),
* the dataPoint are transformed in such a way that the measures values become objects of the form
*
* {'data': 'some value', 'comparisonData': 'other value', 'variation': 'yet another value'}.
*
* In case two dataPoints have the same associated group
* (they then come necessarily from 'data' and 'comparisonData'),
* they are merged into a single dataPoint of the form above.
*
* @param {Object[][]} data
* @param {Object[][]} comparisonData
* @param {string[][]} groupBys
* @returns {Object[][]}
*/
_mergeData: function (data, comparisonData, groupBys) {
if (!this.data.compare) {
@@ -805,6 +820,9 @@ var PivotModel = AbstractModel.extend({
var value, groupIdentifier, dataPoint, m, measureName, measureValue, measureComparisonValue;
for (var index = 0; index < groupBys.length; index++) {
dataPoints = {};
// Consider dataPoints comming from 'data'. The dataPoint measure values are objects with
// zeros values for the 'comparisonData' key since we don't know at this stage
// if the group is represented in the 'comparisonData'.
if (data.length) {
for (var k = 0; k < data[index].length; k++) {
dataPoint = data[index][k];
@@ -816,6 +834,9 @@ var PivotModel = AbstractModel.extend({
if (typeof measureValue === 'boolean') {
measureValue = measureValue ? 1 : 0;
}
if (measureValue === null) {
measureValue = 0;
}
if (!(measureValue instanceof Array) && measureName !== '__count') {
dataPoint[measureName] = {
data: measureValue,
@@ -831,11 +852,11 @@ var PivotModel = AbstractModel.extend({
};
}
}
dataPoint.__count = {
dataPoint.__count = dataPoint.__count ? {
data: dataPoint.__count,
comparisonData: 0,
variation: computeVariation(dataPoint.__count, 0)
};
} : undefined;
dataPoints[groupIdentifier] = dataPoint;
}
}
@@ -845,12 +866,16 @@ var PivotModel = AbstractModel.extend({
value = this._getValue(dataPoint, groupBys[index]);
groupIdentifier = value.join();
if (!dataPoints[groupIdentifier]) {
// Here we know that the group is not represented in 'data'.
for (m=0; m < this.data.measures.length; m++) {
measureName = this.data.measures[m];
measureComparisonValue = dataPoint[measureName];
if (typeof(measureComparisonValue) === 'boolean') {
measureComparisonValue = measureComparisonValue ? 1 : 0;
}
if (measureComparisonValue === null) {
measureComparisonValue = 0;
}
if (!(measureComparisonValue instanceof Array) && measureName !== '__count') {
dataPoint[measureName] = {
data: 0,
@@ -866,21 +891,27 @@ var PivotModel = AbstractModel.extend({
};
}
}
dataPoint.__count = {
dataPoint.__count = dataPoint.__count ? {
data: 0,
comparisonData: dataPoint.__count,
variation: computeVariation(0, dataPoint.__count)
};
} : undefined;
dataPoint.__comparisonCount = dataPoint.__count;
dataPoint.__comparisonDomain = dataPoint.__domain;
dataPoints[groupIdentifier] = _.omit(dataPoint, '__domain');
} else {
// Here we know that the group is represented in 'data'.
// Therefore we modify the corresonding dataPoint:
// we modify the key 'comparisonData' and recompute 'variation'.
for (m=0; m < this.data.measures.length; m++) {
measureName = this.data.measures[m];
measureComparisonValue = dataPoint[measureName];
if (typeof(measureComparisonValue) === 'boolean') {
measureComparisonValue = measureComparisonValue ? 1 : 0;
}
if (measureComparisonValue === null) {
measureComparisonValue = 0;
}
if (!(measureComparisonValue instanceof Array) && measureName !== '__count') {
dataPoints[groupIdentifier][measureName].comparisonData = measureComparisonValue;
dataPoints[groupIdentifier][measureName].variation = computeVariation(
@@ -897,11 +928,14 @@ var PivotModel = AbstractModel.extend({
}
}
dataPoints[groupIdentifier].__count.comparisonData = dataPoint.__count;
dataPoints[groupIdentifier].__count.variation = computeVariation(
dataPoints[groupIdentifier].__count.data,
dataPoint.__count
);
if (dataPoint.__count) {
dataPoints[groupIdentifier].__count.data = dataPoints[groupIdentifier].__count.data || 0;
dataPoints[groupIdentifier].__count.comparisonData = dataPoint.__count;
dataPoints[groupIdentifier].__count.variation = computeVariation(
dataPoints[groupIdentifier].__count.data,
dataPoint.__count
);
}
dataPoints[groupIdentifier].__comparisonCount = dataPoint.__count;
dataPoints[groupIdentifier].__comparisonDomain = dataPoint.__domain;
}
@@ -912,7 +946,7 @@ var PivotModel = AbstractModel.extend({
return allData;
},
/**
* @param {Object} data
* @param {Object[][]} data
*/
_prepareData: function (data) {
var self = this;
@@ -942,9 +976,11 @@ var PivotModel = AbstractModel.extend({
value: self._getValue(dataPoint, groupBys),
domain: dataPoint.__domain,
comparisonDomain: dataPoint.__comparisonDomain,
length: dataPoint.__count.data ? dataPoint.__count.data : dataPoint.__count,
comparisonLength: dataPoint.__count.comparisonData || 0
};
if (dataPoint.__count) {
attrs.length = dataPoint.__count instanceof Object ? dataPoint.__count.data : dataPoint.__count;
attrs.comparisonLength = dataPoint.__comparisonCount;
}
if (j === 0) {
row = this._makeHeader(attrs.value, attrs.domain, attrs.comparisonDomain, attrs.length, attrs.comparisonLength, main_row_header, 0, i);
addons/web/static/tests/views/pivot_tests.js
+71 -4
View File
@@ -1225,8 +1225,8 @@ QUnit.module('Views', {
pivot.destroy();
});
QUnit.test('rendering of pivot view with comparison active', function (assert) {
assert.expect(1);
QUnit.test('rendering of pivot view with comparison', function (assert) {
assert.expect(91);
this.data.partner.records[0].date = '2016-12-15';
this.data.partner.records[1].date = '2016-12-17';
@@ -1243,6 +1243,15 @@ QUnit.module('Views', {
var unpatchDate = patchDate(2016, 11, 20, 1, 0, 0);
var results, i, length;
function checkCellValues (results) {
length = results.length;
for (i = 0; i < length; i++) {
assert.strictEqual($('.o_pivot .o_pivot_cell_value div').eq(i).text().trim(), results.shift());
}
}
// create an action manager to test the interactions with the search view
var actionManager = createActionManager({
@@ -1260,15 +1269,71 @@ QUnit.module('Views', {
res_model: 'partner',
type: 'ir.actions.act_window',
views: [[false, 'pivot']],
flags: {
pivot: {
additionalMeasures: ['product_id'],
}
}
});
// with no data
$('.o_time_range_menu_button').click();
$('.o_time_range_menu .custom-control-label').click();
$('.o_time_range_menu .o_comparison_checkbox').click();
$('.o_time_range_selector').val('today');
$('.o_time_range_menu .o_apply_range').click();
// Test to modify and extend
assert.strictEqual($('.o_pivot p.o_view_nocontent_empty_folder').length, 1);
// with data, no row groupby
$('.o_time_range_menu_button').click();
$('.o_time_range_selector').val('this_month');
$('.o_time_range_menu .o_apply_range').click();
results = [
"13", "0", "100%", "0", "19", "-100%", "13", "19", "-31.58%"
];
checkCellValues(results);
// with data, with row groupby
$('.o_pivot .o_pivot_header_cell_closed').eq(2).click();
$('.o_pivot .o_field_selection a[data-field="product_id"]').click();
results = [
"13", "0", "100%", "0", "19", "-100%", "13", "19", "-31.58%" ,
"12", "0", "100%", "12", "0" , "100%" ,
"1" , "0", "100%", "0", "19", "-100%", "1" , "19" , "-94.74%"
];
checkCellValues(results);
$('.o_control_panel button.btn-primary').eq(0).click();
$('.o_control_panel div.o_pivot_measures_list a[data-field="foo"').click();
$('.o_control_panel div.o_pivot_measures_list a[data-field="product_id"').click();
results = [
"2", "0", "100%", "0", "1", "-100%", "2", "1", "100%" ,
"1", "0", "100%", "1", "0" , "100%" ,
"1" , "0", "100%", "0", "1", "-100%", "1" , "1" , "100%"
];
checkCellValues(results);
$('.o_control_panel button.btn-primary').eq(0).click();
$('.o_control_panel div.o_pivot_measures_list a[data-field="__count"').click();
$('.o_control_panel div.o_pivot_measures_list a[data-field="product_id"').click();
results = [
"2", "0", "100%", "0", "2", "-100%", "2", "2", "0%" ,
"1", "0", "100%", "1", "0" , "100%" ,
"1" , "0", "100%", "0", "2", "-100%", "1" , "2" , "-50%"
];
checkCellValues(results);
$('.o_pivot .o_pivot_header_cell_opened').eq(0).click();
results = [
"2", "2", "0%" ,
"1", "0", "100%" ,
"1", "2", "-50%"
];
checkCellValues(results);
unpatchDate();
actionManager.destroy();
});
@@ -1317,6 +1382,8 @@ QUnit.module('Views', {
// open time range menu
$('.o_control_panel .o_time_range_menu_button').click();
// select 'Today' as range
$('.o_control_panel .o_time_range_selector').val('today');
// check checkbox 'Compare To'
$('.o_control_panel .o_time_range_menu .o_comparison_checkbox').click();
// Click on 'Apply' button