diff --git a/addons/web/static/src/model/relational_model/record.js b/addons/web/static/src/model/relational_model/record.js index 0244663b5d6..3108bd812ee 100644 --- a/addons/web/static/src/model/relational_model/record.js +++ b/addons/web/static/src/model/relational_model/record.js @@ -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() { diff --git a/addons/web/static/src/views/fields/boolean/boolean_field.js b/addons/web/static/src/views/fields/boolean/boolean_field.js index 458bab0086a..7e73704a1c4 100644 --- a/addons/web/static/src/views/fields/boolean/boolean_field.js +++ b/addons/web/static/src/views/fields/boolean/boolean_field.js @@ -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 }); } } diff --git a/addons/web/static/src/views/fields/boolean/boolean_field.xml b/addons/web/static/src/views/fields/boolean/boolean_field.xml index 6b3bd101a62..e61da57a4a8 100644 --- a/addons/web/static/src/views/fields/boolean/boolean_field.xml +++ b/addons/web/static/src/views/fields/boolean/boolean_field.xml @@ -2,7 +2,7 @@ - + diff --git a/addons/web/static/src/views/fields/boolean_toggle/boolean_toggle_field.js b/addons/web/static/src/views/fields/boolean_toggle/boolean_toggle_field.js index c5b63801640..0648281ad4b 100644 --- a/addons/web/static/src/views/fields/boolean_toggle/boolean_toggle_field.js +++ b/addons/web/static/src/views/fields/boolean_toggle/boolean_toggle_field.js @@ -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 }); } diff --git a/addons/web/static/tests/views/fields/boolean_field_tests.js b/addons/web/static/tests/views/fields/boolean_field_tests.js index 08c2237359e..b424d7048ae 100644 --- a/addons/web/static/tests/views/fields/boolean_field_tests.js +++ b/addons/web/static/tests/views/fields/boolean_field_tests.js @@ -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: `
`, + }); + + 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" + ); + }); }); diff --git a/addons/web/static/tests/views/fields/char_field_tests.js b/addons/web/static/tests/views/fields/char_field_tests.js index 46601d33cb4..79eeee289b6 100644 --- a/addons/web/static/tests/views/fields/char_field_tests.js +++ b/addons/web/static/tests/views/fields/char_field_tests.js @@ -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: ` +
+ + `, + }); + + 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: ` +
+ + + `, + }); + 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"); + } + ); }); diff --git a/addons/web/static/tests/views/fields/html_field_tests.js b/addons/web/static/tests/views/fields/html_field_tests.js index 336d4e41192..489ea8775f8 100644 --- a/addons/web/static/tests/views/fields/html_field_tests.js +++ b/addons/web/static/tests/views/fields/html_field_tests.js @@ -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: ` +
+ + + `, + }); + 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"); + } + ); }); diff --git a/addons/web/static/tests/views/fields/one2many_field_tests.js b/addons/web/static/tests/views/fields/one2many_field_tests.js index 45e14cb5b13..88d3427e857 100644 --- a/addons/web/static/tests/views/fields/one2many_field_tests.js +++ b/addons/web/static/tests/views/fields/one2many_field_tests.js @@ -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) { diff --git a/addons/web/static/tests/views/form/form_view_tests.js b/addons/web/static/tests/views/form/form_view_tests.js index 0973e0cdd43..a65bb7c1c75 100644 --- a/addons/web/static/tests/views/form/form_view_tests.js +++ b/addons/web/static/tests/views/form/form_view_tests.js @@ -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"));