From 2d938704ab87576e80b5df4dedfb3d9b9617a10b Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Fri, 9 Jan 2026 12:13:30 +0100
Subject: [PATCH] Bug 41558: Fix and standardize links to tab on opac-user

We have different implemented solution to select a tab on opac-user:
* a "tab" variable (from bug 37972), it's never used
* several variables passed from the controller to the template, depending on the tab to select (it works and is used)
* use of anchors (it does not work and is used)

We should fix and standardize it.

I've linked to several reports that seem to conflict with each others.

This patch suggests to have a "tab" variable

Test plan:
Confirm that the different links to tab to opac-user are not using a
query parameter to pass the tab to select, e.g. from the opac-main page.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
---
 .../en/includes/patron-restrictions.inc       |  2 +-
 .../bootstrap/en/modules/opac-main.tt         |  8 +++----
 .../bootstrap/en/modules/opac-recall.tt       |  2 +-
 .../bootstrap/en/modules/opac-user.tt         | 24 ++++++++-----------
 opac/opac-article-request-cancel.pl           |  2 +-
 opac/opac-modrequest-suspend.pl               |  2 +-
 opac/opac-modrequest.pl                       |  2 +-
 opac/opac-request-article.pl                  |  2 +-
 opac/opac-reserve.pl                          |  2 +-
 opac/opac-user.pl                             | 23 ++++++++----------
 10 files changed, 31 insertions(+), 38 deletions(-)

diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/patron-restrictions.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/patron-restrictions.inc
index 7b2453b7b3..5ca6ea5909 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/patron-restrictions.inc
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/patron-restrictions.inc
@@ -21,7 +21,7 @@
         [% END %]
         <li>
             <span>Usually the reason for freezing an account is old overdues or damage fees. If it appears that your account is clear, please contact the library.</span>
-            <a href="/cgi-bin/koha/opac-account.pl">Check your charges page.</a> <a href="/cgi-bin/koha/opac-user.pl?opac-user-overdues=1">Check your overdues.</a>
+            <a href="/cgi-bin/koha/opac-account.pl">Check your charges page.</a> <a href="/cgi-bin/koha/opac-user.pl?tab=opac-user-overdues">Check your overdues.</a>
         </li>
     </ul>
 </li>
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt
index c41dc43e9f..84859f99d1 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt
@@ -234,7 +234,7 @@
                                     <ul id="user_summary_shortcuts">
                                         [% IF checkouts && checkouts > 0 %]
                                             <li>
-                                                <a href="/cgi-bin/koha/opac-user.pl#opac-user-checkouts">
+                                                <a href="/cgi-bin/koha/opac-user.pl?tab=opac-user-checkouts">
                                                     <span class="user_checkouts_count count_label">[% checkouts | html %]</span>
                                                     [% tn('checkout', 'checkouts', checkouts ) | html %]
                                                 </a>
@@ -242,7 +242,7 @@
                                         [% END %]
                                         [% IF overdues && overdues > 0 %]
                                             <li>
-                                                <a href="/cgi-bin/koha/opac-user.pl#opac-user-overdues">
+                                                <a href="/cgi-bin/koha/opac-user.pl?tab=opac-user-overdues">
                                                     <span class="user_overdues_count count_label">[% overdues | html %]</span>
                                                     [% tn('overdue', 'overdues', overdues ) | html %]
                                                 </a>
@@ -250,7 +250,7 @@
                                         [% END %]
                                         [% IF holds_pending && holds_pending > 0 %]
                                             <li>
-                                                <a href="/cgi-bin/koha/opac-user.pl#opac-user-holds">
+                                                <a href="/cgi-bin/koha/opac-user.pl?tab=opac-user-holds">
                                                     <span class="user_holds_pending_count count_label">[% holds_pending | html %]</span>
                                                     [% tn('hold pending', 'holds pending', holds_pending ) | html %]
                                                 </a>
@@ -258,7 +258,7 @@
                                         [% END %]
                                         [% IF holds_waiting && holds_waiting > 0 %]
                                             <li>
