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

[FIX] web: multi edition in editable listviews error handling

Browse Source 
Fixed crashes/incorrect behaviours in multi edition in the list views.
The solution to these problems was mainly to refactor the function
used to save multiple records.

Before this commit:
- Cancelling the confirmation dialog in multi edition caused
the edited record to be editable but disabled
- Saving an empty field in multi edition triggered a JS error
- Static confirmation message ("saving 0 valid records")

After this commit:
- Ability to cancel confirmation
- Proper error display when trying to save empty fields
- Message consistent with the situation
This commit is contained in:
Julien Mougenot
2019-08-16 09:03:49 +00:00
parent 291dad6bc8
commit dd27b2e9c6
3 changed files with 112 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web/static/src/js/views/list/list_controller.js
+54 -34
View File
@@ -49,6 +49,7 @@ var ListController = BasicController.extend({
this.editable = params.editable;
this.noLeaf = params.noLeaf;
this.selectedRecords = params.selectedRecords || [];
this.multipleRecordsSavingPromise = null;
},
//--------------------------------------------------------------------------
@@ -346,15 +347,6 @@ var ListController = BasicController.extend({
var record = this.model.get(this.handle);
return _.extend(env, {domain: record.getDomain()});
},
/**
* @private
* @returns {boolean}
*/
_inMultipleRecordEdition: function (recordId) {
var record = this.model.get(recordId, { raw: true });
var recordIds = _.union([recordId], this.selectedRecords);
return recordIds.length > 1 && record.res_id;
},
/**
* Only display the pager when there are data to display.
*
@@ -366,10 +358,16 @@ var ListController = BasicController.extend({
return !!state.count;
},
/**
* Saves multiple records at once. This method is called by the _onFieldChanged method
* since the record must be confirmed as soon as the focus leaves a dirty cell.
* Pseudo-validation is performed with registered modifiers.
* Returns a promise that is resolved when confirming and rejected in any other case.
*
* @private
* @param {string} recordId
* @param {Object} node
* @param {Object} changes
* @returns {Promise}
*/
_saveMultipleRecords: function (recordId, node, changes) {
var self = this;
@@ -383,22 +381,42 @@ var ListController = BasicController.extend({
}
return result;
}, []);
var message = _.str.sprintf(
_t('Do you want to set the value on the %d valid selected records?'),
validRecordIds.length);
if (recordIds.length !== validRecordIds.length) {
var nbInvalid = recordIds.length - validRecordIds.length;
message += ' ' + _.str.sprintf(_t('(%d invalid)'), nbInvalid);
}
Dialog.confirm(this, message, {
confirm_callback: function () {
self.model.saveRecords(recordId, validRecordIds)
.then(function () {
self._updateButtons('readonly');
var state = self.model.get(self.handle);
self.renderer.updateState(state, {});
});
},
const nbInvalid = recordIds.length - validRecordIds.length;
return new Promise((resolve, reject) => {
const rejectAndDiscard = () => {
this.model.discardChanges(recordId);
return this._confirmSave(recordId).then(reject);
};
if (validRecordIds.length > 0) {
let message;
if (nbInvalid === 0) {
message = _.str.sprintf(
_t("Do you want to set the value on the %d selected records?"),
validRecordIds.length);
} else {
message = _.str.sprintf(
_t("Do you want to set the value on the %d valid selected records? (%d invalid)"),
validRecordIds.length, nbInvalid);
}
Dialog.confirm(this, message, {
confirm_callback: () => {
this.model.saveRecords(recordId, validRecordIds)
.then(() => {
this._updateButtons('readonly');
const state = this.model.get(this.handle);
this.renderer.updateState(state, {});
resolve();
})
.guardedCatch(rejectAndDiscard);
},
cancel_callback: rejectAndDiscard,
});
} else {
Dialog.alert(this, _t("No valid record to save"), {
confirm_callback: rejectAndDiscard,
});
}
});
},
/**
@@ -409,10 +427,11 @@ var ListController = BasicController.extend({
*/
_saveRecord: function (recordId) {
var record = this.model.get(recordId, { raw: true });
if (record.isDirty() && this._inMultipleRecordEdition(recordId)) {
if (record.isDirty() && this.renderer.inMultipleRecordEdition(recordId)) {
// do not save the record (see _saveMultipleRecords)
return Promise.resolve();
const prom = this.multipleRecordsSavingPromise || Promise.reject();
this.multipleRecordsSavingPromise = null;
return prom;
}
return this._super.apply(this, arguments);
},
@@ -621,14 +640,15 @@ var ListController = BasicController.extend({
*/
_onFieldChanged: function (ev) {
ev.stopPropagation();
var self = this;
const recordId = ev.data.dataPointID;
if (this._inMultipleRecordEdition(ev.data.dataPointID)) {
if (this.renderer.inMultipleRecordEdition(recordId)) {
// deal with edition of multiple lines
var _onSuccess = ev.data.onSuccess;
ev.data.onSuccess = function () {
Promise.resolve(_onSuccess()).then(function () {
self._saveMultipleRecords(ev.data.dataPointID, ev.target.__node, ev.data.changes);
const _onSuccess = ev.data.onSuccess;
ev.data.onSuccess = () => {
Promise.resolve(_onSuccess()).then(() => {
const savedRecordsPromise = this._saveMultipleRecords(ev.data.dataPointID, ev.target.__node, ev.data.changes);
this.multipleRecordsSavingPromise = savedRecordsPromise;
});
};
}
addons/web/static/src/js/views/list/list_editable_renderer.js
+12
View File
@@ -288,6 +288,18 @@ ListRenderer.include({
}
return null;
},
/**
* Returns whether the list is in multiple record edition from a given record.
*
* @private
* @param {string} recordId
* @returns {boolean}
*/
inMultipleRecordEdition: function (recordId) {
const record = this._getRecord(recordId) || {};
const recordIds = [...new Set([recordId, ...this.selection])];
return this.editable && recordIds.length > 1 && record.res_id;
},
/**
* Removes the line associated to the given recordID (the index of the row
* is found thanks to the old state), then updates the state.
addons/web/static/tests/views/list_tests.js
+46 -5
View File
@@ -4448,7 +4448,7 @@ QUnit.module('Views', {
});
QUnit.test('editable list view: multi edition', async function (assert) {
assert.expect(19);
assert.expect(21);
var list = await createView({
View: ListView,
@@ -4491,23 +4491,64 @@ QUnit.module('Views', {
assert.verifySteps(['default_get']);
await testUtils.fields.editInput(list.$('.o_selected_row .o_field_widget[name=int_field]'), 123);
assert.containsNone($, '.modal', "the multi edition should not be triggered during creation");
assert.containsNone(document.body, '.modal', "the multi edition should not be triggered during creation");
await testUtils.dom.click($('.o_list_button_save'));
assert.verifySteps(['create', 'read']);
// edit a field
// We deliberately unselect the first row to check the multi edit union (recordId + selectedRecordIds)
await testUtils.dom.click(list.$('.o_data_row:eq(0) .o_list_record_selector input'));
await testUtils.dom.click(list.$('.o_data_row:eq(0) .o_data_cell:eq(1)'));
await testUtils.fields.editInput(list.$('.o_field_widget[name=int_field]'), 666);
assert.containsOnce($, '.modal', "there should be an opened modal");
assert.ok($('.modal').text().includes('2 valid'), "the number of records should be correctly displayed");
await testUtils.dom.click(list.$('.o_data_row:eq(0) .o_data_cell:eq(0)'));
assert.containsOnce(document.body, '.modal', "modal appears when switching cells");
await testUtils.dom.click($('.modal .btn:contains(Cancel)'));
assert.strictEqual(list.$('.o_data_row:eq(0) .o_data_cell').text(), 'yop10',
"changes have been discarded and row is back to readonly");
await testUtils.dom.click(list.$('.o_data_row:eq(0) .o_data_cell:eq(1)'));
await testUtils.fields.editInput(list.$('.o_field_widget[name=int_field]'), 666);
await testUtils.dom.click(list.$('.o_data_row:eq(1) .o_data_cell:eq(0)'));
assert.ok($('.modal').text().includes('2 selected records'), "the number of records should be correctly displayed");
await testUtils.dom.click($('.modal .btn-primary'));
assert.verifySteps(['write', 'read']);
assert.strictEqual(list.$('.o_data_row:eq(0) .o_data_cell').text(), "yop666",
"the first row should be updated");
assert.strictEqual(list.$('.o_data_row:eq(1) .o_data_cell').text(), "blip666",
"the second row should be updated");
assert.containsNone(list, '.o_data_cell input.o_field_widget', "no field should be editable anymore");
list.destroy();
});
QUnit.test('editable list view: multi edition error and cancellation handling', async function (assert) {
assert.expect(3);
var list = await createView({
View: ListView,
model: 'foo',
data: this.data,
arch: '<tree editable="bottom">' +
'<field name="foo" required="1"/>' +
'</tree>',
});
// select two records
await testUtils.dom.click(list.$('.o_data_row:eq(0) .o_list_record_selector input'));
await testUtils.dom.click(list.$('.o_data_row:eq(1) .o_list_record_selector input'));
// edit a line and cancel
await testUtils.dom.click(list.$('.o_data_row:eq(0) .o_data_cell:eq(0)'));
await testUtils.fields.editInput(list.$('.o_selected_row .o_field_widget[name=foo]'), "abc");
await testUtils.dom.click($('.modal .btn:contains("Cancel")'));
assert.strictEqual(list.$('.o_data_row:eq(0) .o_data_cell').text(), 'yop', "first cell should have discarded any change");
// edit a line with an invalid value
await testUtils.dom.click(list.$('.o_data_row:eq(0) .o_data_cell:eq(0)'));
await testUtils.fields.editInput(list.$('.o_selected_row .o_field_widget[name=foo]'), "");
assert.containsOnce(document.body, '.modal', "there should be an opened modal");
await testUtils.dom.click($('.modal .btn-primary'));
assert.strictEqual(list.$('.o_data_row:eq(0) .o_data_cell').text(), 'yop', "changes should be discarded");
list.destroy();
});