From 12b2a7ef462e7328a1545334f2ae18634bf40b8e Mon Sep 17 00:00:00 2001
From: Lucas Perais <lpe@odoo.com>
Date: Wed, 25 Jan 2023 09:34:18 +0000
Subject: [PATCH] [IMP] web: Record: improving Record and RelationalModel for
 usability

The Record component becomes more and more used, but its API and the flows
it can support are both a work in progress.

Its specs become a little clearer, we probably want two things:
- be able to display some values via Fields widget and interact with them, in
particular relational fields.
- fetch, display and interact with a proper record in the db.
- disconnect it from the use of model in Views (via the hook useModel), because Record's
feature are simpler.

This commit aims at clarifying at least 2 aspects:
- how to pass the values to the record, that is, without reading anything from the db
=> a `values` props contains server formatted data, it supports changing them via the props

- when to reload the data from the db, when not to.
=> when props did not change, don't reload anything.

We expect further improvements on this component, as the feature is still very basic at this point.

See 475f8aa for earlier improvement

closes odoo/odoo#110122

Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
---
 .../calendar_common_popover.xml               |   2 +-
 addons/web/static/src/views/record.js         |  56 ++++--
 .../web/static/src/views/relational_model.js  |  10 +-
 .../views/fields/many2one_field_tests.js      |   7 +-
 addons/web/static/tests/views/record_tests.js | 174 +++++++++++++++++-
 5 files changed, 219 insertions(+), 30 deletions(-)

diff --git a/addons/web/static/src/views/calendar/calendar_common/calendar_common_popover.xml b/addons/web/static/src/views/calendar/calendar_common/calendar_common_popover.xml
index 521422fce03..361a41d9484 100644
--- a/addons/web/static/src/views/calendar/calendar_common/calendar_common_popover.xml
+++ b/addons/web/static/src/views/calendar/calendar_common/calendar_common_popover.xml
@@ -44,7 +44,7 @@
             </li>
         </ul>
         <ul class="list-group list-group-flush o_cw_popover_fields_secondary">
-            <Record resModel="props.model.resModel" resId="props.record.id" fields="props.model.fields" activeFields="props.model.popoverFields" mode="'readonly'" initialValues="props.record.rawRecord" t-slot-scope="slot">
+            <Record resModel="props.model.resModel" resId="props.record.id" fields="props.model.fields" activeFields="props.model.popoverFields" mode="'readonly'" values="props.record.rawRecord" t-slot-scope="slot">
                 <t t-foreach="slot.record.fieldNames" t-as="fieldName" t-key="fieldName">
                     <t t-if="!slot.record.isInvisible(fieldName)">
                         <t t-set="fieldInfo" t-value="props.model.popoverFields[fieldName]" />
diff --git a/addons/web/static/src/views/record.js b/addons/web/static/src/views/record.js
index c46c9f4db2e..5d44b9f54b0 100644
--- a/addons/web/static/src/views/record.js
+++ b/addons/web/static/src/views/record.js
@@ -1,7 +1,6 @@
 /** @odoo-module **/
 
-import { useService } from "@web/core/utils/hooks";
-import { useModel } from "@web/views/model";
+import { useBus, useService } from "@web/core/utils/hooks";
 import { RelationalModel } from "@web/views/relational_model";
 import { Component, xml, onWillStart, onWillUpdateProps } from "@odoo/owl";
 
