From ae1a30e618e071877a561377f544934ee2a89417 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Sun, 23 Apr 2017 08:29:34 +0200
Subject: [PATCH] [FIX] web: use correct id when using form buttons

With new records, it could happen that clicking on a button in the
header was followed by actions using the wrong id.  This was caused by a
field name="id" in the view (see sale order view, in the
delivery.view_order_form_with_carrier inherited view).

In that case, the id was registered in the changes list, and caused
invalid data in the record: its data.id was set to null.
---
 .../static/src/js/views/basic/basic_model.js  |  6 ++-
 addons/web/static/tests/views/form_tests.js   | 49 +++++++++++++++++++
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/addons/web/static/src/js/views/basic/basic_model.js b/addons/web/static/src/js/views/basic/basic_model.js
index e737df9cc9e..efc7fbc1971 100644
--- a/addons/web/static/src/js/views/basic/basic_model.js
+++ b/addons/web/static/src/js/views/basic/basic_model.js
@@ -662,6 +662,10 @@ var BasicModel = AbstractModel.extend({
             }
             var shouldReload = 'reload' in options ? options.reload : true;
             var method = self.isNew(record_id) ? 'create' : 'write';
+            if (record._changes) {
+                // id never changes, and should not be written
+                delete record._changes.id;
+            }
             var changes = self._generateChanges(record);
 
             if (method === 'create') {
@@ -673,8 +677,6 @@ var BasicModel = AbstractModel.extend({
                     }
                 });
             }
-            // make sure we don't write an undefined id
-            delete changes.id;
 
             // in the case of a write, only perform the RPC if there are changes to save
             if (method === 'create' || Object.keys(changes).length) {
diff --git a/addons/web/static/tests/views/form_tests.js b/addons/web/static/tests/views/form_tests.js
index d0662959290..617dc221651 100644
--- a/addons/web/static/tests/views/form_tests.js
+++ b/addons/web/static/tests/views/form_tests.js
@@ -787,6 +787,55 @@ QUnit.module('Views', {
         form.destroy();
     });
 
+    QUnit.test('buttons in form view, new record, with field id in view', function (assert) {
+        assert.expect(6);
+        // buttons in form view are one of the rare example of situation when we
+        // save a record without reloading it immediately, because we only care
+        // about its id for the next step.  But at some point, if the field id
+        // is in the view, it was registered in the changes, and caused invalid
+        // values in the record (data.id was set to null)
+
+        var resID;
+
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form string="Partners">' +
+                    '<header>' +
+                        '<button name="post" class="p" string="Confirm" type="object"/>' +
+                    '</header>' +
+                    '<sheet>' +
+                        '<group>' +
+                            '<field name="id" invisible="1"/>' +
+                            '<field name="foo"/>' +
+                        '</group>' +
+                    '</sheet>' +
+                '</form>',
+            mockRPC: function (route, args) {
+                assert.step(args.method);
+                if (args.method === 'create') {
+                    return this._super.apply(this, arguments).then(function (result) {
+                        resID = result;
+                        return resID;
+                    });
+                }
+                return this._super.apply(this, arguments);
+            },
+        });
+
+        testUtils.intercept(form, 'execute_action', function (event) {
+            assert.step('execute_action');
+            assert.strictEqual(event.data.record_id, resID,
+                "execute action should be done on correct record id");
+            event.data.on_success();
+        });
+        form.$('.o_form_statusbar button.p').click();
+
+        assert.verifySteps(['default_get', 'create', 'execute_action', 'read']);
+        form.destroy();
+    });
+
     QUnit.test('change and save char', function (assert) {
         assert.expect(6);
         var form = createView({