tpac: conjoined items are not supported

[Kathy Lussier]

From the list of JSPAC Features Missing from TPAC at http://evergreen-ils.org/dokuwiki/doku.php?id=dev:opac:template-toolkit:plan&#jspac_features_missing_from_tpac.

TPAC does not support the display of conjoined items.

[Bill Ott (bott)]

I created some initial work, although I'm not certain it's the most efficient approach.

code here:
  http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/bott/working_branch

visuals here:
  http://demo.grpl.org/

 HINT: search "nook" or "wicked" There are currently only 4 bibs on this new master install

[Ben Shum (bshum)]

Assigning to next 2.5 milestone, let's take a look at including this or similar.

[Dan Scott (denials)]

Trying this out with the "concerto" record set, things look okay-ish when displaying record #15, but seem to go off the rails displaying any of the peer bibs. See "peerbibs_okay_display.png" for what looks okay-ish (although I think I would use an unordered list with some sort of title such as "Linked items" to give users a bit more of a clue as to why these titles are appearing underneath this copy).

[Dan Scott (denials)]

And here's "peer_bib_not_okay_display.png" for what happens when you follow one of those links in the Concerto data set. Not exactly what we're looking for, I think. I hope to dig into this soon...

[Dan Scott (denials)]

Oh, one other problem as I dig deeper into the code is that the retrieval mechanism is not efficient. With the Concerto data set, for example, the record display for record 97 goes from an average of about 0.75 seconds (via the TPAC timelog) to 2.65 seconds. Here's an example of the timelog with some additional entries:

At 0.0000: New page
At 0.1265: Initial load
At 0.1519: load_record() began
At 0.1583: past added content stage 1
At 0.1584: past staff saved searches
At 0.1584: Loading results
At 0.1585: Getting search parameters
At 0.1585: Tag circed items?
At 0.1585: Got search parameters
At 0.1592: Firing off the multiclass query
At 0.1728: Returned from the multiclass query
At 0.1728: past related search info
At 0.2546: get_records_and_facets(): about to call multisession
At 0.2564: get_records_and_facets(): about to call unapi.bre via json_query (rec_ids has 1
At 0.2571: get_records_and_facets():almost ready to fetch facets
At 0.4519: get_records_and_facets(): got response content
At 0.4529: get_records_and_facets(): parsed xml
At 0.4531: get_records_and_facets(): end of success handler
At 0.4531: get_records_and_facets():past session wait
At 0.4532: past get_records_and_facets()
At 0.4694: past copy note retrieval call
At 0.4995: past peer bib id retrieval
At 0.4996: get_records_and_facets(): about to call multisession
At 0.5007: get_records_and_facets(): about to call unapi.bre via json_query (rec_ids has 1
At 0.5013: get_records_and_facets():almost ready to fetch facets
At 0.6561: get_records_and_facets(): got response content
At 0.6570: get_records_and_facets(): parsed xml
At 0.6571: get_records_and_facets(): end of success handler
At 0.6572: get_records_and_facets():past session wait
At 0.6573: past peer bib record retrieval
At 0.6697: past copy note retrieval call
At 0.6986: past peer bib id retrieval

... peer bib retrieval / facet retrieval repeats for each copy ...

At 2.5865: get_records_and_facets():past session wait
At 2.5866: past peer bib record retrieval
At 2.5866: past store copy retrieval call
At 2.6113: past get_hold_copy_summary()
At 2.6527: past serials holding stuff
At 2.6528: past expandies
At 2.6528: past added content stage 2

So yes, we definitely have to tighten this up before committing it :/

[Dan Scott (denials)]

Okay, with http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dbs/conjoined_tpac the display time for record #97 is down to an average display time of 0.95 seconds, and record #15 (which has to display the four linked child bibs) is around 1.05 seconds given repeated tests. I restructured the bib retrieval code to cache bibs that we already had retrieved in this page display request and to short-circuit in some cases. I also fixed up whitespace.

Also, I modified the display a fair bit, if you're displaying a child bib (like record #97), the top of the copy table displays the foreign items, as follows (with new CSS):

[Branch name] (foreign item)
  * [Peer type]: [foreign bib title] / [foreign bib author]

I think that before this branch gets pushed, we should probably just squash all of the commits down into one commit; no sense in bringing the churn into mainline repo. Bill, would my branch meet GRPL's needs?

[Dan Scott (denials)]

Of note, something about foreign item display that might irk people is that foreign items will be displayed no matter what the scope is set to. For example, if the search scope is "System 2", the foreign item for Branch 1 still shows up.

The JSPAC implementation also lacked the concept of search scope, with the only difference being that in JSPAC you had to explicitly ask to see the foreign items by clicking the "Linked titles" tab, then clicking "Copy details" for the linked title.

[Bill Ott (bott)]

Catching up on this, the caching is a very welcome addition, as efficiency was my initial concern. Because we run a substantially different bit of CSS and TPAC display in general, I had made changes there as well for our production code. Overall, I think it accomplishes what we need.

One thing that I had not included here, but would certainly be worthy in regard to working toward a single commit, is including changes in Search.pm to facilitate search results, instead of just individual record display.

I'd need to parse this out of our local changes, but I believe it's something along the lines of a call to open-ils.search.peer_bibs, in load_rresults. That allows for display in expanded_results, which we use by default.

[Dan Scott (denials)]

Bill: I would be in favour of getting this in ASAP and working on a separate bug to address display of conjoined items in search results; in theory the right way to do that would be via the unapi database function, rather than issuing separate calls on a per-bib / per-copy basis (where I once again fear performance degradation).

It's different enough that I think this branch (in some sort of squashed fashion) is a win on its own. Glad to hear that it seems to be doing what you need, I think it does what we would need too.

[Dan Scott (denials)]

Added one more commit to provide saner CSS and markup for the copy table display (given that the "copy library" column may not be insanely wide due to the title / author info).

[Dan Scott (denials)]

I went ahead and put the "pullrequest" tag on this. If someone wants to squash it down, cool. If not, also cool.

[Ben Shum (bshum)]

Reviewing this today.

[Ben Shum (bshum)]

Works for me. Picked to master, thanks Bill and Dan!