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

[FIX] web: keep empty string value for char/text fields

Browse Source 
For char/text fields, the server can return false or "" (empty
string) depending if the field isn't set in db (NULL) or set to
the empty string. This makes no difference in the UI, but it
matters when evaluating modifiers.

Before this commit, the value of char/text fields was automatically
set to the empty string if it was falsy, as it doesn't impact the
UI, and it is cleaner type-wise. Unfortunately, we cannot do that
for the eval context, as we must be able to make the difference
between False and "" evaluating modifiers. A solution would have
been to stop fallbacking on the empty string when the value was
false, but we wanted to keep the clean API. So this commit fixes
the issue by keeping an internal structure in Record to track
the exact values we received from the server for those fields, and
use that structure to generate the eval context.

Part of task~3179751

Part-of: odoo/odoo#114024
This commit is contained in:
Aaron Bohy
2023-07-24 20:17:52 +02:00
parent 0917241ff7
commit a748e3c4df
8 changed files with 95 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web/static/src/model/relational_model/record.js
+42 -12
View File
@@ -38,10 +38,8 @@ export class Record extends DataPoint {
this._closeInvalidFieldsNotification = () => {};
const missingFields = this.fieldNames.filter((fieldName) => !(fieldName in data));
const vals = this._parseServerValues({
...this._getDefaultValues(missingFields),
...data,
});
data = { ...this._getDefaultValues(missingFields), ...data };
const vals = this._parseServerValues(data);
if (this.resId) {
this._values = markRaw(vals);
this._changes = markRaw({});
@@ -49,11 +47,19 @@ export class Record extends DataPoint {
this._values = markRaw({});
this._changes = markRaw(vals);
}
this.data = { ...this._values, ...this._changes };
// In db, char, text and html fields can be not set (NULL) and set to the empty string. In
// the UI, there's no difference, but in the eval context, it's not the same. The next
// structure keeps track of the server values we received for those fields (which can thus
// be false or a string). This allows us to properly build the eval context, and to always
// expose string values (false fallbacks on the empty string) in this.data.
this._textValues = markRaw({});
this._setTextValues(data);
this._savePoint = markRaw({
dirty: false,
changes: { ...this._changes },
textValues: { ...this._textValues },
});
this.data = { ...this._values, ...this._changes };
const parentRecord = this._parentRecord;
if (parentRecord) {
@@ -146,7 +152,8 @@ export class Record extends DataPoint {
this.model._updateConfig(this.config, { resId: false }, { noReload: true });
this.dirty = false;
this._changes = this._parseServerValues(this._getDefaultValues());
this._values = {};
this._values = markRaw({});
this._textValues = markRaw({});
this.data = { ...this._changes };
this._setEvalContext();
}
@@ -259,6 +266,7 @@ export class Record extends DataPoint {
_addSavePoint() {
this._savePoint.dirty = this.dirty;
Object.assign(this._savePoint.textValues, this._textValues);
Object.assign(this._savePoint.changes, this._changes);
for (const fieldName in this._changes) {
if (["one2many", "many2many"].includes(this.fields[fieldName].type)) {
@@ -272,6 +280,7 @@ export class Record extends DataPoint {
this._changes[fieldName] = changes[fieldName];
this.data[fieldName] = changes[fieldName];
}
this._setTextValues(changes);
this._setEvalContext();
this._removeInvalidFields(Object.keys(changes));
}
@@ -291,6 +300,7 @@ export class Record extends DataPoint {
_applyValues(values) {
Object.assign(this._values, this._parseServerValues(values));
Object.assign(this.data, this._values, this._changes);
this._setTextValues(Object.assign({}, values, this._changes));
this._setEvalContext();
}
@@ -353,8 +363,8 @@ export class Record extends DataPoint {
for (const fieldName in data) {
const value = data[fieldName];
const field = this.fields[fieldName];
if (["char", "text"].includes(field.type)) {
dataContext[fieldName] = value !== "" ? value : false;
if (["char", "text", "html"].includes(field.type)) {
dataContext[fieldName] = this._textValues[fieldName];
} else if (["one2many", "many2many"].includes(field.type)) {
dataContext[fieldName] = value.resIds;
} else if (value && field.type === "date") {
@@ -407,8 +417,9 @@ export class Record extends DataPoint {
}
}
this.dirty = this._savePoint.dirty;
this._changes = { ...this._savePoint.changes };
this._changes = markRaw({ ...this._savePoint.changes });
this.data = { ...this._values, ...this._changes };
this._textValues = markRaw({ ...this._savePoint.textValues });
this._setEvalContext();
this._invalidFields.clear();
this._closeInvalidFieldsNotification();
@@ -523,14 +534,22 @@ export class Record extends DataPoint {
if (this.resId) {
this.model._updateSimilarRecords(this, values);
this._values = this._parseServerValues(values);
this._changes = {};
this._changes = markRaw({});
this._setTextValues(values);
} else {
this._values = {};
this._changes = this._parseServerValues({ ...this._getDefaultValues(), ...values });
this._values = markRaw({});
const allVals = { ...this._getDefaultValues(), ...values };
this._changes = this._parseServerValues(allVals);
this._setTextValues(allVals);
}
this.dirty = false;
this.data = { ...this._values, ...this._changes };
this._setEvalContext();
this._savePoint = markRaw({
dirty: false,
changes: { ...this._changes },
textValues: { ...this._textValues },
});
this._invalidFields.clear();
}
@@ -816,6 +835,17 @@ export class Record extends DataPoint {
this._invalidFields.add(fieldName);
}
_setTextValues(values) {
for (const fieldName in values) {
if (!this.activeFields[fieldName]) {
continue;
}
if (["char", "text", "html"].includes(this.fields[fieldName].type)) {
this._textValues[fieldName] = values[fieldName];
}
}
}
_switchMode(mode) {
this.model._updateConfig(this.config, { mode }, { noReload: true });
if (mode === "readonly") {
addons/web/static/src/model/relational_model/utils.js
+3 -3
View File
@@ -333,15 +333,15 @@ export function parseServerValue(field, value) {
case "text": {
return value || "";
}
case "html": {
return markup(value || "");
}
case "date": {
return value ? deserializeDate(value) : false;
}
case "datetime": {
return value ? deserializeDateTime(value) : false;
}
case "html": {
return markup(value || "");
}
case "selection": {
if (value === false) {
// process selection: convert false to 0, if 0 is a valid key
addons/web/static/src/views/fields/ace/ace_field.js
+1 -2
View File
@@ -4,7 +4,6 @@ import { _lt } from "@web/core/l10n/translation";
import { registry } from "@web/core/registry";
import { useBus, useService } from "@web/core/utils/hooks";
import { effect } from "@web/core/utils/reactive";
import { formatText } from "../formatters";
import { standardFieldProps } from "../standard_field_props";
import { CodeEditor } from "@web/core/code_editor/code_editor";
@@ -36,7 +35,7 @@ export class AceField extends Component {
effect(
async ({ record, mode, name, readonly }) => {
if (this.lastSetValue !== record.data[name]) {
this.state.value = formatText(record.data[name]);
this.state.value = record.data[name];
}
this.state.mode = mode === "xml" ? "qweb" : mode;
this.state.readonly = readonly;
addons/web/static/src/views/fields/formatters.js
+2 -16
View File
@@ -133,10 +133,7 @@ export function formatBoolean(value) {
}
/**
* Returns a string representing a char. If the value is false, then we return
* an empty string.
*
* @param {string|false} value
* @param {string} value
* @param {Object} [options] additional options
* @param {boolean} [options.escape=false] if true, escapes the formatted value
* @param {boolean} [options.isPassword=false] if true, returns '********'
@@ -144,7 +141,6 @@ export function formatBoolean(value) {
* @returns {string}
*/
export function formatChar(value, options) {
value = typeof value === "string" ? value : "";
if (options && options.isPassword) {
return "*".repeat(value ? value.length : 0);
}
@@ -487,16 +483,6 @@ export function formatSelection(value, options = {}) {
return option ? option[1] : "";
}
/**
* Returns the value or an empty string if it's falsy.
*
* @param {string | false} value
* @returns {string}
*/
export function formatText(value) {
return value || "";
}
export function formatJson(value) {
return (value && JSON.stringify(value)) || "";
}
@@ -524,4 +510,4 @@ registry
.add("properties_definition", formatProperties)
.add("reference", formatReference)
.add("selection", formatSelection)
.add("text", formatText);
.add("text", (value) => value);
addons/web/static/tests/helpers/mock_server.js
+12 -2
View File
@@ -898,7 +898,6 @@ export class MockServer {
return [id, name];
}
/**
* Simulate a 'name_search' operation.
*
@@ -949,6 +948,17 @@ export class MockServer {
onchangeValues[fName] = false;
});
const defaultValues = this.mockDefaultGet(modelName, [fieldsFromView], kwargs);
for (const fieldName in defaultValues) {
const fieldType = this.models[modelName].fields[fieldName].type;
if (["one2many", "many2many"].includes(fieldType)) {
const subSpec = specification[fieldName];
for (const command of defaultValues[fieldName]) {
if (command[0] === 0 || command[0] === 1) {
command[2] = pick(command[2], ...Object.keys(subSpec.fields));
}
}
}
}
Object.assign(onchangeValues, defaultValues);
}
fields.forEach((field) => {
@@ -1033,7 +1043,7 @@ export class MockServer {
} else if (field.type === "one2many" || field.type === "many2many") {
result[fieldName] = record[fieldName] || [];
} else {
result[fieldName] = record[fieldName] || false;
result[fieldName] = record[fieldName] !== undefined ? record[fieldName] : false;
}
}
records.push(result);
addons/web/static/tests/views/fields/many2one_field_tests.js
+1 -1
View File
@@ -2396,7 +2396,7 @@ QUnit.module("Fields", (hooks) => {
assert.expect(3);
serverData.models.partner.fields.timmy.default = [
[0, 0, { display_name: "brandon is the new timmy", name: "brandon" }],
[0, 0, { display_name: "brandon is the new timmy" }],
];
serverData.models.partner.onchanges.timmy = (obj) => {
obj.int_field = obj.timmy.length;
addons/web/static/tests/views/form/form_view_tests.js
+28
View File
@@ -1236,6 +1236,34 @@ QUnit.module("Views", (hooks) => {
}
);
QUnit.test("invisible attrs char fields", async function (assert) {
// For a char/text field, the server can return false or "" (empty string),
// depending if the field isn't set in db (NULL) or set to the empty string.
// This makes no difference in the UI, but it matters when evaluating modifiers.
serverData.models.partner.records[0].display_name = false;
serverData.models.partner.records[0].foo = "";
await makeView({
type: "form",
resModel: "partner",
serverData,
arch: `
<form>
<div class="a" attrs='{"invisible": [("foo", "=", False)]}'>a</div>
<div class="b" attrs='{"invisible": [("foo", "=", "")]}'>b</div>
<div class="c" attrs='{"invisible": [("display_name", "=", False)]}'>c</div>
<div class="d" attrs='{"invisible": [("display_name", "=", "")]}'>d</div>
<field name="foo" invisible="1"/>
</form>`,
resId: 1,
});
assert.containsOnce(target, "div.a");
assert.containsNone(target, "div.b");
assert.containsNone(target, "div.c");
assert.containsOnce(target, "div.d");
});
QUnit.test(
"properly handle modifiers and attributes on notebook tags",
async function (assert) {
addons/web/static/tests/views/kanban/kanban_view_tests.js
+6 -6
View File
@@ -5282,18 +5282,18 @@ QUnit.module("Views", (hooks) => {
QUnit.test("Ensuring each progress bar has some space", async (assert) => {
serverData.models.partner.records = [
({
{
id: 1,
foo: "blip",
state: "def",
}),
({
},
{
id: 2,
foo: "blip",
state: "abc",
}),
},
];
for (let i = 0; i < 20; i++) {
serverData.models.partner.records.push({
id: 3 + i,
@@ -5301,7 +5301,7 @@ QUnit.module("Views", (hooks) => {
state: "ghi",
});
}
await makeView({
type: "kanban",
resModel: "partner",