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

[FIX] web: Record.update with onChange

Browse Source 
The goal of this commit is to resolve:
=====================================
1. When you edit a Char or Boolean field with an onChange that returns
   the value before editing, you must display the value of the onChange.
2. In the model, html fields are always Markup fields. But when we evaluate
   a modifier containing an html field, we use the server value (a string or false).
3. When we edit a field and its onChange returns an server error, we want to
   revert its value to the valuea before edition.

Solution:
=========
1. Always render when applying a change, even if the value does not change,
   because onChange returns the initial value. To do this, we'll apply
   the changes to record.data, then apply the changes from onChange.
   The value will therefore change, which will cause all the components
   observing the value in record.data to render.
2. When we update an html field, its value will always be markup and
   we'll store it as a string in _textValue so that we can evaluate the
   modifiers without the markup. For changes coming from the onChange, we
   will store the value in _textValue and apply markup on it for record.data.
3. When an onChange returns an server error, we will apply the changes and then
   revert them directly. This will have the effect of triggering a rendering
   on all the components observing to the record.data corresponding to
   the changes and so displaying the values before editing.

How to reproduce:
=================
Case 1:
------
- Go to a form view containing a boolean field with an onChange
- Check the boolean field
- The onChange returns with the value false

Before this commit:
    The boolean field is always checked

After this commit:
    The boolean field is no longer checked

Case 2:
------
- Go to a form view containing an html field and an field "x" with
  invisible ="not html_field" and an onChange
- Edit the field "x"
- The onChange returns the value false for "html_field".

Before this commit:
    The field "x" is always displayed

After this commit
    The field "x" is no longer displayed

Case 3:
------
- Go to a form view containing a char field with an onChange
- Edit the field
- onChange returns a server error

Before this commit:
    The field is marked as invalid and contains the value after editing.

After this commit:
    The field contains the pre-edit value

closes odoo/odoo#136924

