wishlist: Improvements to Hopeless Holds

[Andrea Neiman]

Equinox has been contracted by MassLNC to perform development work to improve Evergreen's handling of so-called "hopeless" holds, i.e., holds that have no remaining eligible copies in hold_copy_map.

This work will create a new interface that will show holds in a "hopeless" state and provide a set of actions to be taken upon them, including:
 * Cancel Hold
 * Find Another Target
 * Retrieve Patron
 * Transfer to Marked Title
 * Add Items
 * View/Place Orders

MassLNC's full requirements can be seen here: http://masslnc.org/node/3389

Equinox's full specifications can be seen here: https://yeti.esilibrary.com/dev/public/techspecs/hopeless_holds.pdf

Note that MassLNC has opted to include Add-On A: Hopeless-Prone Statuses with this work. The add on is being funded by NOBLE.

[Jason Etheridge (phasefx)]

top 7 commits from collab/phasefx/lp1811710-hopeless-holds

If the whole thing doesn't go in right way, we may want to cherry-pick some of the incidental bug fixes sooner, so I tried to be smart with my commit squashing.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/phasefx/lp1811710-hopeless-holds

I think "lp1811710: hide Pre-Fetch for now" is optional; I'm not 100% sure that it makes a difference with this particular interface and I opted for less UI clutter.

[Jason Etheridge (phasefx)]

Hrmm, needs documentation.

[Jason Etheridge (phasefx)]

Pushed an 8th commit with release notes.

[Jason Etheridge (phasefx)]

Realized the upgrade scripts are missing the changes that went into 002.schema.config.sql and 090.schema.action.sql, will force push a new branch shortly

[Jason Etheridge (phasefx)]

updated

[Bill Erickson (berick)]

Jason, I started reviewing the code, but was quickly stymied by the merge conflicts with current master, the modified date handling in the FormatService, in particular. It should be possible to remove the date parsing additions and let Moment parse the dates for us.

[Jason Etheridge (phasefx)]

Thanks Bill; the Firefox specific commit isn't critical for the rest of it; I'm testing things without it to see if Moment works.

[Jason Etheridge (phasefx)]

Not sure about Firefox yet, but date filtering no longer works with the interface under the new regime with any browser. I'll try to poke at it.

[Jason Etheridge (phasefx)]

I may have jumped the gun assuming it no longer worked (hold targeter quite naturally changed the data out from under me so I saw what I feared to see). Date filtering appears to work still in Chrome.

I do see an error changing the org unit to CONS for the first time, but it appears to be harmless:
OrgSelectComponent.html:13 ERROR Error: ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: 'ng-untouched: true'. Current value: 'ng-untouched: false'.

As for Firefox, I'm having new problems with master and Firefox preventing me from testing date handling. In Firefox, whenever I go to an /eg2 interface, it shunts me to the register workstation interface. Hrmm.

[Jason Etheridge (phasefx)]

Bill, I'm going to ignore the Firefox issues (and/or open new bugs as needed).

Here's a branch without the date parsing tweak:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/phasefx/lp1811710-hopeless-holds-minus-firefox-date-fix

collab/phasefx/lp1811710-hopeless-holds-minus-firefox-date-fix

I'm not going to put a pullrequest back up yet; I realized that I should add some automated tests first.

[Jason Etheridge (phasefx)]

Added a re-based branch with automated tests:

top 8 commits @ collab/phasefx/lp1811710-hopeless-holds-minus-firefox-date-fix-rebased

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/phasefx/lp1811710-hopeless-holds-minus-firefox-date-fix-rebased

[Terran McCanna (tmccanna)]

We've applied this code to a 3.4 test server with a copy of real PINES data on it, and the only problem I've seen so far is that the dropdown box for the Pickup Library isn't sorted correctly (see screenshot).

To do a test:

1) Identify a title that only has one copy attached to it.
2) Place a hold for a patron on that title.
3) After the hold is successfully placed, mark the copy Missing.
4) Go to Administration > Local Administration > Hopeless Holds.
5) Select the pickup library that the hold was placed for, and the title should populate.

[Galen Charlton (gmc)]

