Permalink
Browse files

[IMP] web: group by according to view

Before this commit:
specific group by for pivot view/graph view is not working.

After this commit:
apply group by differently for different view using keys like 'pivot_row_groupby', 'graph_groupbys'
i.e. if pivot_row_group by is pass in context then group by according to this in pivot view not default one

give highest priority to pivot_row_groupby/graph_groupbys then group_by and then field have type from action context for default groupby in pivot/graph view

related Task: #1848289,#36088

Co-authored-by: Mohammed Shekha <msh@openerp.com>
Co-authored-by: Suraj Shukla <suh@odoo.com>
  • Loading branch information...
3 people authored and hch-odoo committed Nov 29, 2018
1 parent aa0e6f9 commit 9649116b6363d4cc3733a6df8b3ed4f2499e2558
@@ -518,6 +518,8 @@ ActionManager.include({
* @param {Object} [searchData.contexts=[]]
* @param {Object} [searchData.domains=[]]
* @param {Object} [searchData.groupbys=[]]
* @param {Object} [searchData.viewGroupBys={}]
* @returns {Object} an object with keys 'context', 'domain', 'groupBy', 'viewGroupBys'
* @returns {Object} an object with keys 'context', 'domain', 'groupBy'
*/
_processSearchData: function (action, searchData) {
@@ -547,6 +549,7 @@ ActionManager.include({
context: context,
domain: results.domain,
groupBy: groupBy,
viewGroupBys: searchData.viewGroupBys,
};
},
/**
@@ -54,8 +54,8 @@ return AbstractModel.extend({
* to keep track of various entities.
*/
load: function (params) {
var groupBys = params.context.graph_groupbys || params.groupBys;
this.initialGroupBys = groupBys;
this.initialGroupBys = _.isString(params.groupBys) ? [params.groupBys] : params.groupBys;
var groupBys = _.isString(params.context.graph_groupbys) ? [params.context.graph_groupbys] : (params.context.graph_groupbys || this.initialGroupBys);
this.fields = params.fields;
this.modelName = params.modelName;
this.chart = {
@@ -113,8 +113,8 @@ return AbstractModel.extend({
if ('domain' in params) {
this.chart.domain = params.domain;
}
if ('groupBy' in params) {
this.chart.groupedBy = params.groupBy.length ? params.groupBy : this.initialGroupBys;
if ('viewGroupBys' in params) {
this.chart.groupedBy = params.viewGroupBys.graphGroupBy.length ? params.viewGroupBys.graphGroupBy : this.initialGroupBys;
}
if ('intervalMapping' in params) {
this.chart.intervalMapping = params.intervalMapping;
@@ -78,14 +78,23 @@ var GraphView = AbstractView.extend({
}
});

// give highest priority to graph_groupbys if given in action context
// then group_by key of action context then type='row/col' fields of graph view for groupBys
if (params.action && params.action.context.graph_groupbys && params.action.context.graph_groupbys.length) {
groupBys = params.action.context.graph_groupbys;
} else if (params.action && params.action.context.group_by && params.action.context.group_by.length) {
groupBys = params.action.context.group_by;
}

this.controllerParams.measures = measures;
this.controllerParams.groupableFields = groupableFields;
this.rendererParams.stacked = this.arch.attrs.stacked !== "False";
this.rendererParams.title = this.arch.attrs.title; // TODO: use attrs.string instead

this.loadParams.mode = this.arch.attrs.type || 'bar';
this.loadParams.measure = measure || '__count__';
this.loadParams.groupBys = groupBys || [];
this.loadParams.groupBys = groupBys;
this.loadParams.groupedBy = params.viewGroupBys ? params.viewGroupBys.graphGroupBy : this.loadParams.groupedBy;
this.loadParams.intervalMapping = intervalMapping;
this.loadParams.fields = this.fields;
this.loadParams.comparisonDomain = params.comparisonDomain;
@@ -362,9 +362,9 @@ var PivotModel = AbstractModel.extend({
*/
load: function (params) {
var self = this;

this.initialDomain = params.domain;
this.initialRowGroupBys = params.context.pivot_row_groupby || params.rowGroupBys;
this.initialRowGroupBys = _.isString(params.rowGroupBys) ? [params.rowGroupBys] : params.rowGroupBys;
var groupBys = _.isString(params.context.pivot_row_groupby) ? [params.context.pivot_row_groupby] : params.context.pivot_row_groupby;
this.fields = params.fields;
this.modelName = params.modelName;
this.data = {
@@ -375,7 +375,7 @@ var PivotModel = AbstractModel.extend({
comparisonTimeRangeDescription: params.comparisonTimeRangeDescription || "",
compare: params.compare || false,
context: _.extend({}, session.user_context, params.context),
groupedBy: params.groupedBy,
groupedBy: params.groupedBy.length ? params.groupedBy : groupBys,
colGroupBys: params.context.pivot_column_groupby || params.colGroupBys,
measures: this._processMeasures(params.context.pivot_measures) || params.measures,
sorted_column: {},
@@ -403,7 +403,6 @@ var PivotModel = AbstractModel.extend({
this.data.colGroupBys = params.context.pivot_column_groupby || this.data.colGroupBys;
this.data.groupedBy = params.context.pivot_row_groupby || this.data.groupedBy;
this.data.measures = this._processMeasures(params.context.pivot_measures) || this.data.measures;
this.defaultGroupedBy = this.data.groupedBy.length ? this.data.groupedBy : this.defaultGroupedBy;
var timeRangeMenuData = params.context.timeRangeMenuData;
if (timeRangeMenuData) {
this.data.timeRange = timeRangeMenuData.timeRange || [];
@@ -425,8 +424,8 @@ var PivotModel = AbstractModel.extend({
} else {
this.data.domain = this.initialDomain;
}
if ('groupBy' in params) {
this.data.groupedBy = params.groupBy.length ? params.groupBy : this.defaultGroupedBy;
if ('viewGroupBys' in params) {
this.data.groupedBy = params.viewGroupBys.pivotRowGroupBy;
}
if (!this.data.has_data) {
return this._loadData();
@@ -436,7 +435,7 @@ var PivotModel = AbstractModel.extend({
var old_col_root = this.data.main_col.root;
return this._loadData().then(function () {
var new_groupby_length;
if (!('groupBy' in params) && !('pivot_row_groupby' in (params.context || {}))) {
if (!('viewGroupBys' in params) && !('pivot_row_groupby' in (params.context || {}))) {
// we only update the row groupbys according to the old groupbys
// if we don't have the key 'groupBy' in params. In that case,
// we want to have the full open state for the groupbys.
@@ -95,13 +95,22 @@ var PivotView = AbstractView.extend({
activeMeasures = ['__count'].concat(activeMeasures);
}

// give highest priority to graph_groupbys if given in action context
// then group_by key of action context then type='row/col' fields of graph view for groupBys
if (params.action && params.action.context.pivot_row_groupby && params.action.context.pivot_row_groupby.length) {
rowGroupBys = params.action.context.pivot_row_groupby;
} else if (params.action && params.action.context.group_by && params.action.context.group_by.length) {
rowGroupBys = params.action.context.group_by;
}

this.loadParams.measures = activeMeasures;
this.loadParams.colGroupBys = colGroupBys;
this.loadParams.rowGroupBys = rowGroupBys;
this.loadParams.fields = this.fields;
this.loadParams.default_order = params.default_order || this.arch.attrs.default_order;

this.rendererParams.widgets = widgets;
this.loadParams.groupedBy = params.viewGroupBys ? params.viewGroupBys.pivotRowGroupBy : this.loadParams.groupedBy;

this.controllerParams.title = params.title || this.arch.attrs.string || _t("Untitled");
this.controllerParams.enableLinking = !this.arch.attrs.disable_linking;
@@ -122,6 +122,12 @@ return Widget.extend({
if (!_.isEmpty(results.group_by)) {
results.context.group_by = results.group_by;
}
if (search.viewGroupBys.pivotRowGroupBy.length) {
results.context.pivot_row_groupby = search.viewGroupBys.pivotRowGroupBy;
}
if (search.viewGroupBys.graphGroupBy.length) {
results.context.graph_groupbys = search.viewGroupBys.graphGroupBy;
}
// Don't save user_context keys in the custom filter, otherwise end
// up with e.g. wrong uid or lang stored *and used in subsequent
// reqs*
@@ -529,6 +529,8 @@ var SearchView = Widget.extend({
* Array of contexts
* groupbys
* Array of domains, in groupby order rather than view order
* viewGroupBys
* Object of groupbys array in groupby order for particular view
*
* @return {Object}
*/
@@ -551,6 +553,7 @@ var SearchView = Widget.extend({
groupbys.push.apply(groupbys, group_by);
}
});
var viewGroupBys = this.prepareViewGroupby(groupbys);
var intervalMappingNormalized = {};
_.each(this.intervalMapping, function (couple) {
var fieldName = couple.groupby.fieldName;
@@ -562,6 +565,7 @@ var SearchView = Widget.extend({
contexts: contexts,
groupbys: groupbys,
intervalMapping: intervalMappingNormalized,
viewGroupBys: viewGroupBys,
};
},
toggle_visibility: function (is_visible) {
@@ -757,6 +761,32 @@ var SearchView = Widget.extend({
renderChangedFacets: function (model, options) {
this.renderFacets(undefined, model, options);
},
/**
* evaluates groupbys and separate group_by, pivot_row_groupby and graph_groupbys for view dependent groupby
*
* @param {Array} groupbys list of group by
* @return {object}
*/
prepareViewGroupby: function (groupbys) {
var self = this;
var pivotRowGroupBy = [];
var graphGroupBy = [];
_.each(groupbys, function (group) {
if (_.isString(group)) {
group = pyUtils.py_eval(group, self.getSession().user_context);
}
// Prepare group by for pivot view if pivot_row_groupby and group_by both are given on faceet
// For pivot view pivot_row_groupby key have highest priority and graph_groupbys key have highest priority in graph view
// if pivot_row_groupby/graph_groupbys is not given then apply group_by for all view
// To maintain sequence of applied group by fields add group_by if pivot_row_groupby/graph_groupbys are not given
pivotRowGroupBy.push(group.pivot_row_groupby || group.group_by);
graphGroupBy.push(group.graph_groupbys || group.group_by);
});
return {
pivotRowGroupBy: _.flatten(pivotRowGroupBy),
graphGroupBy: _.flatten(graphGroupBy),
};
},
// it should parse the arch field of the view, instantiate the corresponding
// filters/fields, and put them in the correct variables:
// * this.search_fields is a list of all the fields,
@@ -1,6 +1,7 @@
odoo.define('web.action_manager_tests', function (require) {
"use strict";

var concurrency = require('web.concurrency');
var ReportClientAction = require('report.client_action');
var NotificationService = require('web.NotificationService');
var AbstractAction = require('web.AbstractAction');
@@ -3674,6 +3675,65 @@ QUnit.module('ActionManager', {

actionManager.destroy();
});

QUnit.test('correctly apply pivot_row_groupby/graph_groupbys from action context when switching view', function (assert) {
var done = assert.async();
assert.expect(4);
_.findWhere(this.actions, {id: 3}).context = "{'pivot_row_groupby': 'foo', 'graph_groupbys': 'bar'}";
this.actions[2].views.splice(1, 1, [false, 'graph']);
this.actions[2].views.splice(2, 1, [false, 'pivot']);
this.archs['partner,false,pivot'] = '<pivot><field name="foo"/></pivot>';
this.archs['partner,false,graph'] = '<graph string="Partners"><field name="foo"/></graph>';
var actionManager = createActionManager({
actions: this.actions,
archs: this.archs,
data: this.data,
});
actionManager.doAction(3);
assert.hasClass($('.o_control_panel .o_cp_switch_list'), 'active', "list button in control panel is active");
assert.doesNotHaveClass($('.o_control_panel .o_cp_switch_graph'), 'active', "graph button in control panel is not active");
// switch to pivot view
testUtils.dom.click($('.o_control_panel .o_cp_switch_pivot'));
assert.strictEqual($('.o_pivot_header_cell_closed').length, 6, "should be grouped by 'foo' in pivot view");
// switch to graph view
testUtils.dom.click( $('.o_control_panel .o_cp_switch_graph'));
actionManager.on_attach_callback();
return concurrency.delay(0).then(function () {
assert.strictEqual($('.nvd3-svg text:contains(Second record)').length, 1, "should be grouped by 'bar' in graph view");
actionManager.destroy();
done();
});
});

QUnit.test('correctly apply group_by if pivot_row_groupby/graph_groupbys not in action context for pivot/graph view', function (assert) {
var done = assert.async();
assert.expect(4);
_.findWhere(this.actions, {id: 3}).context = "{'group_by': 'foo'}";
this.actions[2].views.splice(1, 1, [false, 'graph']);
this.actions[2].views.splice(2, 1, [false, 'pivot']);
this.archs['partner,false,pivot'] = '<pivot><field name="foo"/></pivot>';
this.archs['partner,false,graph'] = '<graph string="Partners"><field name="foo"/></graph>';
var actionManager = createActionManager({
actions: this.actions,
archs: this.archs,
data: this.data,
debug: true,
});
actionManager.doAction(3);
assert.hasClass($('.o_control_panel .o_cp_switch_list'), 'active', "list button in control panel is active");
assert.doesNotHaveClass($('.o_control_panel .o_cp_switch_graph'), 'active', "graph button in control panel is not active");
// switch to pivot view
testUtils.dom.click($('.o_control_panel .o_cp_switch_pivot'));
assert.strictEqual($('.o_pivot_header_cell_closed').length, 6, "should be grouped by 'foo'");
// switch to graph view
testUtils.dom.click( $('.o_control_panel .o_cp_switch_graph'));
actionManager.on_attach_callback();
return concurrency.delay(0).then(function () {
assert.strictEqual($('.nvd3-svg text:contains(yop)').length, 1, "should be grouped by 'foo'");
actionManager.destroy();
done();
});
});
});

});
@@ -340,7 +340,7 @@ QUnit.module('Views', {
assert.notOk(graph.$('text:contains(red)').length,
"should not contain a text element with color in legend");

testUtils.graph.reload(graph, {groupBy: ['color_id']});
testUtils.graph.reload(graph, {viewGroupBys: {graphGroupBy: ['color_id']}});

return concurrency.delay(0);
}).then(function () {
@@ -396,7 +396,7 @@ QUnit.module('Views', {
graph_intervalMapping: {},
}, "context should be correct");

testUtils.graph.reload(graph, {groupBy: ['product_id', 'color_id']}); // change groupbys
testUtils.graph.reload(graph, {viewGroupBys: {graphGroupBy: ['product_id', 'color_id']}}); // change groupbys

return concurrency.delay(0);
}).then(function () {
@@ -622,7 +622,6 @@ QUnit.module('Views', {

QUnit.test('can be grouped with the update function', function (assert) {
assert.expect(4);

var pivot = createView({
View: PivotView,
model: "partner",
@@ -637,7 +636,7 @@ QUnit.module('Views', {
assert.containsOnce(pivot, 'tbody tr',
"should have 1 rows");

testUtils.pivot.reload(pivot, {groupBy: ['product_id']});
testUtils.pivot.reload(pivot, {viewGroupBys: ['product_id']});

assert.containsN(pivot, '.o_pivot_cell_value', 3,
"should have 3 cells");
@@ -701,7 +700,7 @@ QUnit.module('Views', {

// expand on date:days, product
nbReadGroups = 0;
testUtils.pivot.reload(pivot, {groupBy: ['date:days', 'product_id']});
testUtils.pivot.reload(pivot, {viewGroupBys: ['date:days', 'product_id']});

assert.strictEqual(nbReadGroups, 3, "should have done 3 read_group RPCS");
assert.containsN(pivot, 'tbody tr', 8,
@@ -747,7 +746,7 @@ QUnit.module('Views', {
});

// expand on date:days, product
testUtils.pivot.reload(pivot, {groupBy: ['date:days', 'product_id']});
testUtils.pivot.reload(pivot, {viewGroupBys: ['date:days', 'product_id']});

assert.containsN(pivot, 'tbody tr', 8,
"should have 7 rows (total + 3 for December and 2 for October and April)");
@@ -1189,8 +1188,8 @@ QUnit.module('Views', {
});

def = $.Deferred();
testUtils.pivot.reload(pivot, {groupBy: ['product_id']});
testUtils.pivot.reload(pivot, {groupBy: ['product_id', 'customer']});
testUtils.pivot.reload(pivot, {viewGroupBys: ['product_id']});
testUtils.pivot.reload(pivot, {viewGroupBys: ['product_id', 'customer']});
def.resolve();

assert.containsN(pivot, '.o_pivot_cell_value', 6,

0 comments on commit 9649116

Please sign in to comment.