Bug 3492 - Move hold fee setting into circulation rules
Summary: Move hold fee setting into circulation rules
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: PC All
: P3 enhancement
Assignee: Martin Renvoize (ashimema)
QA Contact: Tomás Cohen Arazi (tcohen)
URL:
Keywords:
Depends on:
Blocks: 40808 40817
  Show dependency treegraph
 
Reported: 2009-08-03 12:59 UTC by Jane Wagner
Modified: 2025-10-06 15:50 UTC (History)
13 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Hold fees can now be configured through the circulation rules matrix, providing much more granular control over hold charges based on patron category, item type, and library combinations. Previously, hold fees (reservefee) could only be set at the patron category level in the patron category administration. This enhancement integrates hold fees into the main circulation rules system, where they follow the standard rule hierarchy with automatic fallback handling. **Key changes:** - Hold fees can now be set for specific combinations of patron category, item type, and library - The deprecated `reservefee` field has been removed from patron categories - Hold fee configuration appears as a new column in the circulation rules matrix - Hold fees follow the same rule precedence as other circulation rules (most specific rule wins, with fallback to broader rules) **For administrators:** After upgrading, you may want to review your circulation rules and update hold fees where appropriate. Any existing hold fees previously set at the patron category level will have been migrated to the circulation rules.
Version(s) released in:
Circulation function:


Attachments
Hold fee setup screenshot (49.40 KB, image/jpeg)
2009-08-03 12:59 UTC, Chris Cormack
Details
Hold fee charge screenshot (12.17 KB, image/jpeg)
2009-08-03 12:59 UTC, Chris Cormack
Details
Bug 3492: Begginings of moving holds_fee rule to circulation rules (5.01 KB, patch)
2021-04-20 13:59 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 3492: Database migration for hold fees to circulation rules (5.40 KB, patch)
2025-09-10 21:24 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 3492: DBIC Update (1.57 KB, patch)
2025-09-10 21:24 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 3492: Add hold fees to circulation rules system (8.32 KB, patch)
2025-09-10 21:24 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 3492: Remove from patron categories admin (6.22 KB, patch)
2025-09-10 21:24 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 3492: Modernize hold fee system and migrate to circulation rules (64.68 KB, patch)
2025-09-10 21:24 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 3492: (follow-up) Remove last reservefee references (5.86 KB, patch)
2025-09-11 06:54 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 3492: Database migration for hold fees to circulation rules (5.51 KB, patch)
2025-09-22 13:44 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 3492: DBIC Update (1.68 KB, patch)
2025-09-22 13:44 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 3492: Add hold fees to circulation rules system (8.43 KB, patch)
2025-09-22 13:44 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 3492: Remove from patron categories admin (6.33 KB, patch)
2025-09-22 13:44 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 3492: Modernize hold fee system and migrate to circulation rules (64.68 KB, patch)
2025-09-22 13:44 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 3492: (follow-up) Remove last reservefee references (5.97 KB, patch)
2025-09-22 13:44 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 3492: Database migration for hold fees to circulation rules (5.64 KB, patch)
2025-09-22 14:12 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 3492: DBIC Update (1.82 KB, patch)
2025-09-22 14:12 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 3492: Add hold fees to circulation rules system (8.57 KB, patch)
2025-09-22 14:12 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 3492: Remove from patron categories admin (6.46 KB, patch)
2025-09-22 14:12 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 3492: Modernize hold fee system and migrate to circulation rules (63.96 KB, patch)
2025-09-22 14:12 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 3492: (follow-up) Remove last reservefee references (6.10 KB, patch)
2025-09-22 14:12 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2010-05-21 01:11:21 UTC


---- Reported by jwagner@ptfs.com 2009-08-03 12:59:15 ----

Created an attachment
Hold fee setup screenshot

Sponsored by East Brunswick Public Library, East Brunswick, NJ, USA.

Add an option to set a hold fee by item type, and to assess that fee when the hold is placed.  A new field in the item type table allows entry of a hold fee (separate from the existing rental fee).  A syspref will allow that hold fee to be assessed when the hold is placed.



---- Additional Comments From jwagner@ptfs.com 2009-08-03 12:59:41 ----

Created an attachment
Hold fee charge screenshot





---- Additional Comments From jwagner@ptfs.com 2010-05-12 12:43:57 ----

The fix for this bug (in ptfs-harley) can be pulled from the public git repository at http://github.com/Koha-PTFS, branch http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=3492




--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:11 UTC  ---

This bug was previously known as _bug_ 3492 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=3492
Imported an attachment (id=1356)
Imported an attachment (id=1357)