The issue with OU sorting in the pickup library selector was bug 1857350 and is now fixed.

I've done another rebase; the current branch to look at is:

working/user/gmcharlt/lp1811710_hopeless_holds_rebased_2020-01-03
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/gmcharlt/lp1811710_hopeless_holds_rebased_2020-01-03

[Chris Sharp (chrissharp123)]

We applied this branch to a 3.4.1 test server and were seeing the following errors in the logs:

2020-01-13 12:49:30 next-brick01-head open-ils.cstore: [ERR :7372:oils_sql.c:6993:157891470218856284] open-ils.cstore ERROR No "datatype" attribute for field "org_unit"
2020-01-13 12:49:30 next-brick01-head open-ils.cstore: [ERR :9131:oils_sql.c:6993:157891470218856286] open-ils.cstore ERROR No "datatype" attribute for field "org_unit"
2020-01-13 12:49:30 next-brick01-head open-ils.cstore: [ERR :7372:oils_sql.c:6993:157891470218856288] open-ils.cstore ERROR No "datatype" attribute for field "org_unit"
2020-01-13 12:53:10 next-brick01-head open-ils.cstore: [ERR :7372:oils_sql.c:6993:15789147022062565] open-ils.cstore ERROR No "datatype" attribute for field "org_unit"
2020-01-13 12:53:10 next-brick01-head open-ils.cstore: [ERR :7372:oils_sql.c:6993:15789147022062567] open-ils.cstore ERROR No "datatype" attribute for field "org_unit"
2020-01-13 12:53:10 next-brick01-head open-ils.cstore: [ERR :7372:oils_sql.c:6993:15789147022062569] open-ils.cstore ERROR No "datatype" attribute for field "org_unit"
2020-01-13 12:53:18 next-brick01-head open-ils.cstore: [ERR :7372:oils_sql.c:6993:157891470218856322] open-ils.cstore ERROR No "datatype" attribute for field "org_unit"
2020-01-13 12:53:18 next-brick01-head open-ils.cstore: [ERR :7372:oils_sql.c:6993:157891470218856324] open-ils.cstore ERROR No "datatype" attribute for field "org_unit"
2020-01-13 12:53:18 next-brick01-head open-ils.cstore: [ERR :7372:oils_sql.c:6993:157891470218856326] open-ils.cstore ERROR No "datatype" attribute for field "org_unit"
2020-01-13 12:54:23 next-brick01-head open-ils.cstore: [ERR :7371:oils_sql.c:6993:15789147022053798] open-ils.cstore ERROR No "datatype" attribute for field "org_unit"
2020-01-13 12:54:23 next-brick01-head open-ils.cstore: [ERR :7371:oils_sql.c:6993:157891470220537100] open-ils.cstore ERROR No "datatype" attribute for field "org_unit"
2020-01-13 12:54:23 next-brick01-head open-ils.cstore: [ERR :7371:oils_sql.c:6993:157891470220537102] open-ils.cstore ERROR No "datatype" attribute for field "org_unit"
2020-01-13 14:19:35 next-brick01-head open-ils.cstore: [ERR :18861:oils_sql.c:6993:157891470220628169] open-ils.cstore ERROR No "datatype" attribute for field "org_unit"
2020-01-13 14:19:35 next-brick01-head open-ils.cstore: [ERR :18861:oils_sql.c:6993:157891470220628171] open-ils.cstore ERROR No "datatype" attribute for field "org_unit"
2020-01-13 14:19:35 next-brick01-head open-ils.cstore: [ERR :18861:oils_sql.c:6993:157891470220628173] open-ils.cstore ERROR No "datatype" attribute for field "org_unit"
2020-01-13 14:20:25 next-brick01-head open-ils.cstore: [ERR :18861:oils_sql.c:6993:157891470220626298] open-ils.cstore ERROR No "datatype" attribute for field "hopeless_date"
2020-01-13 14:20:25 next-brick01-head open-ils.cstore: [ERR :9133:oils_sql.c:6993:157891470220626299] open-ils.cstore ERROR No "datatype" attribute for field "hopeless_date"
2020-01-13 14:26:18 next-brick01-head open-ils.cstore: [ERR :9137:oils_sql.c:6993:157891470220625133] open-ils.cstore ERROR No "datatype" attribute for field "hopeless_date"
2020-01-13 14:27:22 next-brick01-head open-ils...

