When OpacRenewalAllowed is set to "Don't Allow," Patron messages in the OPAC regarding renewals should probably not be displayed. (ie, "Since you have more than XXX.XX in fines, you cannot renew your books online. Please pay your fines if you wish to renew your books.") :T
Created attachment 6094 [details] [review] Bug 7110 - Renewal messages still display in OPAC if OpacRenewalAllowed is disabled This patch adds a check for OpacRenewalAllowed to the computation of the renewal_blocked_fines. If OpacRenewalAllowed is off, renewal_blocked_fines won't be computed, because that message shouldn't show in the OPAC anyway. This also removes the necessity to set a very high threshold on OPACFineNoRenewals to prevent the messages from showing with OpacRenewal disabled. To test: Set a patron with fines over the stated limit in OPACFineNoRenewals. With OpacRenewalAllowed set to ALLOW - log into the OPAC and verify the message that renewals are disallowed is displayed - verify that renewals are disallowed in the interface. - remove or pay the fines, verify the message goes away and that OPAC renewals are re-allowed for your patron. With OPACFineNoRenewals set to Don't Allow - verify the message is not shown, even with the patron's fines over the threshold in OPACFineNoRenewals. - remove or pay the fines, verify that renewals are still disallowed
Created attachment 6103 [details] [review] Bug 7110 - Renewal messages still display in OPAC if OpacRenewalAllowed is disabled This patch adds a check for OpacRenewalAllowed to the computation of the renewal_blocked_fines. If OpacRenewalAllowed is off, renewal_blocked_fines won't be computed, because that message shouldn't show in the OPAC anyway. This also removes the necessity to set a very high threshold on OPACFineNoRenewals to prevent the messages from showing with OpacRenewal disabled. To test: Set a patron with fines over the stated limit in OPACFineNoRenewals. With OpacRenewalAllowed set to ALLOW - log into the OPAC and verify the message that renewals are disallowed is displayed - verify that renewals are disallowed in the interface. - remove or pay the fines, verify the message goes away and that OPAC renewals are re-allowed for your patron. With OPACFineNoRenewals set to Don't Allow - verify the message is not shown, even with the patron's fines over the threshold in OPACFineNoRenewals. - remove or pay the fines, verify that renewals are still disallowed Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
QA Comment: warning: 2 lines add whitespace errors. The two tests in opac-user and the template are not completely in harmony. Note that you only set renewal_blocked_fines if fines higher than threshold and opac renewal is allowed, but the template wants to warn also if there are higher fines. Further take into account that flagged could be 1 for another reason and that could result in a wrong message from the else part in the template. So, I think that the test in opac-user should not include OpacRenewal. Leave it as it was. And rewrite the checks in the template: if Renewal allowed and higher fines then message 1 elseif Higher fines then message 2 else nothing. Hopefully, I was clear enough ;) Thanks your further attention to these details.
Created attachment 7709 [details] [review] Bug 7110 - Renewal messages still displays in OPAC if OpacRenewalAllowed is disabled. To test: Set a patron with fines over the stated limit in OPACFineNoRenewals. With OpacRenewalAllowed set to ALLOW - log into the OPAC and verify the message that renewals are disallowed is displayed - verify that renewals are disallowed in the interface. - remove or pay the fines, verify the message goes away and that OPAC renewals are re-allowed for your patron. With OPACFineNoRenewals set to Don't Allow - verify the message is not shown, even with the patron's fines over the threshold in OPACFineNoRenewals. - remove or pay the fines, verify that renewals are still disallowed
Created attachment 7765 [details] [review] [SIGNED-OFF] Bug 7110 - Renewal messages still displays in OPAC if OpacRenewalAllowed is disabled. To test: Set a patron with fines over the stated limit in OPACFineNoRenewals. With OpacRenewalAllowed set to ALLOW - log into the OPAC and verify the message that renewals are disallowed is displayed - verify that renewals are disallowed in the interface. - remove or pay the fines, verify the message goes away and that OPAC renewals are re-allowed for your patron. With OPACFineNoRenewals set to Don't Allow - verify the message is not shown, even with the patron's fines over the threshold in OPACFineNoRenewals. - remove or pay the fines, verify that renewals are still disallowed Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Stepped through the test plan and found no problems.
Template only change providing contextual fallback for newly differentiated situation. Marking Passed QA.
This patch does not apply cleanly to 3.6.x. If it is intended to, please submit a patch rebased on 3.6.x.
If this change made it into 3.8.x it seems to have been overwritten since.
(In reply to comment #8) > If this change made it into 3.8.x it seems to have been overwritten since. Turns out it was overwritten by a follow-up on Bug 8408.
Created attachment 13706 [details] [review] Bug 7110 - Renewal messages still displays in OPAC if OpacRenewalAllowed is disabled The original patch added a message to display if OpacRenewalAllowed is disabled but OPACFineNoRenewals is enabled. This change was reversed by a follow-up on Bug 8408. I think both versions are incorrect: If a patron has fines which exceed the limit set by OPACFineNoRenewals but OPAC renewals are disallowed by OpacRenewalAllowed, no message should be displayed. Information about outstanding fines in this case has no bearing on how the user sees his summary of checkouts. If OpacRenewalAllowed is diabled and a patron's fines exceed the limit set by OPACFineNoRenewals they should see no message.
Created attachment 18052 [details] [review] Bug 7110 - Renewal messages still displays in OPAC if OpacRenewalAllowed is disabled The original patch added a message to display if OpacRenewalAllowed is disabled but OPACFineNoRenewals is enabled. This change was reversed by a follow-up on Bug 8408. I think both versions are incorrect: If a patron has fines which exceed the limit set by OPACFineNoRenewals but OPAC renewals are disallowed by OpacRenewalAllowed, no message should be displayed. Information about outstanding fines in this case has no bearing on how the user sees his summary of checkouts. To test, try various combinations of OpacRenewalAllowed and OPACFineNoRenewals with a patron who has outstanding fines: - OpacRenewalAllowed ON and OPACFineNoRenewals ON (set to be triggered by the test patron's fines): Logging in to the OPAC the patron should see a warning on opac-user.pl about not being able to renew items because of fines. - OpacRenewalAllowed ON and OPACFineNoRenewals OFF (threshold high enough not to trigger a block): No warning appears. - OpacRenewalAllowed OFF and OPACFineNoRenewals ON: No warning appears. - OpacRenewalAllowed OFF and OPACFineNoRenewals OFF: No warning appears. If OpacRenewalAllowed is diabled and a patron's fines exceed the limit set by OPACFineNoRenewals they should see no message.
Created attachment 18488 [details] [review] Bug 7110 - Renewal messages still displays in OPAC if OpacRenewalAllowed is disabled The original patch added a message to display if OpacRenewalAllowed is disabled but OPACFineNoRenewals is enabled. This change was reversed by a follow-up on Bug 8408. I think both versions are incorrect: If a patron has fines which exceed the limit set by OPACFineNoRenewals but OPAC renewals are disallowed by OpacRenewalAllowed, no message should be displayed. Information about outstanding fines in this case has no bearing on how the user sees his summary of checkouts. To test, try various combinations of OpacRenewalAllowed and OPACFineNoRenewals with a patron who has outstanding fines: - OpacRenewalAllowed ON and OPACFineNoRenewals ON (set to be triggered by the test patron's fines): Logging in to the OPAC the patron should see a warning on opac-user.pl about not being able to renew items because of fines. - OpacRenewalAllowed ON and OPACFineNoRenewals OFF (threshold high enough not to trigger a block): No warning appears. - OpacRenewalAllowed OFF and OPACFineNoRenewals ON: No warning appears. - OpacRenewalAllowed OFF and OPACFineNoRenewals OFF: No warning appears. If OpacRenewalAllowed is diabled and a patron's fines exceed the limit set by OPACFineNoRenewals they should see no message. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 19019 [details] [review] Bug 7110 - Renewal messages still displays in OPAC if OpacRenewalAllowed is disabled The original patch added a message to display if OpacRenewalAllowed is disabled but OPACFineNoRenewals is enabled. This change was reversed by a follow-up on Bug 8408. I think both versions are incorrect: If a patron has fines which exceed the limit set by OPACFineNoRenewals but OPAC renewals are disallowed by OpacRenewalAllowed, no message should be displayed. Information about outstanding fines in this case has no bearing on how the user sees his summary of checkouts. To test, try various combinations of OpacRenewalAllowed and OPACFineNoRenewals with a patron who has outstanding fines: - OpacRenewalAllowed ON and OPACFineNoRenewals ON (set to be triggered by the test patron's fines): Logging in to the OPAC the patron should see a warning on opac-user.pl about not being able to renew items because of fines. - OpacRenewalAllowed ON and OPACFineNoRenewals OFF (threshold high enough not to trigger a block): No warning appears. - OpacRenewalAllowed OFF and OPACFineNoRenewals ON: No warning appears. - OpacRenewalAllowed OFF and OPACFineNoRenewals OFF: No warning appears. If OpacRenewalAllowed is diabled and a patron's fines exceed the limit set by OPACFineNoRenewals they should see no message. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Passes koha-ql.pl and perlcritic
Pushed to master. Thanks, Owen! I could see a library wanting the patron summary to display a warning box if the patron's fine balance exceeds a specified threshold, regardless of the OPAC renewal settings, but I think that would be a separate enhancement.
This patch has been pushed to 3.12.x, will be in 3.12.2. Thanks Owen!
Pushed to 3.10.x, will be in 3.10.8