gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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>
This commit is contained in:
Pedro Amorim 2026-09-14 13:30:02 +00:00
parent 324b4d2fbe
commit 8ccba18691
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
3 changed files with 9 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
View File

@ -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 ) %]

8
koha-tmpl/intranet-tmpl/prog/js/holds.js
View File

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

8
reserve/modrequest.pl
View File

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