Allow patron renewal on max fines for some libraries, disallow it for others

Bug #1411819 reported by Jeff Davis
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned

Bug Description

Some of our libraries want to allow their patrons to renew items when they have hit their max fine limit. Other libraries want to block those renewals. This is currently a global setting; we'd like it to be configurable at the org unit level.

Here's my proposal. Currently, when a patron hits the max fine limit, a standing penalty is applied to their account. By default, that penalty (PATRON_EXCEEDS_FINES) is configured to block renewals. If the patron subsequently attempts to renew items, there is a database function (action.item_user_circ_test) that checks the patron's standing penalties to see if any of them should block the renewal. I'd like to modify this database function so that, if the patron has the PATRON_EXCEEDS_FINES penalty, it looks up a new org setting ("circ.max_fine.allow_renew"?) to decide whether to override the block and permit the renewal.

Jeff Godin (jgodin)
Changed in evergreen:
importance: Undecided → Wishlist
Revision history for this message
Jeff Davis (jdavis-sitka) wrote :

Alternative approaches:

1. Mentioned by Thomas Berezansky: Add another "result" field in the circ matrix for ignoring penalty blocks. Then you wouldn't be limited to just "by org unit" but could mix in other details like patron type.

2. Mentioned by Mike Rylander: Add a "penalty event map." Each org could map an event to a penalty, including custom ones. Default to the current hard-coded ones if no penalty is mapped.

Revision history for this message
Jeff Davis (jdavis-sitka) wrote :

Branch user/jeffdavis/lp1411819-permit-renew-when-exceeds-fines in the working repo has an implementation of the approach originally described in this bug:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=commitdiff;h=ca3a554

This adds a new 'circ.permit_renew_when_exceeds_fines' org setting. No release notes yet, but I can write up something if folks find this approach acceptable.

tags: added: pullrequest
Changed in evergreen:
milestone: none → 2.next
Revision history for this message
Kathy Lussier (klussier) wrote :

I'm going to load this code on a VM. In the meantime, looks like we need a release notes entry. I also don't know if this code would benefit from a test.

tags: added: needsreleasenote
Changed in evergreen:
assignee: nobody → Jeff Davis (jdavis-sitka)
Revision history for this message
Galen Charlton (gmc) wrote :

It could indeed benefit from a test, and there's precedent for doing so in the form of live_t/lp1277731_hold_permit_test.pg.

Also, I agree with Thomas and Mike that a somewhat more generalized approach would be beneficial.

tags: added: needstest
Dan Wells (dbw2)
Changed in evergreen:
milestone: 3.next → 3.1-beta
Changed in evergreen:
milestone: 3.1-beta → 3.next
Dan Wells (dbw2)
Changed in evergreen:
milestone: 3.next → 3.3-beta1
Changed in evergreen:
milestone: 3.3-beta1 → 3.3-rc
Changed in evergreen:
milestone: 3.3-rc → 3.next
Remington Steed (rjs7)
Changed in evergreen:
assignee: Jeff Davis (jdavis-sitka) → nobody
Revision history for this message
Jennifer Pringle (jpringle-u) wrote :

We have been using this live since 2015 (now on 3.7)

Revision history for this message
Blake GH (bmagic) wrote :

I ended up creating a new branch that would merge with master. The conflicts on 950_seed_data were hard to reconcile in-branch. I gave Jeff credit on my commit and forged his signature (that might have been the wrong thing).

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/blake/LP1411819_Allow_patron_renewal_on_max_fines_for_some_libraries

Revision history for this message
Christine Burns (christine-burns) wrote :

Tested on bugsquash.mobiusconsortium.org

Confirmed that the setting works to Allow patron renewal on max fines for libraries where this is set to True . Patron renewals on max fines are not allowed where this setting is not set.

I have tested this code and consent to signing off on it with my name, Christine Burns, and my email address, <email address hidden>

tags: added: signedoff
Changed in evergreen:
milestone: 3.next → 3.10-beta
Changed in evergreen:
assignee: nobody → Jane Sandberg (sandbergja)
Changed in evergreen:
assignee: Jane Sandberg (sandbergja) → nobody
status: New → Fix Committed
Revision history for this message
Jane Sandberg (sandbergja) wrote :

Thank you Jeff for this patch! Blake, a special thank you for excavating this PR and getting it ready for 3.10! Christine, I forgot to add your signoff to the commit -- apologies! I'll acknowledge your testing work here instead: thank you!

I added a pgtap test as suggested by Galen, squashed Blake's commits into one, and merged this for inclusion in 3.10.

Changed in evergreen:
status: Fix Committed → Fix Released
Revision history for this message
Steve Callender (stevecallender) wrote :

This change introduced some old code back into the function. See https://bugs.launchpad.net/evergreen/+bug/2024682

It brought back the hard copied copy status check rather than checking config.copy_status

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.