Koha-clms/C4
Martin Renvoize f1b20b645c
Bug 3492: Modernize hold fee system and migrate to circulation rules
This commit completely modernizes the hold fee system by migrating from
the deprecated categories.reservefee column to the circulation rules framework
and implementing modern Koha::Objects patterns.

WHAT CHANGED:

1. MIGRATION TO CIRCULATION RULES:
   - Moved hold fees from categories.reservefee to circulation_rules table
   - Added database migration script with atomic update
   - Maintains existing fee amounts during migration
   - Provides rule hierarchy: category+itemtype+branch specific rules

2. NEW SYSTEM PREFERENCE: TitleHoldFeeStrategy
   - Options: highest, lowest, most_common
   - Addresses inconsistent title-level hold fees when items have different fees
   - Default: 'highest' (safest approach for library revenue)

3. MODERN Koha::Objects API:
   - Koha::Item->holds_fee($patron): Calculate fee for specific item
   - Koha::Hold->calculate_hold_fee(): Enhanced item/title-level logic
   - Koha::Hold->should_charge($stage): Centralized charging logic
   - Koha::Hold->charge_hold_fee(): Modern fee charging with proper accounting
   - Removed C4::Reserves::GetReserveFee and ChargeReserveFee

4. ENHANCED FUNCTIONALITY:
   - Proper ReservesControlBranch logic (patron home vs item home library)
   - Complete HoldFeeMode implementation (placement/collection/not_always)
   - Clear OPAC messaging with "You will be charged..." commitment
   - Comprehensive test coverage for all fee scenarios

5. TECHNICAL IMPROVEMENTS:
   - Object-oriented design following Koha patterns
   - Proper error handling and edge case management
   - Modern test structure with comprehensive coverage
   - Clean separation between fee calculation and charging

TEST PLAN:

A) MIGRATION AND SETUP:
   1. Apply all patches and run database update
   2. Verify existing hold fees migrated to circulation rules:
      - Administration > Circulation and fine rules
      - Previous categories.reservefee values now in "Hold fee" column
   3. Confirm old categories.reservefee column no longer used

B) BASIC FEE CALCULATION:
   1. Set up patron categories with different hold fees in circulation rules
   2. Create items with different item types, each with different hold fees
   3. Test item-level holds charge correct fees based on specific item
   4. Test title-level holds use TitleHoldFeeStrategy preference

C) TITLE-LEVEL HOLD STRATEGIES:
   1. Set TitleHoldFeeStrategy to "highest":
      - Create title with items having fees $1.00, $2.00, $3.00
      - Verify title-level holds charge $3.00
   2. Set TitleHoldFeeStrategy to "lowest":
      - Verify same title charges $1.00
   3. Set TitleHoldFeeStrategy to "most_common":
      - Add multiple items with $2.00 fee
      - Verify title-level holds charge $2.00 (most common)

D) HOLDFEEMODE PREFERENCES:
   1. Set HoldFeeMode to "any_time_is_placed":
      - Place holds and verify immediate fee charging
      - Check patron account for correct hold fee debits
   2. Set HoldFeeMode to "any_time_is_collected":
      - Place holds, verify no immediate charges
      - Check out items to patron, verify fee charged on collection
   3. Set HoldFeeMode to "not_always":
      - With available items: verify no fee charged
      - With all items checked out but no other holds: verify no fee
      - With all items checked out and other holds exist: verify fee charged

E) RULE HIERARCHY TESTING:
   1. Set different fees for different category/itemtype/branch combinations
   2. Verify most specific rule applies:
      - Branch + Category + ItemType (most specific)
      - Category + ItemType
      - Category only (most general)
   3. Test with different patron categories and item types

F) USER INTERFACE TESTING:
   1. OPAC interface:
      - Verify "You will be charged..." messages display correct amounts
      - Test both item-level and title-level hold fee messages
      - Confirm fees match actual charges in patron account
   2. Staff interface:
      - Place holds for patrons and verify fee calculations
      - Check Administration > Circulation rules interface works correctly

G) EDGE CASES AND ERROR HANDLING:
   1. Test $0.00 fees (no charge scenarios)
   2. Test patrons with no applicable fee rules (should default to $0.00)
   3. Test items with no holdable copies (should return $0.00)
   4. Test invalid/missing itemtypes or categories
   5. Test holds on biblios with no items

H) REGRESSION TESTING:
   1. Verify existing hold placement workflows unchanged
   2. Check patron account charging remains accurate
   3. Confirm OPAC and staff interfaces display correctly
   4. Test that existing holds continue to work properly

I) UNIT TESTS:
   Run the following test suites to verify functionality:
   - prove t/db_dependent/Koha/Hold.t
   - prove t/db_dependent/Koha/Item.t
   - prove t/db_dependent/Reserves.t
   - prove t/db_dependent/Circulation.t

BACKWARDS COMPATIBILITY:
- All existing functionality preserved
- Hold fee amounts maintained during migration
- Existing workflows continue unchanged
- Legacy code updated to use new API seamlessly

