From 6d5a993cf74173456734cccd2ea2c13027c04bbd Mon Sep 17 00:00:00 2001
From: "Lucas Perais (lpe)" <lpe@odoo.com>
Date: Wed, 13 Feb 2019 15:44:21 +0000
Subject: [PATCH] [FIX] web: control panel load/save filter sort param

Before this commit, the wrong semantics was implemented in the webclient
concerning the "sort" char field in ir.filter
Loading a filter with a sort with negative order made the webclient crash

This field holds a JSON string, representing a list of field names
If the order of the field is inverse, there should be a "-" before it
for legacy purposes

After this commit, the semantics have changed for consistency
that is, at save time the sql notation is preferred
We support the legacy notation too for backward compatibility

closes odoo/odoo#31102
---
 .../control_panel/control_panel_model.js      | 17 +++-
 .../static/tests/views/control_panel_tests.js | 81 +++++++++++++++++++
 2 files changed, 95 insertions(+), 3 deletions(-)

diff --git a/addons/web/static/src/js/views/control_panel/control_panel_model.js b/addons/web/static/src/js/views/control_panel/control_panel_model.js
index f58fbc93c4f..e59dbfb1036 100644
--- a/addons/web/static/src/js/views/control_panel/control_panel_model.js
+++ b/addons/web/static/src/js/views/control_panel/control_panel_model.js
@@ -927,10 +927,21 @@ var ControlPanelModel = mvc.Model.extend({
                     }
                     var sort = JSON.parse(favorite.sort);
                     var orderedBy = sort.map(function (order) {
-                        var orderTerms = order.split(' ');
+                        var fieldName;
+                        var asc;
+                        var sqlNotation = order.split(' ');
+                        if (sqlNotation.length > 1) {
+                            // regex: \fieldName (asc|desc)?\
+                            fieldName = sqlNotation[0];
+                            asc = sqlNotation[1] === 'asc';
+                        } else {
+                            // legacy notation -- regex: \-?fieldName\
+                            fieldName = order[0] === '-' ? order.slice(1) : order;
+                            asc = order[0] === '-' ? false : true;
+                        }
                         return {
-                            name: orderTerms[0],
-                            asc: orderTerms.length === 2 && orderTerms[1] === 'asc',
+                            name: fieldName,
+                            asc: asc,
                         };
                     });
                     return {
diff --git a/addons/web/static/tests/views/control_panel_tests.js b/addons/web/static/tests/views/control_panel_tests.js
index 7ed9939fdac..50cb3bb00ad 100644
--- a/addons/web/static/tests/views/control_panel_tests.js
+++ b/addons/web/static/tests/views/control_panel_tests.js
@@ -392,5 +392,86 @@ QUnit.module('Views', {
 
         controlPanel.destroy();
     });
+
+    QUnit.test('load filter', async function (assert) {
+        assert.expect(1);
+
+        var controlPanel = await createControlPanel({
+            model: 'partner',
+            arch: "<search></search>",
+            data: this.data,
+            searchMenuTypes: ['filter'],
+            intercepts: {
+                load_filters: function (ev) {
+                    ev.data.on_success([
+                        {
+                            user_id: [2,"Mitchell Admin"],
+                            name: 'sorted filter',
+                            id: 5,
+                            context: {},
+                            sort: "[\"foo\", \"-bar\"]",
+                            domain: "[('user_id', '=', uid)]",
+                        }
+                    ]);
+                }
+            }
+        });
+
+         _.each(controlPanel.exportState().filters, function (filter) {
+            if (filter.type === 'favorite') {
+                assert.deepEqual(filter.orderedBy, 
+                    [{
+                        name: 'foo',
+                        asc: true,
+                    }, {
+                        name: 'bar',
+                        asc: false,
+                    }],
+                    'the filter should have the right orderedBy values');
+            }
+        });
+
+        controlPanel.destroy();
+    });
+
+    QUnit.test('save filter', async function (assert) {
+        assert.expect(1);
+
+        var controlPanel = await createControlPanel({
+            model: 'partner',
+            arch: "<search></search>",
+            data: this.data,
+            searchMenuTypes: ['filter'],
+            intercepts: {
+                create_filter: function (ev) {
+                    assert.strictEqual(ev.data.filter.sort, "[\"foo\",\"bar desc\"]",
+                        'The right format for the string "sort" should be sent to the server');
+                },
+                get_controller_query_params: function (ev) {
+                    ev.data.callback({
+                        orderedBy: [
+                            {
+                                name: 'foo',
+                                asc: true,
+                            }, {
+                                name: 'bar',
+                                asc: false,
+                            }
+                        ]
+                    });
+                }
+            }
+        });
+
+        controlPanel._onNewFavorite({
+            data: {
+                description: 'Morbier',
+                type: 'favorite',
+            },
+            stopPropagation: function () {return;}
+        });
+
+        controlPanel.destroy();
+    });
 });
 });