From 8a5555238da6671320ef4cd46cb122bdabf784c1 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Tue, 16 Jun 2026 07:20:07 +0200
Subject: [PATCH] Bug 41604: Save user decision per column when visibility
 changes

- Modify column-visibility.dt event handler to store the visibility
  preference of the user
- Improve _dt_force_visibility() to correctly pick the correctly visibility value depending
  on visibility_condition, is_hidden, and the user preference
- Add _dt_get_expected_column_visibility() to guess what's the expected visibility
- Force boolean true/false (vs 1/0) using !!
- Rely on a global variable kohaTableUserChoices to store the user
  choice. It will be part of the state and store in localStorage.

Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
---
 .../prog/en/modules/acqui/basket.tt           |   9 +-
 .../prog/en/modules/members/holdshistory.tt   |   3 +-
 .../prog/en/modules/tools/modborrowers.tt     |   9 +-
 koha-tmpl/intranet-tmpl/prog/js/checkouts.js  |  12 +-
 koha-tmpl/intranet-tmpl/prog/js/datatables.js | 157 ++++++++++++------
 .../integration/KohaTable/KohaTable_spec.ts   |  60 ++++++-
 6 files changed, 170 insertions(+), 80 deletions(-)

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
index 81a3a925d0..6685a9d3d2 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
@@ -1450,19 +1450,16 @@
     <script nonce="[% Koha.CSPNonce | $raw %]">
         var orders_table_settings = [% TablesSettings.GetTableSettings( 'acqui', 'basket', 'orders', 'json' ) | $raw %];
         let supplier_report_column = orders_table_settings.columns.find(c => c.columnname == 'supplier_report');
-        const edifact_available = [% Koha.Preference('EDIFACT') && ediaccount ? 1 : 0 | html %];
+        const edifact_available = [% Koha.Preference('EDIFACT') && ediaccount ? 'true' : 'false' | html %];
         supplier_report_column.visibility_condition = edifact_available;
-        supplier_report_column.force_visibility = 1;
 
         let modify_column = orders_table_settings.columns.find(c => c.columnname == 'modify');
-        const modify_column_available = [% active && !closedate ? 1 : 0 | html %];
+        const modify_column_available = [% active && !closedate ? 'true' : 'false' | html %];
         modify_column.visibility_condition = modify_column_available;
-        modify_column.force_visibility = 1;
 
         let cancel_column = orders_table_settings.columns.find(c => c.columnname == 'cancel');
-        const cancel_column_available = [% !closedate || Koha.Preference('CancelOrdersInClosedBaskets') ? 1 : 0 | html %]
+        const cancel_column_available = [% !closedate || Koha.Preference('CancelOrdersInClosedBaskets') ? 'true' : 'false' | html %]
         cancel_column.visibility_condition = cancel_column_available;