[Galen Charlton (gmc)]

Digging into the error a bit:

[1] The error message from line 6993 of oils_sql.c is incorrect; it should be something like 'No "primitive" attribute for field..." as the function showing the error is invoked from get_primitive().
[2] The "primitive" attribute is set by oilsExtendIDL() when cstore and friends initialize and query the database for the types of the columns in each table.
[3] oilsExtendIDL() specifically iterates through each column and uses libdbi's dbi_result_get_field_type_idx() to get the type.
[4] However, it doesn't check every possible condition, so DBI_TYPE_ERROR and DBI_TYPE_XDECIMAL don't get checked. If libdbi/libdbd-pgsql return one of those two, the "primitive" attribute doesn't get set.
[5] The attribute also doesn't get set if the column doesn't exist in the database. However, if that were the problem, it would have been apparent in Chris' logs whenever a cstore create or update was done, as it would have failed outright and complained about a missing column.
[6] get_primitive() defaults to assuming that the column type is string if the attribute hasn't set, so in the case of a timestamp-with-TZ like hopeless_date, updates and creates would still just work.
[7] However, it's weird that oilsExtendIDL() isn't recognizing the type properly.

I would be curious to know exactly what versions of Pg, libdbi, and libdb-pgsql were used.

[Chris Sharp (chrissharp123)]

Here you go:

ii postgresql-9.6 9.6.16-1.pgdg18.04+1 amd64 object-relational SQL database, version 9.6 server
ii postgresql-client-9.6 9.6.16-1.pgdg18.04+1 amd64 front-end programs for PostgreSQL 9.6
ii postgresql-client-common 210.pgdg18.04+1 all manager for multiple PostgreSQL client versions
ii postgresql-common 210.pgdg18.04+1 all PostgreSQL database-cluster manager
ii postgresql-contrib-9.6 9.6.16-1.pgdg18.04+1 amd64 additional facilities for PostgreSQL
ii postgresql-plperl-9.6 9.6.16-1.pgdg18.04+1 amd64 PL/Perl procedural language for PostgreSQL 9.6
ii postgresql-server-dev-9.6 9.6.16-1.pgdg18.04+1 amd64 development files for PostgreSQL 9.6 server-side programming

ii libdbd-pg-perl 3.10.0-2~pgdg18.04+1 amd64 Perl DBI driver for the PostgreSQL database server
ii libdbd-pgsql:amd64 0.9.0-5ubuntu2 amd64 PostgreSQL database server driver for libdbi
ii libdbd-sqlite3-perl 1.56-1 amd64 Perl DBI driver with a self-contained RDBMS
ii libdbi-dev 0.9.0-5 amd64 DB Independent Abstraction Layer for C -- development files
ii libdbi-perl 1.640-1 amd64 Perl Database Interface (DBI)
ii libdbi1:amd64 0.9.0-5 amd64 DB Independent Abstraction Layer for C -- shared library
ii libdbix-contextualfetch-perl 1.03-3 all module to add context aware fetches to DBI

[Galen Charlton (gmc)]

I've pushed a fresh rebase to user/gmcharlt/lp1811710_hopeless_holds_rebased_2020-07-28.

Chris, after digging a bit, I was able to reproduce the error with the following steps:

- install the branch, including fm_IDL.xml
- restart services
- *THEN* apply the schema update

After that, adding a new item status (e.g.) resulted in that error. Consequently, restarting services should be done after the schema update is done in order for cstore and friends to be able to check the Pg data type directly from the database.

[Galen Charlton (gmc)]

I've done a forced push to the new working branch to account for the newly-pushed patch for bug 1889296.

[Terran McCanna (tmccanna)]

This looks good and works well for the various scenarios I've thrown at it, and we're no longer seeing the errors in the logs! My signoff is at:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mccanna/lp1811710_hopeless_holds_signoff

[Chris Sharp (chrissharp123)]

Pushed to master. Thanks all!!