Bug 13919 - Renewal possible with item level hold on item
Summary: Renewal possible with item level hold on item
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
: 14104 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-03-26 16:17 UTC by Katrin Fischer
Modified: 2015-12-03 22:02 UTC (History)
9 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 13919 - Unit Test (2.10 KB, patch)
2015-03-30 14:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13919 - Renewal possible with item level hold on item (4.88 KB, patch)
2015-03-30 14:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13919 - Renewal possible with item level hold on item (4.93 KB, patch)
2015-04-13 11:06 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 13919 - Renewal possible with item level hold on item (4.93 KB, patch)
2015-04-13 11:06 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 13919: Make the tests pass fixing the number of tests (706 bytes, patch)
2015-04-13 11:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
[Signed-off] Bug 13919 - Unit Test (2.19 KB, patch)
2015-04-22 08:49 UTC, Marc Véron
Details | Diff | Splinter Review
[Signed-off] Bug 13919: Make the tests pass fixing the number of tests (860 bytes, patch)
2015-04-22 08:51 UTC, Marc Véron
Details | Diff | Splinter Review
[Signed-off] Bug 13919 - Renewal possible with item level hold on item (5.12 KB, patch)
2015-04-22 09:28 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 13919 - Unit Test (2.24 KB, patch)
2015-04-22 20:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 13919: Make the tests pass fixing the number of tests (911 bytes, patch)
2015-04-22 20:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 13919 - Renewal possible with item level hold on item (5.17 KB, patch)
2015-04-22 20:55 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 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