When you renew an item on hold, it allows you to bypass the hold, even if you don't allow it elsewhere. It also bypasses renewal limits set in Circulation Rules. Does not honor AllowRenewalLimitOverride preference if set to Don't allow. Does not honor AllowRenewalIfOtherItemsAvailable if set to Don't allow.
Created attachment 62339 [details] [review] Bug 18450 - Implemented a check to determine if the 'Override and renew' button is displayed based on the setting of the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences. This will display the 'Override and renew' button only if both preferences are set to 'Allow'. Test plan: 1. In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences are both set to 'Dont allow'. 2. Place a hold on an item from a patron 3. Checkout the item out to a different patron 4. Renew the item and notice the 'Cannot renew: The item is on hold for another patron' 5. Click the 'Override and renew' button and notice that although you can bypass the hold 6. Apply patch 7. Renew the item again and notice that the message does not have a 'Override and renew' button it just has the 'Ignore and continue' button 8. Go back to More->Administration->Global system preferences and set AllowRenewalLimitOverride to 'Allow' 9. Try renewing the same item again and notice that the 'Override and renew' button is still not being displayed. 10. In systems preferences now set the AllowRenewalIfOtherItemsAvailable to 'Allow' 11. Try renewing the item again and notice that now both of these preferences are allowed the 'Override and renew' button is displayed because you will not be violating these preferences. Note: Checking if the renewal exceeds the renewal level, if auto renew is set up, or if renewal is to soon is checked earlier in this conditional if statement earlier and so the check in this patch does not need to check the renewal level, auto renewal, if renewal is too soon.
Created attachment 62447 [details] [review] Bug 18450 - Implemented a check to determine if the 'Override and renew' button is displayed based on the setting of the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences. This will display the 'Override and renew' button only if both preferences are set to 'Allow'. Test plan: 1. In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences are both set to 'Dont allow'. 2. Place a hold on an item from a patron 3. Checkout the item out to a different patron 4. Renew the item and notice the 'Cannot renew: The item is on hold for another patron' 5. Click the 'Override and renew' button and notice that although you can bypass the hold 6. Apply patch 7. Renew the item again and notice that the message does not have a 'Override and renew' button it just has the 'Ignore and continue' button 8. Go back to More->Administration->Global system preferences and set AllowRenewalLimitOverride to 'Allow' 9. Try renewing the same item again and notice that the 'Override and renew' button is still not being displayed. 10. In systems preferences now set the AllowRenewalIfOtherItemsAvailable to 'Allow' 11. Try renewing the item again and notice that now both of these preferences are allowed the 'Override and renew' button is displayed because you will not be violating these preferences. Note: Checking if the renewal exceeds the renewal level, if auto renew is set up, or if renewal is to soon is checked earlier in this conditional if statement earlier and so the check in this patch does not need to check the renewal level, auto renewal, if renewal is too soon. Folloed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Sorry, found a wrinkle. With AllowRenewalLimitOverride set to Don't allow and AllowRenewalIfOtherItemsAvailable set to Allow and you haven't hit the renewal limit, you are still blocked with the hold in place.
Test needs to be updated to include all combinations of these two settings.
Created attachment 62811 [details] [review] Bug 18450 - Implemented check if either AllowRenewLimitOverride or AllowRenewalIfOtherItemsAvaliable were set to allow, so the user is only blocked if both of these sysprefs are set to 'Dont allow' Test plan: 1. In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences are both set to 'Dont allow'. 2. Place a hold on an item from a patron 3. Checkout the item out to a different patron 4. Renew the item and notice the 'Cannot renew: The item is on hold for another patron' warning 5. Click the 'Override and renew' button and notice that you can bypass the hold 6. Apply patch 7. Renew the item again and notice that the message does not have a 'Override and renew' button it just has the 'Ignore and continue' button 8. Go back to More->Administration->Global system preferences and set AllowRenewalLimitOverride to 'Allow' 9. Try renewing the same item again and notice that the 'Override and renew' button is displayed 10. In systems preferences now set the AllowRenewalIfOtherItemsAvailable syspref to 'Allow' and AllowRenewalLimitOverride to 'Dont allow' and notice the 'Override and renew' button is displayed 11. Now set both these sysprefs to 'Allow' and notice the 'Override and renew' button is displayed because you will not be violating these preferences. Note: Checking if the renewal exceeds the renewal level, if auto renew is set up, or if renewal is to soon is checked earlier in this conditional if statement earlier and so the check in this patch does not need to check the renewal level, auto renewal, if renewal is too soon.
Comment on attachment 62811 [details] [review] Bug 18450 - Implemented check if either AllowRenewLimitOverride or AllowRenewalIfOtherItemsAvaliable were set to allow, so the user is only blocked if both of these sysprefs are set to 'Dont allow' Review of attachment 62811 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt @@ +119,5 @@ > + <input type="hidden" name="override_limit" value="1" /> > + <input type="hidden" name="override_holds" value="1" /> > + <button type="submit" class="approve"><i class="fa fa-check"></i> Override and renew</button> > + </form> > + [% ELSIF ((Koha.Preference('AllowRenewalLimitOverride')) && !(Koha.Preference('AllowRenewalIfOtherItemsAvai lable'))) %] wrong copy paste in AllowRenewalIfOtherItemsAvailable preference name
Created attachment 63226 [details] [review] Bug 18450 - Implemented check if either AllowRenewLimitOverride or AllowRenewalIfOtherItemsAvaliable were set to allow, so the user is only blocked if both of these sysprefs are set to 'Dont allow' Test plan: 1. In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences are both set to 'Dont allow'. 2. Place a hold on an item from a patron 3. Checkout the item out to a different patron 4. Renew the item and notice the 'Cannot renew: The item is on hold for another patron' warning 5. Click the 'Override and renew' button and notice that you can bypass the hold 6. Apply patch 7. Renew the item again and notice that the message does not have a 'Override and renew' button it just has the 'Ignore and continue' button 8. Go back to More->Administration->Global system preferences and set AllowRenewalLimitOverride to 'Allow' 9. Try renewing the same item again and notice that the 'Override and renew' button is displayed 10. In systems preferences now set the AllowRenewalIfOtherItemsAvailable syspref to 'Allow' and AllowRenewalLimitOverride to 'Dont allow' and notice the 'Override and renew' button is displayed 11. Now set both these sysprefs to 'Allow' and notice the 'Override and renew' button is displayed because you will not be violating these preferences. Note: Checking if the renewal exceeds the renewal level, if auto renew is set up, or if renewal is to soon is checked earlier in this conditional if statement earlier and so the check in this patch does not need to check the renewal level, auto renewal, if renewal is too soon.
(In reply to Josef Moravec in comment 6) Thanks Josef. I have fixed up that issue now
Created attachment 63292 [details] [review] [SIGNED-OFF] Bug 18450 - Implemented a check to determine if the 'Override and renew' button is displayed based on the setting of the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences. This will display the 'Override and renew' button only if both preferences are set to 'Allow'. Test plan: 1. In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences are both set to 'Dont allow'. 2. Place a hold on an item from a patron 3. Checkout the item out to a different patron 4. Renew the item and notice the 'Cannot renew: The item is on hold for another patron' 5. Click the 'Override and renew' button and notice that although you can bypass the hold 6. Apply patch 7. Renew the item again and notice that the message does not have a 'Override and renew' button it just has the 'Ignore and continue' button 8. Go back to More->Administration->Global system preferences and set AllowRenewalLimitOverride to 'Allow' 9. Try renewing the same item again and notice that the 'Override and renew' button is still not being displayed. 10. In systems preferences now set the AllowRenewalIfOtherItemsAvailable to 'Allow' 11. Try renewing the item again and notice that now both of these preferences are allowed the 'Override and renew' button is displayed because you will not be violating these preferences. Note: Checking if the renewal exceeds the renewal level, if auto renew is set up, or if renewal is to soon is checked earlier in this conditional if statement earlier and so the check in this patch does not need to check the renewal level, auto renewal, if renewal is too soon. Folloed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 63293 [details] [review] [SIGNED-OFF] Bug 18450 - Implemented check if either AllowRenewLimitOverride or AllowRenewalIfOtherItemsAvaliable were set to allow, so the user is only blocked if both of these sysprefs are set to 'Dont allow' Test plan: 1. In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences are both set to 'Dont allow'. 2. Place a hold on an item from a patron 3. Checkout the item out to a different patron 4. Renew the item and notice the 'Cannot renew: The item is on hold for another patron' warning 5. Click the 'Override and renew' button and notice that you can bypass the hold 6. Apply patch 7. Renew the item again and notice that the message does not have a 'Override and renew' button it just has the 'Ignore and continue' button 8. Go back to More->Administration->Global system preferences and set AllowRenewalLimitOverride to 'Allow' 9. Try renewing the same item again and notice that the 'Override and renew' button is displayed 10. In systems preferences now set the AllowRenewalIfOtherItemsAvailable syspref to 'Allow' and AllowRenewalLimitOverride to 'Dont allow' and notice the 'Override and renew' button is displayed 11. Now set both these sysprefs to 'Allow' and notice the 'Override and renew' button is displayed because you will not be violating these preferences. Note: Checking if the renewal exceeds the renewal level, if auto renew is set up, or if renewal is to soon is checked earlier in this conditional if statement earlier and so the check in this patch does not need to check the renewal level, auto renewal, if renewal is too soon. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
If I do not misread the second patch, we have: IF NOT A AND B: Do stuff ELSIF A AND NOT B: Do the same stuff If that the case, we can simplify with: IT NOT A AND B OR A AND NOT B: Do the stuff
Created attachment 63377 [details] [review] Bug 18450 - Implemented check if either AllowRenewLimitOverride or AllowRenewalIfOtherItemsAvaliable were set to allow, so the user is only blocked if both of these sysprefs are set to 'Dont allow' Test plan: 1. In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences are both set to 'Dont allow'. 2. Place a hold on an item from a patron 3. Checkout the item out to a different patron 4. Renew the item and notice the 'Cannot renew: The item is on hold for another patron' warning 5. Click the 'Override and renew' button and notice that you can bypass the hold 6. Apply patch 7. Renew the item again and notice that the message does not have a 'Override and renew' button it just has the 'Ignore and continue' button 8. Go back to More->Administration->Global system preferences and set AllowRenewalLimitOverride to 'Allow' 9. Try renewing the same item again and notice that the 'Override and renew' button is displayed 10. In systems preferences now set the AllowRenewalIfOtherItemsAvailable syspref to 'Allow' and AllowRenewalLimitOverride to 'Dont allow' and notice the 'Override and renew' button is displayed 11. Now set both these sysprefs to 'Allow' and notice the 'Override and renew' button is displayed because you will not be violating these preferences. Note: Checking if the renewal exceeds the renewal level, if auto renew is set up, or if renewal is to soon is checked earlier in this conditional if statement earlier and so the check in this patch does not need to check the renewal level, auto renewal, if renewal is too soon. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
I have just squashed the two previous patches into 1 and attached it to this bug report, and as Jonathan suggested in comment 11 I have implemented the following logic in the renew.tt file: IT NOT A AND B OR A AND NOT B: Do the stuff
Nope, the last patch does: IF A AND B: Do the stuffs ELSIF ( NOT A AND B ) OR ( A AND NOT B ): Do the same stuffs Alex, could you submit another patch replacing this one, update the commit message with a shorter first line (ideally should be < 80chars), remove the signed-off by line and switch the status to "Needs Signoff" please?
Created attachment 63764 [details] [review] Bug 18450 - Implemented check in renew.tt for block and renewal limits Test plan: 1. In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences are both set to 'Dont allow'. 2. Place a hold on an item from a patron 3. Checkout the item out to a different patron 4. Renew the item and notice the 'Cannot renew: The item is on hold for another patron' warning 5. Click the 'Override and renew' button and notice that you can bypass the hold 6. Apply patch 7. Renew the item again and notice that the message does not have a 'Override and renew' button it just has the 'Ignore and continue' button 8. Go back to More->Administration->Global system preferences and set AllowRenewalLimitOverride to 'Allow' 9. Try renewing the same item again and notice that the 'Override and renew' button is displayed 10. In systems preferences now set the AllowRenewalIfOtherItemsAvailable syspref to 'Allow' and AllowRenewalLimitOverride to 'Dont allow' and notice the 'Override and renew' button is displayed 11. Now set both these sysprefs to 'Allow' and notice the 'Override and renew' button is displayed because you will not be violating these preferences. Note: Checking if the renewal exceeds the renewal level, if auto renew is set up, or if renewal is to soon is checked earlier in this conditional if statement earlier and so the check in this patch does not need to check the renewal level, auto renewal, if renewal is too soon.
Hi Jonathan I have done what you asked in comment 14. Were you happy with the if statement logic in the renew.tt file being this: IF A AND B: Do the stuffs ELSIF ( NOT A AND B ) OR ( A AND NOT B ): Do the same stuffs
Created attachment 64402 [details] [review] Bug 18450 - Implemented check in renew.tt for block and renewal limits Test plan: 1. In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences are both set to 'Dont allow'. 2. Place a hold on an item from a patron 3. Checkout the item out to a different patron 4. Renew the item and notice the 'Cannot renew: The item is on hold for another patron' warning 5. Click the 'Override and renew' button and notice that you can bypass the hold 6. Apply patch 7. Renew the item again and notice that the message does not have a 'Override and renew' button it just has the 'Ignore and continue' button 8. Go back to More->Administration->Global system preferences and set AllowRenewalLimitOverride to 'Allow' 9. Try renewing the same item again and notice that the 'Override and renew' button is displayed 10. In systems preferences now set the AllowRenewalIfOtherItemsAvailable syspref to 'Allow' and AllowRenewalLimitOverride to 'Dont allow' and notice the 'Override and renew' button is displayed 11. Now set both these sysprefs to 'Allow' and notice the 'Override and renew' button is displayed because you will not be violating these preferences. Note: Checking if the renewal exceeds the renewal level, if auto renew is set up, or if renewal is to soon is checked earlier in this conditional if statement earlier and so the check in this patch does not need to check the renewal level, auto renewal, if renewal is too soon. Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Two things here: 1 - 'AllowRenewalIfOtherItemsAvailable' - we need to check if other items are available - a record with 1 item with a hold and checked out should not be renewable (it is currently) 2 - As Jonathan mentioned in comment #11 - if we are doing the same stuff we can do "IF A OR B OR C { stuff }" - no need for if/elsif
Created attachment 69475 [details] [review] Bug 18450 - Followup - Implemented 'IF A OR B OR C {stuff}" for checking what to display Implemented a conditional checking if the item that the patron is trying to renew has more than 1 copy available which can be renewed. This means the AllowRebnewalIfOtherItemsAvailable system preference is now appropriately implemented. Test plan: 1. In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences are both set to 'Dont allow'. 2. Place a hold on an item 3. Checkout the item out to a different patron 4. Renew the item and notice the 'Cannot renew: The item is on hold for another patron' warning 5. Click the 'Override and renew' button and notice that you can bypass the hold 6. Apply patch 7. Try to renew an item (with item level hold on it) which belongs to a biblio with either only one item or a biblio with multiple items all of which have holds on them. Notice the only button in the yellow message box displayed is 'Continue without renewing' 8. Repeat step 7 but change the hold to a 'Next available' hold rather than an item level hold. Again notice only the 'Continue without renewing' button is displayed 9. Try to renew an item (with item level hold on it) which belongs to a biblio with mutliple aviable items of it in the catalogue. Notice only the 'Continue without renewing' button is displayed 10. Repeat step 9 but change the hold to a 'Next available' hold rather than an item level hold. Notice only the 'Continue without renewing' button is displayed 11. Set both AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable systempreferences to 'Allow' and repeat step 7 notice only the 'Continue without renewing' button is displayed in the yellow message box 12. Repeat step 8 and notice only the 'Continue without renewing' button is displayed 13. Repeat step 9 and notice only the 'Continue without renewing' button is displayed 14. Repeat step 10 and notice the item is immediately renewed and the yellow message box is not displayed 15. Set the AllowRenewalIfOtheritemsAvailable to 'Dont' Allow' and repeat step 7 and notice that both the 'Override and renew' and 'Continue without renewal' buttons are displayed 16. Repeat step 8 and notice both buttons are displayed 17. Repeat step 9 and notice both buttons are displayed 18. Repeat step 10 and notice both buttons are displayed 19. Set AllowRenewalIfOtherItemsAvailable syspref to 'Allow' and AllowRenewalLimitOverride syspref to 'Dont' Allow' and repeat step 7 and notice only the 'Continue without renewal' button is displayed 20. Repeat step 8 and notice only 'Continue without renewal' button is displayed 21. Repeat step 9 and notice only the 'Continue without renewal' button is displayed 22. Repeat step 10 and notice both buttons are displayed Sponsored-By: Catalyst IT
Created attachment 69476 [details] [review] Bug 18450 - Followup - Implemented a conditional checking if the item that the patron is trying to renew has more than 1 copy available which can be renewed. This means the AllowRebnewalIfOtherItemsAvailable system preference is now appropriately impleme In order to make the conditional statement work correctly checking three conditions each time (two system preferences and the value of the can_renew variable I had to implement a elsif statement. I spent a large amount of time trying to get a single IF statement to check the three conditions in three different combinations but it ignored the value of the can_renew variable each time and so I was forced to split it out into an If and ELSIF Test plan: 1. In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences are both set to 'Dont allow'. 2. Place a hold on an item 3. Checkout the item out to a different patron 4. Renew the item and notice the 'Cannot renew: The item is on hold for another patron' warning 5. Click the 'Override and renew' button and notice that you can bypass the hold 6. Apply patch 7. Try to renew an item (with item level hold on it) which belongs to a biblio with either only one item or a biblio with multiple items all of which have holds on them. Notice the only button in the yellow message box displayed is 'Continue without renewing' 8. Repeat step 7 but change the hold to a 'Next available' hold rather than an item level hold. Again notice only the 'Continue without renewing' button is displayed 9. Try to renew an item (with item level hold on it) which belongs to a biblio with mutliple aviable items of it in the catalogue. Notice only the 'Continue without renewing' button is displayed 10. Repeat step 9 but change the hold to a 'Next available' hold rather than an item level hold. Notice only the 'Continue without renewing' button is displayed 11. Set both AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable systempreferences to 'Allow' and repeat step 7 notice only the 'Continue without renewing' button is displayed in the yellow message box 12. Repeat step 8 and notice only the 'Continue without renewing' button is displayed 13. Repeat step 9 and notice only the 'Continue without renewing' button is displayed 14. Repeat step 10 and notice the item is immediately renewed and the yellow message box is not displayed 15. Set the AllowRenewalIfOtheritemsAvailable to 'Dont' Allow' and repeat step 7 and notice that both the 'Override and renew' and 'Continue without renewal' buttons are displayed 16. Repeat step 8 and notice both buttons are displayed 17. Repeat step 9 and notice both buttons are displayed 18. Repeat step 10 and notice both buttons are displayed 19. Set AllowRenewalIfOtherItemsAvailable syspref to 'Allow' and AllowRenewalLimitOverride syspref to 'Dont' Allow' and repeat step 7 and notice only the 'Continue without renewal' button is displayed 20. Repeat step 8 and notice only 'Continue without renewal' button is displayed 21. Repeat step 9 and notice only the 'Continue without renewal' button is displayed 22. Repeat step 10 and notice both buttons are displayed Sponsored-By: Catalyst IT
Created attachment 69489 [details] [review] Bug 18450 - Followup - Implemented a conditional checking if the item that the patron is trying to renew has more than 1 copy available which can be renewed. This means the AllowRebnewalIfOtherItemsAvailable system preference is now appropriately impleme In order to make the conditional statement work correctly checking three conditions each time (two system preferences and the value of the can_renew variable I had to implement a elsif statement. I spent a large amount of time trying to get a single IF statement to check the three conditions in three different combinations but it ignored the value of the can_renew variable each time and so I was forced to split it out into an If and ELSIF Test plan: 1. In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences are both set to 'Dont allow'. 2. Place a hold on an item 3. Checkout the item out to a different patron 4. Renew the item and notice the 'Cannot renew: The item is on hold for another patron' warning 5. Click the 'Override and renew' button and notice that you can bypass the hold 6. Apply patch 7. Try to renew an item (with item level hold on it) which belongs to a biblio with either only one item or a biblio with multiple items all of which have holds on them. Notice the only button in the yellow message box displayed is 'Continue without renewing' 8. Repeat step 7 but change the hold to a 'Next available' hold rather than an item level hold. Again notice only the 'Continue without renewing' button is displayed 9. Try to renew an item (with item level hold on it) which belongs to a biblio with mutliple aviable items of it in the catalogue. Notice only the 'Continue without renewing' button is displayed 10. Repeat step 9 but change the hold to a 'Next available' hold rather than an item level hold. Notice only the 'Continue without renewing' button is displayed 11. Set both AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable systempreferences to 'Allow' and repeat step 7 notice only the 'Continue without renewing' button is displayed in the yellow message box 12. Repeat step 8 and notice only the 'Continue without renewing' button is displayed 13. Repeat step 9 and notice only the 'Continue without renewing' button is displayed 14. Repeat step 10 and notice the item is immediately renewed and the yellow message box is not displayed 15. Set the AllowRenewalIfOtheritemsAvailable to 'Dont' Allow' and repeat step 7 and notice that both the 'Override and renew' and 'Continue without renewal' buttons are displayed 16. Repeat step 8 and notice both buttons are displayed 17. Repeat step 9 and notice both buttons are displayed 18. Repeat step 10 and notice both buttons are displayed 19. Set AllowRenewalIfOtherItemsAvailable syspref to 'Allow' and AllowRenewalLimitOverride syspref to 'Dont' Allow' and repeat step 7 and notice only the 'Continue without renewal' button is displayed 20. Repeat step 8 and notice only 'Continue without renewal' button is displayed 21. Repeat step 9 and notice only the 'Continue without renewal' button is displayed 22. Repeat step 10 and notice both buttons are displayed Sponsored-By: Catalyst IT
Patch tested with a sandbox, by Laurence Rault <laurence.rault@biblibre.com>
Created attachment 72835 [details] [review] Bug 18450 - Implemented check in renew.tt for block and renewal limits Test plan: 1. In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences are both set to 'Dont allow'. 2. Place a hold on an item from a patron 3. Checkout the item out to a different patron 4. Renew the item and notice the 'Cannot renew: The item is on hold for another patron' warning 5. Click the 'Override and renew' button and notice that you can bypass the hold 6. Apply patch 7. Renew the item again and notice that the message does not have a 'Override and renew' button it just has the 'Ignore and continue' button 8. Go back to More->Administration->Global system preferences and set AllowRenewalLimitOverride to 'Allow' 9. Try renewing the same item again and notice that the 'Override and renew' button is displayed 10. In systems preferences now set the AllowRenewalIfOtherItemsAvailable syspref to 'Allow' and AllowRenewalLimitOverride to 'Dont allow' and notice the 'Override and renew' button is displayed 11. Now set both these sysprefs to 'Allow' and notice the 'Override and renew' button is displayed because you will not be violating these preferences. Note: Checking if the renewal exceeds the renewal level, if auto renew is set up, or if renewal is to soon is checked earlier in this conditional if statement earlier and so the check in this patch does not need to check the renewal level, auto renewal, if renewal is too soon. Signed-off-by: Lee Jamison <ldjamison@marywood.edu> Signed-off-by: Laurence Rault <laurence.rault@biblibre.com>
Created attachment 72836 [details] [review] Bug 18450 - Followup - Implemented a conditional checking if the item that the patron is trying to renew has more than 1 copy available which can be renewed. This means the AllowRebnewalIfOtherItemsAvailable system preference is now appropriately implemen In order to make the conditional statement work correctly checking three conditions each time (two system preferences and the value of the can_renew variable I had to implement a elsif statement. I spent a large amount of time trying to get a single IF statement to check the three conditions in three different combinations but it ignored the value of the can_renew variable each time and so I was forced to split it out into an If and ELSIF Test plan: 1. In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences are both set to 'Dont allow'. 2. Place a hold on an item 3. Checkout the item out to a different patron 4. Renew the item and notice the 'Cannot renew: The item is on hold for another patron' warning 5. Click the 'Override and renew' button and notice that you can bypass the hold 6. Apply patch 7. Try to renew an item (with item level hold on it) which belongs to a biblio with either only one item or a biblio with multiple items all of which have holds on them. Notice the only button in the yellow message box displayed is 'Continue without renewing' 8. Repeat step 7 but change the hold to a 'Next available' hold rather than an item level hold. Again notice only the 'Continue without renewing' button is displayed 9. Try to renew an item (with item level hold on it) which belongs to a biblio with mutliple aviable items of it in the catalogue. Notice only the 'Continue without renewing' button is displayed 10. Repeat step 9 but change the hold to a 'Next available' hold rather than an item level hold. Notice only the 'Continue without renewing' button is displayed 11. Set both AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable systempreferences to 'Allow' and repeat step 7 notice only the 'Continue without renewing' button is displayed in the yellow message box 12. Repeat step 8 and notice only the 'Continue without renewing' button is displayed 13. Repeat step 9 and notice only the 'Continue without renewing' button is displayed 14. Repeat step 10 and notice the item is immediately renewed and the yellow message box is not displayed 15. Set the AllowRenewalIfOtheritemsAvailable to 'Dont' Allow' and repeat step 7 and notice that both the 'Override and renew' and 'Continue without renewal' buttons are displayed 16. Repeat step 8 and notice both buttons are displayed 17. Repeat step 9 and notice both buttons are displayed 18. Repeat step 10 and notice both buttons are displayed 19. Set AllowRenewalIfOtherItemsAvailable syspref to 'Allow' and AllowRenewalLimitOverride syspref to 'Dont' Allow' and repeat step 7 and notice only the 'Continue without renewal' button is displayed 20. Repeat step 8 and notice only 'Continue without renewal' button is displayed 21. Repeat step 9 and notice only the 'Continue without renewal' button is displayed 22. Repeat step 10 and notice both buttons are displayed Sponsored-By: Catalyst IT Signed-off-by: Laurence Rault <laurence.rault@biblibre.com>
The second patch here confuses me - the if/elsif seem to generate the same button for either condition set? In the case of: NextAvailable hold on record with another item available AllowRenewalIfOtherItemsAvailable=Don't Allow I am offered to override and renew but I believe it should be blocked Second commit message needs a cleanup as well
Created attachment 86587 [details] [review] Bug 18450: Mades changes for QA test tool and old code missed
Created attachment 86588 [details] [review] Bug 18450: (Follow up) Only show Override button for renewals according to relevant sysprefs This patch is a follow up which corrects the conditional that uses the system preferences AllowRenewalIfOtherItemsAvailable and AllowRenewalLimitOverridecontrols to control the display of the 'Override and renew' button, in addition to the 'Continue without renewing' button, when renewing an item that has been placed on hold. This patch also adjusts the expected results for each step in the test plan outlined in the previous patches to align better with the system preferences. Test plan: --NOTE-- The following renewals must be completed in Circulation->Renew 1) Apply the patch, and ensure the following steps work as described 2) In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences are both set to 'Don't allow' 3) Item Level/Only one available item: Try to renew an item (with item level hold on it) which belongs to a biblio with only one item. Note the only button in the yellow message box displayed is 'Continue without renewing' 4) Record Level/Only one available item: Try to renew an item (this time with a 'Next Available' hold on the record) which belongs to a biblio with only one item. Note the only button in the yellow message box displayed is 'Continue without renewing' 5) Item Level/Multiple available items: Try to renew an item (with item level hold on it) which belongs to a biblio with multiple available items of it in the catalogue. Note only the 'Continue without renewing' button is displayed 6) Record Level/Multiple available items: Try to renew an item (this time with a 'Next Available' hold on the record) which belongs to a biblio with multiple available items of it in the catalogue. Note only the 'Continue without renewing' button is displayed 7) Set both AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable system preferences to 'Allow' 8) Repeat step 3, and note only the 'Continue without renewing' button is displayed in the yellow message box 9) Repeat step 4, and note the only button in the yellow message box displayed is 'Continue without renewing' 10) Repeat step 5, note only the 'Continue without renewing' button is displayed (because the hold is for that specific item) 11) Repeat step 6, note the item is immediately renewed and the yellow message box is not displayed 12) Set the AllowRenewalIfOtheritemsAvailable to 'Don't Allow' and repeat steps 3-6, and note that only the 'Continue without renewing' button is displayed 13) Set the AllowRenewalIfOtherItemsAvailable system preference back to 'Allow' and AllowRenewalLimitOverride syspref to 'Don't Allow', and repeat steps 3-5 and note only the 'Continue without renewal' button is displayed. 14) Repeat step 6 and note that the item is immediately renewed with no yellow warning box. Sponsored-by: Catalyst IT
Created attachment 86589 [details] [review] Bug 18450: Mades changes for QA test tool and old code missed
Created attachment 86590 [details] [review] Bug 18450: Only show Override button for renewals according to relevant sysprefs This patch is a follow up which corrects the conditional that uses the system preferences AllowRenewalIfOtherItemsAvailable and AllowRenewalLimitOverridecontrols to control the display of the 'Override and renew' button, in addition to the 'Continue without renewing' button, when renewing an item that has been placed on hold. This patch also adjusts the expected results for each step in the test plan outlined in the previous patches to align better with the system preferences. Test plan: --NOTE-- The following renewals must be completed in Circulation->Renew 1) Apply the patch, and ensure the following steps work as described 2) In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable preferences are both set to 'Don't allow' 3) Item Level/Only one available item: Try to renew an item (with item level hold on it) which belongs to a biblio with only one item. Note the only button in the yellow message box displayed is 'Continue without renewing' 4) Record Level/Only one available item: Try to renew an item (this time with a 'Next Available' hold on the record) which belongs to a biblio with only one item. Note the only button in the yellow message box displayed is 'Continue without renewing' 5) Item Level/Multiple available items: Try to renew an item (with item level hold on it) which belongs to a biblio with multiple available items of it in the catalogue. Note only the 'Continue without renewing' button is displayed 6) Record Level/Multiple available items: Try to renew an item (this time with a 'Next Available' hold on the record) which belongs to a biblio with multiple available items of it in the catalogue. Note only the 'Continue without renewing' button is displayed 7) Set both AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvaliable system preferences to 'Allow' 8) Repeat step 3, and note only the 'Continue without renewing' button is displayed in the yellow message box 9) Repeat step 4, and note the only button in the yellow message box displayed is 'Continue without renewing' 10) Repeat step 5, note only the 'Continue without renewing' button is displayed (because the hold is for that specific item) 11) Repeat step 6, note the item is immediately renewed and the yellow message box is not displayed 12) Set the AllowRenewalIfOtheritemsAvailable to 'Don't Allow' and repeat steps 3-6, and note that only the 'Continue without renewing' button is displayed 13) Set the AllowRenewalIfOtherItemsAvailable system preference back to 'Allow' and AllowRenewalLimitOverride syspref to 'Don't Allow', and repeat steps 3-5 and note only the 'Continue without renewal' button is displayed. 14) Repeat step 6 and note that the item is immediately renewed with no yellow warning box. Sponsored-by: Catalyst IT
Created attachment 86591 [details] [review] Bug 18450: Mades changes for QA test tool and old code missed
Did this get fixed somewhere else already.. looks like both patches are applied but I can't seen any reference to them in the git logs.
(In reply to Martin Renvoize from comment #31) > Did this get fixed somewhere else already.. looks like both patches are > applied but I can't seen any reference to them in the git logs. This has not yet been fixed in the master, which I have verified by following the test plan on a branch off the master this morning. I will fix the patch so it applies now so it can be tested. This patch builds off Alex's previous commits, so maybe they're further back in the git log? Unsure.
Created attachment 87337 [details] [review] Bug 18450: Only allow Override for renewals according to relevant sysprefs This patch adds a conditional which refers to the system preferences AllowRenewalIfOtherItemsAvailable and AllowRenewalLimitOverride to control the display of the 'Override and renew' button, in addition to the 'Continue without renewing', when renewing an item that has been placed on hold. Test plan: --NOTE-- The following renewals must be completedd in Circulation->Renew 1) Apply the patch, and ensure the following steps work as described 2) In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvailable preferences are both set to 'Don't allow' 3) Item levelOnly one available item: Try to renew an item (with item level hold on it) which belongs to a biblio with only one item. Note the only button in the yellow alert box displayed is 'Continue without renewing' 4) Record level/Only one available item: Try to renew an item (this time with a 'Next Available' hold on the record) which belongs to a biblio with only one item. Note the only button in the yellow alert box displayed is 'Continue without renewing' 5) Item level/Multiple available items: Try to renew an item (with item level hold on it) which belongs to a biblio with multiple available items of it in the catalogue. Note only the 'Continue without renewing' button is displayed 6) Record level/Multiple available items: Try to renew an item (with item level hold on it) which belongs to a biblio with multiple available items of it in the catalogue. Note only the 'Continue without renewing' button is displayed 7) Set both AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvailable system preferences to 'Allow' 8) Repeat step 3, and note only the 'Continue without renewing' button is displayed in the yellow alert box 9) Repeat step 4, and note the only button in the yellow alert box displayed is 'Continue without renewing' 10) Repeat step 5, not only the 'Continue without renewing' button is displayed (because the hold is for that specific item) 11) Repeat step 6, note the item is immediately renewed and the yellow alert box is not displayed 12) Set the AllowRenewalIfOtherItemsAvailable to 'Don't Allow' and repeat steps 3-6, and note that only the 'Continue without renewing' button is displayed 13) Set the AllowRenewalIfOtherItemsAvailable system preference back to 'Allow' and AllowRenewalLimitOverride syspref to 'Don't Allow', and repeat steps 3-5 and note only the 'Continue without renewal' button is displayed. 14) Repeat step 6 and note that the item is immediately renewed with no yellow warning box. Sponsored-by: Catalyst IT
Created attachment 87338 [details] [review] Bug 18450: (follow up) Fixed forbidden patterns for qa tool Sponsored-by: Catalyst IT
Created attachment 87528 [details] [review] Bug 18450: Only allow Override for renewals according to relevant sysprefs This patch adds a conditional which refers to the system preferences AllowRenewalIfOtherItemsAvailable and AllowRenewalLimitOverride to control the display of the 'Override and renew' button, in addition to the 'Continue without renewing', when renewing an item that has been placed on hold. Test plan: --NOTE-- The following renewals must be completedd in Circulation->Renew 1) Apply the patch, and ensure the following steps work as described 2) In More->Administration->Global system preferences ensure that both the AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvailable preferences are both set to 'Don't allow' 3) Item levelOnly one available item: Try to renew an item (with item level hold on it) which belongs to a biblio with only one item. Note the only button in the yellow alert box displayed is 'Continue without renewing' 4) Record level/Only one available item: Try to renew an item (this time with a 'Next Available' hold on the record) which belongs to a biblio with only one item. Note the only button in the yellow alert box displayed is 'Continue without renewing' 5) Item level/Multiple available items: Try to renew an item (with item level hold on it) which belongs to a biblio with multiple available items of it in the catalogue. Note only the 'Continue without renewing' button is displayed 6) Record level/Multiple available items: Try to renew an item (with item level hold on it) which belongs to a biblio with multiple available items of it in the catalogue. Note only the 'Continue without renewing' button is displayed 7) Set both AllowRenewalLimitOverride and AllowRenewalIfOtherItemsAvailable system preferences to 'Allow' 8) Repeat step 3, and note only the 'Continue without renewing' button is displayed in the yellow alert box 9) Repeat step 4, and note the only button in the yellow alert box displayed is 'Continue without renewing' 10) Repeat step 5, not only the 'Continue without renewing' button is displayed (because the hold is for that specific item) 11) Repeat step 6, note the item is immediately renewed and the yellow alert box is not displayed 12) Set the AllowRenewalIfOtherItemsAvailable to 'Don't Allow' and repeat steps 3-6, and note that only the 'Continue without renewing' button is displayed 13) Set the AllowRenewalIfOtherItemsAvailable system preference back to 'Allow' and AllowRenewalLimitOverride syspref to 'Don't Allow', and repeat steps 3-5 and note only the 'Continue without renewal' button is displayed. 14) Repeat step 6 and note that the item is immediately renewed with no yellow warning box. Sponsored-by: Catalyst IT Signed-off-by: Liz Rea <wizzyrea@gmail.com> that is a well thorough test plan. All went as expected.
Created attachment 87529 [details] [review] Bug 18450: (follow up) Fixed forbidden patterns for qa tool Sponsored-by: Catalyst IT Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Hi Hayley, looking at the conditions I noticed the check on AllowRenewalLimitOverride. I fear things are even a bit more complicated :( AllowRenewalLimitOverride works only, if the patron also is superlibrarian or has the override_renewals permission. Could you take a look?
Is still relevant?
I just tested this on master: * Checked out an item * Placed a hold on it for another user * Tried to renew it using the renewal tab * Got asked to override AllowRenewalOnHoldOverride and AllowRenewalLimitOverride are set to don't allow. So I believe this to be incorrect behaviour.
*** Bug 22676 has been marked as a duplicate of this bug. ***