[FIX] web: don't show optional column included in others

Before 16.0, optional columns used local_storage service that used
JSON.stringify and JSON.parse to store values and so the object were
directly usable.

Since 16.0, we are using the localStorage API directly, so when checking
if a optional field is enabled, we just check for example:

_.contains('currency_id,partner_id', 'id')

which returns true but shouldn't => this causes for example the ID field
to not be hideable.

note: this commit makes makeRAMLocalStorage (used for safari incognito
and tests) consistent with localStorage that stores values as string.

note: the added test without the fix, fails with:

  should have 3 th, 1 for selector, 1 for columns, 1 for optional
  columns ever after listview reload. Expected: 3, Result: 4.

opw-3339416

closes odoo/odoo#131309

X-original-commit: 922bd446708e05c2368a01b2399855845246885b
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
Signed-off-by: Nicolas Lempereur (nle) <nle@odoo.com>

addons/web/static/src/core/browser/browser.js

@@ -85,8 +85,9 @@ export function makeRAMLocalStorage() {
     let store = {};
     return {
         setItem(key, value) {
-            store[key] = value;
-            window.dispatchEvent(new StorageEvent("storage", { key, newValue: value }));
+            const newValue = String(value);
+            store[key] = newValue;
+            window.dispatchEvent(new StorageEvent("storage", { key, newValue }));
         },
         getItem(key) {
             return store[key];

addons/web/static/src/views/list/list_renderer.js

@@ -1017,11 +1017,12 @@ export class ListRenderer extends Component {
 
     getOptionalActiveFields() {
         this.optionalActiveFields = {};
-        const optionalActiveFields = browser.localStorage.getItem(this.keyOptionalFields);
+        let optionalActiveFields = browser.localStorage.getItem(this.keyOptionalFields);
         const optionalColumn = this.allColumns.filter(
             (col) => col.type === "field" && col.optional
         );
         if (optionalActiveFields) {
+            optionalActiveFields = optionalActiveFields.split(",");
             optionalColumn.forEach((col) => {
                 this.optionalActiveFields[col.name] = optionalActiveFields.includes(col.name);
             });

addons/web/static/tests/views/list_view_tests.js

@@ -15797,6 +15797,55 @@ QUnit.module("Views", (hooks) => {
         }
     );
 
+    QUnit.test("optional fields is shown only if enabled", async function (assert) {
+        serverData.actions = {
+            1: {
+                id: 1,
+                name: "Currency Action 1",
+                res_model: "foo",
+                type: "ir.actions.act_window",
+                views: [[1, "list"]],
+            },
+        };
+
+        serverData.views = {
+            "foo,1,list": `
+                    <tree>
+                        <field name="currency_id" optional="show"/>
+                        <field name="company_currency_id" optional="show"/>
+                    </tree>`,
+            "foo,false,search": "<search/>",
+        };
+
+        const webClient = await createWebClient({ serverData });
+        await doAction(webClient, 1);
+
+        assert.containsN(
+            target,
+            "th",
+            4,
+            "should have 4 th, 1 for selector, 2 for columns, 1 for optional columns"
+        );
+
+        // disable optional field
+        await click(target, "table .o_optional_columns_dropdown .dropdown-toggle");
+        await click(target, "div.o_optional_columns_dropdown span.dropdown-item:first-child");
+        assert.containsN(
+            target,
+            "th",
+            3,
+            "should have 3 th, 1 for selector, 1 for columns, 1 for optional columns"
+        );
+
+        await doAction(webClient, 1);
+        assert.containsN(
+            target,
+            "th",
+            3,
+            "should have 3 th, 1 for selector, 1 for columns, 1 for optional columns ever after listview reload"
+        );
+    });
+
     QUnit.test("selection is kept when optional fields are toggled", async function (assert) {
         await makeView({
             type: "list",
@@ -16039,10 +16088,10 @@ 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(value));
+                    assert.step("setItem " + key + " to " + JSON.stringify(String(value)));
                     return super.setItem(arguments);
                 },
             });
@@ -16100,7 +16149,7 @@ QUnit.module("Views", (hooks) => {
 
             // Only a setItem since the list view maintains its own internal state of toggled
             // optional columns.
-            assert.verifySteps(["setItem " + localStorageKey + ' to ["m2o","reference"]']);
+            assert.verifySteps(["setItem " + localStorageKey + ' to "m2o,reference"']);
 
             // 5 th (1 for checkbox, 3 for columns, 1 for optional columns)
             assert.containsN(target, "th", 5, "should have 5 th");