Related: odoo/enterprise#48515
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
This commit is contained in:
FrancoisGe
2023-10-06 09:39:24 +00:00
parent 3b4e8ea422
commit c830d9bc9d
9 changed files with 279 additions and 88 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web/static/src/model/relational_model/record.js
+129 -76
View File
@@ -7,7 +7,6 @@ import { serializeDate, serializeDateTime } from "@web/core/l10n/dates";
import { _t } from "@web/core/l10n/translation";
import { x2ManyCommands } from "@web/core/orm_service";
import { evaluateBooleanExpr } from "@web/core/py_js/py";
import { pick } from "@web/core/utils/objects";
import { escape } from "@web/core/utils/strings";
import { DataPoint } from "./datapoint";
import {
@@ -173,7 +172,7 @@ export class Record extends DataPoint {
} else {
this.model._updateConfig(this.config, { resId: false }, { reload: false });
this.dirty = false;
this._changes = this._parseServerValues(this._getDefaultValues());
this._changes = markRaw(this._parseServerValues(this._getDefaultValues()));
this._values = markRaw({});
this._textValues = markRaw({});
this.data = { ...this._changes };
@@ -256,7 +255,12 @@ export class Record extends DataPoint {
if (this.model._urgentSave) {
return this._update(changes, { save: false }); // save is already scheduled
}
return this.model.mutex.exec(() => this._update(changes, { save }));
return this.model.mutex.exec(async () => {
await this._update(changes, { withoutOnchange: save });
if (save) {
return this._save();
}
});
}
urgentSave() {
@@ -283,14 +287,49 @@ export class Record extends DataPoint {
}
}
_applyChanges(changes) {
_applyChanges(changes, serverChanges = {}) {
// We need to generate the undo function before applying the changes
const fieldNameChanges = [...Object.keys({ ...changes, ...serverChanges })];
const initialTextValues = { ...this._textValues };
const initialChanges = { ...this._changes };
const initialData = { ...toRaw(this.data) };
const invalidFields = toRaw(this._invalidFields);
const invalidFieldNames = fieldNameChanges.filter((fieldName) =>
invalidFields.has(fieldName)
);
const undoChanges = () => {
for (const fieldName of invalidFieldNames) {
this.setInvalidField(fieldName);
}
Object.assign(this.data, initialData);
this._changes = markRaw(initialChanges);
Object.assign(this._textValues, initialTextValues);
this._setEvalContext();
};
// Apply changes
for (const fieldName in changes) {
this._changes[fieldName] = changes[fieldName];
this.data[fieldName] = changes[fieldName];
const change = changes[fieldName];
this._changes[fieldName] = change;
this.data[fieldName] = change;
if (this.fields[fieldName].type === "html") {
this._textValues[fieldName] = change === false ? false : change.toString();
} else if (["char", "text"].includes(this.fields[fieldName].type)) {
this._textValues[fieldName] = change;
}
}
Object.assign(this._textValues, this._getTextValues(changes));
// Apply server changes
const parsedChanges = this._parseServerValues(serverChanges, this.data);
for (const fieldName in parsedChanges) {
this._changes[fieldName] = parsedChanges[fieldName];
this.data[fieldName] = parsedChanges[fieldName];
}
Object.assign(this._textValues, this._getTextValues(serverChanges));
this._setEvalContext();
this._removeInvalidFields(Object.keys(changes));
this._removeInvalidFields(fieldNameChanges);
return undoChanges;
}
_applyDefaultValues() {
@@ -299,7 +338,7 @@ export class Record extends DataPoint {
});
const defaultValues = this._getDefaultValues(fieldNames);
if (this.isNew) {
this._applyChanges(this._parseServerValues(defaultValues));
this._applyChanges({}, defaultValues);
} else {
this._applyValues(defaultValues);
}
@@ -674,14 +713,6 @@ export class Record extends DataPoint {
return parsedValues;
}
async _preprocessChanges(changes) {
await Promise.all([
this._preprocessMany2oneChanges(changes),
this._preprocessReferenceChanges(changes),
this._preprocessX2manyChanges(changes),
]);
}
async _preprocessReferenceChanges(changes) {
const proms = [];
for (const [fieldName, value] of Object.entries(changes)) {
@@ -812,6 +843,38 @@ export class Record extends DataPoint {
}
}
_preprocessPropertiesChanges(changes) {
for (const [fieldName, value] of Object.entries(changes)) {
const field = this.fields[fieldName];
if (field && field.relatedPropertyField) {
const [propertyFieldName, propertyName] = field.name.split(".");
const propertiesData = this.data[propertyFieldName] || [];
if (!propertiesData.find((property) => property.name === propertyName)) {
// try to change the value of a properties that has a different parent
this.model.notification.add(
_t(
"This record belongs to a different parent so you can not change this property."
),
{ type: "warning" }
);
return;
}
changes[propertyFieldName] = propertiesData.map((property) =>
property.name === propertyName ? { ...property, value } : property
);
delete changes[field.name];
}
}
}
_preprocessHtmlChanges(changes) {
for (const [fieldName, value] of Object.entries(changes)) {
if (this.fields[fieldName].type === "html") {
changes[fieldName] = value === false ? false : markup(value);
}
}
}
_removeInvalidFields(fieldNames) {
for (const fieldName of fieldNames) {
this._invalidFields.delete(fieldName);
@@ -911,11 +974,11 @@ export class Record extends DataPoint {
}
this._setData(records[0]);
} else {
this._values = { ...this._values, ...this._changes };
this._values = markRaw({ ...this._values, ...this._changes });
if ("id" in this.activeFields) {
this._values.id = records[0].id;
}
this._changes = {};
this._changes = markRaw({});
this.data = { ...this._values };
this.dirty = false;
}
@@ -979,63 +1042,57 @@ export class Record extends DataPoint {
}
}
async _update(changes, { withoutOnchange, withoutParentUpdate, save } = {}) {
async _getOnchangeValues(changes) {
const onChangeFields = Object.keys(changes).filter(
(fieldName) => this.activeFields[fieldName] && this.activeFields[fieldName].onChange
);
if (!onChangeFields.length) {
return {};
}
const localChanges = this._getChanges(
{ ...this._changes, ...changes },
{ withReadonly: true }
);
if (this.config.relationField) {
localChanges[this.config.relationField] = this._parentRecord._getChanges();
if (!this._parentRecord.isNew) {
localChanges[this.config.relationField].id = this._parentRecord.resId;
}
}
return this.model._onchange(this.config, {
changes: localChanges,
fieldNames: onChangeFields,
evalContext: toRaw(this.evalContext),
onError: (e) => {
// We apply changes and revert them after to force a render of the Field components
const undoChanges = this._applyChanges(changes);
undoChanges();
throw e;
},
});
}
async _update(changes, { withoutOnchange, withoutParentUpdate } = {}) {
this.dirty = true;
const prom = this._preprocessChanges(changes);
if (prom && !this.model._urgentSave) {
const prom = Promise.all([
this._preprocessMany2oneChanges(changes),
this._preprocessReferenceChanges(changes),
this._preprocessX2manyChanges(changes),
this._preprocessPropertiesChanges(changes),
this._preprocessHtmlChanges(changes),
]);
if (!this.model._urgentSave) {
await prom;
}
if (this.selected && this.model.multiEdit) {
this._applyChanges(changes);
return this.model.root._multiSave(this);
}
for (const [fieldName, value] of Object.entries(changes)) {
const field = this.fields[fieldName];
if (field && field.relatedPropertyField) {
const [propertyFieldName, propertyName] = field.name.split(".");
const propertiesData = this.data[propertyFieldName] || [];
if (!propertiesData.find((property) => property.name === propertyName)) {
// try to change the value of a properties that has a different parent
this.model.notification.add(
_t(
"This record belongs to a different parent so you can not change this property."
),
{ type: "warning" }
);
return;
}
changes[propertyFieldName] = propertiesData.map((property) =>
property.name === propertyName ? { ...property, value } : property
);
delete changes[field.name];
}
}
const onChangeFields = Object.keys(changes).filter(
(fieldName) => this.activeFields[fieldName] && this.activeFields[fieldName].onChange
);
if (onChangeFields.length && !this.model._urgentSave && !withoutOnchange && !save) {
const localChanges = this._getChanges(
{ ...this._changes, ...changes },
{ withReadonly: true }
);
if (this.config.relationField) {
localChanges[this.config.relationField] = this._parentRecord._getChanges();
if (!this._parentRecord.isNew) {
localChanges[this.config.relationField].id = this._parentRecord.resId;
}
}
const otherChanges = await this.model._onchange(this.config, {
changes: localChanges,
fieldNames: onChangeFields,
evalContext: this.evalContext,
onError: (e) => {
for (const fieldName in localChanges) {
this._setInvalidField(fieldName);
}
throw e;
},
});
Object.assign(changes, this._parseServerValues(otherChanges, this.data));
let onchangeServerValues = {};
if (!this.model._urgentSave && !withoutOnchange) {
onchangeServerValues = await this._getOnchangeValues(changes);
}
// changes inside the record set as value for a many2one field must trigger the onchange,
// but can't be considered as changes on the parent record, so here we detect if many2one
@@ -1049,20 +1106,16 @@ export class Record extends DataPoint {
}
}
}
if (Object.keys(changes).length > 0) {
const initialChanges = pick(this.data, ...Object.keys(changes));
this._applyChanges(changes);
const undoChanges = this._applyChanges(changes, onchangeServerValues);
if (Object.keys(changes).length > 0 || Object.keys(onchangeServerValues).length > 0) {
try {
await this._onUpdate({ withoutParentUpdate });
} catch (e) {
this._applyChanges(initialChanges);
undoChanges();
throw e;
}
await this.model.hooks.onRecordChanged(this, this._getChanges());
}
if (save) {
return this._save();
}
}
_updateChildrenContext() {
addons/web/static/src/views/fields/boolean/boolean_field.js
+13 -5
View File
@@ -1,11 +1,11 @@
/** @odoo-module **/
import { registry } from "@web/core/registry";
import { _t } from "@web/core/l10n/translation";
import { standardFieldProps } from "../standard_field_props";
import { Component, useState } from "@odoo/owl";
import { CheckBox } from "@web/core/checkbox/checkbox";
import { Component } from "@odoo/owl";
import { _t } from "@web/core/l10n/translation";
import { registry } from "@web/core/registry";
import { useRecordObserver } from "@web/model/relational_model/utils";
import { standardFieldProps } from "../standard_field_props";
export class BooleanField extends Component {
static template = "web.BooleanField";
@@ -14,10 +14,18 @@ export class BooleanField extends Component {
...standardFieldProps,
};
setup() {
this.state = useState({});
useRecordObserver((record) => {
this.state.value = record.data[this.props.name];
});
}
/**
* @param {boolean} newValue
*/
onChange(newValue) {
this.state.value = newValue;
this.props.record.update({ [this.props.name]: newValue });
}
}
addons/web/static/src/views/fields/boolean/boolean_field.xml
+1 -1
View File
@@ -2,7 +2,7 @@
<templates xml:space="preserve">
<t t-name="web.BooleanField">
<CheckBox id="props.id" value="props.record.data[props.name] or false" className="'d-inline-block'" disabled="props.readonly" onChange.bind="onChange" />
<CheckBox id="props.id" value="state.value" className="'d-inline-block'" disabled="props.readonly" onChange.bind="onChange" />
</t>
</templates>
addons/web/static/src/views/fields/boolean_toggle/boolean_toggle_field.js
+1
View File
@@ -12,6 +12,7 @@ export class BooleanToggleField extends BooleanField {
};
async onChange(newValue) {
this.state.value = newValue;
const changes = { [this.props.name]: newValue };
await this.props.record.update(changes, { save: this.props.autosave });
}
addons/web/static/tests/views/fields/boolean_field_tests.js
+30
View File
@@ -273,4 +273,34 @@ QUnit.module("Fields", (hooks) => {
"checkbox should still be disabled"
);
});
QUnit.test("onchange return value before toggle checkbox", async function (assert) {
serverData.models.partner.onchanges = {
bar(obj) {
obj.bar = true;
},
};
await makeView({
type: "form",
resModel: "partner",
resId: 1,
serverData,
arch: `<form><field name="bar"/></form>`,
});
assert.containsOnce(
target,
".o_field_boolean input:checked",
"checkbox should still be checked"
);
await click(target, ".o_field_boolean .o-checkbox");
await nextTick();
assert.containsOnce(
target,
".o_field_boolean input:checked",
"checkbox should still be checked"
);
});
});
addons/web/static/tests/views/fields/char_field_tests.js
+60
View File
@@ -795,6 +795,30 @@ QUnit.module("Fields", (hooks) => {
}
);
QUnit.test("onchange return value before editing input", async function (assert) {
serverData.models.partner.onchanges = {
foo(obj) {
obj.foo = "yop";
},
};
await makeView({
type: "form",
resModel: "partner",
resId: 1,
serverData,
arch: `
<form>
<field name="foo" />
</form>`,
});
assert.strictEqual(target.querySelector("[name='foo'] input").value, "yop");
await editInput(target, "[name='foo'] input", "tralala");
assert.strictEqual(target.querySelector("[name='foo'] input").value, "yop");
});
QUnit.test(
"input field: change value before pending onchange renaming",
async function (assert) {
@@ -1050,4 +1074,40 @@ QUnit.module("Fields", (hooks) => {
"Placeholder"
);
});
QUnit.test(
"char field: correct value is used to evaluate the modifiers",
async function (assert) {
serverData.models.partner.onchanges = {
foo: (obj) => {
if (obj.foo === "a") {
obj.display_name = false;
} else if (obj.foo === "b") {
obj.display_name = "";
}
},
};
serverData.models.partner.records[0].foo = false;
serverData.models.partner.records[0].display_name = false;
await makeView({
type: "form",
resModel: "partner",
serverData,
resId: 1,
arch: `
<form>
<field name="foo" />
<field name="display_name" invisible="'' == display_name"/>
</form>`,
});
assert.containsOnce(target, "[name='display_name'] input");
await editInput(target, "[name='foo'] input", "a");
assert.containsOnce(target, "[name='display_name'] input");
await editInput(target, "[name='foo'] input", "b");
assert.containsNone(target, "[name='display_name'] input");
}
);
});
addons/web/static/tests/views/fields/html_field_tests.js
+37
View File
@@ -353,4 +353,41 @@ QUnit.module("Fields", ({ beforeEach }) => {
await clickSave(target);
}
);
QUnit.test(
"html field: correct value is used to evaluate the modifiers",
async function (assert) {
serverData.models.partner.fields.foo = { string: "foo", type: "char" };
serverData.models.partner.onchanges = {
foo: (obj) => {
if (obj.foo === "a") {
obj.txt = false;
} else if (obj.foo === "b") {
obj.txt = "";
}
},
};
serverData.models.partner.records[0].foo = false;
serverData.models.partner.records[0].txt = false;
await makeView({
type: "form",
resModel: "partner",
serverData,
resId: 1,
arch: `
<form>
<field name="foo" />
<field name="txt" invisible="'' == txt"/>
</form>`,
});
assert.containsOnce(target, "[name='txt'] textarea");
await editInput(target, "[name='foo'] input", "a");
assert.containsOnce(target, "[name='txt'] textarea");
await editInput(target, "[name='foo'] input", "b");
assert.containsNone(target, "[name='txt'] textarea");
}
);
});
addons/web/static/tests/views/fields/one2many_field_tests.js
+5 -3
View File
@@ -8930,7 +8930,7 @@ QUnit.module("Fields", (hooks) => {
);
QUnit.test("display correct value after validation error", async function (assert) {
assert.expect(5);
assert.expect(4);
serviceRegistry.add("error", errorService);
function validationHandler(env, error, originalError) {
@@ -8986,8 +8986,10 @@ QUnit.module("Fields", (hooks) => {
await editInput(target, ".o_field_widget[name=turtle_foo] input", "pinky");
await click(target, ".o_form_view");
assert.strictEqual(target.querySelector(".o_data_row .o_data_cell").textContent, "foo");
assert.hasClass(target.querySelector(".o_field_widget[name=turtles]"), "o_field_invalid");
assert.ok(target.querySelector(".o_form_button_save").disabled);
// we make sure here that when we save, the values are the current
// values displayed in the field.
await clickSave(target);
});
QUnit.test("propagate context to sub views without default_* keys", async function (assert) {
addons/web/static/tests/views/form/form_view_tests.js
+3 -3
View File
@@ -6362,9 +6362,9 @@ QUnit.module("Views", (hooks) => {
document.body.querySelector(".modal-body").textContent,
"Some business message"
);
assert.hasClass(
target.querySelector('.o_field_widget[name="int_field"]'),
"o_field_invalid"
assert.strictEqual(
target.querySelector('.o_field_widget[name="int_field"] input').value,
"9"
);
await click(target.querySelector(".modal .btn-close"));