From 29e8b2415e78b71b9d6ef4752e53162b4cce70c2 Mon Sep 17 00:00:00 2001 From: Laura_Escamilla Date: Wed, 5 Nov 2025 19:08:04 +0000 Subject: [PATCH] Bug 35612: Recorded branch context in accountlines.branchcode for OVERDUE, LOST, and PROCESSING fees. --- C4/Accounts.pm | 60 +++++++++++++++++++++--------------- C4/Circulation.pm | 48 +++++++++++++++++++++++------ C4/Overdues.pm | 17 ++++++++-- Koha/Checkout.pm | 47 ++++++++++++++++++++++++++++ misc/cronjobs/longoverdue.pl | 21 ++++++++++++- 5 files changed, 156 insertions(+), 37 deletions(-) diff --git a/C4/Accounts.pm b/C4/Accounts.pm index 43f1b568b7..3a5877abbb 100644 --- a/C4/Accounts.pm +++ b/C4/Accounts.pm @@ -24,7 +24,10 @@ use C4::Members; use Koha::Account; use Koha::Account::Lines; use Koha::Account::Offsets; +use Koha::CirculationRules; +use Koha::Checkouts; use Koha::Items; +use Koha::Patrons; use vars qw(@ISA @EXPORT); @@ -67,12 +70,17 @@ FIXME : if no replacement price, borrower just doesn't get charged? sub chargelostitem { my $dbh = C4::Context->dbh(); - my ( $borrowernumber, $itemnumber, $replacementprice, $description ) = @_; + my ( $borrowernumber, $itemnumber, $replacementprice, $description, $opts ) = @_; + $opts ||= {}; + my $patron = Koha::Patrons->find($borrowernumber); my $item = Koha::Items->find($itemnumber); my $itype = $item->itemtype; $replacementprice //= 0; - my $defaultreplacecost = $itype->defaultreplacecost; + + my $defaultreplacecost = $itype->defaultreplacecost; + my $usedefaultreplacementcost = C4::Context->preference("useDefaultReplacementCost"); + my $processingfeenote = C4::Context->preference("ProcessingFeeNote"); my $lost_control_pref = C4::Context->preference('LostChargesControl'); my $lost_control_branch; @@ -87,41 +95,45 @@ sub chargelostitem { : $item->holdingbranch; } - my $processfee = Koha::CirculationRules->get_effective_rule_value( - { - rule_name => "lost_item_processing_fee", - categorycode => undef, - itemtype => $itype->itemtype, - branchcode => $lost_control_branch + my $interface = $opts->{interface} // C4::Context->interface; + my $library_id = defined $opts->{library_id} ? $opts->{library_id} : $lost_control_branch; + + my $issue_id = $opts->{issue_id}; + if ( !defined $issue_id ) { + my $checkout = Koha::Checkouts->find( { itemnumber => $itemnumber } ); + if ( !$checkout && $item->in_bundle ) { + my $host = $item->bundle_host; + $checkout = $host->checkout; } - ) // 0; - my $usedefaultreplacementcost = C4::Context->preference("useDefaultReplacementCost"); - my $processingfeenote = C4::Context->preference("ProcessingFeeNote"); + $issue_id = $checkout ? $checkout->issue_id : undef; + } if ( $usedefaultreplacementcost && $replacementprice == 0 && $defaultreplacecost ) { $replacementprice = $defaultreplacecost; } - my $checkout = Koha::Checkouts->find( { itemnumber => $itemnumber } ); - if ( !$checkout && $item->in_bundle ) { - my $host = $item->bundle_host; - $checkout = $host->checkout; - } - my $issue_id = $checkout ? $checkout->issue_id : undef; my $account = Koha::Account->new( { patron_id => $borrowernumber } ); - # first make sure the borrower hasn't already been charged for this item (for this issuance) my $existing_charges = $account->lines->search( { - itemnumber => $itemnumber, + item_id => $itemnumber, debit_type_code => 'LOST', - issue_id => $issue_id + issue_id => $issue_id, } )->count(); # OK, they haven't unless ($existing_charges) { + my $processfee = Koha::CirculationRules->get_effective_rule_value( + { + rule_name => "lost_item_processing_fee", + categorycode => undef, + itemtype => $itype->itemtype, + branchcode => $library_id, + } + ) // 0; + #add processing fee if ( $processfee && $processfee > 0 ) { my $accountline = $account->add_debit( @@ -131,10 +143,10 @@ sub chargelostitem { note => $processingfeenote, user_id => C4::Context->userenv ? C4::Context->userenv->{'number'} : undef, interface => C4::Context->interface, - library_id => C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef, + library_id => $library_id, type => 'PROCESSING', item_id => $itemnumber, - issue_id => $issue_id, + ( defined $issue_id ? ( issue_id => $issue_id ) : () ), } ); } @@ -148,10 +160,10 @@ sub chargelostitem { note => undef, user_id => C4::Context->userenv ? C4::Context->userenv->{'number'} : undef, interface => C4::Context->interface, - library_id => C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef, + library_id => $library_id, type => 'LOST', item_id => $itemnumber, - issue_id => $issue_id, + ( defined $issue_id ? ( issue_id => $issue_id ) : () ), } ); } diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 0a8df34263..44755a32cd 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -46,6 +46,7 @@ use Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue; use Koha::Biblioitems; use Koha::DateUtils qw( dt_from_string ); use Koha::Calendar; +use Koha::Checkout; use Koha::Checkouts; use Koha::ILL::Requests; use Koha::Items; @@ -4446,22 +4447,49 @@ sub LostItem { or warn "_FixOverduesOnReturn($borrowernumber, $itemnumber...) failed!"; # zero is OK, check defined if ( C4::Context->preference('WhenLostChargeReplacementFee') ) { + my $desc = sprintf( + "%s %s %s", + $issues->{'title'} || q{}, + $issues->{'barcode'} || q{}, + $issues->{'itemcallnumber'} || q{}, + ); + + my $interface = + ( $mark_lost_from && $mark_lost_from eq 'cronjob' ) + ? 'cron' + : C4::Context->interface; + + my $item_obj = Koha::Items->find($itemnumber); + my $issue_obj = Koha::Checkouts->search( + { + itemnumber => $itemnumber, + borrowernumber => $borrowernumber, + } + )->next; + my $rule_branch = Koha::Checkout->branch_for_fee_context( + fee_type => 'LOST', + patron => $patron, + item => $item_obj, + issue => $issue_obj, + ); + my $issue_id = $issues->{issue_id} // ( $issue_obj ? $issue_obj->issue_id : undef ); + C4::Accounts::chargelostitem( $borrowernumber, $itemnumber, - $issues->{'replacementprice'}, - sprintf( - "%s %s %s", - $issues->{'title'} || q{}, - $issues->{'barcode'} || q{}, - $issues->{'itemcallnumber'} || q{}, - ), + ( ( $issues->{'replacementprice'} // 0 ) + 0 ), + $desc, + { + interface => $interface, + library_id => $rule_branch, + issue_id => $issue_id, + } ); - - #FIXME : Should probably have a way to distinguish this from an item that really was returned. - #warn " $issues->{'borrowernumber'} / $itemnumber "; } + #FIXME : Should probably have a way to distinguish this from an item that really was returned. + #warn " $issues->{'borrowernumber'} / $itemnumber "; + MarkIssueReturned( $borrowernumber, $itemnumber, undef, $patron->privacy, $params ) if $mark_returned; } diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 99df5aeb76..420ec8f509 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -30,7 +30,10 @@ use C4::Accounts; use C4::Context; use Koha::Account::Lines; use Koha::Account::Offsets; +use Koha::Checkout; +use Koha::Checkouts; use Koha::DateUtils qw( output_pref ); +use Koha::Items; use Koha::Libraries; use Koha::Recalls; use Koha::Logger; @@ -650,7 +653,17 @@ sub UpdateFine { }; my $desc = $letter ? $letter->{content} : sprintf( "Item %s - due %s", $itemnum, output_pref($due) ); - my $account = Koha::Account->new( { patron_id => $borrowernumber } ); + my $account = Koha::Account->new( { patron_id => $borrowernumber } ); + my $item_obj = Koha::Items->find($itemnum); + my $issue_obj = Koha::Checkouts->find($issue_id); + + my $rule_branch = Koha::Checkout->branch_for_fee_context( + fee_type => 'OVERDUE', + patron => $patron, + item => $item_obj, + issue => $issue_obj, + ); + $accountline = $account->add_debit( { amount => $amount, @@ -658,7 +671,7 @@ sub UpdateFine { note => undef, user_id => undef, interface => C4::Context->interface, - library_id => undef, #FIXME: Should we grab the checkout or circ-control branch here perhaps? + library_id => $rule_branch, type => 'OVERDUE', item_id => $itemnum, issue_id => $issue_id, diff --git a/Koha/Checkout.pm b/Koha/Checkout.pm index 242a71b9e1..967b199a55 100644 --- a/Koha/Checkout.pm +++ b/Koha/Checkout.pm @@ -24,6 +24,7 @@ use DateTime; use Try::Tiny qw( catch try ); use C4::Circulation qw( AddRenewal CanBookBeRenewed LostItem MarkIssueReturned ); +use Koha::Checkout; use Koha::Booking; use Koha::Checkouts::Renewals; use Koha::Checkouts::ReturnClaims; @@ -341,6 +342,52 @@ sub claim_returned { }; } +=head2 branch_for_fee_context + +my $branchcode = Koha::Checkout->branch_for_fee_context( + fee_type => 'OVERDUE' | 'LOST' | 'PROCESSING', + ); + +Determines which branch’s rules should apply to a fee based on system preferences and available context. For LOST/PROCESSING it honors LostChargesControl; for OVERDUE it honors CircControl. It also respects HomeOrHoldingBranch when choosing the item’s branch. Returns a branchcode (string) or undef if it cannot be determined. + +=cut + +sub branch_for_fee_context { + my ( $class, %args ) = @_; + + my $fee_type = $args{fee_type} // 'OVERDUE'; + + # Choose the controlling preference depending on fee type + my $control_pref = + ( $fee_type eq 'LOST' || $fee_type eq 'PROCESSING' ) + ? C4::Context->preference('LostChargesControl') + : C4::Context->preference('CircControl'); + + # Item branch selection (home vs holding) + my $hoh_pref = C4::Context->preference('HomeOrHoldingBranch') // 'home'; + my $use_holding = ( $hoh_pref =~ /holding/i ) ? 1 : 0; + + # Support Koha objects OR plain hashes + my $patron_branch = eval { $args{patron}->branchcode } // ( $args{patron} && $args{patron}->{branchcode} ); + my $issuing_branch = eval { $args{issue}->branchcode } // ( $args{issue} && $args{issue}->{branchcode} ); + my $item_home = eval { $args{item}->homebranch } // ( $args{item} && $args{item}->{homebranch} ); + my $item_holding = eval { $args{item}->holdingbranch } // ( $args{item} && $args{item}->{holdingbranch} ); + my $item_branch = $use_holding ? ( $item_holding // $item_home ) : $item_home; + + # Normalize empty strings to undef so // works as intended + for my $ref ( \$patron_branch, \$issuing_branch, \$item_home, \$item_holding, \$item_branch ) { + $$ref = undef if defined $$ref && $$ref eq ''; + } + + # Resolution order depends on controlling pref + my $resolved = + ( defined $control_pref && $control_pref eq 'PatronLibrary' ) ? $patron_branch + : ( defined $control_pref && $control_pref eq 'PickupLibrary' ) + ? ( $issuing_branch // $patron_branch // $item_branch ) + : $item_branch; # ItemLibrary (default) + return $resolved; +} + =head2 Internal methods =head3 _type diff --git a/misc/cronjobs/longoverdue.pl b/misc/cronjobs/longoverdue.pl index b65467f75b..dd0d36a92e 100755 --- a/misc/cronjobs/longoverdue.pl +++ b/misc/cronjobs/longoverdue.pl @@ -34,6 +34,9 @@ use Pod::Usage qw( pod2usage ); use C4::Circulation qw( LostItem MarkIssueReturned ); use C4::Context; use C4::Log qw( cronlogaction ); +use Koha::Checkout; +use Koha::Checkouts; +use Koha::Items; use Koha::ItemTypes; use Koha::Patron::Categories; use Koha::Patrons; @@ -522,7 +525,23 @@ foreach my $startrange ( sort keys %$lost ) { if ($confirm) { Koha::Items->find( $row->{itemnumber} )->itemlost($lostvalue)->store; if ( $charge && $charge eq $lostvalue ) { - LostItem( $row->{'itemnumber'}, 'cronjob', $mark_returned ); + my $patron = Koha::Patrons->find( $row->{borrowernumber} ); + my $item = Koha::Items->find( $row->{itemnumber} ); + my $issue = Koha::Checkouts->search( + { + itemnumber => $row->{itemnumber}, + borrowernumber => $row->{borrowernumber}, + } + )->next; + + my $rule_branch = Koha::Checkout->branch_for_fee_context( + fee_type => 'LOST', + patron => $patron, + item => $item, + issue => $issue, + ); + + LostItem( $row->{itemnumber}, 'cronjob', $mark_returned, { library_id => $rule_branch } ); } elsif ($mark_returned) { $patron ||= Koha::Patrons->find( $row->{borrowernumber} ); MarkIssueReturned( $row->{borrowernumber}, $row->{itemnumber}, undef, $patron->privacy ); -- 2.39.5