Autorenewals should not be attempted on circs where auto_renewal_remaining is NULL

Bug #1835953 reported by Michele Morgan
36
This bug affects 7 people
Affects Status Importance Assigned to Milestone
Evergreen
Undecided
Unassigned

Bug Description

Evergreen 3.2.4

We have begun using autorenewals and find that transactions that have a NULL auto_renewal_remaining are being treated the same as transactions with 0 auto_renewal_remaining. Transactions of this type are not renewable at all and autorenewals are failing with MAX_RENEWALS_REACHED. Patrons are getting a notification about the failed renewal on a non-renewable transaction which is causing confusion.

When using autorenewals, rows in action.circulation can have the following for the auto_renewal_remaining field:

auto_renewal_remaining > 0
auto_renewal_remaining = 0
auto_renewal_remaining is NULL

For the first case, the autorenewal should be attempted, and the patron notified of success or failure.

For the second case, the autorenewal should be attempted and the patron notified that there are no renewals remaining.

For the third case, I would argue that the item is not renewable and no attempt should be made to autorenew.

Adding

"-not" : [ { "auto_renewal_remaining" : null } ]

to a custom a_t_filter will prevent renewal attempts on circs where auto_renewal_remaining is NULL, but a baked in solution would be preferable.

Revision history for this message
Michele Morgan (mmorgan) wrote :

Adding a link to relevant IRC discussion:

http://irc.evergreen-ils.org/evergreen/2019-07-09#i_411573

Revision history for this message
Bill Erickson (berick) wrote :

Adding the NULL check to the filter is most efficient way to handle this. Maybe this is a documentation / sample data issue?

A secondary check in the code to mark such events invalid probably wouldn't hurt, though.

I'd like to suggest a slight tweak for the second case (above) where auto_renewal_remaining = 0. We should teach the AutoRenew reactor to bypass the actual circulation back-end call, but still generate the notification.

As I understand it, autorenew batches can take some time to process. This would allow the server to avoid a lot of unnecessary work processing items that are known in advance to be non-auto-renewable.

Revision history for this message
Michele Morgan (mmorgan) wrote :

Perhaps we should have a dedicated hook for autorenewals which includes the NULL check?

Regarding the second case, I like the idea of teaching the reactor to bypass the autorenew attempt and just send a notification when auto_renewal_remaining = 0

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

Other bug subscribers