From b610f86595853c52a204d07b39f10af0f3a2eb7f Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Date: Tue, 14 Jul 2026 11:43:06 +0100
Subject: [PATCH] Bug 42915: (QA follow-up) Declare all_libraries once,
 unconditionally, in request.tt

holds_table.inc is INCLUDEd in a loop from reserve/request.tt whenever
HoldsSplitQueue splits the holds queue by branch/itemtype, or when
holds for more than one biblio are shown at once (multi_hold). Each
inclusion appended its own <script> block declaring `all_libraries`,
and since top-level `let`/`const` in separate inline <script> tags
share the page's global lexical scope, the second and subsequent
occurrences threw "Identifier 'all_libraries' has already been
declared", breaking those script blocks and the pickup-library
filtering that depends on `all_libraries` being set.

Worse, that script block is only emitted when a biblio's holds are
actually rendered (biblioloo.total_holds > 0), but holds.js is loaded
unconditionally by reserve/request.tt and reads `all_libraries` at
top-level parse time. Placing the very first hold on a title (0
existing holds) therefore threw:

    ReferenceError: all_libraries is not defined

breaking every hold-management handler defined below that line in
holds.js.

Fix both by declaring `all_libraries` exactly once, unconditionally,
in request.tt's existing page-global script block (alongside the
other unconditional globals such as HidePatronName and
HoldsSplitQueue), before Asset.js("js/holds.js") loads, instead of
duplicating it inside the per-title/per-split holds_table.inc
include.

To test:
1. Search for a title with no existing holds and start placing one
2. Without this patch: browser console shows "ReferenceError:
   all_libraries is not defined" as soon as the page loads
3. With this patch: no console errors
4. Set HoldsSplitQueue to "pickup library", place holds on a title
   for two different pickup libraries, confirm both split tables
   render with working patron-name and pickup-library filtering and
   no console errors
5. Confirm the patron's own Holds tab (moremember.pl/circulation.pl)
   still loads without console errors

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
---
 koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc  | 8 --------
 .../intranet-tmpl/prog/en/includes/patron-detail-tabs.inc | 2 +-
 .../intranet-tmpl/prog/en/modules/reserve/request.tt      | 5 +++++
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
index 9f5b18eba5..1614ec2c2f 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
@@ -1,7 +1,6 @@
 [% USE Koha %]
 [% SET hold_cancellation = AuthorisedValues.GetAuthValueDropbox('HOLD_CANCELLATION') %]
 [% USE AuthorisedValues %]
-[% USE raw %]
 
 <table id="patron_holds_table_[% biblio_id | html %]_[% split_data | html %]" class="holds_table" data-total-holds="[% total_holds | html %]">
     <thead>
@@ -22,10 +21,3 @@
         </tr>
     </thead>
 </table>
-<script nonce="[% Koha.CSPNonce | $raw %]">
-    let all_libraries  = [% To.json(Branches.all) | $raw %].map(e => {
-        e['_id'] = e.branchcode;
-        e['_str'] = e.branchname;
-        return e;
-    });
-</script>
diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-detail-tabs.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-detail-tabs.inc
index 81074d4a12..5c3937c8b4 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-detail-tabs.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-detail-tabs.inc
@@ -267,7 +267,7 @@
 [% END # /WRAPPER tabs %]
 <!-- /patron-detail-tabs.inc -->
 <script nonce="[% Koha.CSPNonce | $raw %]">
-    let all_libraries  = [% To.json(Branches.all) | $raw %].map(e => {
+    var all_libraries = [% To.json(Branches.all) | $raw %].map(e => {
         e['_id'] = e.branchcode;
         e['_str'] = e.branchname;
         return e;
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
index 187080a02e..1fdfb1fc41 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
@@ -1574,6 +1574,11 @@
         var AutoResumeSuspendedHolds = [% ( Koha.Preference('AutoResumeSuspendedHolds') ) ? 1 : 0 | html %];
         var AllowHoldDateInFuture = [% ( Koha.Preference('AllowHoldDateInFuture') ) ? 1 : 0 | html %];
         var HidePatronName = [% ( Koha.Preference('HidePatronName') ) ? 1 : 0 | html %];
+        var all_libraries = [% To.json(Branches.all) | $raw %].map(e => {
+            e['_id'] = e.branchcode;
+            e['_str'] = e.branchname;
+            return e;
+        });
         var CAN_user_reserveforothers_modify_holds_priority = [%CAN_user_reserveforothers_modify_holds_priority ? 1 : 0 | html %];
         var borrowernumber = "[% patron.borrowernumber | $raw %]";
         var patron_homebranch = "[% To.json( Branches.GetName( patron.branchcode ) ) | $raw %]";