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.
We hope also, that branchcode information for all customer fees lines.
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.
Created attachment 186567 [details] [review] Bug 35612: Added new AccountLinesBranchSource system preference
Created attachment 186568 [details] [review] Bug 35612: Added branch code logic
Created attachment 186569 [details] [review] Bug 35612: sysprefs.sql updates
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.
Created attachment 186571 [details] [review] Bug 35612: Tests run perl t/db_dependent/Koha/Account.t
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.
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.
Created attachment 187919 [details] [review] Bug 35612: Recorded branch context in accountlines.branchcode for OVERDUE, LOST, and PROCESSING fees
Created attachment 187920 [details] [review] Bug 35612: Unit test Circulation.t
Created attachment 187921 [details] [review] Bug 35612: Unit tests for Overdues.t
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.
I forgot to add in my test plan to run prove t/db_dependent/Overdues.t prove t/db_dependent/Circulation.t
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.
Created attachment 189137 [details] [review] Bug 35612: Recorded branch context in accountlines.branchcode for OVERDUE, LOST, and PROCESSING fees.