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

[FIX] web: list rendering was not properly updated after discard

Browse Source 
Before this commit, when hitting the "Discard" button in an editable
list view, the line switched to readonly mode but with the undiscarded
data (even though the data was properly discarded and not saved).

Fortunately, the fix is in fact making the save/discard/update system
more consistent. Here are some reasons why:
- the `_confirmSave` method is now used after a save or a discard
    (while it was before only called on readonly views where a change
    was made by a special field widget).
- Abandonning a record (discarding a new unsaved one), is now the
    responsability of `discardChanges` (and not `_setMode`).
- The renderer `updateState` method is used when its state is updated.
- ...

Note that setting the renderer mode is now taking care of deferreds
properly (the widgets deferred are now considered when switching
a row to readonly/edit mode).
This commit is contained in:
qsm-odoo
2017-05-08 14:02:40 +02:00
parent c8d77f93e5
commit 0764bc41aa
8 changed files with 129 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web/static/src/js/fields/relational_fields.js
+5 -4
View File
@@ -638,7 +638,7 @@ var FieldX2Many = AbstractField.extend({
return this._super();
}
if (this.renderer) {
this.renderer.updateState(this.value);
this.renderer.updateState(this.value, {});
this.pager.updateState({ size: this.value.count });
return $.when();
}
@@ -746,8 +746,8 @@ var FieldX2Many = AbstractField.extend({
onFailure: def.reject.bind(def),
});
} else {
self.renderer.setRowMode(recordID, 'readonly');
def.resolve();
self.renderer.setRowMode(recordID, 'readonly')
.done(def.resolve.bind(def));
}
});
return def;
@@ -791,7 +791,8 @@ var FieldX2Many = AbstractField.extend({
*/
_onEditLine: function (ev) {
ev.stopPropagation();
this.renderer.setRowMode(ev.data.recordID, 'edit');
this.renderer.setRowMode(ev.data.recordID, 'edit')
.done(ev.data.onSuccess);
},
/**
* Updates the given record with the changes.
addons/web/static/src/js/views/abstract_renderer.js
+5 -2
View File
@@ -65,15 +65,18 @@ return Widget.extend({
setLocalState: function (localState) {
},
/**
* update the state of the view. It always retrigger a full rerender.
* Updates the state of the view. It retriggers a full rerender, unless told
* otherwise (for optimization for example).
*
* @param {any} state
* @param {Object} params
* @param {boolean} [params.noRender=false]
* if true, the method only updates the state without rerendering
* @returns {Deferred}
*/
updateState: function (state, params) {
this.state = state;
return this._render();
return params.noRender ? $.when() : this._render();
},
//--------------------------------------------------------------------------
addons/web/static/src/js/views/basic/basic_controller.js
+14 -13
View File
@@ -118,13 +118,18 @@ var BasicController = AbstractController.extend(FieldManagerMixin, {
var self = this;
recordID = recordID || this.handle;
return this.canBeDiscarded(recordID).then(function (needDiscard) {
if (needDiscard) { // Just some optimization
self.model.discardChanges(recordID);
}
if (options && options.readonlyIfRealDiscard && !needDiscard) {
return;
}
self._setMode('readonly', recordID);
if (needDiscard) { // Just some optimization
self.model.discardChanges(recordID);
}
if (self.model.isNew(recordID)) {
self._abandonRecord(recordID);
return;
}
return self._confirmSave(recordID);
});
},
/**
@@ -386,7 +391,7 @@ var BasicController = AbstractController.extend(FieldManagerMixin, {
if (options.stayInEdit) {
return def;
} else {
return def.then(this._setMode.bind(this, 'readonly', recordID));
return def.then(this._confirmSave.bind(this, recordID));
}
},
/**
@@ -397,17 +402,13 @@ var BasicController = AbstractController.extend(FieldManagerMixin, {
* @private
* @param {string} mode - 'readonly' or 'edit'
* @param {string} [recordID]
* @returns {Deferred}
*/
_setMode: function (mode, recordID) {
recordID = recordID || this.handle;
// If trying to make a temporary record readonly, discard the record
if (mode === 'readonly' && this.model.isNew(recordID)) {
this._abandonRecord(recordID);
return;
}
if (recordID === this.handle) {
this.update({mode: mode}, {reload: false});
if ((recordID || this.handle) === this.handle) {
return this.update({mode: mode}, {reload: false});
}
return $.when();
},
/**
* Helper method, to get the current environment variables from the model
addons/web/static/src/js/views/form/form_controller.js
+6 -2
View File
@@ -67,7 +67,7 @@ var FormController = BasicController.extend({
}).then(function (handle) {
self.handle = handle;
self._updateEnv();
self._setMode('edit');
return self._setMode('edit');
});
},
/**
@@ -183,7 +183,11 @@ var FormController = BasicController.extend({
*/
_confirmSave: function (id) {
if (id === this.handle) {
return this.reload();
if (this.mode === 'readonly') {
return this.reload();
} else {
return this._setMode('readonly');
}
} else {
// a subrecord changed, so update the corresponding relational field
// i.e. the one whose value is a record with the given id or a list
addons/web/static/src/js/views/list/list_controller.js
+10 -6
View File
@@ -200,7 +200,7 @@ var ListController = BasicController.extend({
this.model.addDefaultRecord(this.handle, {position: this.editable}).then(function (recordID) {
self._toggleNoContentHelper(false);
var state = self.model.get(self.handle);
self.renderer.updateState(state);
self.renderer.updateState(state, {});
self.renderer.editRecord(recordID);
});
},
@@ -231,13 +231,14 @@ var ListController = BasicController.extend({
*/
_confirmSave: function (id) {
var state = this.model.get(this.handle);
return this.renderer.confirmSave(state, id);
return this.renderer.updateState(state, {noRender: true})
.then(this._setMode.bind(this, 'readonly', id));
},
/**
* @override
* @private
*/
_getSidebarEnv: function() {
_getSidebarEnv: function () {
var env = this._super.apply(this, arguments);
var record = this.model.get(this.handle);
return _.extend(env, {domain: record.getDomain()});
@@ -249,12 +250,14 @@ var ListController = BasicController.extend({
* @private
* @param {string} mode
* @param {string} [recordID] - default to main recordID
* @returns {Deferred}
*/
_setMode: function (mode, recordID) {
this._super.apply(this, arguments);
if ((recordID || this.handle) !== this.handle) {
this.renderer.setRowMode(recordID, mode);
this._updateButtons(mode);
return this.renderer.setRowMode(recordID, mode);
} else {
return this._super.apply(this, arguments);
}
},
/**
@@ -356,7 +359,8 @@ var ListController = BasicController.extend({
*/
_onEditLine: function (ev) {
ev.stopPropagation();
this._setMode('edit', ev.data.recordID);
this._setMode('edit', ev.data.recordID)
.done(ev.data.onSuccess);
},
/**
* Opens the Export Dialog
addons/web/static/src/js/views/list/list_editable_renderer.js
+58 -69
View File
@@ -85,20 +85,6 @@ ListRenderer.include({
}
return this._super(recordID);
},
/**
* This method is called by the controller when the user has clicked 'save',
* and the model as actually saved the current changes. We then need to
* set the correct values for each cell. The difference with confirmChange
* is that the edited line is now in readonly mode, so we can just set its
* new value.
*
* @param {Object} state the new full state for the list
* @param {string} savedRecordID the id for the saved record
*/
confirmSave: function (state, savedRecordID) {
this.state = state;
this.setRowMode(savedRecordID, 'readonly');
},
/**
* Edit a given record in the list
*
@@ -143,6 +129,7 @@ ListRenderer.include({
*
* @param {string} recordID
* @param {string} mode
* @returns {Deferred}
*/
setRowMode: function (recordID, mode) {
var self = this;
@@ -155,40 +142,67 @@ ListRenderer.include({
this.currentRow = editMode ? rowIndex : null;
var $row = this.$('.o_data_row:nth(' + rowIndex + ')');
var $tds = $row.children('.o_data_cell');
var oldWidgets = _.clone(this.allFieldWidgets[record.id]);
// When switching to edit mode, force the dimensions of all cells to
// their current value so that they won't change if their content
// changes, to prevent the view from flickering.
if (editMode) {
// Instantiate column widgets and destroy potential readonly ones
var oldWidgets = _.clone(this.allFieldWidgets[record.id]);
var $tds = $row.children('.o_data_cell');
// Force the size of each cell to its current value so that it
// won't change if its content changes, to prevent the view from
// flickering
$tds.each(function () {
var $td = $(this);
$td.css({width: $td.outerWidth()});
});
_.each(this.columns, function (node, colIndex) {
var $td = $tds.eq(colIndex);
var $newTd = self._renderBodyCell(record, node, colIndex, {
renderInvisible: true,
renderWidgets: true,
});
// TODO this is ugly...
if ($td.hasClass('o_list_button')) {
self._unregisterModifiersElement(node, record, $td.children());
}
$td.empty().append($newTd.contents());
});
_.each(oldWidgets, this._destroyFieldWidget.bind(this, record));
} else {
for (var colIndex = 0; colIndex < this.columns.length; colIndex++) {
this._setCellValue(rowIndex, colIndex);
}
}
$row.toggleClass('o_selected_row', editMode); // Toggle here so that style is applied at the end
// Prepare options for cell rendering (this depends on the mode)
var options = {
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';
}
// Switch each cell to the new mode; note: the '_renderBodyCell'
// function might fill the 'this.defs' variables with multiple deferred
// so we create the array and delete it after the rendering.
var defs = [];
this.defs = defs;
_.each(this.columns, function (node, colIndex) {
var $td = $tds.eq(colIndex);
var $newTd = self._renderBodyCell(record, node, colIndex, options);
// Widgets are unregistered of modifiers data when they are
// destroyed. This is not the case for simple buttons so we have to
// do it here.
if ($td.hasClass('o_list_button')) {
self._unregisterModifiersElement(node, record, $td.children());
}
// For edit mode we only replace the content of the cell with its
// new content (invisible fields, editable fields, ...).
// For readonly mode, we replace the whole cell so that the
// dimensions of the cell are not forced anymore.
if (editMode) {
$td.empty().append($newTd.contents());
} else {
self._unregisterModifiersElement(node, record, $td);
$td.replaceWith($newTd);
}
});
delete this.defs;
// Destroy old field widgets
_.each(oldWidgets, this._destroyFieldWidget.bind(this, record));
// Toggle selected class here so that style is applied at the end
$row.toggleClass('o_selected_row', editMode);
return $.when.apply($, defs);
},
//--------------------------------------------------------------------------
@@ -360,6 +374,7 @@ ListRenderer.include({
* Activates the row at the given row index.
*
* @param {integer} rowIndex
* @returns {Deferred}
*/
_selectRow: function (rowIndex) {
// Do nothing if already selected
@@ -372,40 +387,14 @@ ListRenderer.include({
return this._unselectRow().then(function () {
// Notify the controller we want to make a record editable
var record = self.state.data[rowIndex];
var def = $.Deferred();
self.trigger_up('edit_line', {
recordID: record.id,
onSuccess: def.resolve.bind(def),
});
return def;
});
},
/**
* Set the value of a cell. This method can be called when the value of a
* cell was modified, for example after an onchange.
*
* @param {integer} rowIndex
* @param {integer} colIndex
*/
_setCellValue: function (rowIndex, colIndex) {
var record = this.state.data[rowIndex];
var node = this.columns[colIndex];
var $oldTd = this.$('.o_data_row:nth(' + rowIndex + ') > .o_data_cell:nth(' + colIndex + ')');
var $newTd = this._renderBodyCell(record, node, colIndex, {mode: 'readonly'});
this._unregisterModifiersElement(node, record, $oldTd);
$oldTd.replaceWith($newTd);
// Destroy old cell field widget if any
// TODO this is very inefficient O(n^3) instead of O(n) on row update
// because this method is called for each cell (O(n)), each time we have
// to find the associated record widget (O(n)) and once found, the
// search is performed again by the _destroyFieldWidget function
if (node.tag === 'field') {
var recordWidgets = this.allFieldWidgets[record.id];
var w = _.findWhere(recordWidgets, {name: node.attrs.name}); //
if (w) {
this._destroyFieldWidget(record, w);
}
}
},
/**
* This method is called whenever we click/move outside of a row that was
* in edit mode. This is the moment we save all accumulated changes on that
addons/web/static/src/js/views/list/list_renderer.js
+2 -2
View File
@@ -268,9 +268,7 @@ var ListRenderer = BasicRenderer.extend({
return $td.append(this._renderButton(record, node));
}
if (node.attrs.widget || (options && options.renderWidgets)) {
this.defs = []; // TODO maybe wait for those somewhere ?
var widget = this._renderFieldWidget(node, record, _.pick(options, 'mode'));
delete this.defs;
return $td.append(widget.$el);
}
var name = node.attrs.name;
@@ -534,9 +532,11 @@ var ListRenderer = BasicRenderer.extend({
*/
_renderRow: function (record) {
var self = this;
this.defs = []; // TODO maybe wait for those somewhere ?
var $cells = _.map(this.columns, function (node, index) {
return self._renderBodyCell(record, node, index, {mode: 'readonly'});
});
delete this.defs;
var className = 'o_data_row';
var decorations = this._computeDecorationClassNames(record);
if (decorations.length) {
addons/web/static/tests/views/list_tests.js
+29
View File
@@ -2036,6 +2036,35 @@ QUnit.module('Views', {
list.destroy();
});
QUnit.test('discarding changes in a row properly updates the rendering', function (assert) {
assert.expect(3);
var list = createView({
View: ListView,
model: 'foo',
data: this.data,
arch:
'<tree editable="top">' +
'<field name="foo"/>' +
'</tree>',
});
assert.strictEqual(list.$('.o_data_cell:first').text(), "yop",
"first cell should contain 'yop'");
list.$('.o_data_cell:first').click();
list.$('input[name="foo"]').val("hello").trigger('input');
list.$buttons.find('.o_list_button_discard').click();
assert.strictEqual($('.modal:visible').length, 1,
"a modal to ask for discard should be visible");
$('.modal:visible .btn-primary').click();
assert.strictEqual(list.$('.o_data_cell:first').text(), "yop",
"first cell should still contain 'yop'");
list.destroy();
});
});
});