Web Client - Holds Address Sometimes Missing From Transit Slip

[Jennifer Pringle]

Evergreen 3.1

On transit slips sometimes the holds address prints and other times it says "We do not have a holds address for this library" even though it does have a holds address in the system.

This is happening at multiple libraries and we have slips for items being sent to the same library where one slip has the destination library's hold address and the other slip doesn't.

We have double checked and the libraries have holds addresses filled in under Administration -> Server Administration -> Organizational Units.

[John Amundson (jamundson)]

I'm confirming this because I have seen it in Evergreen 3.0.10.

However, I've only seen this occur when the item being checked in has multiple active transits on it, (see comment #1 on bug 1786104). I am unsure if this is the only cause.

When the 'no holds address' message appeared, I refreshed the Check-In screen. Upon the check in of the item again, the holds address did appear on the dialog/slip.

[Deborah Luchenbill (deborah)]

Also confirmed in 3.0.9

[Jennifer Pringle (jpringle-u)]

Refreshing the check in screen and re-scanning the item is working for us too. Thanks for posting that John!

One of our libraries has reported that after doing the refresh they are able to scan in two items and the holds address appears on both transit slips without requiring an additional refresh. As they check in outgoing transits they'll see how many they can check in without needing an additional refresh.

[Jennifer Pringle (jpringle-u)]

Confirming that this is still an issue in 3.3

[Jennifer Pringle (jpringle-u)]

It appears that the check in screen can only handle retrieving the address for the library of the first transit done since the check in screen was opened.

For example:

1. I open the check in screen at Library A.
2. I scan an item belonging to Library B. A transit slip appears and displays the address for Library B.
3. I scan an item belonging to Library C. A transit slip appears and says "We do not have a holds address for this library" for Library C.
4. I scan a different item belonging to Library B. A transit slip appears and displays the address for Library B.
5. I scan an item belonging to Library D. A transit slip appears and says "We do not have a holds address for this library" for Library D.

I can reproduce this consistently on our 3.3.4 test server. The address for whichever library the first transit is going to will consistently appear for items going to that library while addresses for any other library displays as "We do not have a holds address for this library"

Refreshing the tab or opening the check in screen again in the same tab doesn't consistently change what the first transit address is. When I open a completely new tab, the first scan consistently displays the address of wherever the first transit is going to
.

[Jennifer Pringle (jpringle-u)]

Confirming that this continues to be an issue in 3.7.

[Jennifer Pringle (jpringle-u)]

This continues to be an issue in 3.9 and 3.10beta. The first transit slip pop-up shows an address, and subsequent slips going to other locations don't show an address.

In 3.10beta experimental circulation the transit slip pop-up never shows an address. It just says "We do not have a holds address for this library." even though there is definitely a holds address.

In all cases the phone number of the library the item is transiting to shows on the slip pop-up.

[Jennifer Pringle (jpringle-u)]

It would be great if an importance could be assigned to this ticket - possibly Medium or High.

This continues to affect the majority of our libraries on a daily basis as we have lots of reciprocal borrowing within our consortium.

[Jennifer Pringle (jpringle-u)]

The new source address added in 3.10beta doesn't seem to be affected - https://bugs.launchpad.net/evergreen/+bug/1983991

The source address continues to appear on the printed slip when the destination address starts displaying as "We do not have a holds address for this library."

[James Fournie (jfournie)]

I did a bit of tracing on this.

I think the issue is in env.absorbList. This is called multiple times throughout this file, including in this context:
https://github.com/evergreen-library-system/Evergreen/blob/25409de175a566c5afd7d901ea5ff9aafdcd0fee/Open-ILS/web/js/ui/default/staff/circ/services/circ.js#L711

It seems like in line 711, the call to env.absorbList is supposed to cache the value of `addr`, then that value is subsequently retrieved from the env.aoa.map in line 712. However in the case of these transit slips, undef is being returned.

env.absorbList doesn't seem to be adding values to env.aoa in this case. It looks like the issue is the first line:
https://github.com/evergreen-library-system/Evergreen/blob/422971f7868151bec3aa703ef88af8b1d72679c0/Open-ILS/web/js/ui/default/staff/services/env.js#L114

It seems the first line is evaluating to true, effectively returning the function early before it has a chance to look at the "list" argument and cache the passed-in values. Removing this line seems to solve this particular bug. However, I have no idea the side effects of doing that. I didn't notice any immediate issues.

