Hold rule mismatches from too much weight applied to requestor.grp

[Ben Shum]

EG Version: 1.6.0.2
OpenSRF Version: 1.2.2
PostgreSQL Version: 8.3
Linux distribution: Debian

When we went live with our first development partner libraries back in March 2010, we noticed that there was a major problem with the way that holds were being matched against the rules set in the config.hold_matrix_matchpoint table. Some hold rules that were supposed to protect certain items from being placed on hold by certain groups were not doing their job correctly. Instead the items in question were being matched to incorrect rules in the matchpoint table that were allowing holds to go through.

Working with Galen Charlton from ESI, we tracked the potential source of the problem to something in how the holds matching logic applied with regards to the requestor.grp field in the hold matrix. As explained to us, the problem was often that once a hold request matched to a particular requesting user group's hold rule, it would fail to recognize a more specific rule that should have matched better elsewhere in the table. Too much weight was being applied to the requestor.grp and that was messing up all our hold placements.

Currently, we have an adjusted hold matrix matching logic that skips matches to the requestor.grp as a temporary measure, but would like to find a better solution with the community. A big question we have is how the current holds logic is applied to finding matchpoints and what weights are assigned to particular fields.

Will try to supply additional information as best I can.

[Jason Stephenson (jstephenson)]

In trunk and in rel_1_6_0, the action.find_hold_matrix_matchpoint loops first over the requestor object's permission group and ancestors and for each group queries the config.hold_matrix_matchpoint for a match. This looping stops when the requestor runs out of ancestor permission groups or when a matchpoint is found as demonstrated by the following code on line 158 for 110.hold_matrix.sql:

EXIT WHEN current_requestor_group.parent IS NULL OR matchpoint.id IS NOT NULL;

This means that any matchpoints applying specifically to the requestor's group will take precedence over any matchpoints that apply to broader permission groups.

This means that you will need to use caution when setting the requestor_grp field in config.hold_matrix_matchpoint, and possibly duplicate any matchpoints applied to ancestor permission groups to ensure that the proper rule is always used.

My suggestion would be to not use the requestor_grp field, setting it always to 1, so that it matches for all users if you can.

Another option might be to remove the OR matchpoint.id IS NOT NULL on line 158. This would cause the function to loop over all matchpoints for the user's group and ancestor groups and then use the matchpoint with the lowest weight score.

[Jason Stephenson (jstephenson)]

As an addendum:

When getting the requestor object's permission group information, action.find_hold_matrix_matchpoint looks only at actor.usr.profile. This poses a problem in cases where staff users are also patrons and have their patron permission group in their profile, but get their work permissions through permission.usr_grp_map. Thus when placing a request through the staff client for a patron, a staff member will still only use their patron-level permission groups though they are acting in the role of staff at the time. This would ignore any special matchpoints for when staff are placing requests on behalf of patrons.

[Ben Shum (bshum)]

In our case, staff logins are separate accounts from regular staff patron accounts (they are two different permission groups). So we do use the requestor_grp field in the matrix to differentiate between staff-placed holds in the client vs. holds placed via the OPAC by the patrons on their own behalf.

Having that sort of granularity is nice, just something wrong with the matching as indicated.

[Jason Stephenson (jstephenson)]

Thomas Berezansky's dynamic weight patch for trunk is aimed at fixing this by allowing the administrator to apply their own weighting to the different fields.

[Mike Rylander (mrylander)]

Addressed by commit against bug #677074 for trunk/2.1