Bug 41977 - Hold fee not charged for title-level holds when all items have negative notforloan status
Summary: Hold fee not charged for title-level holds when all items have negative notfo...
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Martin Renvoize (ashimema)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2026-03-03 13:54 UTC by Martin Renvoize (ashimema)
Modified: 2026-03-03 14:04 UTC (History)
2 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: Sponsored
Comma delimited list of Sponsors: OpenFifth <https://openfifth.co.uk>
Crowdfunding goal: 0
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 41977: Hold fee not charged for title-level holds when all items are on order (4.95 KB, patch)
2026-03-03 14:03 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 41977: Hold fee not charged for title-level holds when all items are on order (4.95 KB, patch)
2026-03-03 14:04 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2026-03-03 13:54:33 UTC
When a hold is placed on a title where all items have a negative notforloan value (e.g. -1 = "Ordered" — the Koha convention for "not for loan, but holdable"), no reservation fee is charged, even when the applicable circulation rule specifies a hold_fee.

The same hold placed on a title where items have notforloan = 0 (available) correctly generates the charge.

Steps to Reproduce

1. Configure a circulation rule with a hold_fee for a patron category and item type (e.g. "Adult Fiction Paperback").
2. Set HoldFeeMode to any_time_is_placed.
3. Create a biblio with one or more items, all with a negative notforloan value (e.g. -1, "Ordered").
4. Place a hold on that biblio for a patron in the matching category.
5. Observe the patron's account — no reservation fee is applied.

Expected: Reservation fee is charged at hold placement, matching the fee that would apply once the item is received.

Actual: No fee is charged. The patron can hold the title free of charge for its entire on-order period, then be charged when it arrives — inconsistent and unfair to patrons who paid for the same hold earlier.

Root Cause

In Koha/Hold.pm, _calculate_title_hold_fee() searches for items to base the fee on using this filter:

  -or => [
      { 'me.notforloan' => 0 },
      { 'me.notforloan' => undef }
  ]

This excludes items with negative notforloan values. When all items on a biblio are in an "Ordered" state (negative notforloan), no items match, @fees is empty, and the method returns 0 — causing charge_hold_fee() to skip the charge entirely.

However, Koha's own holdability convention (documented in IsAvailableForItemLevelRequest() and CheckReserves()) explicitly states:

  "item with negative or zero notforloan value is holdable" (notforloan > 0 blocks holds; notforloan <= 0 allows them)

The fee calculation code does not follow this same convention, causing the mismatch.

Fix

Change the notforloan filter in _calculate_title_hold_fee() (Koha/Hold.pm) from matching only = 0 to matching <= 0:

  # Before
  -or => [
      { 'me.notforloan' => 0 },
      { 'me.notforloan' => undef }
  ]

  # After
  -or => [
      { 'me.notforloan' => { '<=', 0 } },
      { 'me.notforloan' => undef }
  ]

This aligns fee calculation with the holdability logic used consistently elsewhere in the codebase.

Test Plan

1. Apply patch.
2. Configure a hold_fee circulation rule for a patron category and item type.
3. Create a biblio with items having notforloan = -1 (Ordered).
4. Place a title-level hold for a matching patron.
5. Confirm a reservation fee is now added to the patron's account.
6. Confirm that items with notforloan = 1 (positive, truly not holdable) still produce no fee.
7. Run prove t/db_dependent/Koha/Hold.t.
Comment 1 Martin Renvoize (ashimema) 2026-03-03 14:03:32 UTC
Created attachment 194383 [details] [review]
Bug 41977: Hold fee not charged for title-level holds when all items are on order

Koha's convention is that notforloan <= 0 means "holdable" and
notforloan > 0 means "not holdable". This is documented in both
IsAvailableForItemLevelRequest() and CheckReserves():

  "item with negative or zero notforloan value is holdable"

_calculate_title_hold_fee() was only searching for items with
notforloan = 0 or IS NULL, silently excluding items with negative
notforloan values (e.g. -1 = "Ordered"). When all items on a title
had a negative notforloan status, no items were found, @fees was
empty, and the method returned 0 — causing no fee to be charged.

Fix the filter to use <= 0 so it matches the same holdability rule
used consistently elsewhere in the codebase.

Sponsored-by: OpenFifth <https://openfifth.co.uk>
Comment 2 Martin Renvoize (ashimema) 2026-03-03 14:04:12 UTC
Created attachment 194384 [details] [review]
Bug 41977: Hold fee not charged for title-level holds when all items are on order

Koha's convention is that notforloan <= 0 means "holdable" and
notforloan > 0 means "not holdable". This is documented in both
IsAvailableForItemLevelRequest() and CheckReserves():

  "item with negative or zero notforloan value is holdable"

_calculate_title_hold_fee() was only searching for items with
notforloan = 0 or IS NULL, silently excluding items with negative
notforloan values (e.g. -1 = "Ordered"). When all items on a title
had a negative notforloan status, no items were found, @fees was
empty, and the method returned 0 — causing no fee to be charged.

Fix the filter to use <= 0 so it matches the same holdability rule
used consistently elsewhere in the codebase.

Sponsored-by: OpenFifth <https://openfifth.co.uk>