It is possible to have one more checkouts than allowed by rules. To recreate: - Delete all rules in 'Circulation rules and fines'. - Set a Default limit by patron category, e.g. Student 2 current checkouts , 2 on-site-chekouts. - Checkout to a student until the limit is reached (in this case: 2) - Try a 3rd checkout; it is possible (should not) - Try a 4th checkout: No longer possible. Message says: "Too many checked out. 3 checked out, only 2 are allowed." Expected: Message should trigger with 3rd checkout, saying: "Too many checked out. 2 checked out, only 2 are allowed."
Suppose it happens in comparisons in C4/Circulation.pm
I do not recreate, I get the warning on the 3rd checkouts. Are you checking out an on-site checkouts? If so what is the value of ConsiderOnSiteCheckoutsAsNormalCheckouts? How do you create the circ rule? I just filled "Current checkouts allowed" and "Current on-site checkouts allowed", let other values blank.
Created attachment 61588 [details] Screenshot circulation rules (In reply to Jonathan Druart from comment #2) > I do not recreate, I get the warning on the 3rd checkouts. Are you checking > out an on-site checkouts? If so what is the value of > ConsiderOnSiteCheckoutsAsNormalCheckouts? > > How do you create the circ rule? I just filled "Current checkouts allowed" > and "Current on-site checkouts allowed", let other values blank. - I tested with normal checkouts. - Just one circulation rule for students as described in my initial comment. - ConsiderOnSiteCheckoutsAsNormalCheckouts is: Consider
With ConsiderOnSiteCheckoutsAsNormalCheckouts set to 'Don't consider' it is not possible to checkout 3 items, try to checkout the 3rd item, I get the message: "Too many checked out. 2 checked out, only 2 are allowed." => As expected With ConsiderOnSiteCheckoutsAsNormalCheckouts set to 'Consider' I can check out one more item than allowed as described in initial comment.
Would it be possible to get a screencast, or a sandbox set up with your settings? I still do not recreate.
Created attachment 61646 [details] Screenshots to reproduce error Re-testing: - Set ConsiderOnSiteCheckoutsAsNormalCheckouts to 'Consider' - Delete all rules in 'Circulation rules and fines'. - Set a Default limit by patron category, e.g. Student with 2 current checkouts, 2 on-site-chekouts. - Normal (not On-Site) checkout to a student until the limit is reached (in this case: 2) - Try a 3rd checkout; it is possible (should not) - Try a 4th checkout: No longer possible. Message says: "Too many checked out. 3 checked out, only 2 are allowed." - Switch ConsiderOnSiteCheckoutsAsNormalCheckouts to 'Do not consider' - Check all items in - Repeat steps to reproduce - Checkout is blocked when trying to do 3rd checkout (as expected)
Sorry but still works for me. Did you test on a sandbox?
(In reply to Jonathan Druart from comment #7) > Sorry but still works for me. Did you test on a sandbox? No. Anything not clear with my screenshots / description (Comment #6)?
Everything is clear, but works for me :)
Hmm, thinking... The strange thing is that in my tests the behaviour depends on the status of syspref 'ConsiderOnSiteCheckoutsAsNormalCheckouts' With 'Do not consider' it works as expected, with 'Consider' I can check out one more than allowed (normal checkouts, not onsite checkouts). What are your settings for onsite sysprefs? (OnSiteCheckouts, OnSiteCheckoutsForce, SwitchOnSiteCheckouts, ConsiderOnSiteCheckoutsAsNormalCheckouts)
To reproduce, set following sysprefs: - ConsiderOnSiteCheckoutsAsNormalCheckouts: Consider - SwitchOnSiteCheckouts: Switch That triggers a $delta with value 1 to be added to $max_checkouts_allowed in C4/Circulation.pm line 485, allowing one more checkout. I could not figure out what this $delta stands for. It has a value of 1 if SwitchOnSiteCheckouts is set to 'Switch', otherwise it is 0.
Caused by bug 16272.
Created attachment 61759 [details] [review] Bug 18321: Add tests Clean a bit existing tests by adding default values and add a test to highlight the issue.
Created attachment 61760 [details] [review] Bug 18321: Correctly calculate switch_onsite_checkout Silly error here, the $switch_onsite_checkout is not correctly calculated, it should not only be set to the value of the syspref, but meet all the conditions. Test plan: Set a circ rule with number of on-site checkout = 2 and number of checkouts = 2. Switch on both ConsiderOnSiteCheckoutsAsNormalCheckouts and SwitchOnSiteCheckouts Do a regular checkout Do another regular checkout At this point you have reach the maximum number of checkouts allowed. Not try another one. => Without this patch you will not get the warning => With this patch you will see it
Created attachment 61770 [details] [review] Bug 18321: Add tests Clean a bit existing tests by adding default values and add a test to highlight the issue. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 61771 [details] [review] Bug 18321: Correctly calculate switch_onsite_checkout Silly error here, the $switch_onsite_checkout is not correctly calculated, it should not only be set to the value of the syspref, but meet all the conditions. Test plan: Set a circ rule with number of on-site checkout = 2 and number of checkouts = 2. Switch on both ConsiderOnSiteCheckoutsAsNormalCheckouts and SwitchOnSiteCheckouts Do a regular checkout Do another regular checkout At this point you have reach the maximum number of checkouts allowed. Not try another one. => Without this patch you will not get the warning => With this patch you will see it Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Jonathan, this Bug was marked as assigned but the patches work, so I assume that you forgot to set to NSO. Switching to NSO and then to Signed off
(In reply to Marc Véron from comment #17) > Jonathan, this Bug was marked as assigned but the patches work, so I assume > that you forgot to set to NSO. > Switching to NSO and then to Signed off Yes indeed, I forgot to change the status.
Created attachment 62076 [details] [review] Bug 18321: Add tests Clean a bit existing tests by adding default values and add a test to highlight the issue. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 62077 [details] [review] Bug 18321: Correctly calculate switch_onsite_checkout Silly error here, the $switch_onsite_checkout is not correctly calculated, it should not only be set to the value of the syspref, but meet all the conditions. Test plan: Set a circ rule with number of on-site checkout = 2 and number of checkouts = 2. Switch on both ConsiderOnSiteCheckoutsAsNormalCheckouts and SwitchOnSiteCheckouts Do a regular checkout Do another regular checkout At this point you have reach the maximum number of checkouts allowed. Not try another one. => Without this patch you will not get the warning => With this patch you will see it Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
kyle@bywatersolutions.com 2017-04-13 12:28:01 UTC CC kyle@bywatersolutions.com Status Passed QA Patch doesn't apply Re-tested on current master 16.12.00.022, patch applies fine here.
(In reply to Marc Véron from comment #21) > kyle@bywatersolutions.com 2017-04-13 12:28:01 UTC CC > kyle@bywatersolutions.com > Status Passed QA Patch doesn't apply > > Re-tested on current master 16.12.00.022, patch applies fine here. Same here, please retest.
Re-tested on current master, 16.12.00.024 Before applying, head is 36fd404 Bug 17835: [QA Follow-up] Fix opac-search.pl git bz apply 18321 Both patches apply.
Pushed to master for 17.05, thanks Jonathan!
These patches have been pushed to 16.11.x and will be in 16.11.07.