[Bug 41977] New: Hold fee not charged for title-level holds when all items have negative notforloan status
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 Bug ID: 41977 Summary: Hold fee not charged for title-level holds when all items have negative notforloan status Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle@bywatersolutions.com 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. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk Severity|enhancement |normal Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 194383 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194383&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Comma delimited| |OpenFifth list of Sponsors| |<https://openfifth.co.uk> Sponsorship status|--- |Sponsored Status|ASSIGNED |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #194383|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 --- Comment #2 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 194384 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194384&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 OpenFifth Sandboxes <sandboxes@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #194384|0 |1 is obsolete| | --- Comment #3 from OpenFifth Sandboxes <sandboxes@openfifth.co.uk> --- Created attachment 194445 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194445&action=edit 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> Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 Anneli Österman <anneli.osterman@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |anneli.osterman@koha-suomi. | |fi Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 --- Comment #4 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 194731 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194731&action=edit 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> Signed-off-by: Roger fredricks <roger.fredricks@westsussex.gov.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #194445|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #194731|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 194732 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194732&action=edit 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> Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi> Signed-off-by: Roger fredricks <roger.fredricks@westsussex.gov.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |baptiste.wojtkowski@biblibr |y.org |e.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #194732|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 --- Comment #6 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Created attachment 197007 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197007&action=edit 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> Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi> Signed-off-by: Roger fredricks <roger.fredricks@westsussex.gov.uk> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 --- Comment #7 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 26.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |3492 CC| |jacob.omara@openfifth.co.uk Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=3492 [Bug 3492] Move hold fee setting into circulation rules -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Needs documenting --- Comment #8 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- This will not be backported to 25.11.x due to it being deemed an enhancement or a dependancy not being met -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41977 Bug 41977 depends on bug 3492, which changed state. Bug 3492 Summary: Move hold fee setting into circulation rules https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=3492 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org