Cannot place hold on age protected items

Going to try to test this now.

IOn the TPAC side anyway, I'm not seeing why this didn't already work. There already exists an ITEM_AGE_PROTECTED.override permission, and I'm not seeing why that event wouldn't bubble up from the circ service to the TPAC/mod_perl code that checks whether you have an override permission for a given event and offers you the chance to do it.

What it looks like to me is that the TPAC code you've added just acts as if everyone has ITEM_AGE_PROTECTED.override. Is there a reason for circumventing the permission rather than just granting it to everyone at your site if that's what you want?

I'm now testing how this actually plays out in practice (and looking at the jspac side too).

For the most part that I could determine the .override permissions only really apply if the *last* item looked at were to return the error. So if the *first* of 10 items looked at is age protected, and the next 9 are flagged as not holdable at your pickup library outright, the override permission TPac would use for determining "can override" would be related to the holdable policy, not the age protection.

I think the TPac is also only looking at the last error returned on the last item, which for holds may be useless due to looking at a lot of items each with 1 or more errors each that could have come back, and the code I put in says "the *only* failing reason for at least one item was hold protection".

In fact, I would argue that every item would need evaluation, for every error the item returned, and only if you have override permissions for the entire set of errors for at least one item should the override hold be offered. Otherwise you aren't allowing overrides when you should, or are when you shouldn't.

I think I see what you're saying.

If you're right, and the tpac evaluation of hold failure events needs to be corrected, then isn't that the right way to make age_hold_protect overridability work (for the tpac)?

I didn't even realize that there was an age protected override permission, as I had never seen an interface that supported overriding an unplacable hold, and thus had never looked for those permissions. I just assumed that the tpac was doing things based on a potential future permission set.

Beyond that, however, the tpac evaluation of hold failure events is based on what the backend API is returning, to my knowledge. And it only returns one failure code currently. To change the way tpac does overrides in general would require re-writing the entire backend to support returning the full response set from every failed item, and I think that might be a tad too much data to then iterate over for the front end.

That, and how do you decide what message(s) to show? "You can override any of these three copies, each of which were not holdable for two reasons each, because you have all the override permissions needed for any of them". Or "You can't override any of the copies, and there were 19 different messages returned from various parts of the system...."?

Unless we want to teach the *backend* how to check the list of permissions needed to do an override, and have it return that at least one item was permitted based on override permissions? Which gets pretty close to what I did for age protected items anyway.

I think the permissions themselves should be treated differently if they are to exist as well. "You have permission to override these failures on hold placement" should probably include "the hold targeting system will ignore those failure reasons when targeting the hold". Otherwise you are being given permission to place a hold that will never fill because the system ignores the permissions afterwards.

Merged to master. Thanks, Thomas!