Sponsored-by: OpenFifth <https://openfifth.co.uk>
Signed-off-by: Jackie Usher <jackie.usher@westsussex.gov.uk>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-02-09 12:57:56 -07:00
..
AuthoritiesMarc Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Barcodes Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
ClassSortRoutine Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
ClassSplitRoutine Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Creators Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
External Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Form Bug 28308: Remove 0 from "days in advance" for "Advance notice" 2026-01-30 10:54:02 -07:00
Heading Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
ILSDI Bug 40622: Add missing interface parameter 2025-10-06 09:15:27 -06:00
Installer Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Labels Bug 40473: Use better algorithm for calculating Code39 barcode length 2025-09-26 10:36:41 -06:00
Linker Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Members Bug 28308: Remove 0 from "days in advance" for "Advance notice" 2026-01-30 10:54:02 -07:00
OAI Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Output Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Patroncards Bug 40680: Fix C4/Patroncards/Patroncard.pm 2025-12-01 08:33:57 -07:00
Reports Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
SIP Bug 41369: SIP payments have no branchcode 2025-12-17 13:24:30 -07:00
Search Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Serials Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Utils/DataTables Bug 41042: default_save_state is enabled by default 2025-10-23 11:33:32 -06:00
Accounts.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Acquisition.pm Bug 39722: (follow-up) Remove find 2026-02-06 15:17:09 -07:00
Auth.pm Bug 41076: Adjust our perl tidiness for v20250105 2026-02-05 09:03:51 -07:00
Auth_cas_servers.yaml.sample
…
Auth_with_cas.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Auth_with_ldap.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Auth_with_shibboleth.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
AuthoritiesMarc.pm Bug 35423: Fix substr outside of string warning 2026-01-23 07:29:01 -07:00
Barcodes.pm Bug 18584: Remove trailing spaces in Barcodes.pm 2025-11-18 06:56:42 -07:00
Biblio.pm Bug 41076: Adjust our perl tidiness for v20250105 2026-02-05 09:03:51 -07:00
Breeding.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Budgets.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Calendar.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Charset.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Circulation.pm Bug 7376: (follow-up) Add tests on no_return 2026-02-06 15:17:23 -07:00
ClassSortRoutine.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
ClassSource.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
ClassSplitRoutine.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Context.pm Bug 41102: Error 500 on the About page when biblioserver Zebra configuration is missing 2026-01-12 10:25:55 -07:00
Contract.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
CourseReserves.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Creators.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
HTML5Media.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Heading.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
HoldsQueue.pm Bug 41781: Holds queue builder ( build_holds_queue.pl ) fails if HoldsQueueParallelLoopsCount is greater than 1 2026-02-06 07:21:03 -07:00
ImportBatch.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
ImportExportFramework.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
InstallAuth.pm Bug 41076: Adjust our perl tidiness for v20250105 2026-02-05 09:03:51 -07:00
Installer.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
ItemCirculationAlertPreference.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Items.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Koha.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Labels.pm Bug 21395: Make perlcritic happy 2020-06-29 12:37:02 +02:00
Languages.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Letters.pm Bug 41684: Make `notices_content` hook be used with `get_enabled_plugins()` 2026-02-06 15:17:16 -07:00
Linker.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Log.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
MarcModificationTemplates.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Matcher.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Members.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Message.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Output.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Overdues.pm Bug 41315: (follow-up) Rename statements for clarity 2026-01-28 14:24:04 -07:00
Patroncards.pm Bug 38664: Tidy the whole codebase 2025-02-11 14:58:24 +01:00
Record.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Reports.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Reserves.pm Bug 3492: Modernize hold fee system and migrate to circulation rules 2026-02-09 12:57:56 -07:00
Ris.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
RotatingCollections.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
SMS.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Scheduler.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Scrubber.pm Bug 39860: Update scrubber profile and add Scrubber.t test 2025-11-03 14:13:47 -07:00
Search.pm Bug 41287: (QA follow-up) Fix HTML syntax and improve logic consistency 2025-12-29 09:56:09 -07:00
Serials.pm Bug 38061: (QA follow-up) Add default falsy value for subscription creation/editing 2026-01-22 10:02:19 -07:00
Service.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
ShelfBrowser.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
SocialData.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Stats.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Suggestions.pm Bug 39722: Remove GetSuggestionFromBiblionumber from C4/Suggestions.pm 2026-02-06 15:17:09 -07:00
TTParser.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
Tags.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
Templates.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
TmplToken.pm Bug 40163: Fix several http addresses in the code docs 2025-09-23 16:21:29 -06:00
TmplTokenType.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
UsageStats.pm Bug 30331: Rename RenewalPeriodBase as ManualRenewalPeriodBase and add AutomaticRenewalPeriodBase sys pref 2026-01-30 10:54:20 -07:00
XISBN.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00
XSLT.pm Bug 40680: Move EXPORT_OK right after Modern::Perl 2025-12-01 08:33:56 -07:00