Bug 5675 - Returned items always trigger hold notification
Summary: Returned items always trigger hold notification
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: unspecified
Hardware: All All
: PATCH-Sent (DO NOT USE) major (vote)
Assignee: Chris Cormack
QA Contact: Bugs List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 5609
  Show dependency treegraph
 
Reported: 2011-01-31 17:29 UTC by Jared Camins-Esakov
Modified: 2012-10-25 23:09 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 5675 : CheckReserves not honouring item level itemtypes preferences (1.82 KB, patch)
2011-08-28 23:22 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 5675 : CheckReserves not honouring item level itemtypes preferences (2.12 KB, patch)
2011-09-18 18:58 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jared Camins-Esakov 2011-01-31 17:29:02 UTC
It seems that when an item is checked in for a biblio that has a biblio-level hold, the message notifying the librarian that the book is on hold always comes up, even when holds are not allowed on that itemtype. The cause seems to be that returns.pl does not check the circulation rules, just whether or not there is a hold. There should be some sort of sanity testing to confirm that an item really is allowed to have a hold on it.

Example:
1. Create a biblio with one item of a type that can have holds on it.
2. Place a hold on that biblio.
3. Create a new item of a type that cannot have a hold.
4. Check in new item.
Comment 1 Jared Camins-Esakov 2011-03-19 20:36:25 UTC
This also applies to damaged items- damaged items will trigger holds even if AllowHoldsOnDamagedItems is set to "Don't allow" if there is another item that can have holds on the biblio.
Comment 2 Chris Cormack 2011-08-25 01:21:42 UTC
Working on this now
Comment 3 Chris Cormack 2011-08-28 21:29:38 UTC
It is restricted to biblio level holds, if you place an item specific hold, it works ok.

Working on fix now
Comment 4 Chris Cormack 2011-08-28 23:13:12 UTC
Yep, its C4::Reserves::CheckReserves .. and the issue is only with biblio level holds
Comment 5 Chris Cormack 2011-08-28 23:17:07 UTC
The problem query is

   SELECT items.biblionumber,
           items.biblioitemnumber,
           itemtypes.notforloan,
           items.notforloan AS itemnotforloan,
           items.itemnumber
    FROM   items
    LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber
    LEFT JOIN itemtypes   ON biblioitems.itemtype   = itemtypes.itemtype

This is wrong, because it doesnt use the itemlevel itype. It should check the syspref and use the right level. Either biblio or item level
Comment 6 Chris Cormack 2011-08-28 23:22:55 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2011-09-18 18:58:08 UTC
Created attachment 5452 [details] [review]
Bug 5675 : CheckReserves not honouring item level itemtypes preferences

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>

With item level itypes activated only the item for which holds were
allowed in circulation rules triggered the hold notice on checkin.

Also checked that biblio level itype still trigered correctly.
Comment 8 Ian Walls 2011-09-20 11:04:59 UTC
Change to SQL only, with a little logic around selecting which version of the query to utilize (depending on item-level_itypes).  Easy to read, marking as Passed QA.
Comment 9 Chris Cormack 2011-09-20 21:40:41 UTC
Pushed, please test