From 5f5e699baa724d359795d5f0f617cab67bbcf692 Mon Sep 17 00:00:00 2001
From: "Simon Genin (ges)" <ges@odoo.com>
Date: Thu, 30 Sep 2021 11:36:49 +0000
Subject: [PATCH] [FIX] web: Form stat button stays correctly disabled

A stat button set as disabled was used to just show some statistics
without an action on click. However, on tasks executed by the Form,
all the buttons would be temporarly set as disabled then set back to
enabled. This would lose the original state of the button and make them
all clickable.

The way it worked before v14.5 was most likely because the action
manager would not throw an error on an action with those "invalid"
arguments.

Also, a button without a type and name will now be set the disabled
attribute automatically.

closes odoo/odoo#78839

Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
---
 .../src/legacy/js/views/form/form_renderer.js | 31 +++++-
 .../static/tests/legacy/views/form_tests.js   | 97 +++++++++++++++++--
 2 files changed, 114 insertions(+), 14 deletions(-)

diff --git a/addons/web/static/src/legacy/js/views/form/form_renderer.js b/addons/web/static/src/legacy/js/views/form/form_renderer.js
index 9a9c3570c00..80950ec3fca 100644
--- a/addons/web/static/src/legacy/js/views/form/form_renderer.js
+++ b/addons/web/static/src/legacy/js/views/form/form_renderer.js
@@ -52,6 +52,9 @@ var FormRenderer = BasicRenderer.extend({
         // display them (e.g. in Studio, in "show invisible" mode). This flag
         // allows to disable this optimization.
         this.renderInvisible = false;
+        // Keeps track of buttons that are disabled momentarily and need to be renabled.
+        // Needed to compare with buttons that have to stay disabled all the time.
+        this.manuallyDisabledButtons = new Set();
     },
     /**
      * @override
@@ -182,16 +185,24 @@ var FormRenderer = BasicRenderer.extend({
      *
      */
     disableButtons: function () {
-        this.$('.o_statusbar_buttons button, .oe_button_box button')
-            .attr('disabled', true);
+        const allButtons = this.$el[0].querySelectorAll('.o_statusbar_buttons button, .oe_button_box button');
+        for (const button of allButtons) {
+            if (!button.getAttribute("disabled")) {
+                this.manuallyDisabledButtons.add(button)
+                button.setAttribute("disabled", true)
+            }
+        }
     },
     /**
      * Enable statusbar buttons and stat buttons so they can be clicked again
      *
      */
     enableButtons: function () {
-        this.$('.o_statusbar_buttons button, .oe_button_box button')
-            .removeAttr('disabled');
+        const allButtons = this.$el[0].querySelectorAll('.o_statusbar_buttons button, .oe_button_box button');
+        this.manuallyDisabledButtons.forEach((button) => {
+            button.removeAttribute("disabled");
+        });
+        this.manuallyDisabledButtons.clear();
     },
     /**
      * Put the focus on the last activated widget.
@@ -814,11 +825,20 @@ var FormRenderer = BasicRenderer.extend({
         var $button = viewUtils.renderButtonFromNode(node, {
             extraClass: 'oe_stat_button',
         });
+
+        // If there is no type nor name, it will not bind a click listener and will set the button as disabled
+        const buttonDoesStartAnAction = node.attrs.type || node.attrs.name;
+        if (!buttonDoesStartAnAction) {
+            $button[0].setAttribute("disabled", true);
+        }
+
         $button.append(_.map(node.children, this._renderNode.bind(this)));
         if (node.attrs.help) {
             this._addButtonTooltip(node, $button);
         }
-        this._addOnClickAction($button, node);
+        if (buttonDoesStartAnAction) {
+            this._addOnClickAction($button, node);
+        }
         this._handleAttributes($button, node);
         this._registerModifiers(node, this.state, $button);
         return $button;
@@ -1109,6 +1129,7 @@ var FormRenderer = BasicRenderer.extend({
         return Promise.all(defs).then(() => this.__renderView()).then(function () {
             self._postProcessLabels();
             self._updateView($form.contents());
+            self.manuallyDisabledButtons.clear();
             if (self.state.res_id in self.alertFields) {
                 self.displayTranslationAlert();
             }
diff --git a/addons/web/static/tests/legacy/views/form_tests.js b/addons/web/static/tests/legacy/views/form_tests.js
index 4632e96d03e..0ff30a82261 100644
--- a/addons/web/static/tests/legacy/views/form_tests.js
+++ b/addons/web/static/tests/legacy/views/form_tests.js
@@ -1114,7 +1114,7 @@ QUnit.module('Views', {
 
     });
 
-    QUnit.test('rendering stat buttons', async function (assert) {
+    QUnit.test('rendering stat buttons with action', async function (assert) {
         assert.expect(3);
 
         var form = await createView({
@@ -1123,11 +1123,11 @@ QUnit.module('Views', {
             data: this.data,
             arch:'<form string="Partners">' +
                     '<sheet>' +
-                        '<div name="button_box">' +
-                            '<button class="oe_stat_button">' +
+                        '<div name="button_box" class="oe_button_box">' +
+                            '<button class="oe_stat_button" >' +
                                 '<field name="int_field"/>' +
                             '</button>' +
-                            '<button class="oe_stat_button" attrs=\'{"invisible": [["bar", "=", true]]}\'>' +
+                            '<button class="oe_stat_button" name="some_action" type="action" attrs=\'{"invisible": [["bar", "=", true]]}\'>' +
                                 '<field name="bar"/>' +
                             '</button>' +
                         '</div>' +
@@ -1147,7 +1147,86 @@ QUnit.module('Views', {
             count++;
         });
         await testUtils.dom.click('.oe_stat_button');
-        assert.strictEqual(count, 1, "should have triggered a execute action");
+        assert.strictEqual(count, 0, "should have triggered an execute_action");
+        form.destroy();
+    });
+
+    QUnit.test('rendering stat buttons without action', async function (assert) {
+        assert.expect(4);
+
+        var form = await createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch:'<form string="Partners">' +
+                    '<sheet>' +
+                        '<div name="button_box" class="oe_button_box">' +
+                            '<button class="oe_stat_button">' +
+                                '<field name="int_field"/>' +
+                            '</button>' +
+                            '<button class="oe_stat_button" attrs=\'{"invisible": [["bar", "=", true]]}\'>' +
+                                '<field name="bar"/>' +
+                            '</button>' +
+                        '</div>' +
+                        '<group>' +
+                            '<field name="foo"/>' +
+                        '</group>' +
+                    '</sheet>' +
+                '</form>',
+            res_id: 2,
+        });
+
+        assert.containsN(form, 'button.oe_stat_button', 2);
+        assert.containsOnce(form, 'button.oe_stat_button.o_invisible_modifier');
+        assert.containsN(form, 'button.oe_stat_button:disabled', 2);
+
+        var count = 0;
+        await testUtils.mock.intercept(form, "execute_action", function () {
+            count++;
+        });
+        await testUtils.dom.click('.oe_stat_button');
+        assert.strictEqual(count, 0, "should have not triggered an execute_action");
+        form.destroy();
+    });
+
+    QUnit.test('readonly stat buttons stays disabled', async function (assert) {
+        assert.expect(4);
+
+        var form = await createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch:'<form string="Partners">' +
+                    '<sheet>' +
+                        '<div name="button_box" class="oe_button_box">' +
+                            '<button class="oe_stat_button">' +
+                                '<field name="int_field"/>' +
+                            '</button>' +
+                            '<button class="oe_stat_button" type="action" name="some_action">' +
+                                '<field name="bar"/>' +
+                            '</button>' +
+                        '</div>' +
+                        '<group>' +
+                            '<button type="action" name="action_to_perform">Run an action</button>' +
+                        '</group>' +
+                    '</sheet>' +
+                '</form>',
+            res_id: 2,
+        });
+
+        var count = 0;
+        await testUtils.mock.intercept(form, "execute_action", function (event) {
+            if (event.data.action_data.name == "action_to_perform") {
+                assert.containsN(form, 'button.oe_stat_button[disabled]', 2, "While performing the action, both buttons should be disabled.");
+                event.data.on_success();
+            }
+        });
+
+        assert.containsN(form, 'button.oe_stat_button', 2);
+        assert.containsN(form, 'button.oe_stat_button[disabled]', 1);
+        await testUtils.dom.click('button[name=action_to_perform]');
+        assert.containsN(form, 'button.oe_stat_button[disabled]', 1, "After performing the action, only one button should be disabled.");
+        
         form.destroy();
     });
 
@@ -2741,7 +2820,7 @@ QUnit.module('Views', {
             arch:'<form string="Partners">' +
                     '<sheet>' +
                         '<div name="button_box">' +
-                            '<button class="oe_stat_button">' +
+                            '<button class="oe_stat_button" name="some_action" type="action">' +
                                 '<field name="bar"/>' +
                             '</button>' +
                         '</div>' +
@@ -7137,7 +7216,7 @@ QUnit.module('Views', {
                     '</header>' +
                     '<sheet>' +
                         '<div name="button_box" class="oe_button_box">' +
-                            '<button class="oe_stat_button">' +
+                            '<button class="oe_stat_button" name="some_action" type="action">' +
                                 '<field name="bar"/>' +
                             '</button>' +
                         '</div>' +
@@ -7201,7 +7280,7 @@ QUnit.module('Views', {
                     '</header>' +
                     '<sheet>' +
                         '<div name="button_box" class="oe_button_box">' +
-                            '<button class="oe_stat_button">' +
+                            '<button class="oe_stat_button" name="some_action" type="action">' +
                                 '<field name="bar"/>' +
                             '</button>' +
                         '</div>' +
@@ -7314,7 +7393,7 @@ QUnit.module('Views', {
                 'partner,false,form': '<form>' +
                         '<sheet>' +
                             '<div name="button_box" class="oe_button_box">' +
-                                '<button class="oe_stat_button">' +
+                                '<button class="oe_stat_button" name="some_action" type="action">' +
                                     '<field name="bar"/>' +
                                 '</button>' +
                             '</div>' +