From 54a90ca91910ea4345cf4261b80ea1cb7cfbae9d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Sun, 23 Apr 2017 21:03:52 +0200
Subject: [PATCH] [FIX] hr_holidays,web: proper use of toggle_boolean in list
 view

hr_holiday was the only module that used a widget=toggle_boolean on a
button in a list view.  That behaviour was implemented as a column
widget in the previous list view, and was not reimplemented in the new
views. The feature is useful, but this was not done properly: it is
better to use a widget on a field (that was the intent) instead of doing
a weird hack like it was done.

With this commit, we update hr_holidays to use the ToggleBoolean widget,
which is supposed to work on every views.

Also, we fix the toggleboolean widget (it was not properly rendering
tooltips, and changes were not saved in readonly list view).

Note that it as the side effect of being better from the point of rpcs:
before, the list view had to reload itself.  Also, another advantage is
that models do not need to implement custom methods (such as
toggle_payslip_status) just to toggle a boolean...
---
 addons/hr_holidays/models/hr_holidays.py      |  6 --
 .../hr_holidays/views/hr_holidays_views.xml   | 12 +---
 .../web/static/src/js/fields/basic_fields.js  | 58 ++++++++++++++-----
 .../src/js/views/list/list_controller.js      | 14 +++++
 .../static/tests/fields/basic_fields_tests.js | 37 ++++++++++++
 5 files changed, 98 insertions(+), 29 deletions(-)

diff --git a/addons/hr_holidays/models/hr_holidays.py b/addons/hr_holidays/models/hr_holidays.py
index 0b66bbf4f06..083d0d4e15a 100644
--- a/addons/hr_holidays/models/hr_holidays.py
+++ b/addons/hr_holidays/models/hr_holidays.py
@@ -530,12 +530,6 @@ class Holidays(models.Model):
         self._remove_resource_leave()
         return True
 
-    @api.multi
-    def toggle_payslip_status(self):
-        record_to_set_true = self.search([('id', 'in', self.ids), ('payslip_status', '=', False)])
-        record_to_set_false = self - record_to_set_true
-        return record_to_set_true.write({'payslip_status': True}) and record_to_set_false.write({'payslip_status': False})
-
     ####################################################
     # Messaging methods
     ####################################################
diff --git a/addons/hr_holidays/views/hr_holidays_views.xml b/addons/hr_holidays/views/hr_holidays_views.xml
index 2d6f7da7055..34636085377 100644
--- a/addons/hr_holidays/views/hr_holidays_views.xml
+++ b/addons/hr_holidays/views/hr_holidays_views.xml
@@ -166,10 +166,8 @@
                     <field name="date_to" required="1"/>
                     <field name="number_of_days_temp" string="Allocated Days" sum="Remaining Days"/>
                     <field name="state"/>
-                    <button name="toggle_payslip_status" field_name="payslip_status" type="object"
-                        widget="toggle_button" options='{"active": "Reported in last payslips", "inactive": "To Report in Payslip"}' groups="hr_holidays.group_hr_holidays_manager"/>
+                    <field name="payslip_status" widget="toggle_button" options='{"active": "Reported in last payslips", "inactive": "To Report in Payslip"}' groups="hr_holidays.group_hr_holidays_manager"/>
                     <field name="report_note" groups="hr_holidays.group_hr_holidays_manager"/>
-                    <field name="payslip_status" invisible="1"/>
                 </tree>
             </field>
         </record>
@@ -210,10 +208,8 @@
                     <field name="date_to"/>
                     <field name="holiday_status_id"/>
                     <field name="state"/>
-                    <button name="toggle_payslip_status" field_name="payslip_status" type="object"
-                        widget="toggle_button" options='{"active": "Reported in last payslips", "inactive": "To Report in Payslip"}' groups="hr_holidays.group_hr_holidays_manager"/>
+                    <field name="payslip_status" widget="toggle_button" options='{"active": "Reported in last payslips", "inactive": "To Report in Payslip"}' groups="hr_holidays.group_hr_holidays_manager"/>
                     <field name="report_note"/>
-                    <field name="payslip_status" invisible="1"/>
                 </tree>
             </field>
         </record>
@@ -232,9 +228,7 @@
                     <field name="date_to"/>
                     <field name="number_of_days" string="Number of Days" sum="Remaining Days"/>
                     <field name="state"/>
-                    <field name="payslip_status" invisible="1"/>
-                    <button name="toggle_payslip_status" field_name="payslip_status" type="object"
-                        widget="toggle_button" options='{"active": "Reported in last payslips", "inactive": "To Report in Payslip"}' groups="hr_holidays.group_hr_holidays_manager"/>
+                    <field name="payslip_status" widget="toggle_button" options='{"active": "Reported in last payslips", "inactive": "To Report in Payslip"}' groups="hr_holidays.group_hr_holidays_manager"/>
                     <field name="category_id" invisible="1"/>
                     <field name="department_id" invisible="not context.get('set_visible',False)"/>
                     <field name="manager_id" invisible="1"/>
