From b849e7966dc809947d8798a56c395c42dc998851 Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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. --- 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.51.0