Bug 13548

Summary: "High Holds" does not behave correctly when decreaseLoanHighHoldsValue is set to 0.
Product: Koha Reporter: Barton Chittenden <barton>
Component: CirculationAssignee: Eivin Giske Skaaren <eivin>
Status: CLOSED WORKSFORME QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: chris, eivin, gitbot, gmcharlt, kyle.m.hall
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Trivial patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 13548: "High Holds" does not behave correctly when decreaseLoanHighHoldsValue is set to 0.

Description Barton Chittenden 2015-01-09 19:43:24 UTC
Partner wants to decrease the length of loans when *any* items are checked out.

Setting 'decreaseLoahHighHolds' system preferences as follows:

+-------------------------------+------------+
| variable                      | value      |
+-------------------------------+------------+
| decreaseLoanHighHolds         | 1 [Enable] | 
| decreaseLoanHighHoldsDuration | 14         |
| decreaseLoanHighHoldsValue    | 0          |
+-------------------------------+------------+

This shows on the system preferences screen as follows:

[Enable] the reduction of loan period to  [14] days for items with more than [0]  holds.

Checking out *any* item (regardless of whether or not it has holds) will trigger the following message:

  High demand item. Loan period shortened to 0 days (due ). Check out anyway?

According to the system preferences description, this should not be triggered unless there is at least one hold. According to the partner, if decreaseLoanHighHoldsValue is set to 1, High Holds isn't triggered until an item is checked out that has two holds placed, meaning that it's impossible to trigger at exactly 1 hold placed.

when decreaseLoanHighHoldsValue is 0, the loan period is *not* shortened, even though the message pops up.

The description of decreaseLoanHighHoldsDuration is also misleading:

[Enable] the reduction of loan period to  [decreaseLoanHighHoldsDuration] days

should read

[Enable] the reduction of loan period by [decreaseLoanHighHoldsDuration] days

because, the loan period is not set to a constant 'decreaseLoanHighHoldsDuration' days; rather it is decremented by that amount.
Comment 1 Eivin Giske Skaaren 2015-09-07 20:38:21 UTC
Created attachment 42442 [details] [review]
Bug 13548: "High Holds" does not behave correctly when decreaseLoanHighHoldsValue is set to 0.

This solves the problem since there was a boolean check that evaluated to false because of the 0.

To test:

1. Apply patch.
2. Enable decreaseLoanHighHolds and set decreaseLoanHighHoldsValue to 0.
3. Check out an item without holds and see that the behaviour is now correct.
Comment 2 Eivin Giske Skaaren 2016-03-09 22:26:42 UTC
Is this fixed somewhere else? If not this should be very easy for someone to sign off..
Comment 3 Chris Cormack 2016-03-22 20:19:42 UTC
Quite a big conflict in C4::Circulation sorry
Comment 4 Eivin Giske Skaaren 2016-03-22 21:58:20 UTC
I updated to latest master and tested the behaviour.

Setting decreaseLoanHighHoldsValue to 0 does not trigger the message anymore and I can also see in Circulation.pm in the checkHighHolds subroutine that the code has changed and that this issue probably was fixed by that.

So I am closing the ticket as resolved - worksforme.
Comment 5 Eivin Giske Skaaren 2016-03-22 22:05:19 UTC
For reference, it seems like this was mostly solved in Bug 14694.