-                                                <a href="/cgi-bin/koha/opac-user.pl#opac-user-holds">
+                                                <a href="/cgi-bin/koha/opac-user.pl?tab=opac-user-holds">
                                                     <span class="user_holds_waiting_count count_label">[% holds_waiting | html %]</span>
                                                     [% tn('hold waiting', 'holds waiting', holds_waiting ) | html %]
                                                 </a>
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-recall.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-recall.tt
index 9c58aa7343..07e9dba487 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-recall.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-recall.tt
@@ -159,7 +159,7 @@
                                                             [% ELSIF ( hold.waitingdate ) %]
                                                                 <span class="waiting"> Waiting for patron at [% Branches.GetName( hold.branchcode ) | html %] since [% hold.waitingdate | $KohaDates %]. </span>
                                                             [% ELSIF ( hold.borrowernumber == logged_in_user.borrowernumber ) %]
-                                                                <span class="waiting"> You have already placed a <a href="/cgi-bin/koha/opac-user.pl#opac-user-holds">reserve</a> on this item. </span>
+                                                                <span class="waiting"> You have already placed a <a href="/cgi-bin/koha/opac-user.pl?tab=opac-user-holds">reserve</a> on this item. </span>
                                                             [% END # / IF ( item.recall or hold ) %]
                                                         </td>
                                                     </tr>
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
index 739773802e..6519382b99 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
@@ -1050,6 +1050,9 @@
 [% BLOCK jsinclude %]
     [% INCLUDE 'calendar.inc' %]
     [% INCLUDE 'datatables.inc' %]
+    <script>
+        const tab = "[% tab | html %]";
+    </script>
     <script>
         var AR_CAPTION_COUNT = _("(%s total)");
 
@@ -1063,13 +1066,13 @@
             }
         }
         $(document).ready(function(){
-            [% IF ( opac_user_holds ) %]
-                $("#opac-user-views a[href='#opac-user-holds_panel']").tab("show");
-            [% ELSIF ( opac_user_overdues ) %]
-                $("#opac-user-views a[href='#opac-user-overdues_panel']").tab("show");
-            [% ELSIF ( opac_user_article_requests ) %]
-                $("#opac-user-views a[href='#opac-user-article-requests_panel']").tab("show");
-            [% END %]
+            if (tab){
+                $(`#opac-user-views a[href='#${tab}_panel']`).tab("show");
+            }
+            if( $("#opac-user-views .tab-pane.active").length < 1 ){
+                $("#opac-user-views a:first").tab("show");
+            }
+
             $('#article-requests-table caption .count').html(AR_CAPTION_COUNT.format('[% current_article_requests.size | html %]'));
 
             $(".modal-nojs").addClass("modal").addClass("hide").removeClass("modal-nojs");
@@ -1340,13 +1343,6 @@
                 return confirmDelete(_("Are you sure you want to dismiss this message?"));
             });
 
-            [% IF ( tab ) %]
-                $("#opac-user-views #[% tab | html %]-tab").tab("show");
-            [% END %]
-
-            if( $("#opac-user-views .tab-pane.active").length < 1 ){
-                $("#opac-user-views a:first").tab("show");
-            }
         });
 
         function submitNote( title, issue_id, note ){
diff --git a/opac/opac-article-request-cancel.pl b/opac/opac-article-request-cancel.pl
index abc81a6229..591e4b9b77 100755
--- a/opac/opac-article-request-cancel.pl
+++ b/opac/opac-article-request-cancel.pl
@@ -50,4 +50,4 @@ if ($id) {
     $ar->cancel();
 }
 
-print $query->redirect("/cgi-bin/koha/opac-user.pl#opac-user-article-requests");
+print $query->redirect("/cgi-bin/koha/opac-user.pl?tab=opac-user-article-requests");
diff --git a/opac/opac-modrequest-suspend.pl b/opac/opac-modrequest-suspend.pl
index ccd750d2da..48c9a6473b 100755
--- a/opac/opac-modrequest-suspend.pl
+++ b/opac/opac-modrequest-suspend.pl
@@ -54,4 +54,4 @@ if ( $op eq 'cud-suspend' ) {
     );
 }
 