@@ -9,21 +8,44 @@ const defaultActiveField = { attrs: {}, options: {}, domain: "[]", string: "" };
 
 class _Record extends Component {
     setup() {
-        this.model = useModel(RelationalModel, {
-            resId: this.props.info.resId,
-            resModel: this.props.info.resModel,
+        const resModel = this.props.info.resModel;
+        const modelParams = {
+            resModel,
             fields: this.props.fields,
             viewMode: "form",
             rootType: "record",
             activeFields: this.getActiveFields(),
-            mode: this.props.info.mode === "edit" ? "edit" : undefined,
-            initialValues: this.props.info.initialValues,
-        });
-        onWillUpdateProps(async (nextProps) => {
-            await this.model.load({
-                resId: nextProps.info.resId,
-                mode: nextProps.info.mode,
-            });
+        };
+        const modelServices = Object.fromEntries(
+            RelationalModel.services.map((servName) => {
+                return [servName, useService(servName)];
+            })
+        );
+        if (resModel) {
+            modelServices.orm = useService("orm");
+        }
+
+        this.model = new RelationalModel(this.env, modelParams, modelServices);
+        useBus(this.model, "update", () => this.render(true));
+
+        let loadKey;
+        const load = (props) => {
+            const loadParams = {
+                resId: props.info.resId,
+                mode: props.info.mode,
+                values: props.values,
+            };
+            const nextLoadKey = JSON.stringify(loadParams);
+            if (loadKey === nextLoadKey) {
+                return;
+            }
+            loadKey = nextLoadKey;
+            return this.model.load(loadParams);
+        };
+        onWillStart(() => load(this.props));
+        onWillUpdateProps((nextProps) => {
+            // don't wait because the keeplast may make willUpdateProps hang forever
+            load(nextProps);
         });
 
         if (this.props.info.onRecordChanged) {
@@ -54,7 +76,7 @@ class _Record extends Component {
     }
 }
 _Record.template = xml`<t t-slot="default" record="model.root"/>`;
-_Record.props = ["slots", "info", "fields"];
+_Record.props = ["slots", "info", "fields", "values?"];
 
 export class Record extends Component {
     setup() {
@@ -73,16 +95,16 @@ export class Record extends Component {
         }
     }
 }
-Record.template = xml`<_Record fields="fields" slots="props.slots" info="props" />`;
+Record.template = xml`<_Record fields="fields" slots="props.slots" values="props.values" info="props" />`;
 Record.components = { _Record };
 Record.props = [
     "slots",
-    "resModel",
+    "resModel?",
     "fieldNames?",
     "activeFields?",
     "fields?",
     "resId?",
     "mode?",
-    "initialValues?",
+    "values?",
     "onRecordChanged?",
 ];
diff --git a/addons/web/static/src/views/relational_model.js b/addons/web/static/src/views/relational_model.js
index 355e302e0ec..a17539abfd7 100644
--- a/addons/web/static/src/views/relational_model.js
+++ b/addons/web/static/src/views/relational_model.js
@@ -273,7 +273,11 @@ class DataPoint {
 
         this.model = model;
         this.resModel = params.resModel;
-        this.fields = params.fields;
+        this.fields = {
+            id: { name: "id", type: "integer", readonly: true },
+            display_name: { name: "display_name", type: "char" },
+            ...params.fields,
+        };
         this.setActiveFields(params.activeFields);
 
         this.onWillSaveRecord = params.onWillSaveRecord || (() => {});
@@ -1105,7 +1109,6 @@ export class Record extends DataPoint {
         const activeField = this.activeFields[fieldName];
         const { fieldsToFetch, relatedFields = {}, views = {}, viewMode } = activeField;
         const fields = {
-            id: { name: "id", type: "integer", readonly: true },
             ...relatedFields,
             ...fieldsToFetch,
         };
@@ -3512,7 +3515,6 @@ export class RelationalModel extends Model {
             this.rootParams.expand = params.expand;
             this.rootParams.groupsLimit = params.groupsLimit;
         }
-        this.initialValues = params.initialValues;
 
         // this.db = Object.create(null);
         this.root = null;
@@ -3555,7 +3557,7 @@ export class RelationalModel extends Model {
             : this.initialRootState;
 
         const newRoot = this.createDataPoint(this.rootType, rootParams, state);
-        await this.keepLast.add(newRoot.load({ values: this.initialValues }));
+        await this.keepLast.add(newRoot.load({ values: params.values }));
         this.root = newRoot;
         this.rootParams = rootParams;
         this.notify();
diff --git a/addons/web/static/tests/views/fields/many2one_field_tests.js b/addons/web/static/tests/views/fields/many2one_field_tests.js
index edab16595e1..a001516ee6b 100644
--- a/addons/web/static/tests/views/fields/many2one_field_tests.js
+++ b/addons/web/static/tests/views/fields/many2one_field_tests.js
@@ -1606,7 +1606,7 @@ QUnit.module("Fields", (hooks) => {
         }
         Comp.components = { Record, Field };
         Comp.template = owl.xml`
-            <Record resModel="'coucou'" fields="fields" fieldNames="['partner_id']" initialValues="values" mode="'edit'" t-slot-scope="scope">
+            <Record resModel="'coucou'" fields="fields" fieldNames="['partner_id']" values="values" mode="'edit'" t-slot-scope="scope">
                 <Field name="'partner_id'" record="scope.record" canOpen="false" />
             </Record>
         `;
@@ -3251,10 +3251,7 @@ QUnit.module("Fields", (hooks) => {
         await click(target, ".o_field_widget[name=trululu] input");
         await selectDropdownItem(target, "trululu", "Create and edit...");
 
-        assert.strictEqual(
-            target.querySelector(".o_field_widget[name=foo] input").value,
-            "yz"
-        );
+        assert.strictEqual(target.querySelector(".o_field_widget[name=foo] input").value, "yz");
 
         await clickDiscard(target.querySelector(".modal"));
     });
diff --git a/addons/web/static/tests/views/record_tests.js b/addons/web/static/tests/views/record_tests.js
index a9dad462d48..648655b26f6 100644
--- a/addons/web/static/tests/views/record_tests.js
+++ b/addons/web/static/tests/views/record_tests.js
@@ -7,7 +7,9 @@ import { click, getFixture, mount } from "../helpers/utils";
 import { setupViewRegistries } from "../views/helpers";
 
 import { Component, xml, useState } from "@odoo/owl";
-import { editInput } from "@web/../tests/helpers/utils";
+import { editInput, nextTick, patchWithCleanup } from "@web/../tests/helpers/utils";
+import { Many2OneField } from "@web/views/fields/many2one/many2one_field";
+import { AutoComplete } from "@web/core/autocomplete/autocomplete";
 
 let serverData;
 let target;
@@ -168,7 +170,7 @@ QUnit.module("Record Component", (hooks) => {
         }
         Parent.components = { Record, Field };
         Parent.template = xml`
-            <Record resModel="'partner'" fieldNames="['foo']" fields="fields" initialValues="values" t-slot-scope="data">
+            <Record resModel="'partner'" fieldNames="['foo']" fields="fields" values="values" t-slot-scope="data">
                 <Field name="'foo'" record="data.record"/>
             </Record>
         `;
@@ -212,7 +214,7 @@ QUnit.module("Record Component", (hooks) => {
         }
         Parent.components = { Record, Field };
         Parent.template = xml`
-            <Record resModel="'partner'" fieldNames="['foo']" fields="fields" initialValues="values" t-slot-scope="data" onRecordChanged.bind="onRecordChanged">
+            <Record resModel="'partner'" fieldNames="['foo']" fields="fields" values="values" t-slot-scope="data" onRecordChanged.bind="onRecordChanged">
                 <Field name="'foo'" record="data.record"/>
             </Record>
         `;
@@ -230,4 +232,170 @@ QUnit.module("Record Component", (hooks) => {
         assert.verifySteps(["record changed"]);
         assert.strictEqual(target.querySelector("[name='foo'] input").value, "753");
     });
+
+    QUnit.test("handles many2one fields", async function (assert) {
+        patchWithCleanup(AutoComplete, {
+            timeout: 0,
+        });
+
+        serverData.models = {
+            bar: {
+                records: [
+                    { id: 1, display_name: "bar1" },
+                    { id: 3, display_name: "abc" },
+                ],
+            },
+        };
+
+        class Parent extends Component {
+            setup() {
+                this.fields = {
+                    foo: {
+                        name: "foo",
+                        type: "many2one",
+                        relation: "bar",
+                    },
+                };
+                this.values = {
+                    foo: [1, undefined],
+                };
+            }
+
+            onRecordChanged(record, changes) {
+                assert.step("record changed");
+                assert.deepEqual(changes, { foo: 3 });
+                assert.deepEqual(record.data, { foo: [3, "abc"] });
+            }
+        }
+        Parent.components = { Record, Many2OneField };
+        Parent.template = xml`
+            <Record resModel="'partner'" fieldNames="['foo']" fields="fields" values="values" t-slot-scope="data" onRecordChanged.bind="onRecordChanged">
+                <Many2OneField name="'foo'" record="data.record" relation="'bar'" value="data.record.data.foo" update="(value) => data.record.update({ foo: value })"/>
+            </Record>
+        `;
+
+        await mount(Parent, target, {
+            env: await makeTestEnv({
+                serverData,
+                mockRPC(route, args) {
+                    assert.step(route);
+                },
+            }),
+        });
+        assert.verifySteps(["/web/dataset/call_kw/bar/name_get"]);
+        assert.strictEqual(target.querySelector(".o_field_many2one_selection input").value, "bar1");
+        await editInput(target, ".o_field_many2one_selection input", "abc");
+        assert.verifySteps(["/web/dataset/call_kw/bar/name_search"]);
+        await click(target.querySelectorAll(".o-autocomplete--dropdown-item a")[0]);
+        assert.verifySteps(["record changed"]);
+        assert.strictEqual(target.querySelector(".o_field_many2one_selection input").value, "abc");
+    });
+
+    QUnit.test(
+        "supports passing dynamic values -- full control to the user of Record",
+        async (assert) => {
+            class Parent extends Component {
+                setup() {
+                    this.fields = {
+                        foo: {
+                            name: "foo",
+                            type: "char",
+                        },
+                        bar: {
+                            name: "bar",
+                            type: "boolean",
+                        },
+                    };
+                    this.values = owl.useState({
+                        foo: "abc",
+                        bar: true,
+                    });
+                }
+
+                onRecordChanged(record, changes) {
+                    assert.step("record changed");
+                    assert.strictEqual(record.model.constructor.name, "RelationalModel");
+                    assert.deepEqual(changes, { foo: "753" });
+                    this.values.foo = "357";
+                }
+            }
+            Parent.components = { Record, Field };
+            Parent.template = xml`
+            <Record resModel="'partner'" fieldNames="['foo']" fields="fields" values="{ foo: values.foo }" t-slot-scope="data" onRecordChanged.bind="onRecordChanged">
+                <Field name="'foo'" record="data.record"/>
+            </Record>
+        `;
+
+            await mount(Parent, target, {
+                env: await makeTestEnv({
+                    serverData,
+                    mockRPC(route) {
+                        assert.step(route);
+                    },
+                }),
+            });
+            assert.strictEqual(target.querySelector("[name='foo'] input").value, "abc");
+            await editInput(target, "[name='foo'] input", "753");
+            assert.verifySteps(["record changed"]);
+            await nextTick();
+            assert.strictEqual(target.querySelector("[name='foo'] input").value, "357");
+        }
+    );
+
+    QUnit.test("can switch records", async (assert) => {
+        class Parent extends Component {
+            setup() {
+                this.fields = {
+                    foo: {
+                        name: "foo",
+                        type: "char",
+                    },
+                    bar: {
+                        name: "bar",
+                        type: "boolean",
+                    },
+                };
+                this.state = useState({ currentId: 1, num: 0 });
+            }
+
+            next() {
+                this.state.currentId = 5;
+                this.state.num++;
+            }
+        }
+        Parent.components = { Record, Field };
+        Parent.template = xml`
+            <a id="increment" t-on-click="() => state.num++" t-esc="state.num" />
+            <a id="next" t-on-click="next">NEXT</a>
+            <Record resId="state.currentId" resModel="'partner'" fieldNames="['foo']" fields="fields" t-slot-scope="data">
+                <Field name="'foo'" record="data.record"/>
+            </Record>
+        `;
+
+        await mount(Parent, target, {
+            env: await makeTestEnv({
+                serverData,
+                mockRPC(route, args) {
+                    assert.step(`${args.method} : ${JSON.stringify(args.args)}`);
+                },
+            }),
+        });
+
+        assert.verifySteps([`read : [[1],["foo"]]`]);
+        const increment = target.querySelector("#increment");
+        const field = target.querySelector("div[name='foo']");
+        assert.strictEqual(increment.textContent, "0");
+        assert.strictEqual(field.textContent, "yop");
+
+        await click(increment);
+        // No reload when a render from upstream comes
+        assert.verifySteps([]);
+        assert.strictEqual(increment.textContent, "1");
+        assert.strictEqual(field.textContent, "yop");
+
+        await click(target.querySelector("#next"));
+        assert.verifySteps([`read : [[5],["foo"]]`]);
+        assert.strictEqual(increment.textContent, "2");
+        assert.strictEqual(field.textContent, "blop");
+    });
 });