Bug 35612

Summary: Record branch context in accountlines.branchcode for OVERDUE, LOST, and PROCESSING fees
Product: Koha Reporter: Pirkko-Liisa Lauhikari <pirkko-liisa.lauhikari>
Component: Fines and feesAssignee: Laura Escamilla <Laura.escamilla>
Status: ASSIGNED --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: andrew, Laura.escamilla
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39802
GIT URL: Initiative type: ---
Sponsorship status: --- Comma delimited list of Sponsors:
Crowdfunding goal: 0 Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 36506, 39802    
Bug Blocks:    
Attachments: Bug 35612: Added new AccountLinesBranchSource system preference
Bug 35612: Added branch code logic
Bug 35612: sysprefs.sql updates
Bug 35612: Added the system preference within the Circulation and Fines policy in Administration
Bug 35612: Tests
Bug 35612: Recorded branch context in accountlines.branchcode for OVERDUE, LOST, and PROCESSING fees
Bug 35612: Unit test Circulation.t
Bug 35612: Unit tests for Overdues.t
Bug 35612: Updates to Circulation.pm
Bug 35612: Recorded branch context in accountlines.branchcode for OVERDUE, LOST, and PROCESSING fees.

Description Pirkko-Liisa Lauhikari 2023-12-20 11:18:34 UTC
We would like to have branchcode recorded to the OVERDUE fees in accountlines table. Configurability is desired, so that it is possible to choose which library is recorded. Options could be customer's home library, issuing library and item's home library.
Comment 1 Pirkko-Liisa Lauhikari 2024-09-27 08:17:23 UTC
We hope also, that branchcode information for all customer fees lines.
Comment 2 Andrew Fuerste-Henry 2025-05-01 13:58:21 UTC
Agreed it would be helpful to consistently populate the branchcode for accountlines.

For overdues fines, I believe the branch used to calculate the fine is determined by the CircControl and HomeOrHoldingBranch system preferences. Accountlines should record the branch used to calculate the fine.

The longoverdue cron also follows CircControl but fails to record that branch in accountlines. It should do so.
Comment 3 Laura Escamilla 2025-09-18 17:20:06 UTC
Created attachment 186567 [details] [review]
Bug 35612: Added new AccountLinesBranchSource system preference
Comment 4 Laura Escamilla 2025-09-18 17:20:09 UTC
Created attachment 186568 [details] [review]
Bug 35612: Added branch code logic
Comment 5 Laura Escamilla 2025-09-18 17:20:12 UTC
Created attachment 186569 [details] [review]
Bug 35612: sysprefs.sql updates
Comment 6 Laura Escamilla 2025-09-18 17:20:14 UTC
Created attachment 186570 [details] [review]
Bug 35612: Added the system preference within the Circulation and Fines policy in Administration

To test:

1. Set system preferences:
    * FinesMode: Calculate and Charge
    * AccountLinesBranchSource: (this will be tested with different values later)
2. Set circulation rules:
    * Ensure a circulation rule exists that charges overdue fines for the item type and branch you're testing with.
3. Check out an item to a test patron with a due date in the past (to trigger overdue).
4.  Run the fines cronjob: perl /usr/share/koha/bin/cronjobs/fines.pl
5. Verify fines appear on the patron’s Accounting tab in the staff interface.
6. Confirm branchcode is NULL in the database:
    * SELECT accountlines_id, branchcode, debit_type_code FROM accountlines ORDER BY accountlines_id DESC LIMIT 10;
    * Expect to see branchcode = NULL for the OVERDUE debit.
7.  Apply the patch to your Koha instance.
8.  Run updatedatabase
9. Run restart_all
10. In the staff interface, go to Administration → System Preferences, search for AccountLinesBranchSource, and set it to: "the library the item was checked out from" (checkoutbranch)
11.  Repeat steps 3–6.
12.  Confirm that in the SQL output, the branchcode now matches the checkout branch of the item.
13. Repeat steps 10–12 with each of the following AccountLinesBranchSource values:
    * itemhomebranch: The item’s home library
    * patronhomebranch: The patron’s home library
    * checkoutbranch: The checkout library (already tested)
    * none: Should result in NULL branchcode
    * Confirm that: The correct branch is recorded in the branchcode field of accountlines. When set to none, branchcode is again NULL.
Comment 7 Laura Escamilla 2025-09-18 17:20:17 UTC
Created attachment 186571 [details] [review]
Bug 35612: Tests