Unknown operating system Windows 2000. Setting to default OS "All".
Actual time not defined. Setting to 0.0
The original reporter of this bug does not have
   an account here. Reassigning to the person who moved
   it here: chris@bigballofwax.co.nz.
   Previous reporter was jwagner@ptfs.com.
The original assignee of this bug does not have
   an account here. Reassigning to the default assignee
   for the component, gmcharlt@gmail.com.
   Previous assignee was kohaprogrammers@ptfs.com.
The original submitter of attachment 1356 [details] is unknown.
   Reassigning to the person who moved it here: chris@bigballofwax.co.nz.
The original submitter of attachment 1357 [details] is unknown.
   Reassigning to the person who moved it here: chris@bigballofwax.co.nz.

Comment 1 Paul Poulain 2011-03-22 12:43:47 UTC
patch available at: https://github.com/ptfs/Koha-PTFS
May cause problem/incompatibility with #5872.
Comment 2 Nicole C. Engard 2011-03-22 21:09:22 UTC
A patch needs to be submitted based on master for this - and either a link
right to the patch or an attached patch is necessary.

Nicole
Comment 3 Jared Camins-Esakov 2012-02-13 13:17:08 UTC
The Harley integration project has been stalled for some time.
Comment 4 Martin Renvoize (ashimema) 2020-04-17 13:16:01 UTC
This is still a feature request we see fairly often.. re-opening.
Comment 5 Fiona Borthwick 2020-04-17 13:32:38 UTC
This subject often comes up during system implementations - especially with our public libraries customer base.

The hold fee is currently assigned to the patron category but many libraries have varying hold fees dependent upon item type. It is often thought that it would be more useful to have the hold fee linked to the item type instead of the patron category.

For example:
Hold on Adult book by Adult user = £1.00
Hold on Junior book by Adult user = £0.50

If the hold fee in the Adult patron category is set to £1.00, then they will be charged £1.00 for every hold they place - regardless if is an adult or a junior book.

An alternative approach may be to adopt a solution similar to the 'rental fee discount' in the circulation rules. The rental fee is associated with the item type but the system administrators are able to manipulate the circ rules to allow for specific users to borrow items with rental fees at a reduced rate.
Comment 6 Katrin Fischer 2020-04-18 03:54:18 UTC
Maybe we should just move it to the circulation rules?

I think libraries will not be happy with either itemtype or patron category - until we have all the dimensions, things will always not work well for someone.
Comment 7 Fiona Borthwick 2020-04-20 09:57:15 UTC
I think moving this into the circ rules will work well. The downside is that there are already many attributes within the circ rules and there is the potential for libraries to maintain many more policies if they have a lot of combinations. In my experience though there shouldn't be too many variations.
Comment 8 Sally 2020-04-20 10:07:12 UTC
+1

Putting hold fees into the circulation rules would add a lot of flexibility.

It would also be useful if ExpireReservesMaxPickUpDelayCharge (the charge for holds not collected) could pick up the hold fee in the circulation rule, rather than being a blanket charge across all patron categories and itemtypes.

There's a separate bug for that here:  https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18195 - but I mention it in case it's something that can be explored at the same time.
Comment 9 Martin Renvoize (ashimema) 2021-04-20 13:21:10 UTC
Looking into this... Moving to the circulation rules adds an additional question..

