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>
This commit is contained in:
Jonathan Druart 2026-01-09 12:13:30 +01:00 committed by Laura Escamilla
parent 8903df398a
commit 2d938704ab
No known key found for this signature in database
GPG Key ID: 6B1D9CA1AF817BC3
10 changed files with 31 additions and 38 deletions

View File

@ -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>

View File

@ -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>

View File

@ -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>

View File

@ -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 ){

View File

@ -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");

View File

@ -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");

View File

@ -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");

View File

@ -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;
}

View File

@ -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;
}

View File

@ -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