Bugzilla – Attachment 189137 Details for
Bug 35612
Record branch context in accountlines.branchcode for OVERDUE, LOST, and PROCESSING fees
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 35612: Recorded branch context in accountlines.branchcode for OVERDUE, LOST, and PROCESSING fees.
Bug-35612-Recorded-branch-context-in-accountlinesb.patch (text/plain), 14.56 KB, created by
Laura Escamilla
on 2025-11-05 19:09:13 UTC
(
hide
)
Description:
Bug 35612: Recorded branch context in accountlines.branchcode for OVERDUE, LOST, and PROCESSING fees.
Filename:
MIME Type:
Creator:
Laura Escamilla
Created:
2025-11-05 19:09:13 UTC
Size:
14.56 KB
patch
obsolete
>From 29e8b2415e78b71b9d6ef4752e53162b4cce70c2 Mon Sep 17 00:00:00 2001 >From: Laura_Escamilla <laura.escamilla@bywatersolutions.com> >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
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 35612
:
186567
|
186568
|
186569
|
186570
|
186571
|
187919
|
187920
|
187921
|
187922
| 189137