[FIX] web: correctly allow to leave form view

Scenario:
Open a record that is invalid in database (a required field is
empty), and click on the pager to go to another record.

Before this commit, we prevented the user from leaving the record,
marked the field as invalid and displayed a notification. This is
a weird UX experiment, since the user changed nothing (he did not
even enter edit mode). In this situation, since the record hasn't
been modified, the user should be allowed to leave. This is how
the legacy form view behaved.

closes odoo/odoo#100442

Signed-off-by: Géry Debongnie <ged@odoo.com>

addons/web/static/src/views/basic_relational_model.js

@@ -343,15 +343,15 @@ export class Record extends DataPoint {
         return this.model.__bm__.localData[this.__bm_handle__].res_ids;
     }
 
-    // -------------------------------------------------------------------------
-    // Getters
-    // -------------------------------------------------------------------------
+    async askChanges() {
+        const proms = [];
+        this.model.env.bus.trigger("RELATIONAL_MODEL:NEED_LOCAL_CHANGES", { proms });
+        return Promise.all([...proms, this._updatePromise]);
+    }
 
     async checkValidity(urgent) {
         if (!urgent) {
-            const proms = [];
-            this.model.env.bus.trigger("RELATIONAL_MODEL:NEED_LOCAL_CHANGES", { proms });
-            await Promise.all([...proms, this._updatePromise]);
+            await this.askChanges();
         }
         for (const fieldName in this.activeFields) {
             const fieldType = this.fields[fieldName].type;

addons/web/static/src/views/form/form_controller.js

@@ -197,9 +197,13 @@ export class FormController extends Component {
                     limit: 1,
                     total: resIds.length,
                     onUpdate: async ({ offset }) => {
-                        const canProceed = await this.model.root.save({ stayInEdition: true });
+                        await this.model.root.askChanges(); // ensures that isDirty is correct
+                        let canProceed = true;
+                        if (this.model.root.isDirty) {
+                            canProceed = await this.model.root.save({ stayInEdition: true });
+                        }
                         if (canProceed) {
-                            this.model.load({ resId: resIds[offset] });
+                            return this.model.load({ resId: resIds[offset] });
                         }
                     },
                 };

addons/web/static/tests/views/form/form_view_tests.js

@@ -4468,7 +4468,79 @@ QUnit.module("Views", (hooks) => {
         assert.containsOnce(target, ".o_notification_manager .o_notification");
     });
 
-    QUnit.test("keynav switching to another record from a dirty one", async function (assert) {
+    QUnit.test("keynav: switching to another record from an invalid one", async function (assert) {
+        await makeView({
+            type: "form",
+            resModel: "partner",
+            serverData,
+            arch: '<form><field name="foo" required="1"/></form>',
+            resIds: [1, 2],
+            resId: 1,
+            mockRPC(route) {
+                if (route === "/web/dataset/call_kw/partner/write") {
+                    throw new Error("Shouldn't call write as the record is invalid");
+                }
+            },
+        });
+
+        assert.strictEqual(target.querySelector(".breadcrumb").innerText, "first record");
+        assert.hasClass(target.querySelector(".o_field_widget[name=foo]"), "o_required_modifier");
+        assert.strictEqual(target.querySelector(".o_pager_value").textContent, "1");
+        assert.strictEqual(target.querySelector(".o_pager_limit").textContent, "2");
+
+        await click(target.querySelector(".o_form_button_edit"));
+        await editInput(target, ".o_field_widget[name=foo] input", "");
+        triggerHotkey("alt+n");
+        await nextTick();
+        assert.strictEqual(target.querySelector(".breadcrumb").innerText, "first record");
+        assert.strictEqual(target.querySelector(".o_pager_value").textContent, "1");
+        assert.strictEqual(target.querySelector(".o_pager_limit").textContent, "2");
+        assert.hasClass(target.querySelector(".o_field_widget[name=foo]"), "o_field_invalid");
+        assert.containsOnce(target, ".o_notification_manager .o_notification");
+    });
+
+    QUnit.test("switching to another record from an invalid one (2)", async function (assert) {
+        // in this scenario, the record is already invalid in db, so we should be allowed to
+        // leave it
+        serverData.models.partner.records[0].foo = false;
+
+        await makeView({
+            type: "form",
+            resModel: "partner",
+            serverData,
+            arch: `
+                <form>
+                    <field name="foo" required="1"/>
+                </form>`,
+            resIds: [1, 2],
+            resId: 1,
+        });
+
+        assert.strictEqual(target.querySelector(".breadcrumb").innerText, "first record");
+        assert.hasClass(target.querySelector(".o_field_widget[name=foo]"), "o_required_modifier");
+        assert.strictEqual(target.querySelector(".o_pager_value").textContent, "1");
+        assert.strictEqual(target.querySelector(".o_pager_limit").textContent, "2");
+
+        await click(target.querySelector(".o_pager_next"));
+        assert.strictEqual(target.querySelector(".breadcrumb").innerText, "second record");
+        assert.strictEqual(target.querySelector(".o_pager_value").textContent, "2");
+
+        await click(target.querySelector(".o_pager_previous"));
+        assert.strictEqual(target.querySelector(".breadcrumb").innerText, "first record");
+        assert.hasClass(target.querySelector(".o_field_widget[name=foo]"), "o_required_modifier");
+        assert.strictEqual(target.querySelector(".o_pager_value").textContent, "1");
+
+        // same, but in edit mode
+        await click(target.querySelector(".o_form_button_edit"));
+        assert.containsOnce(target, ".o_form_editable");
+
+        await click(target.querySelector(".o_pager_next"));
+        assert.containsOnce(target, ".o_form_editable");
+        assert.strictEqual(target.querySelector(".breadcrumb").innerText, "second record");
+        assert.strictEqual(target.querySelector(".o_pager_value").textContent, "2");
+    });
+
+    QUnit.test("keynav: switching to another record from a dirty one", async function (assert) {
         let nbWrite = 0;
         await makeView({
             type: "form",
@@ -4508,7 +4580,7 @@ QUnit.module("Views", (hooks) => {
         input.value = "new value";
         await triggerEvent(input, null, "input");
 
-        // click on the pager to switch to the next record (will save record)
+        // trigger the pager hotkey to switch to the next record (will save record)
         triggerHotkey("alt+n");
         await nextTick();
         assert.containsNone(document.body, ".modal", "no confirm modal should be displayed");