Whilst I can understand the want to add rules at itemtype and patron category levels.. would there also be a need to do this at a branch level too... and if that is the case.. which branch would govern the looking.. the pickup location, the items home library, the items current holding library?.. it opens a bit of a can of worms ;)
Comment 10 Martin Renvoize (ashimema) 2021-04-20 13:33:41 UTC
Aha, GetReservesControlBranch deals with this for us already.. so it's a non-issue.
Comment 11 Martin Renvoize (ashimema) 2021-04-20 13:52:45 UTC
(In reply to Martin Renvoize from comment #10)
> Aha, GetReservesControlBranch deals with this for us already.. so it's a
> non-issue.

Humm.. actually, it doesn't.

`GetReservesControlBranch` requires an item (for the case where `ReservesControlBranch` is set to 'ItemHomeLibrary'). However, at the time of reserve we don't always have that information yet (Bib level holds).. so for the case where we charge at the point of placing a hold and we allow biblio level holds.. we would explode.

Rethink needed.
Comment 12 Martin Renvoize (ashimema) 2021-04-20 13:59:26 UTC
Created attachment 119909 [details] [review]
Bug 3492: Begginings of moving holds_fee rule to circulation rules
Comment 13 Martin Renvoize (ashimema) 2021-04-20 13:59:48 UTC
This is just a starting point so far.. not complete yet
Comment 14 Katrin Fischer 2023-01-07 22:48:13 UTC
I have updated the bug description per the discussion below.

HoldFeeMode already allows to charge when placing a hold.
Comment 15 Hannah Dunne-Howrie 2025-01-03 15:27:35 UTC
We would also like to see the ability to set a hold fee based on item type instead of patron category. For example, we don't charge for CD holds but we have no way to disable this so we have to mainly waive the charge for patrons when they ask us to do this.
Comment 16 Martin Renvoize (ashimema) 2025-09-10 08:32:12 UTC
I'm working on this again now.. to get around the title level holds challenge, I'm going to add an additional preference to define what route to take.. highest|lowest| most_common fee.
Comment 17 Martin Renvoize (ashimema) 2025-09-10 21:24:06 UTC
Created attachment 186336 [details] [review]
Bug 3492: Database migration for hold fees to circulation rules

This commit adds the database migration to move hold fee configuration
from patron categories to circulation rules, providing more granular
control over hold fees based on patron category, item type, and branch.

The migration:
- Creates circulation rules for existing categories.reservefee values
- Removes the deprecated reservefee column from categories table
Comment 18 Martin Renvoize (ashimema) 2025-09-10 21:24:09 UTC
Created attachment 186337 [details] [review]
Bug 3492: DBIC Update
Comment 19 Martin Renvoize (ashimema) 2025-09-10 21:24:11 UTC
Created attachment 186338 [details] [review]
Bug 3492: Add hold fees to circulation rules system

This commit integrates hold fee configuration into the main circulation
rules system, providing granular control over hold fees based on patron
category, item type, and branch combinations.

Changes:
- Add hold_fee as a new circulation rule type with full scope support
- Remove deprecated reservefee column mapping from Koha::Patron::Category
- Integrate hold fee input into the circulation rules admin interface
- Add hold fee column to the main circulation rules matrix
- Enable proper rule hierarchy resolution for hold fees

Hold fees can now be configured at multiple levels with automatic
fallback handling following standard circulation rule patterns.
Comment 20 Martin Renvoize (ashimema) 2025-09-10 21:24:13 UTC
Created attachment 186339 [details] [review]
Bug 3492: Remove from patron categories admin
Comment 21 Martin Renvoize (ashimema) 2025-09-10 21:24:16 UTC
Created attachment 186340 [details] [review]
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
Comment 22 Martin Renvoize (ashimema) 2025-09-11 06:54:30 UTC
Created attachment 186361 [details] [review]
Bug 3492: (follow-up) Remove last reservefee references
Comment 23 Martin Renvoize (ashimema) 2025-09-11 06:57:05 UTC
Note to testers: You will need to test this locally or using KTD.. if using KTD you will need to start ktd with the main koha branch checked out prior to applying this patch.  We will need a ktd test data update corresponding to this patchset to remove the reservefee field from population scripts as we move the field here.
Comment 24 Tomás Cohen Arazi (tcohen) 2025-09-17 15:33:48 UTC
Hi, all my tests were good. A couple minor issues remain:

* The db_rev is weird. Should be using the built-in column_exists. Please fix inline.
* I expected to see the HoldFeeMode system preference in the holds area. It is in the 'Fines policy' one, which I believe is incorrect.

Please fix that and we should find testers so I can do the final QA.
Comment 25 Martin Renvoize (ashimema) 2025-09-22 13:44:46 UTC
Created attachment 186691 [details] [review]
Bug 3492: Database migration for hold fees to circulation rules

This commit adds the database migration to move hold fee configuration
from patron categories to circulation rules, providing more granular
control over hold fees based on patron category, item type, and branch.

The migration:
- Creates circulation rules for existing categories.reservefee values
- Removes the deprecated reservefee column from categories table

Sponsored-by: OpenFifth <https://openfifth.co.uk>
Signed-off-by: Jackie Usher <jackie.usher@westsussex.gov.uk>
Comment 26 Martin Renvoize (ashimema) 2025-09-22 13:44:49 UTC
Created attachment 186692 [details] [review]
Bug 3492: DBIC Update

Sponsored-by: OpenFifth <https://openfifth.co.uk>
Signed-off-by: Jackie Usher <jackie.usher@westsussex.gov.uk>
Comment 27 Martin Renvoize (ashimema) 2025-09-22 13:44:51 UTC
Created attachment 186693 [details] [review]
Bug 3492: Add hold fees to circulation rules system

This commit integrates hold fee configuration into the main circulation
rules system, providing granular control over hold fees based on patron
category, item type, and branch combinations.

Changes:
- Add hold_fee as a new circulation rule type with full scope support
- Remove deprecated reservefee column mapping from Koha::Patron::Category
- Integrate hold fee input into the circulation rules admin interface
- Add hold fee column to the main circulation rules matrix
- Enable proper rule hierarchy resolution for hold fees

Hold fees can now be configured at multiple levels with automatic
fallback handling following standard circulation rule patterns.

Sponsored-by: OpenFifth <https://openfifth.co.uk>
Signed-off-by: Jackie Usher <jackie.usher@westsussex.gov.uk>
Comment 28 Martin Renvoize (ashimema) 2025-09-22 13:44:54 UTC
Created attachment 186694 [details] [review]
Bug 3492: Remove from patron categories admin

Sponsored-by: OpenFifth <https://openfifth.co.uk>
Signed-off-by: Jackie Usher <jackie.usher@westsussex.gov.uk>
Comment 29 Martin Renvoize (ashimema) 2025-09-22 13:44:56 UTC
Created attachment 186695 [details] [review]
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>
Comment 30 Martin Renvoize (ashimema) 2025-09-22 13:44:59 UTC
Created attachment 186696 [details] [review]
Bug 3492: (follow-up) Remove last reservefee references

Sponsored-by: OpenFifth <https://openfifth.co.uk>
Signed-off-by: Jackie Usher <jackie.usher@westsussex.gov.uk>
Comment 31 Martin Renvoize (ashimema) 2025-09-22 13:46:39 UTC
(In reply to Tomás Cohen Arazi (tcohen) from comment #24)
> Hi, all my tests were good. A couple minor issues remain:
> 
> * The db_rev is weird. Should be using the built-in column_exists. Please
> fix inline.
> * I expected to see the HoldFeeMode system preference in the holds area. It
> is in the 'Fines policy' one, which I believe is incorrect.
> 
> Please fix that and we should find testers so I can do the final QA.

DB update fixed inline.. not sure what came over me there.
Not sure about the HoldFeeMode.. that one pre-exists this patchset.. I do add TitleHoldsFeeStrategy just beneath HoldFeeMode though.. happy to have it wherever, really.
Comment 32 Tomás Cohen Arazi (tcohen) 2025-09-22 14:12:30 UTC
Created attachment 186711 [details] [review]
Bug 3492: Database migration for hold fees to circulation rules

This commit adds the database migration to move hold fee configuration
from patron categories to circulation rules, providing more granular
control over hold fees based on patron category, item type, and branch.

The migration:
- Creates circulation rules for existing categories.reservefee values
- Removes the deprecated reservefee column from categories table

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>
Comment 33 Tomás Cohen Arazi (tcohen) 2025-09-22 14:12:34 UTC
Created attachment 186712 [details] [review]
Bug 3492: DBIC Update

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>
Comment 34 Tomás Cohen Arazi (tcohen) 2025-09-22 14:12:37 UTC
Created attachment 186713 [details] [review]
Bug 3492: Add hold fees to circulation rules system

This commit integrates hold fee configuration into the main circulation
rules system, providing granular control over hold fees based on patron
category, item type, and branch combinations.

Changes:
- Add hold_fee as a new circulation rule type with full scope support
- Remove deprecated reservefee column mapping from Koha::Patron::Category
- Integrate hold fee input into the circulation rules admin interface
- Add hold fee column to the main circulation rules matrix
- Enable proper rule hierarchy resolution for hold fees

Hold fees can now be configured at multiple levels with automatic
fallback handling following standard circulation rule patterns.

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>
Comment 35 Tomás Cohen Arazi (tcohen) 2025-09-22 14:12:41 UTC
Created attachment 186714 [details] [review]
Bug 3492: Remove from patron categories admin

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>
Comment 36 Tomás Cohen Arazi (tcohen) 2025-09-22 14:12:44 UTC
Created attachment 186715 [details] [review]
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>
Comment 37 Tomás Cohen Arazi (tcohen) 2025-09-22 14:12:48 UTC
Created attachment 186716 [details] [review]
Bug 3492: (follow-up) Remove last reservefee references

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>
Comment 38 Lucas Gass (lukeg) 2025-10-03 13:49:29 UTC
In the DB update I see that existing rules are migrated from categories to their new home in circ rules. In the release notes I notice:

After upgrading, you'll need to review your circulation rules and configure hold fees where appropriate. Any existing hold fees previously set at the patron category level will need to be migrated to the circulation rules.

This makes it sould like rules are NOT migrated. Can you clarify?