Bugzilla – Attachment 187682 Details for
Bug 40866
Corrections to override logging
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40866: Implement Alternative pattern for holds override logging (Part 4)
Bug-40866-Implement-Alternative-pattern-for-holds-.patch (text/plain), 10.48 KB, created by
Andrew Fuerste-Henry
on 2025-10-09 15:21:27 UTC
(
hide
)
Description:
Bug 40866: Implement Alternative pattern for holds override logging (Part 4)
Filename:
MIME Type:
Creator:
Andrew Fuerste-Henry
Created:
2025-10-09 15:21:27 UTC
Size:
10.48 KB
patch
obsolete
>From 1fefb5d2b61d1e14b4d4e8742e7d7781e29198fd Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >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 <andrew@bywatersolutions.com> >--- > 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' %] >+ <span>Hold policy override</span> > [% CASE 'INVALID_DATE' %] > <span>Invalid date confirmed</span> > [% 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 @@ > <div class="confirmed"> [% PROCESS translate_log_override override=confirmation %] </div> > [% END %] > </div> >+ [% ELSIF ( loopro.module == 'HOLDS' && loopro.json_found ) %] >+ <div class="loginfo" id="loginfo[% loopro.action_id | html %]"> >+ <div> >+ <a href="/cgi-bin/koha/reserve/request.pl?biblionumber=[% loopro.json.biblionumber | uri %]" title="Display holds for this record" >+ >Hold [% loopro.json.hold | html %] on biblio [% loopro.json.biblionumber | html %]</a >+ > >+ [% IF loopro.json.itemnumber %] >+ - >+ <a >+ href="/cgi-bin/koha/catalogue/moredetail.pl?itemnumber=[% loopro.json.itemnumber | uri %]&biblionumber=[% loopro.json.biblionumber | uri %]#item[% loopro.json.itemnumber | uri %]" >+ title="Display detail for this item" >+ >Item [% loopro.json.itemnumber | html %]</a >+ > >+ [% END %] >+ </div> >+ [% FOREACH force IN loopro.json.forced %] >+ <div class="forced"> [% PROCESS translate_log_override override=force %] </div> >+ [% END %] >+ [% FOREACH confirmation IN loopro.json.confirmations %] >+ <div class="confirmed"> [% PROCESS translate_log_override override=confirmation %] </div> >+ [% END %] >+ </div> > [% ELSIF loopro.module == "SYSTEMPREFERENCE" || loopro.module == "REPORTS" || loopro.module == "NEWS" %] > <div class="loginfo" id="loginfo[% loopro.action_id | html %]">[% loopro.info | trim | html %]</div> > <div class="compare_info" id="compare_info[% loopro.action_id | 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40866
:
186985
|
186986
|
187063
|
187116
|
187627
|
187628
|
187629
|
187630
|
187631
|
187632
|
187633
|
187675
|
187676
|
187677
|
187678
|
187679
|
187680
|
187681
|
187682
|
187683
|
187684
|
187685
|
187686
|
187687
|
187688
|
187689
|
187690
|
187691
|
187692
|
187693
|
187694
|
187707
|
187725