[FIX] web: prevent useless discard dialog from showing up

Here is a scenario that could cause an issue:
- open a form view with a many2one
- click on edit button
- click on small external button for many2one. this opens a modal form
  view
- click on save in the modal (without changing anything)
- click on save in the main record
- exit form view. this opens a discard dialog

There were two issues here:
- saving the modal dialog automatically marks the many2one field as changed.
  This was necessary to force reloading the data, because editing a sub value
  in the modal form view could have changed the display_name of the manyone
  field.  However, this is not necessary when no change was done.
- when saving a record, the _isDirty flag was reset to false only if an
  actual rpc was done.  However, it may happen that the flag is set to
  dirty (for example, when modifying a value inside a many2one), but the
  main record has no changed fields.

In this commit, we also remove the on_save handler in the
formviewdialog.  This is a small refactoring in a stable version, but no code
currently use it, and I believe that it will make the code much easier
to maintain (the previous code was really awkward), so I think that the
tradeoff is acceptable.
This commit is contained in:
Géry Debongnie
2017-11-08 10:50:12 +01:00
parent 324d4bfb56
commit 9e97ad8dce
4 changed files with 107 additions and 19 deletions
@@ -502,9 +502,11 @@ var FieldMany2One = AbstractField.extend({
title: _t("Open: ") + self.string,
view_id: view_id,
readonly: !self.can_write,
on_saved: function () {
self._setValue(self.value.data, {forceChange: true});
self.trigger_up('reload', {db_id: self.value.id});
on_saved: function (record, changed) {
if (changed) {
self._setValue(self.value.data, {forceChange: true});
self.trigger_up('reload', {db_id: self.value.id});
}
},
}).open();
});
@@ -896,6 +896,9 @@ var BasicModel = AbstractModel.extend({
return def.resolve(changedFields);
}
def.then(function () {
record._isDirty = false;
});
// in the case of a write, only perform the RPC if there are changes to save
if (method === 'create' || changedFields.length) {
var args = method === 'write' ? [[record.data.id], changes] : [changes];
@@ -917,7 +920,6 @@ var BasicModel = AbstractModel.extend({
// Erase changes as they have been applied
record._changes = {};
record._isDirty = false;
// Update the data directly or reload them
if (shouldReload) {
+7 -15
View File
@@ -81,9 +81,9 @@ var FormViewDialog = ViewDialog.extend({
* @param {Object} [options.fields_view] optional form fields_view
* @param {boolean} [options.readonly=false] only applicable when not in
* creation mode
* @param {function} [options.on_save] callback to execute when clicking on
* 'Save' (form view's 'saveRecord' by default)
* @param {function} [options.on_saved] callback executed after on_save
* @param {function} [options.on_saved] callback executed after saving a
* record. It will be called with the record data, and a boolean which
* indicates if something was changed
* @param {BasicModel} [options.model] if given, it will be used instead of
* a new form view model
* @param {string} [options.recordID] if given, the model has to be given as
@@ -211,24 +211,16 @@ var FormViewDialog = ViewDialog.extend({
_save: function () {
var self = this;
var def;
if (this.options.on_save) {
if (this.form_view.canBeSaved()) {
return $.Deferred().reject();
}
def = this.options.on_save(this.form_view.model.get(this.form_view.handle));
} else {
def = this.form_view.saveRecord(this.form_view.handle, {
return this.form_view.saveRecord(this.form_view.handle, {
stayInEdit: true,
reload: false,
savePoint: this.shouldSaveLocally,
viewType: 'form',
});
}
return $.when(def).then(function () {
}).then(function (changedFields) {
// record might have been changed by the save (e.g. if this was a new record, it has an
// id now), so don't re-use the copy obtained before the save
self.on_saved(self.form_view.model.get(self.form_view.handle));
var record = self.form_view.model.get(self.form_view.handle);
self.on_saved(record, !!changedFields.length);
});
},
});
@@ -210,6 +210,98 @@ QUnit.module('relational_fields', {
form.destroy();
});
QUnit.test('editing a many2one, but not changing anything', function (assert) {
assert.expect(2);
var form = createView({
View: FormView,
model: 'partner',
data: this.data,
arch: '<form string="Partners">' +
'<sheet>' +
'<field name="trululu"/>' +
'</sheet>' +
'</form>',
archs: {
'partner,false,form': '<form string="Partners"><field name="display_name"/></form>',
},
res_id: 1,
mockRPC: function (route, args) {
if (args.method === 'get_formview_id') {
assert.deepEqual(args.args[0], [4], "should call get_formview_id with correct id");
return $.when(false);
}
return this._super(route, args);
},
viewOptions: {
ids: [1, 2],
},
});
form.$buttons.find('.o_form_button_edit').click();
// click on the external button (should do an RPC)
form.$('.o_external_button').click();
// save and close modal
$('.modal .modal-footer .btn-primary:first').click();
// save form
form.$buttons.find('.o_form_button_save').click();
// click next on pager
form.pager.$('.o_pager_next').click();
// this checks that the view did not ask for confirmation that the
// record is dirty
assert.strictEqual(form.pager.$el.text().trim(), '2 / 2',
'pager should be at second page');
form.destroy();
});
QUnit.test('editing a many2one (with form view opened with external button)', function (assert) {
assert.expect(1);
var form = createView({
View: FormView,
model: 'partner',
data: this.data,
arch: '<form string="Partners">' +
'<sheet>' +
'<field name="trululu"/>' +
'</sheet>' +
'</form>',
archs: {
'partner,false,form': '<form string="Partners"><field name="foo"/></form>',
},
res_id: 1,
mockRPC: function (route, args) {
if (args.method === 'get_formview_id') {
return $.when(false);
}
return this._super(route, args);
},
viewOptions: {
ids: [1, 2],
},
});
form.$buttons.find('.o_form_button_edit').click();
// click on the external button (should do an RPC)
form.$('.o_external_button').click();
$('.modal input[name="foo"]').val('brandon').trigger('input');
// save and close modal
$('.modal .modal-footer .btn-primary:first').click();
// save form
form.$buttons.find('.o_form_button_save').click();
// click next on pager
form.pager.$('.o_pager_next').click();
// this checks that the view did not ask for confirmation that the
// record is dirty
assert.strictEqual(form.pager.$el.text().trim(), '2 / 2',
'pager should be at second page');
form.destroy();
});
QUnit.test('many2ones in form views with show_adress', function (assert) {
assert.expect(4);
var form = createView({