wishlist: Improvements to Hopeless Holds

Bug #1811710 reported by Andrea Neiman on 2019-01-14
20
This bug affects 4 people
Affects Status Importance Assigned to Milestone
Evergreen
Wishlist
Unassigned

Bug Description

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.

Andrea Neiman (aneiman) on 2019-01-14
tags: added: webstaffclient
Jason Etheridge (phasefx) wrote :

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.

Changed in evergreen:
assignee: nobody → Jason Etheridge (phasefx)
Jason Etheridge (phasefx) wrote :

Hrmm, needs documentation.

tags: added: needsreleasenote pullrequest
Jason Etheridge (phasefx) wrote :

Pushed an 8th commit with release notes.

tags: removed: needsreleasenote
Andrea Neiman (aneiman) on 2019-08-16
Changed in evergreen:
milestone: none → 3.4-beta1
Changed in evergreen:
assignee: Jason Etheridge (phasefx) → nobody
Jason Etheridge (phasefx) wrote :

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) wrote :

updated

Bill Erickson (berick) wrote :

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) wrote :

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.

Changed in evergreen:
assignee: nobody → Jason Etheridge (phasefx)
Jason Etheridge (phasefx) wrote :

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.

tags: removed: pullrequest
Jason Etheridge (phasefx) wrote :

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) wrote :

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.

Changed in evergreen:
assignee: Jason Etheridge (phasefx) → nobody
Jason Etheridge (phasefx) wrote :

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

tags: added: pullrequest
Galen Charlton (gmc) on 2019-09-11
Changed in evergreen:
milestone: 3.4-beta1 → 3.next
Terran McCanna (tmccanna) wrote :

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) on 2020-01-03
Changed in evergreen:
assignee: nobody → Galen Charlton (gmc)
status: New → Confirmed
milestone: 3.next → 3.5-alpha
Galen Charlton (gmc) wrote :

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

Changed in evergreen:
assignee: Galen Charlton (gmc) → nobody
Chris Sharp (chrissharp123) wrote :
Download full text (4.1 KiB)

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...

Read more...

Galen Charlton (gmc) wrote :

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) wrote :

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

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers