FOLIO: Display bound-with titles #3094

maccabeelevine · 2023-09-15T13:43:54Z

"Bound-with" titles are a situation where multiple independent works / bibliographic titles are bound together into the same physical item. This breaks the most common ILS convention where a single work can contain multiple holdings/items but not vice-versa. In a bound-with, a single item physically contains multiple works. It is sometimes even a many-to-many relationship, where a single bibliographic title is bound into multiple bound-with items, each of which also contains many other works.

In VuFind, a patron looking at a bibliographic record page, on the holdings tab, needs to be able to see if the record is bound-with other titles, and what those titles are. A bound-with item often has no printed table of contents, and may have dozens of works bound together, sometimes with no relationship to each other and no obviously inherent order. Yes this is truly annoying.

The original FOLIO data model did not support this, as each single item can only be linked to one holdingsRecord, and from there to one instance. A parallel bound-with-parts API was developed and implemented over the last few years. Each bound-with-part links a holdingsRecord to an item. See the orange lines in this diagram. Support for this data model has been built over the last year into the FOLIO Inventory app such that you can view, delete and create bound-with relationships.

This PR uses the FOLIO bound-with data model to add bound-with titles to the holdings tab.

Future improvements

Optimize getBoundWithRecords to bulk query for holdings records and instances, batched for URL length.

TODO

Update ILS driver spec when merging
Update changelog (FOLIO BC breaks) when merging

maccabeelevine · 2023-09-15T13:46:56Z

UX developed with input from Lehigh librarians, and meeting with U Chicago bound-with experts for a second perspective this afternoon.

maccabeelevine · 2023-09-15T14:28:41Z

module/VuFind/src/VuFind/ILS/Driver/Folio.php