run perl t/db_dependent/Koha/Account.t
Comment 8 Andrew Fuerste-Henry 2025-09-18 18:56:17 UTC
Rather than adding a system preference to determine which branch is used or recorded for a debit, the goal here is to allow the existing system preferences  -- CircControl, LostChargesControl (see bug 39802) -- to determine which branch's rules should be used to calculate the fee and then record that context in accountlines.
Comment 10 Andrew Fuerste-Henry 2025-10-02 20:17:00 UTC
I got so busy thinking about lost items I forgot that Pirkko-Liisa started this bug with OVERDUEs. OVERDUEs also fail to record the branch used to determine the fee and should also be covered in this work.
Comment 11 Laura Escamilla 2025-10-15 16:10:51 UTC
Created attachment 187919 [details] [review]
Bug 35612: Recorded branch context in accountlines.branchcode for OVERDUE, LOST, and PROCESSING fees
Comment 12 Laura Escamilla 2025-10-15 16:10:54 UTC
Created attachment 187920 [details] [review]
Bug 35612: Unit test Circulation.t
Comment 13 Laura Escamilla 2025-10-15 16:10:57 UTC
Created attachment 187921 [details] [review]
Bug 35612: Unit tests for Overdues.t
Comment 14 Laura Escamilla 2025-10-15 16:10:59 UTC
Created attachment 187922 [details] [review]
Bug 35612: Updates to Circulation.pm

To test:

There are dependencies for this patch so you’ll need to apply bugs 36506 and 39802

1. Create three test libraries — I used the following existing branches in K-T-D:
    1. FPL (use as item’s home library)
    2. CPL (use as checkout/holding library)
    3. PVL (use as patron home library)
2. Create a patron with home library PVL — Olga Rivera is an example patron.
3. Create an item with:
    1. homebranch = FPL
    2. holdingbranch = CPL
4. System preferences:
    1. FinesMode: set to Calculate and charge.
    2. Within Circulation rules ensure a rule exists that charges overdue fines for the item type/branch you’re using.
5. Check out the item you created or selected to your patron and set the due date to a past date so that it is overdue. 
6. Run fines: perl /usr/share/koha/bin/cronjobs/fines.pl
7. Confirm an OVERDUE fine appears on the patron’s account. 
8. SQL check (expect branchcode is NULL for the OVERDUE line):
SELECT accountlines_id, branchcode, debit_type_code
FROM accountlines
ORDER BY accountlines_id DESC
LIMIT 10; 
9. Apply the patch, run updatedatabase, then restart_all.
10. Enable the system preference CircControl: select an option of PatronLibrary, PickupLibrary, or ItemHomeLibrary
11. Enable the HomeOrHoldingBranch system preference: homebranch or holdingbranch (only matters when CircControl = ItemHomeLibrary)
12. Repeat steps 3-6 for the following scenarios. 
    1. CircControl = PatronLibrary → expect PVL (patron’s home).
    2. CircControl = PickupLibrary → expect CPL (checkout/issuing branch).
    3. CircControl = ItemHomeLibrary + HomeOrHoldingBranch = homebranch → expect FPL (item’s home).
    4. CircControl = ItemHomeLibrary + HomeOrHoldingBranch = holdingbranch → expect CPL (item’s holding).
13. Next we’re going to test this with ‘Lost Items’
14. Make sure WhenLostChargeReplacementFee syspref is set to Charge
15. Set the LostChargesControl syspref to one of the following PatronLibrary, PickupLibrary, or ItemHomeLibrary
16. Replacement price on the item must be > 0, otherwise no LOST debit is created.
17. First test this manually by setting the item to lost. 
18. Start by confirming that an item is currently checked out to the test patron (checked out at CPL).
19. Set preferences for the scenario.
20. Mark the checked out item as lost. 
21. SQL check (find the most recent LOST row):
SELECT accountlines_id, branchcode, debit_type_code
FROM accountlines
ORDER BY accountlines_id DESC
LIMIT 10;
22. Next test this by using the longoverdue cron
23. Re-checkout the item (as above), adjust longoverdue rules so cron will mark it lost.
24. Run perl /usr/share/koha/bin/cronjobs/longoverdue.pl --lost 30=1 --charge 1 --confirm --verbose
25. SQL check again for the new LOST row.
26. Test for the following scenarios:
    1. LostChargesControl = PatronLibrary → expect PVL (patron’s home).
    2. LostChargesControl = PickupLibrary → expect CPL (checkout/issuing branch).
    3. LostChargesControl = ItemHomeLibrary + HomeOrHoldingBranch = homebranch → expect FPL (item’s home).
    4. LostChargesControl = ItemHomeLibrary + HomeOrHoldingBranch = holdingbranch → expect CPL (item’s holding).
27. Make sure that if the replacement price is 0 → no LOST debit should be created.
28. And ensure that if WhenLostChargeReplacementFee = Don’t charge → no LOST debit should be created.
29. Turn FinesMode OFF (i.e., not “Calculate and charge”), run fines again → no new OVERDUE debits should be added.
Comment 15 Laura Escamilla 2025-10-15 16:13:33 UTC
I forgot to add in my test plan to run 

prove t/db_dependent/Overdues.t 

prove t/db_dependent/Circulation.t
Comment 16 Andrew Fuerste-Henry 2025-10-15 18:34:45 UTC
This all worked great for OVERDUE and LOST fees. But with the patches from this bug applied I'm not getting PROCESSING fees generated at all when items are lost. I do get processing fees with just BZ 36506 and BZ 39802 applied.
Comment 17 Laura Escamilla 2025-11-05 19:09:13 UTC
Created attachment 189137 [details] [review]
Bug 35612: Recorded branch context in accountlines.branchcode for OVERDUE, LOST, and PROCESSING fees.