Renewals based on hold-ratio doesn't take age protection into account

[Terran McCanna]

Evergreen 3.8+

We have the circulation policy for "Minimum Available Copy/Hold Ratio" set to 1 so that renewals are allowed if there are more available copies than pending holds.

In most cases this works fine, but we run into problems when it comes to items under age protection.

Example scenario 1:
- Joe has a new book checked out that is under 3 month age protection at his home library.
- 3 other people have that book on hold for pickup at Joe's location, but the library doesn't have any other copies, so their holds can only be filled by Joe's copy.
- There are 10 other copies available at other locations that are also under age protection, so they can't fulfill these pending holds. There are no other holds aside from the ones at Joe's library.
- This situation allows Joe to renew the book because there are available status at other libraries even though they couldn't actually fill the pending holds, only Joe's copy could.
- Ideally, Joe's renewal would be blocked because there are other patrons waiting for his specific copy.

Example scenario 2:
- Joe has a new book checked out that is under 3 month age protection at his home library.
- Nobody else at Joe's library has the book on hold, but 3 people have it on hold at other locations (where the copies there are also under age protection).
- This situation prevents Joe from renewing the book even though no other pending holds could target his copy.
- Ideally, Joe's renewal would be allowed because the other patrons that are waiting cannot have their holds filled by his copy.

(There may also be other situations that create this confusion such as shelving locations that are non-holdable, but I haven't specifically encountered them yet.)

[Benjamin Kalish (bkalish)]

My library system (CW MARS) allows renewals regardless of the number of holds but I think we would be a lot more likely to implement this kind of feature if weird behaviors like this were fixed.

[Steven Mayo (stmayo)]

Been stalking this one for a while and I'm going to try to put down my thoughts as understandably as possible.

Technically, this problem lies in the SQL function action.copy_related_hold_stats(copy_id BIGINT) at /Open-ILS/src/sql/Pg/100.circ_matrix.sql - but the scope of addressing it keeps zooming out the more I think.
action.copy_related_hold_stats() is given the ID of a row in asset.copy and then attempts to return the number of available copies of that item divided by the number of holds targeting that item.
So all we need to do is add a check to see whether or not the item is age protected to the statement to the lines tallying up the available copies, right?

Wrong. Just because the item HAS age protection enabled doesn't mean it WOULD be age protected - it might have expired. And... it depends on where you are. Age protection not only cares how long it has been, but also whether the item would go to an org_unit outside of the range of its protection - there are different values to allow items to go different distances for different durations. The concerto settings allow you to restrict items to 3 months within the same library or 6 months within the same library system, and different consortiums could use different distances and durations as well.

Okay, well, then I'll just take the logic for age protection from... hold_matrix.sql? Age protection, as it currently is, is used to prevent a hold from being placed, not to prevent circulations from happening. This makes sense - if a patron physically found the item, it shouldn't have gotten to a library different than its home library unless it had been shipped there - and the only reason (I think) items are shipped AWAY from their home libraries is because they've been placed on hold.

So, I try to extract the logic for checking if age protection would stop a hold. It's in 110.hold_matrix_sql in the function action.hold_request_permit_test(). Hopefully, I can make it check all the things it needs to know in any context, to make copy_related_hold_stats() and hold_request_permit_test use only one function. I named it action.does_age_protection_allow_use_at() Except...

Age protection needs to know how to calculate distance.
When an item is part of a floating collection, one might measure the distance of how far one org_unit is from another using a different library than where it stays: its owning library. Where is this stored? On the hold_matrix, with all the rules about when a hold is allowed. If I pulled the check for age protection out, it would need to find the hold matchpoint again.

At this point, I take a step back and think about the big picture. In order to figure out if ONE renewal is allowed to happen, you would need to search the entire hold_matrix table as many times as there are holds on that item in order to find the rules that apply to each hold on that item. Agh, my performance! That's O(n^2) on something that should be O(n)!

But that's... kinda the expected behavior, right? It sounds like the purpose in the ticket is that everyone who wants a turn gets a turn. That only the people who COULD get their hands on this item are counted. That means that the copy_related_ho...