Bug 11258

Summary: Holds queue is still making transfer requests that contradict the branch holds policy
Product: Koha Reporter: Kyle M Hall <kyle>
Component: Hold requestsAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: fridolin.somers, gmcharlt, jonathan.druart, katrin.fischer, mcoalwell
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 11258 - Holds queue is still making transfer requests that contradict the branch holds policy
Bug 11258 - Holds queue is still making transfer requests that contradict the branch holds policy
Bug 11258 - Holds queue is still making transfer requests that contradict the branch holds policy
Bug 11258 - Holds queue is still making transfer requests that contradict the branch holds policy
Bug 11258 - Holds queue is still making transfer requests that contradict the branch holds policy
[PASSED QA] Bug 11258 - Holds queue is still making transfer requests that contradict the branch holds policy

Description Kyle M Hall 2013-11-15 16:27:36 UTC
The holds queue generator is still making requests that are not allowed. Previously it was making transfer requests that were not allowed. Currently, it is still making requests where the pickup branch is the same as the item's branch but not the patron's branch, even if there is a "Default holds policy by item type"
rule that states this item can only fill holds for patron's of the same branch as the item.
Comment 1 Kyle M Hall 2013-11-15 16:41:18 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2013-11-15 17:49:09 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2013-11-15 17:50:00 UTC Comment hidden (obsolete)
Comment 4 Liz Rea 2013-11-19 02:36:08 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2014-01-12 09:04:17 UTC
I am sorry, but I can't seem to reproduce the bug on current master. I followed the test plan, but my holds queue entry is the same before and after applying the patch. 

>1) Create a test record with 2 items with different itemtypes

Branches, itemtype, barcode
Midway, Books, t1
Midway, Computer files, t2

>2) Set the Default holds policy by item type for the first
>   item to "From any library"

Midway: Books, from any library

>3) Set the Default holds policy by item type for the second
>   item to "From home library"

Midway: Computer files, from home library

>4) Place a record level hold for a patron from another library,
>   but for pickup at the same library as the item is from

Switched to Riverside, placed a title level hold for a Riverside patron, 
pickup location is Midway.

>5) Rebuild the holds queue

Entry reads:
Books, t1 or any available

>6) View the holds queue, note the item is listed, though this
>   patron cannot place a hold on this item

I couldn't reproduce this.

>7) Apply this patch
>8) Repeat step 5, note the hold is no longer in the queue
Comment 6 Mickey Coalwell 2014-01-21 19:13:50 UTC
I set up test plan in production as described in ticket. Results positive. Available item with Local Hold was correctly x'ed out in holdings table, and hold did not show up on pick list for patron wanting pickup location at owning library. Checked in another item without hold restriction, which DID show up correctly on pick list. So the behavior is now as expected, and desired. Patch eliminated bug.
Comment 7 Jonathan Druart 2014-01-24 13:02:16 UTC
(In reply to Katrin Fischer from comment #5)
> >6) View the holds queue, note the item is listed, though this
> >   patron cannot place a hold on this item
> 
> I couldn't reproduce this.

And me I see it before and after applying the patch (I rebuilt the holds queue).
Comment 8 Katrin Fischer 2014-01-24 13:03:08 UTC
Kyle, I think we need help here - could it be something is missing from the test plan?
Comment 9 Kyle M Hall 2014-01-31 14:53:59 UTC
(In reply to Katrin Fischer from comment #8)
> Kyle, I think we need help here - could it be something is missing from the
> test plan?

Nope, I followed the test plan with a fresh database and was able to recreate the bug, and also show the patch resolves it.

Here is a video demonstration: http://screencast.com/t/0PBAfJkf5

After I demonstrate the bug, apply the patch and rebuild the holds queue, you'll see no items in the holds queue. This is because I forgot that Reference items are not holdable. So post-patch, neither item is holdable by that patron, and the holds queue *should* be empty. After that, I make Reference items holdable, rebuild the holds queue again, and a hold request for the Reference item shows up, which is the correct behavior as well.
Comment 10 Kyle M Hall 2014-02-13 16:42:35 UTC Comment hidden (obsolete)
Comment 11 Katrin Fischer 2014-04-17 18:19:40 UTC
Created attachment 27267 [details] [review]
[PASSED QA] Bug 11258 - Holds queue is still making transfer requests that contradict the branch holds policy

The holds queue generator is still making requests that are not allowed.
Previously it was making transfer requests that were not allowed.
Currently, it is still making requests where the pickup branch is the
same as the item's branch but not the patron's branch,
even if there is a "Default holds policy by item type" rule that states
this item can only fill holds for patron's of the same branch as the item.

Test Plan:
1) Create a test record with 2 items with different itemtypes
2) Set the Default holds policy by item type for the first
   item to "From any library"
3) Set the Default holds policy by item type for the second
   item to "From home library"
4) Place a record level hold for a patron from another library,
   but for pickup at the same library as the item is from
5) Rebuild the holds queue
6) View the holds queue, note the item is listed, though this
   patron cannot place a hold on this item
7) Apply this patch
8) Repeat step 5, note the hold is no longer in the queue

Signed-off-by: Liz Rea <liz@catalyst.net.nz>
automated tests pass, functional tests pass. Bug replicated, eradicated by patch.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
I finally managed to reproduce this, patch works as described.
Passes tests and QA script, provided tests fail without patch, but
succeed with the patch.
Comment 12 Galen Charlton 2014-04-18 15:51:40 UTC
Pushed to master.  Thanks, Kyle!
Comment 13 Fridolin Somers 2014-08-06 09:56:50 UTC
Pushed to 3.14.x, will be in 3.14.10
Comment 14 Kyle M Hall 2014-08-13 14:30:30 UTC
Picked for 3.12.15!