diff --git a/addons/web/static/src/js/fields/basic_fields.js b/addons/web/static/src/js/fields/basic_fields.js
index 1477d017852..33728565cf5 100644
--- a/addons/web/static/src/js/fields/basic_fields.js
+++ b/addons/web/static/src/js/fields/basic_fields.js
@@ -1668,29 +1668,59 @@ var FieldProgressBar = AbstractField.extend({
 });
 
 /**
-    This widget is intended to be used on boolean fields. It toggles a button
-    switching between a green bullet / gray bullet.
+ * This widget is intended to be used on boolean fields. It toggles a button
+ * switching between a green bullet / gray bullet.
 */
 var FieldToggleBoolean = AbstractField.extend({
     template: "toggle_button",
     events: {
-        'click': 'set_toggle_button'
+        'click': '_onToggleButton'
     },
     supportedFieldTypes: ['boolean'],
-    _render: function () {
-        var class_name = this.value ? 'o_toggle_button_success' : 'text-muted';
-        this.$('i').attr('class', ('fa fa-circle ' + class_name));
-    },
-    set_toggle_button: function () {
-        var toggle_value = !this.value;
-        this._setValue(toggle_value);
-        if (this.mode === 'edit') {
-            this._render();
-        }
-    },
+
+    //--------------------------------------------------------------------------
+    // Public
+    //--------------------------------------------------------------------------
+
+    /**
+     * A boolean field is always set since false is a valid value.
+     *
+     * @override
+     */
     isSet: function () {
         return true;
     },
+
+    //--------------------------------------------------------------------------
+    // Private
+    //--------------------------------------------------------------------------
+
+    /**
+     * @override
+     * @private
+     */
+    _render: function () {
+        var class_name = this.value ? 'o_toggle_button_success' : 'text-muted';
+        this.$('i').attr('class', ('fa fa-circle ' + class_name));
+        var title = this.value ? this.attrs.options.active : this.attrs.options.inactive;
+        this.$el.attr('title', title);
+    },
+
+    //--------------------------------------------------------------------------
+    // Handlers
+    //--------------------------------------------------------------------------
+
+    /**
+     * Toggle the button
+     *
+     * @private
+     * @param {MouseEvent} event
+     */
+    _onToggleButton: function (event) {
+        event.stopPropagation();
+        this._setValue(!this.value);
+    },
+
 });
 
 var JournalDashboardGraph = AbstractField.extend({
diff --git a/addons/web/static/src/js/views/list/list_controller.js b/addons/web/static/src/js/views/list/list_controller.js
index d4d1fe9debe..06c27a61e94 100644
--- a/addons/web/static/src/js/views/list/list_controller.js
+++ b/addons/web/static/src/js/views/list/list_controller.js
@@ -268,6 +268,20 @@ var ListController = BasicController.extend({
         var record = this.model.get(this.handle);
         new DataExport(this, record).open();
     },
+    /**
+     * This method comes from the field manager mixin.
+     * @todo: there is a very similar method in kanban and form controllers.
+     * This should be moved in basic controller, and shared between basic views.
+     *
+     * @override
+     * @param {OdooEvent} event
+     */
+    _onFieldChanged: function (event) {
+        if (this.renderer.mode === 'readonly') {
+            event.data.force_save = true;
+        }
+        this._super.apply(this, arguments);
+    },
     /**
      * changes of the list editable are automatically saved when unselecting the
      * row, which is done when clicking on 'Save' (anywhere outside the row
diff --git a/addons/web/static/tests/fields/basic_fields_tests.js b/addons/web/static/tests/fields/basic_fields_tests.js
index d9e3f55a646..c4884658508 100644
--- a/addons/web/static/tests/fields/basic_fields_tests.js
+++ b/addons/web/static/tests/fields/basic_fields_tests.js
@@ -258,6 +258,43 @@ QUnit.module('basic_fields', {
         form.destroy();
     });
 
+    QUnit.module('FieldToggleButton');
+
+    QUnit.test('use toggle_button in list view', function (assert) {
+        assert.expect(6);
+
+        var list = createView({
+            View: ListView,
+            model: 'partner',
+            data: this.data,
+            arch: '<tree>' +
+                    '<field name="bar" widget="toggle_button" ' +
+                        'options="{&quot;active&quot;: &quot;Reported in last payslips&quot;, &quot;inactive&quot;: &quot;To Report in Payslip&quot;}"/>' +
+                '</tree>',
+            res_id: 2,
+        });
+
+        assert.strictEqual(list.$('button i.fa.fa-circle.o_toggle_button_success').length, 4,
+            "should have 4 green buttons");
+        assert.strictEqual(list.$('button i.fa.fa-circle.text-muted').length, 1,
+            "should have 1 muted button");
+
+        assert.strictEqual(list.$('button').first().attr('title'), "Reported in last payslips",
+            "active buttons should have proper tooltip");
+        assert.strictEqual(list.$('button').last().attr('title'), "To Report in Payslip",
+            "inactive buttons should have proper tooltip");
+
+        // clicking on first button to check the state is properly changed
+        list.$('button').first().click();
+        assert.strictEqual(list.$('button i.fa.fa-circle.o_toggle_button_success').length, 3,
+            "should have 3 green buttons");
+
+        list.$('button').first().click();
+        assert.strictEqual(list.$('button i.fa.fa-circle.o_toggle_button_success').length, 4,
+            "should have 4 green buttons");
+        list.destroy();
+    });
+
 
     QUnit.module('FieldFloat');