From ddba01a8d439b2cebe078b9eb3d9aa4685faed3e Mon Sep 17 00:00:00 2001
From: abd-msyukyu-odoo <abd@odoo.com>
Date: Fri, 9 Feb 2024 15:00:26 +0000
Subject: [PATCH] [FIX] web: conserve empty opt. columns headers for list view

The string value of an empty array is `""` which is falsy. Prior to this commit,
if all optional columns headers were disabled in a list view, it would result in
a reset of those when the view is mounted the next time, instead of keeping them
disabled like in prior versions.

task-3692178

closes odoo/odoo#153831

X-original-commit: e600ed2fa2f2e612dfca20872356ae926dde0a54
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
Signed-off-by: Damien Abeloos (abd) <abd@odoo.com>
---
 .../static/src/views/list/list_renderer.js    |  2 +-
 .../web/static/tests/views/list_view_tests.js | 85 ++++++++++++++++++-
 2 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/addons/web/static/src/views/list/list_renderer.js b/addons/web/static/src/views/list/list_renderer.js
index da8b92fec4b..8fbf3544d49 100644
--- a/addons/web/static/src/views/list/list_renderer.js
+++ b/addons/web/static/src/views/list/list_renderer.js
@@ -1056,7 +1056,7 @@ export class ListRenderer extends Component {
             optionalColumn.forEach((col) => {
                 this.optionalActiveFields[col.name] = optionalActiveFields.includes(col.name);
             });
-        } else {
+        } else if (optionalActiveFields !== "") {
             for (const col of optionalColumn) {
                 this.optionalActiveFields[col.name] = col.optional === "show";
             }
diff --git a/addons/web/static/tests/views/list_view_tests.js b/addons/web/static/tests/views/list_view_tests.js
index 5dcb44bf18d..4990be6264d 100644
--- a/addons/web/static/tests/views/list_view_tests.js
+++ b/addons/web/static/tests/views/list_view_tests.js
@@ -16561,11 +16561,11 @@ QUnit.module("Views", (hooks) => {
             patchWithCleanup(browser.localStorage, {
                 getItem(key) {
                     assert.step("getItem " + key);
-                    return forceLocalStorage ? "m2o" : super.getItem(arguments);
+                    return forceLocalStorage ? "m2o" : super.getItem(...arguments);
                 },
                 setItem(key, value) {
                     assert.step("setItem " + key + " to " + JSON.stringify(String(value)));
-                    return super.setItem(arguments);
+                    return super.setItem(...arguments);
                 },
             });
 
@@ -16641,6 +16641,87 @@ QUnit.module("Views", (hooks) => {
         }
     );
 
+    QUnit.test(
+        "list view with optional fields from local storage being the empty array",
+        async function (assert) {
+            patchWithCleanup(browser.localStorage, {
+                getItem(key) {
+                    assert.step("getItem " + key);
+                    return super.getItem(...arguments);
+                },
+                setItem(key, value) {
+                    assert.step("setItem " + key + " to " + JSON.stringify(String(value)));
+                    super.setItem(...arguments);
+                },
+            });
+
+            const verifyHeaders = (namedHeaders) => {
+                const headers = [...target.querySelectorAll(".o_list_table thead th")];
+                assert.hasClass(headers[0], "o_list_record_selector");
+                assert.hasClass(headers[headers.length - 1], "o_list_actions_header");
+                assert.equal(
+                    headers.length,
+                    namedHeaders.length + 2,
+                    `list has ${namedHeaders.length + 2} headers`
+                );
+                for (let i = 1; i < headers.length - 1; i++) {
+                    assert.equal(
+                        headers[i].dataset.name,
+                        namedHeaders[i - 1],
+                        `header at index ${i} is ${namedHeaders[i - 1]}`
+                    );
+                }
+            };
+
+            serverData.actions = {
+                1: {
+                    id: 1,
+                    name: "Action 1",
+                    res_model: "foo",
+                    type: "ir.actions.act_window",
+                    views: [[42, "list"]],
+                    search_view_id: [1, "search"],
+                },
+            };
+            serverData.views = {
+                "foo,1,search": "<search></search>",
+                "foo,42,list": `
+                    <tree>
+                        <field name="foo"/>
+                        <field name="m2o" optional="hide"/>
+                        <field name="reference" optional="show"/>
+                    </tree>`,
+            };
+            const localStorageKey = "optional_fields,foo,list,42,foo,m2o,reference";
+            const webClient = await createWebClient({ serverData });
+            await doAction(webClient, 1);
+
+            // verify initialization
+            assert.verifySteps(["getItem " + localStorageKey]);
+            verifyHeaders(["foo", "reference"]);
+            // open optional columns headers dropdown
+            await click(target.querySelector("table .o_optional_columns_dropdown button"));
+            assert.containsN(
+                target,
+                "div.o_optional_columns_dropdown span.dropdown-item",
+                2,
+                "dropdown has 2 optional column headers"
+            );
+            // disable optional field "reference" (no optional column enabled)
+            await click(
+                target.querySelectorAll(
+                    "div.o_optional_columns_dropdown span.dropdown-item input"
+                )[1]
+            );
+            assert.verifySteps(["setItem " + localStorageKey + ' to ""']);
+            verifyHeaders(["foo"]);
+            // mount again to ensure that active optional columns will not be reset while empty
+            await doAction(webClient, 1);
+            assert.verifySteps(["getItem " + localStorageKey]);
+            verifyHeaders(["foo"]);
+        }
+    );
+
     QUnit.test("quickcreate in a many2one in a list", async function (assert) {
         await makeView({
             type: "list",