From 8ccba18691853c6aae072fe5c5aff84102440a0a Mon Sep 17 00:00:00 2001
From: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Date: Mon, 14 Sep 2026 13:30:02 +0000
Subject: [PATCH] Bug 31391: (RM follow-up) Enforce recalls permission when
 converting holds to recalls

The previous follow-up only blanked the button's label via JS for
users without the recalls permission; the button still rendered,
was still clickable, and modrequest.pl's cud-convertall had no
permission check at all, so the conversion still succeeded.

Adds CAN_user_recalls to the button's template condition (matching
the existing "Move selected holds" button) so it doesn't render at
all without the permission, drops the now-redundant JS check in
holds.js, and adds a manage_recalls check in modrequest.pl itself.
checkauth()'s userid is the login username, not a borrowernumber, so
this uses C4::Auth::haspermission() directly rather than
Koha::Patrons->find().

Test plan:
1) Set a default recalls rule in admin/smart-rules.pl.
2) Set UseRecalls to 'staff' or 'opac_and_staff'.
3) Give a test patron 'catalogue' + 'reserveforothers' but not
   'recalls'.
4) Check out an item, place a hold on it for another patron.
5) As the test patron, go to that record's Holds tab, confirm
   'Convert selected to recall' doesn't appear.
6) Since the button is gone, confirm the server side is enforced too:
   post op=cud-convertall directly to modrequest.pl and confirm it no
   longer converts the hold.
7) Grant 'recalls' and confirm the button reappears and works.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
---
 .../intranet-tmpl/prog/en/modules/reserve/request.tt      | 3 +--
 koha-tmpl/intranet-tmpl/prog/js/holds.js                  | 8 +++-----
 reserve/modrequest.pl                                     | 8 +++++---
 3 files changed, 9 insertions(+), 10 deletions(-)

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 aa0aaf2359..f38cd906f1 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
@@ -1391,7 +1391,7 @@
                             </ul>
                         </div>
                     [% END %]
-                    [% IF Koha.Preference('UseRecalls').match('staff') %]
+                    [% IF Koha.Preference('UseRecalls').match('staff') && CAN_user_recalls %]
                         <div class="btn-group">
                             <button class="btn convert_selected_holds" data-bulk="true"></button>
                         </div>
@@ -1605,7 +1605,6 @@
             return e;
         });
         var CAN_user_reserveforothers_modify_holds_priority = [%CAN_user_reserveforothers_modify_holds_priority ? 1 : 0 | html %];
-        var CAN_user_recalls = [%CAN_user_recalls ? 1 : 0 | html %];
         var borrowernumber = "[% patron.borrowernumber | $raw %]";
         var patron_homebranch = "[% To.json( Branches.GetName( patron.branchcode ) ) | $raw %]";
         var override_items = {[% FOREACH biblio_info IN biblioloop %][% FOREACH itemloo IN biblio_info.itemloop %][% IF ( itemloo.override ) %]
diff --git a/koha-tmpl/intranet-tmpl/prog/js/holds.js b/koha-tmpl/intranet-tmpl/prog/js/holds.js
index a6d9ee2fc1..ed6e80cb19 100644
--- a/koha-tmpl/intranet-tmpl/prog/js/holds.js
+++ b/koha-tmpl/intranet-tmpl/prog/js/holds.js
@@ -1983,11 +1983,9 @@ async function load_patron_holds_table(biblio_id, split_data) {
             $("#convert_hold_alert").html(
                 MSG_CONVERT_ALERT.format(selectedCount, selectedCount)
             );
-            if (CAN_user_recalls) {
-                $(".convert_selected_holds").html(
-                    MSG_RECALL_SELECTED.format(selectedCount)
-                );
-            }
+            $(".convert_selected_holds").html(
+                MSG_RECALL_SELECTED.format(selectedCount)
+            );
             if (selectedCount > 0) {
                 $("#cancel_hold_alert").show();
                 $("#convert_hold_alert").show();
diff --git a/reserve/modrequest.pl b/reserve/modrequest.pl
index 12b2101dcd..253a664610 100755
--- a/reserve/modrequest.pl
+++ b/reserve/modrequest.pl
@@ -29,12 +29,12 @@ use Try::Tiny;
 
 use C4::Output;
 use C4::Reserves qw( ModReserve FixPriority );
-use C4::Auth     qw( checkauth );
+use C4::Auth     qw( checkauth haspermission );
 use Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue;
 
 my $query = CGI->new;
 
-checkauth( $query, 0, { reserveforothers => '*' }, 'intranet' );
+my $userid = checkauth( $query, 0, { reserveforothers => '*' }, 'intranet' );
 
 my $op              = $query->param('op') || 'cud-modifyall';
 my @reserve_id      = $query->multi_param('reserve_id');
@@ -102,7 +102,9 @@ if ( $op eq 'cud-cancelall' || $op eq 'cud-modifyall' ) {
     }
 
 } elsif ( $op eq 'cud-convertall' ) {
-    for ( my $i = 0 ; $i < $count ; $i++ ) {
+    my $can_convert = haspermission( $userid, { recalls => 'manage_recalls' } );
+
+    for ( my $i = 0 ; $can_convert && $i < $count ; $i++ ) {
         undef $itemnumber[$i] if !$itemnumber[$i];
         my $cancellation_reason = $query->param("cancellation-reason");
         my $params              = {