Hanging transits can cause checkins to fail

[Michele Morgan]

If a Checked out item with a hanging transit is checked in, and needs to go in transit, the checkin will fail with no visual indication to the user. Logs show an error creating the transit:

2019-03-11 13:14:23 brick1 open-ils.circ: [ERR :93454:CStoreEditor.pm:139:15523243949351311] editor[1|61] request error open-ils.cstore.direct.action.hold_transit_copy.create : <new object> : Exception: OpenSRF::DomainObject::oilsMethodException 2019-03-11T17:14:23 OpenILS::Utils::CStoreEditor /usr/local/share/perl/5.20.2/OpenILS/Utils/CStoreEditor.pm:465 <500> INSERT error -- please see the error log for more details

Bug 1787274 resolved many issues where duplicate transits were being created at checkin. Part of this fix was adding a unique-per-copy constraint to action.transit_copy. Unfortunately, other circumstances can lead to hanging transits where an item has an associated open transit, but does not have In transit status. The following query can be used to identify such items.

select copy.id, copy.status, transit.id from action.transit_copy transit
join asset.copy copy on transit.target_copy = copy.id
where copy.deleted is false
and copy.status != 6
and transit.dest_recv_time is null
and transit.cancel_time is null

Since a given item can have only one active transit at a time, perhaps a check for an existing transit could be performed, and if found could be cancelled prior to attempting to create a new transit.

[Robert J Jackson (rjackson-deactivatedaccount)]

Related to bug: https://bugs.launchpad.net/evergreen/+bug/1529969 ?

[John Amundson (jamundson)]

Related: Bug 1794575

[Michele Morgan (mmorgan)]

See comments on duplicate bug 1529969, which are more relevant to this bug than the original report.

[Michele Morgan (mmorgan)]

Adding links to additional bugs which may contribute to the frequency of hanging transits, hence failed checkins:

bug 1780315 - Web Client: Cancelling hold should prompt to cancel associated transit
bug 1789296 - Wishlist: Option to cancel hold transits when a hold is canceled
bug 1756441 - Web client: Mark Item Damaged function fails silently on in transit items

And one that may reduce the frequency of hanging transits:

bug 1779467 - Enhance Mark Item Discard/Weed Functionality

[Jason Etheridge (phasefx)]

Michele, are you still able to reproduce this? In Concerto, I checked in CONC90000436 (a BM1 item) at BR1, created a transit, then in the database set the item status to Available and tried checking it in again at BR1, BR2, and BM1, all with no error. I also checked out a version of EG from from 2019-09 and still could not duplicate the problem.

For fun, I also placed a copy level hold (pickup BR1) on the item mid-transit and it made no difference. I then let the transit complete and tried with the resulting hold transit back to BR1 and had no trouble.

[Jason Etheridge (phasefx)]

Hrmm, but I didn't enforce the status change on the hold transit. Will revisit.

[John Amundson (jamundson)]

Jason,

I have mainly seen this behavior when the item is Checked Out to a patron and also In Transit.

Using your method, after updating the item to Available, check it out to patron record. Because of Bug 1794575, you will not be prompted to cancel the transit. Then try to check it in.

It is also easy to test on a system pre 3.4, (Mark Missing improvements - bug 1779467).
Send an item in transit.
Mark item missing from Item Status. Won't prompt to cancel transit.
Check item out to patron. Won't prompt to cancel transit.
Check item in. Check in fails.

[Jason Etheridge (phasefx)]

Thanks John! I'll give this a shot.

[Jason Etheridge (phasefx)]

Interesting. Per bug 1794575, put item into transit, tried to Mark it Damaged, and get an uncaught exception in the console: TypeError: Cannot read property 'charge' of undefined at circ.js:1375

Put item into transit, changed status to Available directly in the DB, check it out to user, and indeed, no cancel transit prompt. Checked item in, get a klaxxon. Perfect. DATABASE_UPDATE_FAILED at circ.js:1740

ERROR: Copy id=401 is already in transit

I'd be tempted to insert a transit search in Circulate.pm:sub do_copy_checks where it's doing the following:

    if( $stat == OILS_COPY_STATUS_IN_TRANSIT ) {
        return $self->bail_on_events(OpenILS::Event->new('COPY_IN_TRANSIT'));
    }

However, there are several places in the code where it's checking for OILS_COPY_STATUS_IN_TRANSIT. We could change the transit_copy_is_unique_check on action.transit_copy to replace old transits with new transits, but that's the sort of logic that allowed multiple redundant calls in the XUL client. Maybe we repair the item status at the point of first comparison. I'll mull this over. Thanks!

[Jason Etheridge (phasefx)]

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/phasefx/lp1819542-broken-transits

This defends against, at least, the scenario I've been using to trigger the bug:

    1) transit an item (CONC90000436 in Concerto, BR1->BM1)
    2) artificially change its status directly in the database (for
       example, to Available)
    3) check it out to a patron (99999376864 in Concerto), noting
       that the Cancel Transit prompt does not get triggered
    4) check in the item and get a warning klaxxon and some errors
       in the logs

Please give it a spin and let me know if it breaks anything or fails to catch anything related that breaks things. Thanks!

[Rogan Hamby (rogan-hamby)]

sign off at user/rogan/lp1819542_hanging_transits

I took the example test case and manually screwed up a bunch of other transits and they worked now as expected

[Jason Stephenson (jstephenson)]

I want to add that John at CW MARS is also looking at the branch on this bug.

[John Amundson (jamundson)]

This code looks good to me, too. I'll add my sign off to the mix.

I have tested this code and consent to signing off on it with my name, John Amundson and my email address, <email address hidden>.

Thanks for your work on this, Jason.

[Jane Sandberg (sandbergja)]

Pushed to master, rel_3_5, and rel_3_4. Thanks, Michele, Jason, Rogan, and John.