Allow Item Renewal on Expired Patron Record

[Joan Kranich]

Currently using Evergreen 3.2.8 production, 3.4.1 training
Chrome

We would like renewals allowed on an expired patron record. To be clear, the actor.usr.expire_date is in the past.

Allowing renewals on an expired patron record would not only be helpful but reduce the number of auto renewal failures. If a patron record has items out eligible for renewals then likely the patron record has not been expired for very long.

The actor.org_unit_setting table has a setting circ.holds.expired_patron_block which allows expired patron records to place holds. I do not see a similar setting to allow renewals.

[Joan Kranich (jkranich)]

C/W MARS has contracted with Catalyte for this development.

[Kyle Huckins (khuckins)]

I've pushed a branch here: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/khuckins/lp1861319-expired-patron-item-renewal

[Joan Kranich (jkranich)]

Browser: Chrome
Evergreen 3.4.2

Here are the results of our testing. We continue to test auto renewals.

1. If the Library Setting Block renewal request if renewal recipient privileges have expired is not enabled/set then initial checkouts are also allowed on an expired patron record and with no warning. The setting should only apply to renewals.

2. If the Library Setting is set for a specific library, the setting is using the patron Home Library for criteria. The checkout library (circ_lib from action.circulation) is a better criteria to use. Any items checked out by x library.

3. The Library Setting defaults to allowing the renewals for an expired patron record. Libraries will need to be aware of this when implementing the release containing this feature.

[Kyle Huckins (khuckins)]

Thanks for the feedback! I've force-pushed an update to the branch addressing the noted issues. I've changed up the YAOUS to be enable to allow, instead of enable to block. While this is inconsistent with the similar YAOUS for holds, it should be a bit easier for libraries to work with. Checkouts should no longer be affected by the YAOUS, and continue to fail when the setting is enabled. It now also checks the circ lib rather than the Patron's home library.

[John Amundson (jamundson)]

Thank you for your work on this, Kyle.

Here are our current testing results:

What works:
- The library setting has been renamed and works well in general.
- Initial checkouts are now correctly being blocked.
- Staff client renewals are looking at the workstation/renewal library to determine renewability on expired counts. This behaves well.

What doesn't:
- Both OPAC and Auto-renewals are using the patron's home library to determine if a renewal can occur on an expired account. Instead, both should be looking at the global flag used to determine renewal library in the OPAC. (This is based on the desired behavior expressed by Joan in #2 in Comment #3 above).

[Rogan Hamby (rogan-hamby)]

Since it's been tagged as needing repatch I've removed the pullrequest tag.

[Kyle Huckins (khuckins)]

Looks like I missed this bit of feedback. I'm looking into why OPAC/Autorenewals would be ignoring the circ_lib requirement. I've noticed that in Application/Trigger/Validator.pm, this line appears to be feeding in $user->home_ou(). This shouldn't have an affect on the functionality, as it takes the circ_lib from the circ object, I can't rule this out as the cause for autorenewals:
$U->create_events_for_hook('autorenewal', $circ, $user->home_ou(), 'system_autorenewal', \%user_data);

As for OPAC renewals using the home library, I believe it's related to a setting that determines whether OPAC renewals should utilize the home_ou or the circulation's original circ_lib(see bug 806049).

Will be looking into ways to ensure the circ_lib is preserved by the time it makes it to mk_env, where the check is being made.

[Kyle Huckins (khuckins)]

I'm having trouble testing auto-renewals, but the changes here _should_ work: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/khuckins/lp1861312-item-renwal-expired-patron

[Jason Stephenson (jstephenson)]

Hi, Kyle. Thanks for your efforts on this.

With your latest branch applied to a test server, we get an internal server error whenever we try to renew from the OPAC. The culprit appears to be this change:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=commitdiff;h=74b53f6c5941b4316d2d01fb7e58918ef150b4dc

It looks like you are trying to access $self->circ in a position where that object may not be initialized/defined, yet. Our error message would indicate so:

Call to [open-ils.circ.renew] failed for session [1606316647.9015115338.4185859235], thread trace [1]:#012Can't call method "circ_lib" on an undefined value at /usr/local/share/perl/5.26.1/OpenILS/Application/Circ/Circulate.pm line 1035.

Perusing the code, it appears that the mk_env method only initializes $self->circ when the patron id and patron barcode are not present as a means of retrieving the patron. The $self->circ is not guaranteed to be set until later in do_renew or do_checkin for ordinary renewals and checkins. (This could be considered a bug in its own right, depending.)

I hope the above information is helpful.

[Kyle Huckins (khuckins)]

I've pushed a commit which appears on our end to have fixed the issue - mk_env will now determine if the item can be renewed, and follow up on checking which library to use in do_renew, just under where the circ object is fetched

[Jason Stephenson (jstephenson)]

The latest changes to the code were still not working for us at CW MARS. We took the liberty of going back to Kyle's code from November and then modified it to get the desired behavior. We also added release notes and a file of Perl live tests for the new setting and renewal feature.

This code has been signed off by CW MARS staff and pushed to a collab branch:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/dyrcona/lp1861319-expired-patron-item-renewal

[Galen Charlton (gmc)]

Does the thing, so pushed to master for inclusion in 3.7. Thanks Kyle, Jason, and Jason!