Bug 7110 - Renewal messages still displays in OPAC if OpacRenewalAllowed is disabled.
Summary: Renewal messages still displays in OPAC if OpacRenewalAllowed is disabled.
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: 3.14
Hardware: All All
: P3 normal (vote)
Assignee: Owen Leonard
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-27 20:49 UTC by Thatcher Rea
Modified: 2014-12-07 20:02 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 7110 - Renewal messages still display in OPAC if OpacRenewalAllowed is disabled (3.56 KB, patch)
2011-10-28 16:07 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 7110 - Renewal messages still display in OPAC if OpacRenewalAllowed is disabled (3.57 KB, patch)
2011-10-28 19:42 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 7110 - Renewal messages still displays in OPAC if OpacRenewalAllowed is disabled. (2.07 KB, patch)
2012-02-17 13:59 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7110 - Renewal messages still displays in OPAC if OpacRenewalAllowed is disabled. (2.19 KB, patch)
2012-02-19 18:38 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7110 - Renewal messages still displays in OPAC if OpacRenewalAllowed is disabled (2.17 KB, patch)
2012-11-27 16:34 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 7110 - Renewal messages still displays in OPAC if OpacRenewalAllowed is disabled (3.40 KB, patch)
2013-05-09 20:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 7110 - Renewal messages still displays in OPAC if OpacRenewalAllowed is disabled (3.42 KB, patch)
2013-05-30 04:15 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 7110 - Renewal messages still displays in OPAC if OpacRenewalAllowed is disabled (3.50 KB, patch)
2013-06-14 16:37 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Thatcher Rea 2011-10-27 20:49:13 UTC
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
Comment 1 Liz Rea 2011-10-28 16:07:31 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2011-10-28 19:42:22 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2011-11-24 13:01:24 UTC
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.
Comment 4 Kyle M Hall 2012-02-17 13:59:32 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2012-02-19 18:38:30 UTC Comment hidden (obsolete)
Comment 6 Ian Walls 2012-02-24 20:35:26 UTC
Template only change providing contextual fallback for newly differentiated situation.  Marking Passed QA.
Comment 7 Jared Camins-Esakov 2012-05-23 23:37:06 UTC
This patch does not apply cleanly to 3.6.x. If it is intended to, please submit a patch rebased on 3.6.x.
Comment 8 Owen Leonard 2012-11-27 16:07:36 UTC
If this change made it into 3.8.x it seems to have been overwritten since.
Comment 9 Owen Leonard 2012-11-27 16:28:16 UTC
(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.
Comment 10 Owen Leonard 2012-11-27 16:34:04 UTC Comment hidden (obsolete)
Comment 11 Owen Leonard 2013-05-09 20:00:52 UTC Comment hidden (obsolete)
Comment 12 Chris Cormack 2013-05-30 04:15:27 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall 2013-06-14 16:37:12 UTC
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
Comment 14 Galen Charlton 2013-06-19 16:46:25 UTC
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.
Comment 15 Tomás Cohen Arazi 2013-07-15 16:42:50 UTC
This patch has been pushed to 3.12.x, will be in 3.12.2.

Thanks Owen!
Comment 16 Bernardo Gonzalez Kriegel 2013-07-16 11:52:22 UTC
Pushed to 3.10.x, will be in 3.10.8