From 1fefb5d2b61d1e14b4d4e8742e7d7781e29198fd Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 9 Oct 2025 07:53:19 +0100 Subject: [PATCH] Bug 40866: Implement Alternative pattern for holds override logging (Part 4) This patch implements proper override logging for holds following the Alternative implementation pattern used for checkouts and renewals. Instead of just removing the orphaned logging code from the original Bug 9762 implementation, this patch properly implements override logging by updating AddReserve to accept and log override information, similar to how AddIssue handles confirmations and forced overrides. Changes: C4/Reserves.pm: - Added JSON qw(to_json) import - Updated AddReserve to accept confirmations and forced parameters - Enhanced the existing logaction call to include override information in JSON format when overrides are present - Log format matches the Alternative implementation pattern: * hold ID * branchcode * biblionumber * itemnumber * confirmations array * forced array Koha/REST/V1/Holds.pm: - Added confirmations array building in the add() method - Pass 'HOLD_POLICY_OVERRIDE' confirmation when override_all is true - Pass confirmations parameter to AddReserve call reserve/request.pl: - Removed orphaned original implementation logging code - Removed unused variables and imports (message, @message, user, etc.) - Removed C4::Log import koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc: - Added 'HOLD_POLICY_OVERRIDE' case to translate_log_override block - Displays as "Hold policy override" in log viewer koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt: - Added JSON display handling for HOLDS module (similar to CIRCULATION) - Displays hold ID, biblionumber, and optional itemnumber with links - Shows forced and confirmation overrides using translate_log_override This ensures hold overrides are logged consistently with the Alternative implementation pattern, providing proper audit trails for policy overrides that are properly displayed in the log viewer. Signed-off-by: Andrew Fuerste Henry --- C4/Reserves.pm | 24 ++++++++++++++-- Koha/REST/V1/Holds.pm | 7 +++++ .../prog/en/includes/action-logs.inc | 2 ++ .../prog/en/modules/tools/viewlog.tt | 22 +++++++++++++++ reserve/request.pl | 28 ------------------- 5 files changed, 53 insertions(+), 30 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 7d6c106ceae..b559c8b9b12 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -22,6 +22,8 @@ package C4::Reserves; use Modern::Perl; +use JSON qw( to_json ); + use C4::Accounts; use C4::Biblio qw( GetMarcFromKohaField ); use C4::Circulation qw( CheckIfIssuedToPatron GetAgeRestriction GetBranchItemRule ); @@ -193,6 +195,8 @@ sub AddReserve { my $non_priority = $params->{non_priority}; my $item_group_id = $params->{item_group_id}; my $hold_group_id = $params->{hold_group_id}; + my $confirmations = $params->{confirmations}; + my $forced = $params->{forced}; $resdate ||= dt_from_string; @@ -263,8 +267,24 @@ sub AddReserve { # record patron activity $hold->patron->update_lastseen('hold'); - logaction( 'HOLDS', 'CREATE', $hold->id, $hold ) - if C4::Context->preference('HoldsLog'); + # Log the hold creation + if ( C4::Context->preference('HoldsLog') ) { + my $info = $hold->id; + if ( defined($confirmations) || defined($forced) ) { + $info = to_json( + { + hold => $hold->id, + branchcode => $hold->branchcode, + biblionumber => $hold->biblionumber, + itemnumber => $hold->itemnumber, + confirmations => $confirmations, + forced => $forced + }, + { pretty => 1, canonical => 1 } + ); + } + logaction( 'HOLDS', 'CREATE', $hold->id, $info ); + } my $reserve_id = $hold->id(); diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm index 4a9093f0fc8..23bd4ead8f9 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -191,6 +191,12 @@ sub add { my $priority = C4::Reserves::CalculatePriority($biblio_id); + # Build confirmations array for logging overrides + my $confirmations = []; + if ($override_all) { + push @{$confirmations}, 'HOLD_POLICY_OVERRIDE'; + } + my $hold_id = C4::Reserves::AddReserve( { branchcode => $pickup_library_id, @@ -206,6 +212,7 @@ sub add { itemtype => $item_type, non_priority => $non_priority, item_group_id => $item_group_id, + confirmations => $confirmations, } ); diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc index 774e05477f6..57092d75e7d 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc @@ -179,6 +179,8 @@ [% BLOCK translate_log_override %] [% SWITCH override %] + [% CASE 'HOLD_POLICY_OVERRIDE' %] + Hold policy override [% CASE 'INVALID_DATE' %] Invalid date confirmed [% CASE 'DEBT_GUARANTEES' %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt index 44e6e4b20e2..03efca0532b 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt @@ -349,6 +349,28 @@
[% PROCESS translate_log_override override=confirmation %]
[% END %] + [% ELSIF ( loopro.module == 'HOLDS' && loopro.json_found ) %] +
+
+ Hold [% loopro.json.hold | html %] on biblio [% loopro.json.biblionumber | html %] + [% IF loopro.json.itemnumber %] + - + Item [% loopro.json.itemnumber | html %] + [% END %] +
+ [% FOREACH force IN loopro.json.forced %] +
[% PROCESS translate_log_override override=force %]
+ [% END %] + [% FOREACH confirmation IN loopro.json.confirmations %] +
[% PROCESS translate_log_override override=confirmation %]
+ [% END %] +
[% ELSIF loopro.module == "SYSTEMPREFERENCE" || loopro.module == "REPORTS" || loopro.module == "NEWS" %]
[% loopro.info | trim | html %]
diff --git a/reserve/request.pl b/reserve/request.pl index faabb74b775..a00a6de53e1 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -40,7 +40,6 @@ use C4::Serials qw( CountSubscriptionFromBiblionumber ); use C4::Circulation qw( _GetCircControlBranch GetBranchItemRule ); use Koha::DateUtils qw( dt_from_string ); use C4::Search qw( enabled_staff_search_views ); -use C4::Log qw( logaction ); use Koha::Biblios; use Koha::Checkouts; @@ -624,33 +623,6 @@ if ( ( $findborrower && $borrowernumber_hold || $findclub && $club_hold ) $num_alreadyheld++; } - my $borrower = $patron; - my $borrowernumber = $patron->borrowernumber; - my $user = C4::Context->userenv->{number}; - my $branchcode = C4::Context->userenv->{branch}; - my $message = "Override limitation of hold"; - my @message = ("Override limitation of hold"); - my $barcode = $item->{barcode}; - - my $infos = ( - { - message => \@message, - borrowernumber => $borrowernumber, - barcode => $barcode, - manager_id => $user, - branchcode => $branchcode, - } - ); - - my $json_infos = JSON->new->utf8->pretty->encode($infos); - $json_infos =~ s/"/'/g; - - logaction( - "HOLD", "ISSUE", - $borrower->{'borrowernumber'}, - $json_infos, - ) if C4::Context->preference("IssueLog"); - push( @available_itemtypes, $item->{itype} ); } else { -- 2.39.5