[FIX] web: do no drop invalid record in list on discard

Revision on b09f0c99

The intent of the previous commit was to no drop records in a list
on discard in the following cases:

    - when they have been created from a `default_get`
    - when they have been created from an `onchange` that result
      from a `default_get`

Before the commit above, the kind of records that were abandoned
in those cases are stated as invalid, which happens when a record
has a required field that is empty.

The case of an `onchange` that is not involved in a `default_get`
was not considered in the fixes above. In fact, this is the intent
behaviour in most cases. However, if the items in the list are
created from such an `onchange`, we should not drop them.

This commit fixes the bug where records in a list that are created
from an `onchange` are wrongly abandoned. Here is the new logic to
abandon a record on discard in a list:

    - the record that is marked as "do not abandon" should not be
      abandoned;
    - a record that is registered as a "not new addition" in the
      list should not be dropped;
    - a record that is registered as a "new addition" in the list,
      and has been updated afterwards, should not be dropped;
    - a record that is not new should not be dropped;

opw-806650
This commit is contained in:
Alexandre Kühn
2018-06-15 17:36:27 +02:00
parent b09b3d54d4
commit c53b0ac188
2 changed files with 156 additions and 12 deletions
@@ -573,13 +573,16 @@ var BasicModel = AbstractModel.extend({
/**
* Returns true if a record can be abandoned.
*
* A record can be abandonned if it is a new record, except if
* this datapoint has been specifically tagged as "do not abandon".
* Case for not abandoning the record:
*
* Example:
* 1. flagged as 'no abandon' (i.e. during a `default_get`, including any
* `onchange` from a `default_get`)
* 2. registered in a list on addition
* 2.1. registered as non-new addition
* 2.2. registered as new additon on update
* 3. record is not new
*
* - Discard record from "Add an item" => "New" record => abandon
* - Discard record from `default_get`/`onchange` => do not abandon
* Otherwise, the record can be abandoned.
*
* This is useful when discarding changes on this record, as it means that
* we must keep the record even if some fields are invalids (e.g. required
@@ -589,7 +592,29 @@ var BasicModel = AbstractModel.extend({
* @returns {boolean}
*/
canBeAbandoned: function (id) {
return !this.localData[id]._noAbandon && this.isNew(id);
// 1. no drop if flagged
if (this.localData[id]._noAbandon) {
return false;
}
// 2. no drop in a list on "ADD in some cases
var record = this.localData[id];
var parent = this.localData[record.parentID];
if (parent) {
var entry = _.findWhere(parent._savePoint, {operation: 'ADD', id: id});
if (entry) {
// 2.1. no drop on non-new addition in list
if (!entry.isNew) {
return false;
}
// 2.2. no drop on new addition on "UPDATE"
var lastEntry = _.last(parent._savePoint);
if (lastEntry.operation === 'UPDATE' && lastEntry.id === id) {
return false;
}
}
}
// 3. drop new records
return this.isNew(id);
},
/**
* Returns true if a record is dirty. A record is considered dirty if it has
@@ -1179,7 +1179,7 @@ QUnit.module('relational_fields', {
form.destroy();
});
QUnit.test('list in form: discard newly added element with empty required field (default_get)', function (assert) {
QUnit.test('item not dropped on discard with empty required field (default_get)', function (assert) {
// This test simulates discarding a record that has been created with
// one of its required field that is empty. When we discard the changes
// on this empty field, it should not assume that this record should be
@@ -1323,7 +1323,7 @@ QUnit.module('relational_fields', {
form.destroy();
});
QUnit.test('list in form: discard newly added element with empty required field (onchange in default_get)', function (assert) {
QUnit.test('list in form: item not dropped on discard with empty required field (onchange in default_get)', function (assert) {
// variant of the test "list in form: discard newly added element with
// empty required field (default_get)", in which the `default_get`
// performs an `onchange` at the same time. This `onchange` may create
@@ -1393,11 +1393,13 @@ QUnit.module('relational_fields', {
form.destroy();
});
QUnit.test('list in form: discard newly added element with empty required field (onchange after default_get)', function (assert) {
QUnit.test('list in form: item not dropped on discard with empty required field (onchange on list after default_get)', function (assert) {
// discarding a record from an `onchange` in a `default_get` should not
// abandon the record. However, any `onchange` after `default_get`
// should be abandoned on discard
assert.expect(6);
// abandon the record. This should not be the case for following
// `onchange`, except if an onchange make some changes on the list:
// in particular, if an onchange make changes on the list such that
// a record is added, this record should not be dropped on discard
assert.expect(8);
var M2O_DELAY = relationalFields.FieldMany2One.prototype.AUTOCOMPLETE_DELAY;
relationalFields.FieldMany2One.prototype.AUTOCOMPLETE_DELAY = 0;
@@ -1449,10 +1451,127 @@ QUnit.module('relational_fields', {
// click off so that the required field still stay empty
$('body').click();
// record should not be dropped
assert.strictEqual(form.$('.o_data_row').length, 1,
"should not have dropped record in the editable list");
assert.strictEqual($('td.o_data_cell').first().text(), "entry",
"should still have the correct displayed name");
assert.strictEqual($('td.o_data_cell.o_required_modifier').text(), "",
"should still have empty string in the required field");
relationalFields.FieldMany2One.prototype.AUTOCOMPLETE_DELAY = M2O_DELAY;
form.destroy();
});
QUnit.test('item dropped on discard with empty required field with "Add an item" (invalid on "ADD")', function (assert) {
// when a record in a list is added with "Add an item", it should
// always be dropped on discard if some required field are empty
// at the record creation.
assert.expect(6);
var form = createView({
View: FormView,
model: 'partner',
data: this.data,
arch: '<form>' +
'<field name="p">' +
'<tree editable="bottom">' +
'<field name="display_name"/>' +
'<field name="trululu" required="1"/>' +
'</tree>' +
'</field>' +
'</form>',
});
// Click on "Add an item"
form.$('.o_field_x2many_list_row_add a').click();
var charField = form.$('.o_field_widget.o_field_char[name="display_name"]');
var requiredField = form.$('.o_field_widget.o_required_modifier[name="trululu"]');
charField.val("some text");
assert.strictEqual(charField.length, 1,
"should have a char field 'display_name' on this record");
assert.notOk(charField.hasClass('o_required_modifier'),
"the char field should not be required on this record");
assert.strictEqual(charField.val(), "some text",
"should have entered text in the char field on this record");
assert.strictEqual(requiredField.length, 1,
"should have a required field 'trululu' on this record");
assert.strictEqual(requiredField.val().trim(), "",
"should have empty string in the required field on this record");
// click on empty required field in editable list record
requiredField.click();
// click off so that the required field still stay empty
$('body').click();
// record should be dropped
assert.strictEqual(form.$('.o_data_row').length, 0,
"should have dropped record in the editable list");
form.destroy();
});
QUnit.test('item not dropped on discard with empty required field with "Add an item" (invalid on "UPDATE")', function (assert) {
// when a record in a list is added with "Add an item", it should
// be temporarily added to the list when it is valid (e.g. required
// fields are non-empty). If the record is updated so that the required
// field is empty, and it is discarded, then the record should not be
// dropped.
assert.expect(8);
var M2O_DELAY = relationalFields.FieldMany2One.prototype.AUTOCOMPLETE_DELAY;
relationalFields.FieldMany2One.prototype.AUTOCOMPLETE_DELAY = 0;
var form = createView({
View: FormView,
model: 'partner',
data: this.data,
arch: '<form>' +
'<field name="p">' +
'<tree editable="bottom">' +
'<field name="display_name"/>' +
'<field name="trululu" required="1"/>' +
'</tree>' +
'</field>' +
'</form>',
});
assert.strictEqual(form.$('.o_data_row').length, 0,
"should initially not have any record in the list");
// Click on "Add an item"
form.$('.o_field_x2many_list_row_add a').click();
assert.strictEqual(form.$('.o_data_row').length, 1,
"should have a temporary record in the list");
var $inputEditMode = form.$('.o_field_widget.o_required_modifier[name="trululu"] input');
assert.strictEqual($inputEditMode.length, 1,
"should have a required field 'trululu' on this record");
assert.strictEqual($inputEditMode.val(), "",
"should have empty string in the required field on this record");
// add something to required field and leave edit mode of the record
$inputEditMode.click();
$('li.ui-menu-item').first().click();
$('body').click(); // leave edit mode on the line
var $inputReadonlyMode = form.$('.o_data_cell.o_required_modifier');
assert.strictEqual(form.$('.o_data_row').length, 1,
"should not have dropped valid record when leaving edit mode");
assert.strictEqual($inputReadonlyMode.text(), "first record",
"should have put some content in the required field on this record");
// remove the required field and leave edit mode of the record
$('.o_data_row').click(); // enter edit mode on the line
$inputEditMode.click();
$inputEditMode.val("");
$('body').click();
assert.strictEqual(form.$('.o_data_row').length, 1,
"should not have dropped record in the list on discard (invalid on UPDATE)");
assert.strictEqual($inputReadonlyMode.text(), "first record",
"should keep previous valid required field content on this record");
relationalFields.FieldMany2One.prototype.AUTOCOMPLETE_DELAY = M2O_DELAY;
form.destroy();
});