-        cancel_column.force_visibility = 1;
 
         let skip_confirm_reopen = [% skip_confirm_reopen ? 1 : 0 | html %];
     </script>
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt
index 7773823b8a..41333bdfce 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt
@@ -191,8 +191,7 @@
     <script nonce="[% Koha.CSPNonce | $raw %]">
         $(document).ready(function () {
             let item_type_column = table_settings.columns.find(c => c.columnname == "itemtype");
-            item_type_column.force_visibility = 1;
-            item_type_column.visibility_condition = show_itemtype_column;
+            item_type_column.visibility_condition = !!show_itemtype_column;
 
             let current_holds_table = build_holds_table("#table_holdshistory");
             let old_holds_table = build_holds_table("#table_oldholdshistory", 1);
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt
index 75e6e625e6..07999fc2b5 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt
@@ -519,21 +519,18 @@
                 let selections_column = table_settings.columns.find(c => c.columnname == "batch_patron_edit_selections");
                 const selections_column_available = op == "show";
                 selections_column.cannot_be_modified = 1;
-                selections_column.visibility_condition = selections_column_available;
-                selections_column.force_visibility = 1;
+                selections_column.visibility_condition = !!selections_column_available;
 
                 if (!Koha.permissions.CanUpdatePasswordExpiration) {
                     let password_expiration_date_column = table_settings.columns.find(c => c.columnname == "batch_patron_edit_password_expiration_date");
                     password_expiration_date_column.cannot_be_modified = 1;
-                    password_expiration_date_column.visibility_condition = 0;
-                    password_expiration_date_column.force_visibility = 1;
+                    password_expiration_date_column.visibility_condition = false;
                 }
 
                 if (!Koha.permissions.CanUpdateProtectPatron) {
                     let protected_column = table_settings.columns.find(c => c.columnname == "batch_patron_edit_protected");
                     protected_column.cannot_be_modified = 1;
-                    protected_column.visibility_condition = 0;
-                    protected_column.force_visibility = 1;
+                    protected_column.visibility_condition = false;
                 }
 
                 let patron_table = $("#borrowerst").kohaTable(
diff --git a/koha-tmpl/intranet-tmpl/prog/js/checkouts.js b/koha-tmpl/intranet-tmpl/prog/js/checkouts.js
index eea14770f2..a306d1c6f9 100644
--- a/koha-tmpl/intranet-tmpl/prog/js/checkouts.js
+++ b/koha-tmpl/intranet-tmpl/prog/js/checkouts.js
@@ -40,26 +40,22 @@ function LoadIssuesTable() {
     let renew_column = table_settings_issues_table.columns.find(
         c => c.columnname == "renew"
     );
-    renew_column.visibility_condition = AllowCirculate;
-    renew_column.force_visibility = 1;
+    renew_column.visibility_condition = !!AllowCirculate;
 
     let checkin_column = table_settings_issues_table.columns.find(
         c => c.columnname == "checkin"
     );
-    checkin_column.visibility_condition = AllowCirculate;
-    checkin_column.force_visibility = 1;
+    checkin_column.visibility_condition = !!AllowCirculate;
 
     let claims_returned_column = table_settings_issues_table.columns.find(
         c => c.columnname == "claims_returned"
     );
-    claims_returned_column.visibility_condition = ClaimReturnedLostValue;
-    claims_returned_column.force_visibility = 1;
+    claims_returned_column.visibility_condition = !!ClaimReturnedLostValue;
 
     let export_column = table_settings_issues_table.columns.find(
         c => c.columnname == "export"
     );
-    export_column.visibility_condition = exports_enabled;
-    export_column.force_visibility = 1;
+    export_column.visibility_condition = !!exports_enabled;
 
     issuesTable = $("#issues-table").kohaTable(
         {
diff --git a/koha-tmpl/intranet-tmpl/prog/js/datatables.js b/koha-tmpl/intranet-tmpl/prog/js/datatables.js
index 1a9fb711b6..7929210b73 100644
--- a/koha-tmpl/intranet-tmpl/prog/js/datatables.js
+++ b/koha-tmpl/intranet-tmpl/prog/js/datatables.js
@@ -75,6 +75,7 @@ var dataTablesDefaults = {
     },
 };
 DataTable.defaults.column.orderSequence = ["asc", "desc"];
+var kohaTableUserChoices = {};
 
 function toggledClearFilter(searchText, tableId) {
     let clear_filter_button = $("#" + tableId + "_wrapper").find(
@@ -983,43 +984,72 @@ function _dt_buttons(params) {
     return buttons;
 }
 
+function _dt_get_expected_column_visibility(column) {
+    // Determine expected visibility based on table settings
+    // Priority: is_hidden > visibility_condition
+    if (column.is_hidden) {
+        return false;
+    }
+    if (column.visibility_condition !== undefined) {
+        return !!column.visibility_condition;
+    }
+    return true; // default to visible
+}
+
 function _dt_force_visibility(table_settings, table_dt, state) {
     var columns_settings = table_settings.columns;
-    let i = 0;
-    let force_vis_columns = table_settings.columns.filter(
-        c => c.force_visibility
-    );
-    if (!force_vis_columns.length) return state;
-
     let use_names = $(table_dt.table().node()).data("bKohaColumnsUseNames");
-    if (use_names) {
-        table_dt
-            .columns(
-                force_vis_columns
-                    .map(c => "[data-colname='%s']".format(c.columnname))
-                    .join(",")
-            )
-            .every(function () {
-                let column = table_settings.columns.find(
-                    c =>
-                        c.columnname ==
-                        this.header().getAttribute("data-colname")
-                );
-                let index = this.index();
-                if (column.visibility_condition !== undefined) {
-                    state.columns[index].visible =
-                        !!column.visibility_condition;
-                    state.columns[index].cannot_be_toggled = true;
-                } else if (column.is_hidden && column.cannot_be_toggled) {
-                    state.columns[index].visible = false;
-                    state.columns[index].cannot_be_toggled = true;
-                }
-            });
-    } else {
+    if (!use_names) {
         throw new Error(
             "Cannot force column visibility without bKohaColumnsUseNames!"
         );
     }
+
+    table_dt.columns().every(function () {
+        let column = table_settings.columns.find(
+            c => c.columnname == this.header().getAttribute("data-colname")
+        );
+        let index = this.index();
+        let expected_visible = _dt_get_expected_column_visibility(column);
+
+        const user_choice =
+            state.user_choices && state.user_choices[index] !== undefined
+                ? state.user_choices[index]
+                : undefined;
+
+        // Determine visibility based on priorities
+        if (column.cannot_be_toggled) {
+            // Admin lock => always use config
+            state.columns[index].visible = expected_visible;
+            state.columns[index].cannot_be_toggled = true;
+        } else if (column.visibility_condition !== undefined) {
+            // visibility_condition is set
+            if (column.visibility_condition === true) {
+                // If the column is displayed
+                if (user_choice !== undefined) {
+                    // We read user's choice
+                    // The following condition is needed, it is not the same as `state.columns[index].visible = user_choice`!
+                    if (user_choice === false) {
+                        state.columns[index].visible = false;
+                    } else {
+                        state.columns[index].visible = true;
+                    }
+                } else {
+                    state.columns[index].visible = expected_visible;
+                }
+            } else {
+                // If the column cannot be displayed, we keep it hidden
+                state.columns[index].visible = false;
+                state.columns[index].cannot_be_toggled = true;
+            }
+        } else if (user_choice !== undefined) {
+            state.columns[index].visible = user_choice;
+        } else {
+            // No user decision, apply table settings
+            state.columns[index].visible = expected_visible;
+        }
+    });
+
     return state;
 }
 
@@ -1030,21 +1060,6 @@ function _dt_visibility(table_settings, table_dt) {
         let i = 0;
         let use_names = $(table_dt.table().node()).data("bKohaColumnsUseNames");
         if (use_names) {
-            // Deal with visibility_condition (needed when state is not saved)
-            table_settings.columns = table_settings.columns.map(c => {
-                if (
-                    c.visibility_condition !== undefined &&
-                    !c.visibility_condition
-                ) {
-                    return {
-                        ...c,
-                        is_hidden: true,
-                        cannot_be_toggled: true,
-                    };
-                }
-                return c;
-            });
-
             let hidden_columns = table_settings.columns.filter(
                 c => c.is_hidden
             );
@@ -1238,16 +1253,21 @@ function _dt_save_restore_state(table_settings, external_filter_nodes = {}) {
     };
 
     function set_default(table_settings, table_dt) {
-        let columns = new Array(table_dt.columns()[0].length).fill({
-            visible: true,
-        });
+        let columns = Array.from(
+            { length: table_dt.columns()[0].length },
+            (_, i) => ({
+                visible: true,
+            })
+        );
         let hidden_ids = _dt_visibility(table_settings, table_dt);
         hidden_ids.forEach((id, i) => {
-            columns[id] = { visible: false };
+            columns[id].visible = false;
         });
+
         // State is not loaded if time is not passed
         return { columns, time: new Date() };
     }
+
     let stateLoadCallback = function (settings) {
         // Load state from URL
         const url = new URL(window.location.href);
@@ -1264,9 +1284,11 @@ function _dt_save_restore_state(table_settings, external_filter_nodes = {}) {
 
         state = JSON.parse(state);
 
-        if (default_save_state_search) {
+        if (default_save_state || default_save_state_search) {
             $("#" + settings.nTable.id).data("loaded_from_state", true);
-        } else {
+        }
+
+        if (!default_save_state_search) {
             delete state.search;
             state.columns.forEach(c => delete c.search);
         }
@@ -1301,6 +1323,9 @@ function _dt_save_restore_state(table_settings, external_filter_nodes = {}) {
             },
             {}
         );
+
+        const tableId = settings.nTable.id;
+        data.user_choices = kohaTableUserChoices[tableId] || {};
     };
     let stateLoadParams = function (settings, data) {
         if (!$("#" + settings.nTable.id).data("loaded_from_state")) return;
@@ -1336,6 +1361,11 @@ function _dt_save_restore_state(table_settings, external_filter_nodes = {}) {
                 }
             });
         }
+
+        if (data.user_choices) {
+            const tableId = settings.nTable.id;
+            kohaTableUserChoices[tableId] = data.user_choices;
+        }
     };
 
     return {
@@ -1554,11 +1584,28 @@ function update_search_description(
             _dt_add_filters(this, table_dt, filters_options);
         }
 
-        table_dt.on("column-visibility.dt", function () {
-            if (add_filters) {
-                _dt_add_filters(this, table_dt, filters_options);
+        table_dt.on(
+            "column-visibility.dt",
+            function (e, settings, column, visibility) {
+                if (add_filters) {
+                    _dt_add_filters(this, table_dt, filters_options);
+                }
+
+                // Mark the column as user-decided when visibility changes
+                // Only set for the specific column that was toggled
+                let current_state = table_dt.state();
+                if (
+                    current_state &&
+                    current_state.columns &&
+                    current_state.columns[column]
+                ) {
+                    const tableId = settings.nTable.id;
+                    kohaTableUserChoices[tableId] ??= {};
+                    kohaTableUserChoices[tableId][column] = visibility;
+                    table_dt.state.save();
+                }
             }
-        });
+        );
 
         table_dt.on("search.dt", function (e, settings) {
             // When the DataTables search function is triggered,
diff --git a/t/cypress/integration/KohaTable/KohaTable_spec.ts b/t/cypress/integration/KohaTable/KohaTable_spec.ts
index 16277ea215..8fab186b15 100644
--- a/t/cypress/integration/KohaTable/KohaTable_spec.ts
+++ b/t/cypress/integration/KohaTable/KohaTable_spec.ts
@@ -336,7 +336,7 @@ describe("kohaTable (using REST API)", () => {
             build_libraries().then(() => {
                 cy.visit("/cgi-bin/koha/admin/branches.pl");
 
-                // default settings: hide "Code"
+                // default settings: Show "Code" and cannot be toggled, but feature is off (visibility_condition=false)
                 cy.mock_table_settings({
                     default_save_state: 1,
                     columns: {
@@ -360,8 +360,8 @@ describe("kohaTable (using REST API)", () => {
                 });
 
                 // Do not touch anything
-                // But turn on a syspref that would display the column
-                // Should be kept hidden after reload
+                // But turn on the feature that would display the column
+                // Should be displayed after reload
                 // This is the behaviour for the Export column of the checkouts table, and syspref ExportCircHistory
                 cy.mock_table_settings({
                     default_save_state: 1,
@@ -376,12 +376,66 @@ describe("kohaTable (using REST API)", () => {
 
                 cy.visit("/cgi-bin/koha/admin/branches.pl");
 
+                cy.get("@columns").then(columns => {
+                    cy.get(`#${table_id} th`).should(
+                        "have.length",
+                        columns.length
+                    );
+                    cy.get(`#${table_id} th`).contains("Name");
+                    cy.get(`#${table_id} th`).contains("Code");
+                });
+            });
+        });
+
+        it("visibility_condition modified, can be toggled", () => {
+            build_libraries().then(() => {
+                cy.visit("/cgi-bin/koha/admin/branches.pl");
+
+                // default settings: Show "Code" and can be toggled, but feature is off (visibility_condition=false)
+                cy.mock_table_settings({
+                    default_save_state: 1,
+                    columns: {
+                        library_code: {
+                            visibility_condition: false,
+                            is_hidden: false,
+                            cannot_be_toggled: false,
+                        },
+                    },
+                });
+
                 cy.get("@columns").then(columns => {
                     cy.get(`#${table_id} th`).should(
                         "have.length",
                         columns.length - 1
                     );
                     cy.get(`#${table_id} th`).contains("Name");
+                    cy.get(`#${table_id} th`)
+                        .contains("Code")
+                        .should("not.exist");
+                });
+
+                // Do not touch anything
+                // But turn on a syspref that would display the column
+                // Should be kept hidden after reload
+                cy.mock_table_settings({
+                    default_save_state: 1,
+                    columns: {
+                        library_code: {
+                            visibility_condition: true,
+                            is_hidden: false,
+                            cannot_be_toggled: false,
+                        },
+                    },
+                });
+
+                cy.visit("/cgi-bin/koha/admin/branches.pl");
+
+                cy.get("@columns").then(columns => {
+                    cy.get(`#${table_id} th`).should(
+                        "have.length",
+                        columns.length
+                    );
+                    cy.get(`#${table_id} th`).contains("Name");
                     cy.get(`#${table_id} th`).contains("Code");
                 });
             });