Bug 12811

Summary: Patron 'Details' and 'Check out' pages not working properly for staff users which don't have renewal override permissions
Product: Koha Reporter: Jacek Ablewicz <abl>
Component: CirculationAssignee: Jacek Ablewicz <abl>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: gmcharlt, katrin.fischer, kyle.m.hall, kyle, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12915
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12878
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 11703    
Bug Blocks: 12920    
Attachments: Bug 12811 - Patron 'Details' and 'Check out' pages not working properly for staff users which don't have renewal override permissions
[SIGNED-OFF] Bug 12811 - Patron 'Details' and 'Check out' pages not working for staff users without renewal override permissions
[PASSED QA] Bug 12811 - Patron 'Details' and 'Check out' pages not working for staff users without renewal override permissions

Description Jacek Ablewicz 2014-08-23 14:28:10 UTC
In case when the staff user doesn't have (circulate) -> (override_renewals) permission granted, this code part

   var AllowRenewalLimitOverride = [% CAN_user_circulate_override_renewals && AllowRenewalLimitOverride %];

in circ/circulation.tt and members/moremember.tt leads to javascript error, because TT statement evaluates to empty string.

To reproduce:

- set AllowRenewalLimitOverride syspref to "Don't allow" (this step is possibly redundant / not quite relevant)
- for testing purposes, log in as some (sample) staff user whitch does not have permission granted for "(override_renewals) Override blocked renewals"
- have a look at some patron accounts (preferably, ones with 1+ check-out), observe that page layout is incorrect in "Check Out" and "Details" tab.
Comment 1 Jacek Ablewicz 2014-08-23 14:46:04 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2014-08-27 17:00:56 UTC Comment hidden (obsolete)
Comment 3 Jacek Ablewicz 2014-09-06 08:55:51 UTC
*** Bug 12878 has been marked as a duplicate of this bug. ***
Comment 4 Katrin Fischer 2014-09-07 21:08:23 UTC
Created attachment 31439 [details] [review]
[PASSED QA] Bug 12811 - Patron 'Details' and 'Check out' pages not working for staff users without renewal override permissions

In case when the staff user doesn't have (circulate) ->
(override_renewals) permission granted, this code part

   var AllowRenewalLimitOverride = [%
CAN_user_circulate_override_renewals && AllowRenewalLimitOverride %];

in circ/circulation.tt and members/moremember.tt leads to javascript
error, because TT statement evaluates to empty string.

To reproduce:

- set AllowRenewalLimitOverride syspref to "Don't allow" (this step is
  possibly redundant / not quite relevant),
- for testing purposes, log in as some (sample) staff user whitch does
  not have permission granted for "(override_renewals) Override blocked
  renewals",
- have a look at some patron accounts (preferably, ones with 1+
  check-out), observe that page layout is incorrect in "Check Out" and
  "Details" tab.

To test:

- apply patch,
- retest and ensure that this issue is no longer reproductible,
- make sure that there are no regressions of any kinds regarding renewal
  override permission for staff users (i.e, that it still works like
  intended).

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, passes QA script and tests.
Comment 5 Kyle M Hall 2014-09-08 17:12:55 UTC
Just an fyi, but I believe AllowRenewalLimitOverride should actually be Koha.Preference('AllowRenewalLimitOverride'). My patch ( in the duplicate bug ) resolves this issue. So we should either push that patch instead, or file a further bug so we can fix that in a followup report.
Comment 6 Jacek Ablewicz 2014-09-09 07:05:39 UTC
(In reply to Kyle M Hall from comment #5)
> Just an fyi, but I believe AllowRenewalLimitOverride should actually be
> Koha.Preference('AllowRenewalLimitOverride'). My patch ( in the duplicate
> bug ) resolves this issue. 

I agree it would be better to use Koha.Preference() in both affected templates, but patch in bug 12878 appears to be incomplete:

- circ/circulation.tt: renewal override is not working properly with this patch applied (due to missing [% USE Koha %] ?),
- Koha.Preference() call is not being used in patched members/moremember.tt - any particular reason to treat this template differently then circ/circulation.tt?
Comment 7 Tomás Cohen Arazi 2014-09-16 19:12:31 UTC
Patch pushed to master.

Thanks Jacek!

Note: I'd prefer that each new patch uses Koha.Preference instead. Yours doesn't introduce the syspref-from-pl situation, so I'm pushing it because is a sensitive issue for the stable branches.

Thanks!
Comment 8 Galen Charlton 2014-09-23 20:35:34 UTC
Pushed to 3.16.x for inclusion in 3.16.4.