-print $query->redirect("/cgi-bin/koha/opac-user.pl?opac-user-holds=1");
+print $query->redirect("/cgi-bin/koha/opac-user.pl?tab=opac-user-holds");
diff --git a/opac/opac-modrequest.pl b/opac/opac-modrequest.pl
index 2cf6712685..8611137040 100755
--- a/opac/opac-modrequest.pl
+++ b/opac/opac-modrequest.pl
@@ -71,4 +71,4 @@ if ( $reserve_id && $borrowernumber ) {
     }
 }
 
-print $query->redirect("/cgi-bin/koha/opac-user.pl?opac-user-holds=1");
+print $query->redirect("/cgi-bin/koha/opac-user.pl?tab=opac-user-holds");
diff --git a/opac/opac-request-article.pl b/opac/opac-request-article.pl
index fa1015d4ab..a20f068c50 100755
--- a/opac/opac-request-article.pl
+++ b/opac/opac-request-article.pl
@@ -96,7 +96,7 @@ if ( $op eq 'cud-create' ) {
     };
 
     if ($success) {
-        print $cgi->redirect("/cgi-bin/koha/opac-user.pl?opac-user-article-requests=1");
+        print $cgi->redirect("/cgi-bin/koha/opac-user.pl?tab=opac-user-article-requests");
         exit;
     }
 
diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
index d78e0942fc..e69562481e 100755
--- a/opac/opac-reserve.pl
+++ b/opac/opac-reserve.pl
@@ -316,7 +316,7 @@ if ( $op eq 'cud-place_reserve' ) {
 
     print $query->redirect( "/cgi-bin/koha/opac-user.pl?"
             . ( @failed_holds ? "failed_holds=" . join( '|', @failed_holds ) : q|| )
-            . "&opac-user-holds=1" );
+            . "&tab=opac-user-holds" );
     exit;
 }
 
diff --git a/opac/opac-user.pl b/opac/opac-user.pl
index d4ce85dce8..d7000158fb 100755
--- a/opac/opac-user.pl
+++ b/opac/opac-user.pl
@@ -419,19 +419,16 @@ if ( C4::Context->preference("ArticleRequests") ) {
 }
 
 $template->param(
-    patron_messages            => $patron_messages,
-    opacnote                   => $borr->{opacnote},
-    patronupdate               => $patronupdate,
-    OpacRenewalAllowed         => C4::Context->preference("OpacRenewalAllowed"),
-    userview                   => 1,
-    tab                        => scalar $query->param('tab') || 0,
-    SuspendHoldsOpac           => C4::Context->preference('SuspendHoldsOpac'),
-    AutoResumeSuspendedHolds   => C4::Context->preference('AutoResumeSuspendedHolds'),
-    OpacHoldNotes              => C4::Context->preference('OpacHoldNotes'),
-    failed_holds               => scalar $query->param('failed_holds'),
-    opac_user_holds            => scalar $query->param('opac-user-holds')            || 0,
-    opac_user_overdues         => scalar $query->param('opac-user-overdues')         || 0,
-    opac_user_article_requests => scalar $query->param('opac-user-article-requests') || 0,
+    patron_messages          => $patron_messages,
+    opacnote                 => $borr->{opacnote},
+    patronupdate             => $patronupdate,
+    OpacRenewalAllowed       => C4::Context->preference("OpacRenewalAllowed"),
+    userview                 => 1,
+    tab                      => scalar $query->param('tab') || q{},
+    SuspendHoldsOpac         => C4::Context->preference('SuspendHoldsOpac'),
+    AutoResumeSuspendedHolds => C4::Context->preference('AutoResumeSuspendedHolds'),
+    OpacHoldNotes            => C4::Context->preference('OpacHoldNotes'),
+    failed_holds             => scalar $query->param('failed_holds'),
 );
 
 # if not an empty string this indicates to return