+                    '/inventory/items-by-holdings-id',
                    $query
                ) as $item
            ) {
+                if ($item->discoverySuppress) {
+                    continue;
+                }


/inventory/items-by-holdings-id is an optimized call to retrieve both directly linked and bound-with items at once. They are distinguished in the result set by the isBoundWith attribute. The downside is this API does not support passing the discoverySuppress parameter so that filter is done to the results intead.

themes/bootstrap3/templates/RecordTab/holdingsils.phtml

maccabeelevine · 2023-09-15T14:42:32Z

themes/bootstrap3/templates/RecordTab/holdingsils/boundwith.phtml

+        <?php foreach ($boundWithRecords as $record): ?>
+            <li class="<?=($record['bibId']==$driver->getUniqueId()) ? 'direct-item' : 'bound-with-item'?>">
+                <a href="<?=$this->escapeHtmlAttr($record['bibId'])?>">
+                    <?=$this->escapeHtml($this->truncate($record['title'], 45)) ?>


This truncation is only necessary because the list of titles is displayed in the second column of this items table, taking up half its width, and approx 1/3 of total width including the sidebar column.

I would prefer it to be full-width on the table, but couldn't think of a way to do that while preserving the th / td structure for accessibility.

If I simply put this info in a separate <tr> following the main one for this item, it messes up the alternating color CSS.

I could alternately just adjust the column withs to something like 25%/75%, but the table has fixed layout and it might look odd for it to be relative, especially if there are other item tables on the same record (i.e. for other locations) that continue to have fixed width.

I can't just display the list outside of the table itself, because it's semantically relevant to the specific item -- not to the bib/instance record.

I also suspect individual institutions will want to change that specific number. I could move that to a config file but don't see a good design pattern for using config (beyond the base config.ini) within templates. Alternately I could do the truncation earlier in Folio.ini. But again I'd prefer to just have a wider column.

Ideas welcome.

I would be in favor of changing the table layout away from a fixed 50/50%. It might be best to give these tables a class like holdings-table since the fixed layout is coming from the Bootstrap framework.

As far as truncating goes, make sure to test it on the sandal theme for landing on a number. It has the largest font and padding.

EDIT: it occurred to me that having the full title would actually be a good option. The list will format it clearly. If we still want to truncate, a title or making this an abbr element would give the user an option to see the full title.

@crhallberg I've submitted a separate PR #3096 to adjust the columns on all holdings tab tables to 75/25 as we discussed in the zoom on Friday. I've changed the truncation here to a level that seems to work fine (with a bit of margin to spare) even on Sandal. Where it wraps on mobile widths it still doesn't seem to exceed 2 lines each.

module/VuFind/src/VuFind/ILS/Driver/Folio.php

maccabeelevine · 2023-09-15T15:33:43Z

First example screenshot, an bib record with two items, the second being a bound-with. In the list of contents, the bolded item is this bib record.

crhallberg

Everything looks accessibly marked up and clearly presented.

crhallberg · 2023-09-15T16:12:48Z

themes/bootstrap3/templates/RecordTab/holdingsils/boundwith.phtml

+        <?php foreach ($boundWithRecords as $record): ?>
+            <li class="<?=($record['bibId']==$driver->getUniqueId()) ? 'direct-item' : 'bound-with-item'?>">
+                <a href="<?=$this->escapeHtmlAttr($record['bibId'])?>">
+                    <?=$this->escapeHtml($this->truncate($record['title'], 45)) ?>


I would be in favor of changing the table layout away from a fixed 50/50%. It might be best to give these tables a class like holdings-table since the fixed layout is coming from the Bootstrap framework.

crhallberg · 2023-09-15T16:15:07Z

themes/bootstrap3/templates/RecordTab/holdingsils/boundwith.phtml

+        <?php foreach ($boundWithRecords as $record): ?>
+            <li class="<?=($record['bibId']==$driver->getUniqueId()) ? 'direct-item' : 'bound-with-item'?>">
+                <a href="<?=$this->escapeHtmlAttr($record['bibId'])?>">
+                    <?=$this->escapeHtml($this->truncate($record['title'], 45)) ?>


As far as truncating goes, make sure to test it on the sandal theme for landing on a number. It has the largest font and padding.

EDIT: it occurred to me that having the full title would actually be a good option. The list will format it clearly. If we still want to truncate, a title or making this an abbr element would give the user an option to see the full title.

maccabeelevine · 2023-09-18T13:18:45Z

themes/bootstrap3/templates/RecordTab/holdingsils/boundwith.phtml

+        <?=$this->transEsc(
+    $callNumberCount < 2 ? 'bound_with_heading_one_callnumber' : 'bound_with_heading_multiple_callnumbers',
+    ['%%callnumber%%' => $this->escapeHtml($callNumber)]
+)


I really don't like the way this is (not) indented, but it's what php-cs-fixer insists on.

Sometimes it makes bad decisions in templates. You might be able to get better results by leaving the <?= on its own line, then indenting the content on the next line, then closing with ?> below that.

I see that my earlier comment got a bit garbled -- that'll teach me to write something in a rush from my phone. What I was trying to suggest was something like this:

<?= $this->transEsc( $callNumberCount < 2 ? 'bound_with_heading_one_callnumber' : 'bound_with_heading_multiple_callnumbers', ['%%callnumber%%' => $this->escapeHtml($callNumber)] ) ?>

I think with this basic arrangement, php-cs-fixer might tweak the indentation a little bit, but it shouldn't completely de-indent anything in ugly ways.

Yes that worked! Much cleaner.

maccabeelevine · 2023-09-18T13:33:37Z

UX developed with input from Lehigh librarians, and meeting with U Chicago bound-with experts for a second perspective this afternoon.

The UX feedback from U Chicago folks and @crhallberg was generally positive. I made a few changes based on the conversation:

The bound_with_intro paragraph is now optional (not even an empty <p> will display if it's left out.
It turns out that the order of bound-with titles is not necessarily the actual bound order (even though it is for Lehigh). I changed the <ol> to a <ul> and changed the intro text to match.
The group agreed on the general layout but preferred changing all holdings tab tables to a wider first column. I did that in Narrow the first holdings tab column to 25% width #3096 since it has much wider scope than bound-with and may need more input.

I think the UX is now reasonably good, so ready for full review.

demiankatz

Thanks, @maccabeelevine -- this looks pretty reasonable to me. I've left some feedback and replies to your comments below.

I gather from @crhallberg that there is some desire to get this into release 9.1 if possible. We're rapidly closing in on the freeze on new translation strings, however, so I'm not sure how realistic that is. Since it looks like there are some potential BC breaks in the FOLIO driver changes, I don't want to rush this without proper consideration of the implications.

If you do want to increase the likelihood of this getting into 9.1, one approach might be to implement the Demo driver version of the functionality first, instead of FOLIO. Then we could merge the UI changes (including new language strings) along with the Demo functionality first before the freeze, and spend a little more time discussing the FOLIO implementation details.

If you think it's okay to wait until 10.0, though, there's no need to make things more complicated -- just brainstorming options.

module/VuFind/src/VuFind/ILS/Driver/Folio.php

demiankatz · 2023-09-21T12:14:34Z

module/VuFind/src/VuFind/ILS/Driver/Folio.php

+     * @param string $bibId            Current bibliographic ID
+     * @param array  $holdingDetails   Holding details produced by
+     *                                 getHoldingDetailsForItem()
+     * @param object $item             FOLIO item record (decoded from JSON)


It looks to me like you're retrieving $item from a different API than the previous code used, which is necessitating some additional changes (like the way effective location ID is retrieved below). Should this comment reflect exactly where the item came from? Also, it seems like we'll need a changelog note about this, because this could be a breaking change if people have customized formatHoldingItem, and the values they rely on are no longer available or formatted differently. (I didn't dig in to see how many potential differences there are -- but it seems like a possible issue).

demiankatz · 2023-09-21T12:15:17Z

module/VuFind/src/VuFind/ILS/Driver/Folio.php

                    $query
                ) as $item
            ) {
+                if ($item->discoverySuppress ?? false) {


Is it no longer possible to pre-filter because of the API change? Is this a possible performance concern?

Yes, unfortunately the /inventory/items-by-holdings-id API does not support the discoverySuppress query parameter. I can submit that as a FOLIO Jira if it's a concern.

The post-filtering itself is not a performance concern, I think, because in practice I'm not sure how often there will be an unsuppressed instance record, with an unsuppressed holdings record, with a significant number of suppressed item records. Even if that were the case, we immediately ignore any suppressed records after the API call. Eliminating the filter in the call itself might actually speed things up.

That said, the switch from an item-storage API to the higher-level inventory API does have a negative performance impact. In my very ad-hoc testing it adds an average of maybe 100ms to the call, ranging from almost zero difference to ~200ms at worst. So then yeah, if there are 5 holdings records on the instance, that could in theory add a full second to the retrieval. Again I am not sure if this is a practical concern, and there are probably other advantages to having the full (not storage-only) item record. But if needed I could add a config param loadFullItem and allow the code to work both ways for the (probably large) majority of institutions that won't use bound-with.

module/VuFind/src/VuFind/ILS/Driver/Folio.php

themes/bootstrap3/templates/RecordTab/holdingsils.phtml

demiankatz · 2023-09-21T12:25:43Z

themes/bootstrap3/templates/RecordTab/holdingsils/boundwith.phtml

+        <?=$this->transEsc(
+    $callNumberCount < 2 ? 'bound_with_heading_one_callnumber' : 'bound_with_heading_multiple_callnumbers',
+    ['%%callnumber%%' => $this->escapeHtml($callNumber)]
+)


I see that my earlier comment got a bit garbled -- that'll teach me to write something in a rush from my phone. What I was trying to suggest was something like this:

<?= $this->transEsc( $callNumberCount < 2 ? 'bound_with_heading_one_callnumber' : 'bound_with_heading_multiple_callnumbers', ['%%callnumber%%' => $this->escapeHtml($callNumber)] ) ?>

I think with this basic arrangement, php-cs-fixer might tweak the indentation a little bit, but it shouldn't completely de-indent anything in ugly ways.

themes/bootstrap3/templates/RecordTab/holdingsils/standard.phtml

maccabeelevine · 2023-09-21T12:42:29Z

If you think it's okay to wait until 10.0, though, there's no need to make things more complicated -- just brainstorming options.

@demiankatz I have no problem with this going into 10.0. Actually I'm less concerned with the reasons you mentioned than that this effectively relies on #3096 to not look horrible, and that PR may need more time for discussion (despite @crhallberg and I agreeing) since it's a noticeable visual change.

demiankatz · 2023-09-21T12:50:08Z

Thanks, @maccabeelevine -- I've given this a 10.0 milestone so it won't slip through the cracks, and I'll try to give #3096 some priority in the meantime.

demiankatz · 2023-11-28T12:56:31Z

@maccabeelevine, do you want to sort out the conflicts here now that #3096 is merged?

module/VuFind/src/VuFind/ILS/Driver/Demo.php

demiankatz

Thanks, @maccabeelevine, I think this is nearly done! Just one minor formatting complaint, plus I pushed up a small Demo driver improvement that I'd welcome your input on if you have opinions.

themes/bootstrap3/templates/RecordTab/holdingsils/boundwith.phtml

module/VuFind/src/VuFind/ILS/Driver/Demo.php

maccabeelevine · 2023-11-28T18:37:38Z

Thanks, @maccabeelevine, I think this is nearly done! Just one minor formatting complaint, plus I pushed up a small Demo driver improvement that I'd welcome your input on if you have opinions.

I like the percentage configuration; hadn't considered that.

demiankatz

Looks good to me! Thanks, @maccabeelevine!

demiankatz · 2023-11-28T19:35:10Z

@maccabeelevine, I've updated the driver spec (see bottom of table). Please feel free to adjust if you can improve upon this!

maccabeelevine · 2023-11-28T19:40:29Z

@maccabeelevine, I've updated the driver spec (see bottom of table). Please feel free to adjust if you can improve upon this!

I think your description is good -- this is such a cataloging niche, anyone who doesn't know what the phrase means won't actually care. :)

maccabeelevine added 3 commits September 14, 2023 20:35

FOLIO: Support bound-with items

1e99cab

Adjust heading if there are multiple call numbers

de018ce

Move class attribute up to list item.

c000250

maccabeelevine added 2 commits September 15, 2023 14:16

Remove unused code

f0bfade

Revert change no longer needed

e6f5f38

maccabeelevine commented Sep 15, 2023

View reviewed changes

themes/bootstrap3/templates/RecordTab/holdingsils.phtml Show resolved Hide resolved

maccabeelevine commented Sep 15, 2023

View reviewed changes

module/VuFind/src/VuFind/ILS/Driver/Folio.php Show resolved Hide resolved

crhallberg reviewed Sep 15, 2023

View reviewed changes

maccabeelevine added 2 commits September 15, 2023 16:35

Bracket the call number

43c36e1

Hide the bound_with_intro if not in locale file

089d27e

maccabeelevine mentioned this pull request Sep 15, 2023

Narrow the first holdings tab column to 25% width #3096

Merged

maccabeelevine added 5 commits September 18, 2023 12:03

Change truncation length to 85, with vufind-org#3096

a05fff3

Remove statement that the list is in order

bd682a3

Merge branch 'dev' into folio-bound-with

75607c9

Fix tests

86b0723

Fix code sniffer stuff

8a5a32b

maccabeelevine commented Sep 18, 2023

View reviewed changes

Fix order of en.ini

6594f9e

maccabeelevine marked this pull request as ready for review September 18, 2023 13:33

maccabeelevine requested a review from crhallberg September 18, 2023 13:34

demiankatz requested changes Sep 21, 2023

View reviewed changes

demiankatz added this to the 10.0 milestone Sep 21, 2023

sturkel89 added the new feature label Oct 25, 2023

maccabeelevine added 3 commits November 21, 2023 14:16

Merge branch 'dev' into folio-bound-with

abdc022

Trick php-cs-fixer into not formatting this ugly

c27c45b

Add bound-withs to extended holdings template

6346533

maccabeelevine added 2 commits November 28, 2023 13:22

Merge branch 'dev' into folio-bound-with

2b82cb9

Add support to demo driver

5a87933

maccabeelevine commented Nov 28, 2023

View reviewed changes

module/VuFind/src/VuFind/ILS/Driver/Demo.php Outdated Show resolved Hide resolved

maccabeelevine commented Nov 28, 2023

View reviewed changes

module/VuFind/src/VuFind/ILS/Driver/Demo.php Show resolved Hide resolved

maccabeelevine and others added 2 commits November 28, 2023 17:17

Merge branch 'dev' into folio-bound-with

36de6b8

Make bound-with probability configurable.

5fa007b

demiankatz requested changes Nov 28, 2023

View reviewed changes

themes/bootstrap3/templates/RecordTab/holdingsils/boundwith.phtml Outdated Show resolved Hide resolved

module/VuFind/src/VuFind/ILS/Driver/Demo.php Outdated Show resolved Hide resolved

module/VuFind/src/VuFind/ILS/Driver/Demo.php Show resolved Hide resolved

Fix indentation

1af66c3

demiankatz approved these changes Nov 28, 2023

View reviewed changes

demiankatz removed the request for review from crhallberg November 28, 2023 19:27

demiankatz merged commit 33978d5 into vufind-org:dev Nov 28, 2023
7 checks passed

maccabeelevine deleted the folio-bound-with branch November 28, 2023 19:38

maccabeelevine mentioned this pull request Nov 30, 2023

FOLIO: Fix getStatus() after getHolding() change #3243

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FOLIO: Display bound-with titles #3094

FOLIO: Display bound-with titles #3094

maccabeelevine commented Sep 15, 2023 •

edited by demiankatz

maccabeelevine commented Sep 15, 2023

maccabeelevine Sep 15, 2023

maccabeelevine Sep 15, 2023 •

edited

crhallberg Sep 15, 2023

crhallberg Sep 15, 2023 •

edited

maccabeelevine Sep 18, 2023

maccabeelevine commented Sep 15, 2023

crhallberg left a comment

crhallberg Sep 15, 2023

crhallberg Sep 15, 2023 •

edited

maccabeelevine Sep 18, 2023

demiankatz Sep 18, 2023 •

edited

demiankatz Sep 21, 2023

maccabeelevine Nov 21, 2023

maccabeelevine commented Sep 18, 2023

demiankatz left a comment

demiankatz Sep 21, 2023

demiankatz Sep 21, 2023

maccabeelevine Nov 21, 2023 •

edited

demiankatz Sep 21, 2023

maccabeelevine commented Sep 21, 2023

demiankatz commented Sep 21, 2023

demiankatz commented Nov 28, 2023

demiankatz left a comment

maccabeelevine commented Nov 28, 2023

demiankatz left a comment

demiankatz commented Nov 28, 2023

maccabeelevine commented Nov 28, 2023

FOLIO: Display bound-with titles #3094

FOLIO: Display bound-with titles #3094

Conversation

maccabeelevine commented Sep 15, 2023 • edited by demiankatz

Future improvements

TODO

maccabeelevine commented Sep 15, 2023

Choose a reason for hiding this comment

maccabeelevine Sep 15, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crhallberg Sep 15, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maccabeelevine commented Sep 15, 2023

crhallberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crhallberg Sep 15, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

demiankatz Sep 18, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maccabeelevine commented Sep 18, 2023

demiankatz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maccabeelevine Nov 21, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maccabeelevine commented Sep 21, 2023

demiankatz commented Sep 21, 2023

demiankatz commented Nov 28, 2023

demiankatz left a comment

Choose a reason for hiding this comment

maccabeelevine commented Nov 28, 2023

demiankatz left a comment

Choose a reason for hiding this comment

demiankatz commented Nov 28, 2023

maccabeelevine commented Nov 28, 2023

maccabeelevine commented Sep 15, 2023 •

edited by demiankatz

maccabeelevine Sep 15, 2023 •

edited

crhallberg Sep 15, 2023 •

edited

crhallberg Sep 15, 2023 •

edited

demiankatz Sep 18, 2023 •

edited

maccabeelevine Nov 21, 2023 •

edited