Collections API users_of_interest can crash on users with null card values

[Michael Peters]

Evergreen Master
OpenSRF Master

If the open-ils.collections.users_of_interest API encounters a patron that
has a null "card" value in actor.usr the API will crash with an error like:

Received Exception:
Name: osrfMethodException
Status: *** Call to [open-ils.collections.users_of_interest.retrieve] failed for session [1416515351.333446.1416515351406], thread trace [1]:
Can't call method "barcode" on an undefined value at /usr/local/share/perl/5.14.2/OpenILS/Application/Collections.pm line 299.

This patches users_of_interest to be sure that the user has a defined value
for actor.usr.card before proceeding to pull out the barcode for the results
of the API call.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrpeters/open-ils_collections_users_of_interest_test_defined_card

[Michael Peters (mrpeters)]

For potential testers -- an easy srfsh command to test is below. Set a user in your system to have a null actor.usr.card value and make sure they will end up in the results of this srfsh command.

request open-ils.collections open-ils.collections.users_of_interest.retrieve, "your auth token from a login", "number of days back", "total fine amount", "shortname of home library"

Something like:

srfsh# login admin open-ils

Received Data: "071fa4b8850e680842df18d3e7f53b43"

------------------------------------
Request Completed Successfully
Request Time in seconds: 0.013642
------------------------------------

Received Data: {
  "ilsevent":0,
  "textcode":"SUCCESS",
  "desc":"Success",
  "pid":3111,
  "stacktrace":"oils_auth.c:531",
  "payload":{
    "authtoken":"192173272b39ac051961b5dd7944201e",
    "authtime":420
  }
}

------------------------------------
Request Completed Successfully
Request Time in seconds: 0.301600
------------------------------------
Login Session: 192173272b39ac051961b5dd7944201e. Session timeout: 420.000000

srfsh# request open-ils.collections open-ils.collections.users_of_interest.retrieve, "192173272b39ac051961b5dd7944201e", "15", "4.99", "DCPL"

[Bill Erickson (berick)]

Thanks, Mike. I have a suggestion for the patch. Instead of setting "There is no card for this user" as the value for the barcode when no card is present, I would recommend setting it to undef. That way it's (programmatically) evident to the caller whether a card is present or not. For example:

=> $u->card ? $u->card->barcode : undef

The caller can then display a human-friendly, locale-aware message if they choose to.

[Galen Charlton (gmc)]

I agree with Bill, though since open-ils.collections.users_of_interest.retrieve doesn't have any callers within Evergreen itself at present -- just third-party collections agencies -- a different reason for passing undef rather than a string is to avoid making "There is no card for this user" into a magic value that callers have to care about.

Marking bug as "confirmed", but removing pull request.

[Ben Shum (bshum)]

We're testing Bill's approach now with Unique Management (who reported this issue to us when trying to retrieve users of interest from one of our libraries). So far, so good, though I made a small typo by not including an ending comma to the undef line and that busted our Collections.pm for a day...

If it works for Unique, we'll likely go ahead and put Bill's suggested line change into mrpeters' branch and get it ship-shape to be pushed to a future maintenance release of Evergreen.

Thanks everyone!

[Ben Shum (bshum)]

Adding to this bug from another earlier bug on the same subject. Bill Ott found that deleted users were being retrieved by this users_of_interest API call by Collections.pm.

He offered this change from their 2.1 system:

Specific change in money.pm of 2.1
  https://github.com/grpl-eg/rel_2_1/commit/185e85494ef14310ff43fb641ecc9223997dfc1b

We should revamp that and apply both issues towards a new branch for master and backports to supported releases to fully resolve the problems.

[Ben Shum (bshum)]

Okay, this is a working branch that brings in both mrpeters, berick, and bott's work under one place. Top two commits.

user/bshum/lp1394989_collections_bug_fixes

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/bshum/lp1394989_collections_bug_fixes

We're testing with Unique to check out the results of those changes, but expect that things should work better with them in place.

[Ben Shum (bshum)]

Testing with Unique is complete, they seem pleased with the resolution of this API call issue using the approaches undertaken in the working branch I put together using code from all of you guys.

Thanks! Will wait a bit before pushing this myself unless someone else gets it moved along first.

[Bill Erickson (berick)]

Nothing like real-world testing. Code looks good. Merged to 2.7+. Thanks, all.