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

[FIX] web: better mode handling, prevent useless work in some cases

Browse Source 
Before this commit, the way mode was handled was not optimal.  The end
result was correct (the widget were properly created/displayed in
edit/readonly as required), but useless work was done.  In particular,
if we have list/x2manys with widgets and modifiers, it could happen that
widgets were created in edit mode, then destroyed and recreated in
readonly mode.

This could happen in some important views, for example, any view with a
field many2many_tags with a readonly modifiers (pretty much all views
handling taxes)

In this commit, we add a 'editable' attribute to list renderer.  This is
necessary, because the list renderer is not actually editable when it is
in a x2many in a form view which is in readonly mode.  Also, we need to
manage the distinction between the fact that a list view is editable,
and the fact that some widgets are created in edit and other in readonly
mode, depending on modifiers, and on which line is being edited.

We also add a benchmark (courtesy of CHM) to be able to measure the
performance impact of edit/readonly/onchange in a larger form view.
Before this commit, running this form benchmark on my machine gave a
number of 0.4 op/sec, and after, it increases to 1.26 op/sec, so a
pretty large improvement.
This commit is contained in:
Géry Debongnie
2017-12-18 17:29:14 +01:00
parent 0c98951f1c
commit dbdd742d20
9 changed files with 174 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web/static/src/js/fields/relational_fields.js
+1 -1
View File
@@ -839,7 +839,7 @@ var FieldX2Many = AbstractField.extend({
this.currentColInvisibleFields = this._evalColumnInvisibleFields();
this.renderer = new ListRenderer(this, this.value, {
arch: arch,
mode: this.mode,
editable: this.mode === 'edit' && arch.attrs.editable,
addCreateLine: !this.isReadonly && this.activeActions.create,
addTrashIcon: !this.isReadonly && this.activeActions.delete,
viewType: viewType,
addons/web/static/src/js/views/basic/basic_renderer.js
+26 -21
View File
@@ -251,13 +251,12 @@ var BasicRenderer = AbstractRenderer.extend({
function _apply(element) {
// If the view is in edit mode and that a widget have to switch
// its "readonly" state, we have to re-render it completely
if ('readonly' in modifiers
&& self.mode === "edit"
&& element.widget
&& (element.widget.mode === 'readonly') !== modifiers.readonly)
{
self._rerenderFieldWidget(element.widget, record);
return; // Rerendering already applied the modifiers, no need to go further
if ('readonly' in modifiers && element.widget) {
var mode = modifiers.readonly ? 'readonly' : modifiersData.baseMode;
if (mode !== element.widget.mode) {
self._rerenderFieldWidget(element.widget, record, mode);
return; // Rerendering already applied the modifiers, no need to go further
}
}
// Toggle modifiers CSS classes if necessary
@@ -429,6 +428,12 @@ var BasicRenderer = AbstractRenderer.extend({
this.allModifiersData.push(modifiersData);
}
}
// we register here the base mode of the node. This is a field widget
// specific settings which represents the generic mode for the widget,
// regardless of its modifiers. The interesting case is the list view:
// all widgets are supposed to be in the baseMode 'readonly', except the
// ones that are in the line that is currently being edited.
modifiersData.baseMode = (options && options.mode) || this.mode;
// Evaluate if necessary
if (!modifiersData.evaluatedModifiers[record.id]) {
@@ -454,7 +459,7 @@ var BasicRenderer = AbstractRenderer.extend({
}
modifiersData.elementsByRecord[record.id].push(newElement);
this._applyModifiers(modifiersData, record, newElement);
this._applyModifiers(modifiersData, record, newElement, options);
}
return modifiersData.evaluatedModifiers[record.id];
@@ -490,22 +495,20 @@ var BasicRenderer = AbstractRenderer.extend({
* @param {Object} node
* @param {Object} record
* @param {Object} [options]
* @param {Object} [modifiersOptions]
* @returns {AbstractField}
*/
_renderFieldWidget: function (node, record, options, modifiersOptions) {
_renderFieldWidget: function (node, record, options) {
var fieldName = node.attrs.name;
// Register the node-associated modifiers
var modifiers = this._registerModifiers(node, record);
var mode = options && options.mode || this.mode;
var modifiers = this._registerModifiers(node, record, null, options);
// Initialize and register the widget
// Readonly status is known as the modifiers have just been registered
var Widget = record.fieldsInfo[this.viewType][fieldName].Widget;
var widget = new Widget(this, fieldName, record, _.extend({
mode: modifiers.readonly ? 'readonly' : this.mode,
var widget = new Widget(this, fieldName, record, {
mode: modifiers.readonly ? 'readonly' : mode,
viewType: this.viewType,
}, options || {}));
});
// Register the widget so that it can easily be found again
if (this.allFieldWidgets[record.id] === undefined) {
@@ -526,15 +529,16 @@ var BasicRenderer = AbstractRenderer.extend({
// associated to new widget)
var self = this;
def.then(function () {
self._registerModifiers(node, record, widget, _.extend({
self._registerModifiers(node, record, widget, {
callback: function (element, modifiers, record) {
element.$el.toggleClass('o_field_empty', !!(
record.data.id
&& (modifiers.readonly || self.mode === 'readonly')
&& (modifiers.readonly || mode === 'readonly')
&& !element.widget.isSet()
));
},
}, modifiersOptions || {}));
mode: mode
});
self._postProcessField(widget, node);
});
@@ -602,11 +606,12 @@ var BasicRenderer = AbstractRenderer.extend({
* @private
* @param {Widget} widget
* @param {Object} record
* @param {string} mode either 'readonly' or 'edit'
* @returns {AbstractField}
*/
_rerenderFieldWidget: function (widget, record) {
_rerenderFieldWidget: function (widget, record, mode) {
// Render the new field widget
var newWidget = this._renderFieldWidget(widget.__node, record);
var newWidget = this._renderFieldWidget(widget.__node, record, {mode: mode});
widget.$el.replaceWith(newWidget.$el);
// Destroy the old widget and position the new one at the old one's
addons/web/static/src/js/views/list/list_editable_renderer.js
+3 -8
View File
@@ -54,7 +54,7 @@ ListRenderer.include({
* @returns {Deferred}
*/
start: function () {
if (this.mode === 'edit') {
if (this._isEditable()) {
this.$el.css({height: '100%'});
core.bus.on('click', this, this._onWindowClicked.bind(this));
}
@@ -295,12 +295,7 @@ ListRenderer.include({
renderInvisible: editMode,
renderWidgets: editMode,
};
if (!editMode) {
// Force 'readonly' mode for widgets in readonly rows as
// otherwise they default to the view mode which is 'edit' for
// an editable list view
options.mode = 'readonly';
}
options.mode = editMode ? 'edit' : 'readonly';
// Switch each cell to the new mode; note: the '_renderBodyCell'
// function might fill the 'this.defs' variables with multiple deferred
@@ -417,7 +412,7 @@ ListRenderer.include({
* @returns {boolean}
*/
_isEditable: function () {
return this.mode === 'edit' && !this.state.groupedBy.length && this.arch.attrs.editable;
return !this.state.groupedBy.length && this.editable;
},
/**
* Move the cursor on the end of the previous line, if possible.
addons/web/static/src/js/views/list/list_renderer.js
+2 -1
View File
@@ -60,6 +60,7 @@ var ListRenderer = BasicRenderer.extend({
this.hasSelectors = params.hasSelectors;
this.selection = [];
this.pagers = []; // instantiated pagers (only for grouped lists)
this.editable = params.editable;
},
//--------------------------------------------------------------------------
@@ -261,7 +262,7 @@ var ListRenderer = BasicRenderer.extend({
// We register modifiers on the <td> element so that it gets the correct
// modifiers classes (for styling)
var modifiers = this._registerModifiers(node, record, $td);
var modifiers = this._registerModifiers(node, record, $td, _.pick(options, 'mode'));
// If the invisible modifiers is true, the <td> element is left empty.
// Indeed, if the modifiers was to change the whole cell would be
// rerendered anyway.
addons/web/static/src/js/views/list/list_view.js
+1 -1
View File
@@ -48,7 +48,7 @@ var ListView = BasicView.extend({
this.rendererParams.arch = arch;
this.rendererParams.hasSelectors =
'hasSelectors' in params ? params.hasSelectors : true;
this.rendererParams.mode = mode;
this.rendererParams.editable = params.readonly ? false : arch.attrs.editable;
this.loadParams.limit = this.loadParams.limit || 80;
this.loadParams.type = 'list';
addons/web/static/tests/fields/relational_fields_tests.js
+42
View File
@@ -4164,6 +4164,48 @@ QUnit.module('relational_fields', {
testUtils.unpatch(AbstractField);
});
QUnit.test('editable one2many with sub widgets are rendered in readonly', function (assert) {
assert.expect(2);
var editableWidgets = 0;
testUtils.patch(AbstractField, {
init: function () {
this._super.apply(this, arguments);
if (this.mode === 'edit') {
editableWidgets++;
}
},
});
var form = createView({
View: FormView,
model: 'partner',
data: this.data,
arch:'<form string="Partners">' +
'<field name="turtles">' +
'<tree editable="bottom">' +
'<field name="turtle_foo" widget="char" attrs="{\'readonly\': [(\'turtle_int\', \'==\', 11111)]}"/>' +
'<field name="turtle_int"/>' +
'</tree>' +
'</field>' +
'</form>',
res_id: 1,
viewOptions: {
mode: 'edit',
},
});
assert.strictEqual(editableWidgets, 1,
"o2m is only widget in edit mode");
form.$('tbody td.o_field_x2many_list_row_add a').click();
assert.strictEqual(editableWidgets, 3,
"3 widgets currently in edit mode");
form.destroy();
testUtils.unpatch(AbstractField);
});
QUnit.test('one2many editable list with onchange keeps the order', function (assert) {
assert.expect(2);
addons/web/static/tests/views/form_benchmarks.js
+95
View File
@@ -0,0 +1,95 @@
odoo.define('web.list_benchmarks', function (require) {
"use strict";
var FormView = require('web.FormView');
var testUtils = require('web.test_utils');
var createView = testUtils.createView;
QUnit.module('Form View', {
beforeEach: function () {
this.data = {
foo: {
fields: {
many2many: { string: "bar", type: "many2many", relation: 'bar'},
},
records: [
{ id: 1, many2many: []},
],
onchanges: {}
},
bar: {
fields: {
char: {string: "char", type: "char"},
many2many: { string: "pokemon", type: "many2many", relation: 'pokemon'},
},
records: [],
onchanges: {}
},
pokemon: {
fields: {
name: {string: "Name", type: "char"},
},
records: [],
onchanges: {}
},
};
this.arch = null;
this.run = function (assert, done, cb) {
var data = this.data;
var arch = this.arch;
var viewOptions = this.viewOptions;
new Benchmark.Suite({})
.add('form', function () {
var list = createView({
View: FormView,
model: 'foo',
data: data,
arch: arch,
res_id: 1,
});
if (cb) {
cb(list);
}
list.destroy();
})
.on('cycle', function(event) {
assert.ok(true, String(event.target));
})
.on('complete', done)
.run({ 'async': true });
};
}
}, function () {
QUnit.test('x2many with 250 rows, 2 fields (with many2many_tags, and modifiers), onchanges, and edition', function (assert) {
var done = assert.async();
assert.expect(1);
this.data.foo.onchanges.many2many = function (obj) {
obj.many2many = [5].concat(obj.many2many);
};
for (var i = 2; i < 500; i) {
this.data.bar.records.push({
id: i,
char: "automated data",
});
this.data.foo.records[0].many2many.push(i);
}
this.arch =
'<form string="Partners">'
'<field name="many2many">'
'<tree editable="top" limit="250">'
'<field name="char"/>'
'<field name="many2many" widget="many2many_tags" attrs="{\'readonly\': [(\'char\', \'==\', \'toto\')]}"/>'
'</tree>'
'</field>'
'</form>';
this.run(assert, done, function (form) {
form.$buttons.find('.o_form_button_edit').click();
form.$('.o_data_cell:first').click();
form.$('input:first').val("tralala").trigger('input');
});
});
});
});
addons/web/static/tests/views/list_tests.js
+3 -3
View File
@@ -474,13 +474,13 @@ QUnit.module('Views', {
var n = 0;
testUtils.intercept(list, "field_changed", function () {
n = 1;
n += 1;
});
$td.click();
$td.find('input').val('abc').trigger('input');
assert.strictEqual(n, 1, "field_changed should not have been triggered");
list.$('td:not(.o_list_record_selector)').eq(2).click();
assert.strictEqual(n, 1, "field_changed should have been triggered");
list.$('td:not(.o_list_record_selector)').eq(2).click();
assert.strictEqual(n, 1, "field_changed should not have been triggered");
list.destroy();
});
addons/web/views/webclient_templates.xml
+1
View File
@@ -586,6 +586,7 @@
<script type="text/javascript" src="/web/static/tests/views/list_benchmarks.js"></script>
<script type="text/javascript" src="/web/static/tests/views/kanban_benchmarks.js"></script>
<script type="text/javascript" src="/web/static/tests/views/form_benchmarks.js"></script>
</t>
<div id="qunit"/>