Bug 9922 - holds queue shows home library with AutomaticReturn
Summary: holds queue shows home library with AutomaticReturn
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Kyle M Hall
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 5911
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-25 13:29 UTC by Nicole C. Engard
Modified: 2019-06-27 09:24 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 9922 - holds queue shows home library with AutomaticReturn (1.45 KB, patch)
2013-03-25 13:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9922 - holds queue shows home library with AutomaticReturn (1.60 KB, patch)
2013-03-27 03:06 UTC, Liz Rea
Details | Diff | Splinter Review
[PASSED QA] Bug 9922 - holds queue shows home library with AutomaticReturn (1.82 KB, patch)
2013-03-29 08:05 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 Nicole C. Engard 2013-03-25 13:29:41 UTC
It looks like the syspref AutomaticReturn controls whether the holds queue acts upon the holdingbranch or the homebranch.  This makes no sense whatsoever the one should have nothing to do with the other and the Holds Queue should always be on Holding branch because that's who can pull the item from the shelf.
Comment 1 Kyle M Hall 2013-03-25 13:32:53 UTC
This error is caused by line 403 of C4::HoldsQueue and appears to be part of the transport cost matrix dev. There is no explanation for this behavior documented.
Comment 2 Kyle M Hall 2013-03-25 13:38:24 UTC Comment hidden (obsolete)
Comment 3 Liz Rea 2013-03-26 02:41:53 UTC
I'm going to be honest - I think it was a mistake to do anything with automatic item return in the hold queue, and any checking for it should be removed.

My understanding of the purpose for automaticitemreturn was to automatically send an item home when nothing away from home was left to do with it, as an example, a library system probably wouldn't want to send an item home in between every hold, it would want to send the item on to the next hold from the item's current holding branch. AutomaticItemReturn only factors in when there are no holds left, and the library wants the book to return home automatically at the end of it's run.

I'm not sure I understand why it's even a factor on the hold queue - the hold queue should always pick items from the holding branch, regardless of the AIR setting.

Unless it has something to do with the Cost Matrix calculations, let's just get rid of checking for automaticitemreturn all together.
Comment 4 Srdjan Jankovic 2013-03-27 03:03:04 UTC
It was a long time ago (2011-03-24) so i can only speculate, but the nost probable vesion is that I saw it in Circ::AddReturn, and I thought I needed to put it in the cost matrix generated queue as well.
Comment 5 Liz Rea 2013-03-27 03:06:48 UTC Comment hidden (obsolete)
Comment 6 Liz Rea 2013-03-27 03:10:16 UTC
Thanks for helping us understand Srdjan. :)
Comment 7 Katrin Fischer 2013-03-27 06:25:10 UTC
Right now, the test plan does not work for me. Not sure what is going wrong.

I have an item level hold on an item with home branch Demo and holding branch Fairfield.

I am logged in for Demo and I no matter which value I have for AIR, the item always shows up for Fairfield.

Is this only a problem in earlier versions of Koha?
Comment 8 Katrin Fischer 2013-03-29 08:05:07 UTC
Created attachment 16996 [details] [review]
[PASSED QA] Bug 9922 - holds queue shows home library with AutomaticReturn

Test Plan:
 1) Enable AutomaticItemReturn
 2) Place a reserve on a record where the holding and home branches differ
    for the available items
 3) Rebuild the holds queue
 4) Check the holds queue, verify the item is listed in the items to pull for the item's home branch
 5) Disable AutomaticItemReturn
 6) Rebuild the holds queue
 7) Verify the item is listed in the items to pull for the item's holding branch
 8) Enable AutomaticItemReturn
 9) Apply patch
10) Rebuild the holds queue
11) Verify the item is listed in the items to pull for the item's holding branch

Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Tested per plan, and the patch seems sane. Functionality of the hold queue is restored to previous behaviour.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Amended test plan to make clear it has to be a record level
hold for the test plan to work.
All tests and QA script pass.
Comment 9 Jared Camins-Esakov 2013-03-30 02:25:44 UTC
This patch has been pushed to master.
Comment 10 Chris Cormack 2013-03-30 07:27:53 UTC
Pushed to 3.10.x will be in 3.10.5