From adcde74532737d472edbbf095bd17e007ef6034f Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Fri, 16 Sep 2022 19:33:52 +0000 Subject: [PATCH] [FIX] web: correctly allow to leave form view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../src/views/basic_relational_model.js | 12 +-- .../static/src/views/form/form_controller.js | 8 +- .../tests/views/form/form_view_tests.js | 76 ++++++++++++++++++- 3 files changed, 86 insertions(+), 10 deletions(-) diff --git a/addons/web/static/src/views/basic_relational_model.js b/addons/web/static/src/views/basic_relational_model.js index 3b11f772188..d9f9e5e1798 100644 --- a/addons/web/static/src/views/basic_relational_model.js +++ b/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; diff --git a/addons/web/static/src/views/form/form_controller.js b/addons/web/static/src/views/form/form_controller.js index 5c02f2a35c0..3e6a81c2360 100644 --- a/addons/web/static/src/views/form/form_controller.js +++ b/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] }); } }, }; 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 02d766c9a26..d166ec06d7d 100644 --- a/addons/web/static/tests/views/form/form_view_tests.js +++ b/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: '
', + 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: ` +
+ + `, + 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");