The early function return line was introduced by this commit: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=commitdiff;h=849cfabaca74bd350867166740be6f47ddf6588b
This commit fixes LP1706107 -- adds a big chunk of code related to offline mode, and modified absorbTree and absorbList. It also added a similar early return line #99 to absorbTree, and added code which makes calls to absorbTree and absorbList.

It's a mystery to me what line 114 (and 99) in env.js are trying to do. My guess is something to do with offline mode, and maybe the call to .loaded is something to do with offline mode data being loaded or not.

In circ.js, the calls to absorbList seem to be passing items as the first "list" argument, intending to add them to a cached list. In that context, I'm not sure why you'd ever want to return only service[class_] immediately. It seems like the absorb methods are often used just to initially cache an entire thing (org tree, all circ mods, etc), so maybe it was a way of speeding up things in that context because it's just checking for the existence of ie: env.aou and then returning it rather than recreating it.

The offline commit also added reconstituteTree and reconstituteList, which I'm not yet sure what they do but also each call the absorb functions.

That's about as far as I could get for now, this rabbithole was way deeper than I expected

[Jeff Davis (jdavis-sitka)]

Thanks, James! I agree it seems wrong that the absorb functions return the cached version before updating the cache. Here's a branch that removes the relevant line from absorbList and absorbTree:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jeffdavis/lp1778567-always-update-cached-list-or-tree

This branch needs further testing to ensure it doesn't cause performance problems or break any other UIs where absorbList and absorbTree are used. However, I've confirmed that it fixes the issue reported in this bug, using the following steps to test:

1. Make sure the libraries in your test environment have hold addresses.
2. Sign into the web client at Library A.
3. Check in an item belonging to Library B.
4. Check in an item belonging to Library C. Expected behavior: The hold address displays on the transit slip. Actual behavior: The transit slip says "We do not have a holds address for this library."
5. Apply the fix and clear your browser cache.
6. Check in an item belonging to Library B.
7. Check in an item belonging to Library C. The hold address should now correctly display on the transit slip.

[James Fournie (jfournie)]

just thought I'd add some more info here:

I'm going to ignore than line in absorbTree, and focus on absorbList.

Most absorbList calls look something like this:

            return egCore.pcrud.retrieveAll('cit', {}, {atomic : true})
            .then(function(types) {
                egCore.env.absorbList(types, 'cit')
                service.ident_types = types
            });

They are retrieving all of an object from pcrud, and passing it into absorbList. absorbList then caches it so it can be accessed in this case by calling env.cit. In the various calls, in some cases this retrieves "all" and in other cases it's just all for the workstation. So in that context, this line makes sense:

        if (service[class_] && service[class_].loaded) return service[class_];

So when absorbList is called, it's going to see if there is already value (in env.cit, etc.) and if it's there, don't try to re-cache. But it seems strange to me that despite this effort to avoid extra caching, we've already made a call to pcrud which is going to likely be more expensive than running the absorbList javascript again? Is that even worthwhile? Am I misunderstanding something here?

Anyway, so whereas most of the time we're retrieving "all" of a type of thing, sometimes we aren't. Here are the exceptions I found:

- circ/services/holds.js
        line 683 - seems to be fetching/caching copy locations ad hoc
        line 711,726 - fetching/caching addresses, the original bug report of this
- services/env.js - in absorbTree, which is a whole other can of worms in itself
- services/lovefield.js - used by reconstituteList not sure what this is doing
- circ/patron/regctl.js
        line 550 - not sure what's happening here, but seems to be getting all perms probably
        line 611 - comment seems to indicate this was added on a whim, can't find call to env.cust anywhere

A hackish easy fix might be to just address those cases with a separate function or something I guess? That said, I think just removing that line might be another quick fix. The main thing I don't get is what .loaded is indicating and if that's important. Like maybe the first time aoa values get cached, it's setting that to true and it shouldn't etc.

[Brett French (bsfrench)]

I have tested this bug on https://bugsquash.mobiusconsortium.org/eg/staff and every slip has displayed with the correct address. I'm no longer getting the "We do not have a holds address for this library." notification.

[James Fournie (jfournie)]

I can confirm Brett's finding that it works there, but this issue persists on demo.evergreencatalog.com

[Ruth Frasur Davis (redavis)]

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

[Galen Charlton (gmc)]

Pushed down to rel_3_9. Thanks Jeff, James, and Ruth!