Bug 13919

Summary: Renewal possible with item level hold on item
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: CirculationAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: barton, chris, gmcharlt, jonathan.druart, katrin.fischer, kyle.m.hall, nicolas.legrand, tomascohen, veron
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 13919 - Unit Test
Bug 13919 - Renewal possible with item level hold on item
Bug 13919 - Renewal possible with item level hold on item
Bug 13919 - Renewal possible with item level hold on item
Bug 13919: Make the tests pass fixing the number of tests
[Signed-off] Bug 13919 - Unit Test
[Signed-off] Bug 13919: Make the tests pass fixing the number of tests
[Signed-off] Bug 13919 - Renewal possible with item level hold on item
[PASSED QA] Bug 13919 - Unit Test
[PASSED QA] Bug 13919: Make the tests pass fixing the number of tests
[PASSED QA] Bug 13919 - Renewal possible with item level hold on item

Description Katrin Fischer 2015-03-26 16:17:20 UTC
Tested in 3.18.5.1:

- item is checked out to patron A
- patron B places an item level hold on the item
- patron A renews all - item is renewed :(

Can someone confirm? I think there is no configuration option that should allow this, but maybe I am missing something.
Comment 1 Katrin Fischer 2015-03-26 16:22:25 UTC
This also affects the staff interface: when checking out the item again to another user, I get a warning that there is a hold. I override that and check it out anyway - and I am offfered the option to renew it.

Soemthing is seriously going wrong here.
Comment 2 Katrin Fischer 2015-03-26 16:41:19 UTC
In my setup it's AllowRenewalIfOtherItemsAvailable that makes the difference.

Switching that makes the difference between seeing the checkbox in the OPAC and the 'on hold' message in the staff client.

I understood it should only allow renewal if items are available to fill the hold - so I think this is a bug, as the other existing item can't fill the hold here.
Comment 3 Kyle M Hall 2015-03-30 14:18:11 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2015-03-30 14:18:40 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2015-04-13 10:48:34 UTC
The "renew all" button appears (same as intranet), should not we remove it?
Comment 6 Jonathan Druart 2015-04-13 10:50:17 UTC
I had 2 items, 1 with an hold (not renewable).
I clicked on "renew all" and got "Please note: Your loan renewal failed because of the following reason(s): This item is on hold for another patron."

1 has been renewed (Renewed!) not the other.

Could we get another tester on this one please?
Comment 7 Katrin Fischer 2015-04-13 10:56:29 UTC
Hm, but only the item without the hold was renewed? Or was the item with the hold renewed? 

I think the confusing message stems is what I noticed on bug 13913 - it just lists without telling you how many or which item.
Comment 8 Nicolas Legrand 2015-04-13 11:06:13 UTC Comment hidden (obsolete)
Comment 9 Nicolas Legrand 2015-04-13 11:06:44 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2015-04-13 11:14:46 UTC
(In reply to Katrin Fischer from comment #7)
> Hm, but only the item without the hold was renewed?

Yes
Comment 11 Jonathan Druart 2015-04-13 11:32:28 UTC Comment hidden (obsolete)
Comment 12 Marc Véron 2015-04-22 08:49:54 UTC Comment hidden (obsolete)
Comment 13 Marc Véron 2015-04-22 08:51:22 UTC Comment hidden (obsolete)
Comment 14 Marc Véron 2015-04-22 09:28:55 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2015-04-22 20:55:46 UTC
Created attachment 38377 [details] [review]
[PASSED QA] Bug 13919 - Unit Test

Tested both unit test patches together.
Without main patch applied: do not pass as expected / with main patch pass OK as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Katrin Fischer 2015-04-22 20:55:49 UTC
Created attachment 38378 [details] [review]
[PASSED QA] Bug 13919: Make the tests pass fixing the number of tests

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Katrin Fischer 2015-04-22 20:55:52 UTC
Created attachment 38379 [details] [review]
[PASSED QA] Bug 13919 - Renewal possible with item level hold on item

Test Plan:
1) Apply this patch
2) Enable AllowRenewalIfOtherItemsAvailable
3) Check out an item from a record with multiple holdable items
4) Place an item level hold on the checked out item
5) Verify the item can not be renewed from the opac

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 18 Katrin Fischer 2015-04-22 20:56:50 UTC
Tested with various combinations of item level/record level holds and syspref settings - no problems found!
Comment 19 Tomás Cohen Arazi 2015-04-28 19:48:34 UTC
Patches pushed to master.

Thanks Kyle!
Comment 20 Katrin Fischer 2015-05-15 06:30:43 UTC
*** Bug 14104 has been marked as a duplicate of this bug. ***
Comment 21 Chris Cormack 2015-05-17 04:19:48 UTC
Pushed to 3.18.x will be in 3.18.7