Bug 4850 - CheckReserves does not respect holds circulation policies
Summary: CheckReserves does not respect holds circulation policies
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P3 major (vote)
Assignee: Ian Walls
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-03 21:25 UTC by Ian Walls
Modified: 2020-05-21 09:26 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Patch for Bug 4850 (2.64 KB, patch)
2010-06-04 15:47 UTC, Ian Walls
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ian Walls 2010-06-03 21:25:42 UTC
Scenario:

A title has two items, one of an item type that allows holds, one of a type that does not.  The rules defining this are entered into smart-rules.pl, and stored in the branch_item_rules, default_branch_item_rules and default_branch_circ_rules tables in the database.  Patron A checks out the item that does not allow holds (Item A).  Patron B then places a title-level (next available) hold on that title.  Patron B must wait until Item B is available for their hold to be filled, as per our policy.

When Patron A looks at their account in the OPAC, they'll see that Item A is on hold, and cannot be renewed.  Similarly, when a staff member views their record in the staff client, they will see the item as unrenewal for the same reason (pending hold).

When Patron A returns Item A, a message will come up to the staff member checking in that there is a hold on this item for Patron B.  This is incorrect, since Item A cannot be held, and Patron B must continue to wait until Item B is available to have their hold filled.


The three tables defined above (branch_item_rules, default_branch_item_rules and default_branch_circ_rules) are not consulted anywhere in the codebase except for GetBranchItemRule in C4/Circulation.pm, which in turn is only consulted in opac-reserve.pl and reserve/request.pl.
Comment 1 Ian Walls 2010-06-04 15:47:01 UTC
Created attachment 2218 [details] [review]
Patch for Bug 4850
Comment 2 Ian Walls 2010-06-08 14:53:21 UTC
Previous patch insufficient.  Requires more testing.
Comment 3 Jared Camins-Esakov 2012-02-13 13:41:00 UTC
Marking Failed QA per Ian's last comment.
Comment 4 Jared Camins-Esakov 2013-01-02 02:05:11 UTC
Is this still an issue? I think this may have been fixed by other work on the holds system, but I'm not entirely clear on what the problem is.
Comment 5 Marc Véron 2015-12-20 15:59:34 UTC
Still valid?
Comment 6 Koha Team University Lyon 3 2016-01-25 10:46:01 UTC
I confirm that it's still an issue on master.
If your item-level_itypes syspref is on item level, you can choose different itemtypes for the same biblio : some of them can be hold and the other not.
If you choose to deal holds at a biblio level, you can encounter the problem described by Ian Walls :
If there a hold on a biblio with 2 items that are checked out, if the first one to be checked in is not an item type that can be reserve, the circulation rules are not verified and the hold is placed on this item.
Comment 7 Koha Team University Lyon 3 2016-01-26 10:10:55 UTC
In fact, it's just a problem of settings. 
Filling the "Defining circulation and fine rules for all libraries" table by setting 0 in "Holds allowed" column isn't sufficient.
You have to fill the "Default holds policy by item type" table (default_branch_item_rules or  branch_item_rules) and then, circulation rules are respected at a itemtype level for holds.

So for me, this isn't an issue anymore.
Comment 8 Marc Véron 2016-08-22 08:35:45 UTC
Regarding comments #6 and #7:

Maybe there could be a hint in the description of syspref 'item-level_itypes' and/or in 'Circulation and fine rule' for table 'Default holds policy by item type'?

If yes, what would be a good wording?
Comment 9 Hugh Rundle 2018-04-20 01:58:13 UTC
I disagree that this isn't a problem.

Comment 7 is true, but only if the only thing you care about is item types.

Imagine you have a circ rule for borrower category A plus item type Z, and another rule for borrower category B plus item type Z. You want Category A to be allowed to place holds on Z items, and you don't want category B to be able to place holds on Z items.

"Default holds policy by item type" is a blanket rule based on the item type only. So it's impossible to apply different hold rules on a combination of item type and borrower category.
Comment 10 Alex Buckley 2019-07-09 05:31:53 UTC
Hi all, 

I have written a related fix to what Ian wrote on this bug report. Mine is slightly different in that it checks the issuingrules database table to check the reservesallowed, holds_per_record, holds_per_day for a specific patron category/itemtype combination. 

This addresses Hugh's point as in my fix the return of an item now checks and respects the more granular hold rule on different patron category/item type combinations.

Ian's patch will be great for checking blanket branch/itemtype combinations as it calls/checks C4::Circulation::GetBranchItemRule but it will not check/respect patron category/item type combination rules defined in the issuingrules table. 

If you could take a look at and if possible test Bug 23172 ( https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23172 ) that would be wonderful and much appreciated.

Many thanks, 
Alex