Bugzilla – Attachment 187688 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: Remove original Bug 9762 implementation remnants (Part 1)
Bug-40866-Remove-original-Bug-9762-implementation-.patch (text/plain), 9.97 KB, created by
Paul Derscheid
on 2025-10-09 15:36:47 UTC
(
hide
)
Description:
Bug 40866: Remove original Bug 9762 implementation remnants (Part 1)
Filename:
MIME Type:
Creator:
Paul Derscheid
Created:
2025-10-09 15:36:47 UTC
Size:
9.97 KB
patch
obsolete
>From d7a549c1e229996e6b686f376253e13d984aa3e2 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Wed, 8 Oct 2025 21:51:11 +0100 >Subject: [PATCH] Bug 40866: Remove original Bug 9762 implementation remnants > (Part 1) > >This patch removes the duplicate logging code from the original >Bug 9762 implementation that was not properly removed when the >Alternative implementation was merged. > >The original implementation added separate logaction() calls in >CanBookBeIssued and circulation.pl that created additional log >entries with a different JSON format. This caused double logging >when the Alternative implementation (which logs via AddIssue) was >merged on top. > >Changes in this patch: > >C4/Circulation.pm: >- Removed @message_log array and all pushes to it >- Removed the separate logaction() call at the end of CanBookBeIssued >- Removed 5th return value (@message_log) from CanBookBeIssued >- Removed all if ($issueconfirmed) blocks that were populating @message_log > >circ/circulation.pl: >- Removed the problematic line that added OVERRIDDEN to every confirmed > checkout: $needsconfirmation->{'OVERRIDDEN'} = $issueconfirmed > >This addresses the double logging issue where circulation overrides >were being logged twice with different formats. > >Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> >Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> >--- > C4/Circulation.pm | 76 +-------------------------------------------- > circ/circulation.pl | 3 +- > 2 files changed, 2 insertions(+), 77 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index f3b5f276373..0a8df342638 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -827,15 +827,11 @@ sub CanBookBeIssued { > my $circ_library = Koha::Libraries->find( _GetCircControlBranch( $item_object, $patron ) ); > > my $now = dt_from_string(); >- my @message_log; > > $duedate ||= CalcDateDue( $now, $effective_itemtype, $circ_library->branchcode, $patron ); > > if ( DateTime->compare( $duedate, $now ) == -1 ) { # duedate cannot be before now > $needsconfirmation{INVALID_DATE} = $duedate; >- if ($issueconfirmed) { >- push( @message_log, "sticky due date is invalid or due date in the past" ); >- } > } > > my $fees = Koha::Charges::Fees->new( >@@ -881,9 +877,6 @@ sub CanBookBeIssued { > } > if ( $patron->is_debarred ) { > $issuingimpossible{DEBARRED} = 1; >- if ($issueconfirmed) { >- push( @message_log, "borrower is restricted" ); >- } > } > > if ( $patron->is_expired ) { >@@ -956,16 +949,10 @@ sub CanBookBeIssued { > } else { > if ( $patron_borrowing_status->{noissuescharge}->{overlimit} && $allowfineoverride ) { > $needsconfirmation{DEBT} = $patron_borrowing_status->{noissuescharge}->{charge}; >- if ($issueconfirmed) { >- push( @message_log, "borrower had amend" ); >- } > } elsif ( $patron_borrowing_status->{noissuescharge}->{overlimit} && !$allowfineoverride ) { > $issuingimpossible{DEBT} = $patron_borrowing_status->{noissuescharge}->{charge}; > } elsif ( $patron_borrowing_status->{noissuescharge}->{charge} > 0 && $allfinesneedoverride ) { > $needsconfirmation{DEBT} = $patron_borrowing_status->{noissuescharge}->{charge}; >- if ($issueconfirmed) { >- push( @message_log, "borrower had amend" ); >- } > } > } > >@@ -1046,9 +1033,6 @@ sub CanBookBeIssued { > $needsconfirmation{issued_surname} = $patron->surname; > $needsconfirmation{issued_cardnumber} = $patron->cardnumber; > $needsconfirmation{issued_borrowernumber} = $patron->borrowernumber; >- if ($issueconfirmed) { >- push( @message_log, "item is checked out for someone else" ); >- } > } > } > } >@@ -1076,9 +1060,6 @@ sub CanBookBeIssued { > $needsconfirmation{current_loan_count} = $toomany->{count}; > $needsconfirmation{max_loans_allowed} = $toomany->{max_allowed}; > $needsconfirmation{circulation_rule_TOO_MANY} = $toomany->{circulation_rule}; >- if ($issueconfirmed) { >- push( @message_log, "too many checkout" ); >- } > } else { > $issuingimpossible{TOO_MANY} = $toomany->{reason}; > $issuingimpossible{current_loan_count} = $toomany->{count}; >@@ -1111,9 +1092,6 @@ sub CanBookBeIssued { > } else { > $needsconfirmation{NOT_FOR_LOAN_FORCING} = 1; > $needsconfirmation{item_notforloan} = $item_object->notforloan; >- if ($issueconfirmed) { >- push( @message_log, "item not for loan" ); >- } > } > } else { > >@@ -1159,9 +1137,6 @@ sub CanBookBeIssued { > my $code = $av->count ? $av->next->lib : ''; > $needsconfirmation{ITEM_LOST} = $code if ( C4::Context->preference("IssueLostItem") eq 'confirm' ); > $alerts{ITEM_LOST} = $code if ( C4::Context->preference("IssueLostItem") eq 'alert' ); >- if ($issueconfirmed) { >- push( @message_log, "item lost" ); >- } > } > if ( C4::Context->preference("IndependentBranches") ) { > my $userenv = C4::Context->userenv; >@@ -1262,13 +1237,6 @@ sub CanBookBeIssued { > $needsconfirmation{'resbranchcode'} = $res->{branchcode}; > $needsconfirmation{'reswaitingdate'} = $res->{'waitingdate'}; > $needsconfirmation{'reserve_id'} = $res->{reserve_id}; >- >- if ($issueconfirmed) { >- push( >- @message_log, >- "item is on reserve and waiting, but has been reserved by some other patron" >- ); >- } > } elsif ( $restype eq "Reserved" ) { > > # The item is on reserve for someone else. >@@ -1280,10 +1248,6 @@ sub CanBookBeIssued { > $needsconfirmation{'resbranchcode'} = $res->{branchcode}; > $needsconfirmation{'resreservedate'} = $res->{reservedate}; > $needsconfirmation{'reserve_id'} = $res->{reserve_id}; >- >- if ($issueconfirmed) { >- push( @message_log, "item is on reserve for someone else" ); >- } > } elsif ( $restype eq "Transferred" ) { > > # The item is determined hold being transferred for someone else. >@@ -1295,10 +1259,6 @@ sub CanBookBeIssued { > $needsconfirmation{'resbranchcode'} = $res->{branchcode}; > $needsconfirmation{'resreservedate'} = $res->{reservedate}; > $needsconfirmation{'reserve_id'} = $res->{reserve_id}; >- >- if ($issueconfirmed) { >- push( @message_log, "item is determined hold being transferred for someone else" ); >- } > } elsif ( $restype eq "Processing" ) { > > # The item is determined hold being processed for someone else. >@@ -1310,10 +1270,6 @@ sub CanBookBeIssued { > $needsconfirmation{'resbranchcode'} = $res->{branchcode}; > $needsconfirmation{'resreservedate'} = $res->{reservedate}; > $needsconfirmation{'reserve_id'} = $res->{reserve_id}; >- >- if ($issueconfirmed) { >- push( @message_log, "item is determined hold being processed for someone else" ); >- } > } > } > } >@@ -1357,9 +1313,6 @@ sub CanBookBeIssued { > if ( $restriction_age && $patron->dateofbirth && $restriction_age > $patron->get_age() ) { > if ( C4::Context->preference('AgeRestrictionOverride') ) { > $needsconfirmation{AGE_RESTRICTION} = "$agerestriction"; >- if ($issueconfirmed) { >- push( @message_log, "age restriction" ); >- } > } else { > $issuingimpossible{AGE_RESTRICTION} = "$agerestriction"; > } >@@ -1418,34 +1371,7 @@ sub CanBookBeIssued { > } > } > >- my $borrower = $patron; >- my $user = C4::Context->userenv->{number}; >- my $branchcode = C4::Context->userenv->{branch}; >- >- # action, cardnumber, barcode, date, heure, user, branche >- if ($issueconfirmed) { >- >- my $infos = ( >- { >- message => \@message_log, >- borrowernumber => $borrower->borrowernumber, >- barcode => $barcode, >- manager_id => $user, >- branchcode => $branchcode, >- } >- ); >- >- my $json_infos = JSON->new->utf8->pretty->encode($infos); >- $json_infos =~ s/"/'/g; >- >- logaction( >- "CIRCULATION", "ISSUE", >- $borrower->{'borrowernumber'}, >- $json_infos, >- ) if C4::Context->preference("IssueLog"); >- } >- >- return ( \%issuingimpossible, \%needsconfirmation, \%alerts, \%messages, \@message_log ); >+ return ( \%issuingimpossible, \%needsconfirmation, \%alerts, \%messages ); > } > > =head2 CanBookBeReturned >diff --git a/circ/circulation.pl b/circ/circulation.pl >index 13a4aff0b4e..dddd2a1f5f9 100755 >--- a/circ/circulation.pl >+++ b/circ/circulation.pl >@@ -547,8 +547,7 @@ if ( @$barcodes && $op eq 'cud-checkout' ) { > if ( my $booked = $needsconfirmation->{BOOKED_EARLY} // $alerts->{BOOKED} ) { > $datedue = $booked->end_date; > } >- $needsconfirmation->{'DEBT'} = $needsconfirmationDEBT if ($debt_confirmed); >- $needsconfirmation->{'OVERRIDDEN'} = $issueconfirmed if ($issueconfirmed); >+ $needsconfirmation->{'DEBT'} = $needsconfirmationDEBT if ($debt_confirmed); > my $issue = AddIssue( > $patron, $barcode, $datedue, > $cancelreserve, >-- >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