From dd72a4092646f58dabe3d183e64e33396573f8fb Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Tue, 13 Sep 2022 09:56:53 +0000
Subject: [PATCH] [FIX] web: can't save o2m with not new dirty invalid record
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Imagine a form view with a one2many editable list, containing a
required field. Open an existing record, s.t. in the one2many
there's already a sub record. Modify that subrecord, s.t. the
required field is unset (e.g. empty it, or it can be unset from
the beginning, but then modify another field). Then save the main
record. Before this commit, the record could be saved.

In this situation, the modified one2many record isn't valid, so
we shouldn't be allowed to save. The view should remain in edition
and the invalid field should be indicated.

This commit fixes this issue.

closes odoo/odoo#100059

Signed-off-by: Géry Debongnie <ged@odoo.com>
---
 .../src/views/basic_relational_model.js       |  2 +-
 .../views/fields/one2many_field_tests.js      | 35 +++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/addons/web/static/src/views/basic_relational_model.js b/addons/web/static/src/views/basic_relational_model.js
index 60ced130e8a..4d9c58d4d64 100644
--- a/addons/web/static/src/views/basic_relational_model.js
+++ b/addons/web/static/src/views/basic_relational_model.js
@@ -453,7 +453,7 @@ export class Record extends DataPoint {
     async checkX2ManyValidity(fieldName) {
         const list = this.data[fieldName];
         const record = list.editedRecord;
-        if (record && record.isNew && !(await record.checkValidity())) {
+        if (record && !(await record.checkValidity())) {
             if (record.canBeAbandoned && !record.isDirty) {
                 list.abandonRecord(record.id);
             } else {
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 489137e9c1a..df81de8b74d 100644
--- a/addons/web/static/tests/views/fields/one2many_field_tests.js
+++ b/addons/web/static/tests/views/fields/one2many_field_tests.js
@@ -3654,6 +3654,41 @@ QUnit.module("Fields", (hooks) => {
         }
     );
 
+    QUnit.test("save a record with not new, dirty and invalid subrecord", async function (assert) {
+        serverData.models.partner.records[0].p = [2];
+        serverData.models.partner.records[1].display_name = ""; // invalid record
+
+        await makeView({
+            type: "form",
+            resModel: "partner",
+            serverData,
+            arch: `
+                <form>
+                    <field name="p">
+                        <tree editable="bottom">
+                            <field name="display_name" required="1"/>
+                            <field name="int_field"/>
+                        </tree>
+                    </field>
+                </form>`,
+            resId: 1,
+            mockRPC(route, args) {
+                if (args.method === "write") {
+                    throw new Error("Should not call write as record is invalid");
+                }
+            },
+            mode: "edit",
+        });
+
+        assert.containsOnce(target, ".o_form_editable");
+        await click(target.querySelector(".o_data_cell")); // edit the first row
+        assert.hasClass(target.querySelector(".o_data_row"), "o_selected_row");
+        await editInput(target, ".o_field_widget[name=int_field] input", 44);
+        await click(target.querySelector(".o_form_button_save"));
+        assert.containsOnce(target, ".o_form_editable");
+        assert.containsOnce(target, ".o_invalid_cell");
+    });
+
     QUnit.test("editable one2many list, adding, discarding, and pager", async function (assert) {
         serverData.models.partner.records[0].turtles = [1];