| Summary: | Hold fee not charged for title-level holds when all items have negative notforloan status | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Martin Renvoize (ashimema) <martin.renvoize> |
| Component: | Circulation | Assignee: | Martin Renvoize (ashimema) <martin.renvoize> |
| Status: | Needs Signoff --- | QA Contact: | Testopia <testopia> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | gmcharlt, kyle |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| 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
Bug 41977: Hold fee not charged for title-level holds when all items are on order |
||
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> 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> |
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.