Bug 6598 - OPACFineNoRenewals syspreference does not stop user renewing in opac
Summary: OPACFineNoRenewals syspreference does not stop user renewing in opac
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: 3.6
Hardware: All All
: P3 normal (vote)
Target Milestone: ---
Assignee: Owen Leonard
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-18 11:03 UTC by Colin Campbell
Modified: 2013-12-05 19:59 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Proposed Patch (6.51 KB, patch)
2011-07-18 11:52 UTC, Colin Campbell
Details | Diff | Splinter Review
Proposed fix (7.77 KB, patch)
2011-10-11 19:17 UTC, Owen Leonard
Details | Diff | Splinter Review
Signed-off (7.78 KB, patch)
2011-11-04 06:04 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Revised fix (7.15 KB, patch)
2012-01-17 21:09 UTC, Owen Leonard
Details | Diff | Splinter Review
Screenshot showing view without the patch (17.13 KB, image/png)
2012-02-13 14:29 UTC, Owen Leonard
Details
Screenshot showing view after the patch (17.31 KB, image/png)
2012-02-13 14:31 UTC, Owen Leonard
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Colin Campbell 2011-07-18 11:03:34 UTC
This option is intended to stop users with fines from using opac to renew their books. Because the global variable patron_flagged is mistakenly taken to be a variable within larger structures in a couple of places the restriction is never applied. The opac display is also affected (a bogus table column is added and an empty message box is displayed)

Fixing the above logic reveals two further bugs to address:
The Renew Selected button is displayed even although you cannot select any for renewal.

The option cannot be turned off. When editing the syspref it says 'leave blank to disable' as that is read in the script as the value 0.00 it disables renewing for all borrowers with fines. It needs to be set to high value to allow (relatively) unrestricted opac renewals
Comment 1 Colin Campbell 2011-07-18 11:52:31 UTC Comment hidden (obsolete)
Comment 2 Nicole C. Engard 2011-07-18 12:04:48 UTC
Actually it fixed another bug as well which was that the columns were mislabeled if you have the barcode and the 'links' column appearing. I have signed off on this patch!
Comment 3 Chris Cormack 2011-08-01 01:21:56 UTC
Easily tested, works well, pushing. Please test and mark resolved
Comment 4 Nicole C. Engard 2011-10-11 18:05:42 UTC
I think this bug is back ... 

Patron was allowed to renew at home with late items. Our system setting is set
to no renew if more than .01.

The patron was at home and used IE. She told staff the it said it would not
renew because of fine, but then when she hit OK it renewed. That's all the info
I have.

I tested in FF7 on Mac and it prevents renewals, so I think it's a browser compatibility issue.
Comment 5 Owen Leonard 2011-10-11 19:17:22 UTC Comment hidden (obsolete)
Comment 6 Brendan Gallagher 2011-11-04 06:04:31 UTC Comment hidden (obsolete)
Comment 7 Marcel de Rooy 2011-12-01 08:06:45 UTC
Patch does not apply anymore. Automerge still works, but result needs some editing since a few warnings have been added in the meantime..
Please send a new version; leave the status at Signed Off.
Comment 8 Paul Poulain 2011-12-13 14:05:42 UTC
changed to "does not apply", once you've submitted an updated bug, switch back to signed-off, no problem with that
Comment 9 Owen Leonard 2012-01-17 21:09:51 UTC
Created attachment 7204 [details] [review]
Revised fix
Comment 10 Marcel de Rooy 2012-02-03 10:51:51 UTC
QA Comment: Looks good (without testing it). Well documented change, improving code. Marking as Passed QA.
Comment 11 Paul Poulain 2012-02-03 14:10:19 UTC
Sorry, I wanted to test before pushing, and could not understand which test data I must have to see how the fix works.

Please give a full test plan and switch back to passed QA to reach me directly.
Comment 12 Owen Leonard 2012-02-13 14:29:36 UTC
Created attachment 7619 [details]
Screenshot showing view without the patch

Before this patch the user logged in to the OPAC is correctly told that they cannot renew items online because of their fine, but renewal-related controls are displayed (renew selected, renew all).
Comment 13 Owen Leonard 2012-02-13 14:31:24 UTC
Created attachment 7620 [details]
Screenshot showing view after the patch

After the patch renewal controls are no longer displayed.
Comment 14 Jared Camins-Esakov 2012-05-23 13:01:06 UTC
This fix was included in the 3.6.x branch prior to 3.6.4.