additional browse improvements

Bug #1741997 reported by Galen Charlton on 2018-01-08
This bug affects 1 person
Affects Status Importance Assigned to Milestone

Bug Description

This bug is for a grab-bag of issues related to the authority browse work in bug 1638299:

* related headings links in the browse list should direct to the heading, not bibliographic results for that heading
* the browse list was not taking OPAC visibility into account for related headings

Evergreen master

Galen Charlton (gmc) on 2018-01-08
Changed in evergreen:
milestone: none → 3.0.3
importance: Undecided → Medium
status: New → Confirmed
Galen Charlton (gmc) wrote :
tags: added: pullrequest
Mike Rylander (mrylander) wrote :


All the code looks sane (with one caveat below), but I have a question. Should we take a NULLable depth parameter into account in the bib-is-visible testing function? That might be necessary anyway, if the user were to select "at all libraries" from the result page, and may make it more broadly useful for other purposes where we know a depth selection is possible.

The caveat: I believe you'll want to OR ("|") the default patron filter with any supplied filter that ends up being supplied in the b_attrs context. I ran into exactly that problem over on bug 1736419 which was limiting Located URI records. Testing would tell...

Changed in evergreen:
milestone: 3.0.3 → 3.0.4
Changed in evergreen:
milestone: 3.0.4 → 3.0.5
Changed in evergreen:
milestone: 3.0.5 → 3.0.6
Changed in evergreen:
milestone: 3.0.6 → 3.0.7
Changed in evergreen:
milestone: 3.0.7 → 3.0.8
Kathy Lussier (klussier) wrote :

I'm going to test this branch as we do need these fixes for our browse search. However, I have a couple of questions:

- Will these fixes require an authority reingest?
- Does the branch need more work based on Mike's question and comment in comment #2?

Kathy Lussier (klussier) wrote :

I'm having trouble testing this branch, not necessarily because of the code in this branch, but because I've been able to get cross-references to display even without the page on my test systems. I've tried in master, a recent pull of 3.0, and in 3.0.1 (checking to ensure that no new code has broken this display). I've also tried on clean installs and upgraded databases. Given that I see other 3.0.x catalogs that are successfully displaying cross references, I have to believe I am missing a step in getting them to display. I've linked authorities, linked bibs to authorities, and enabled the global flag to show related references.

Having said that, I did notice that the COMMIT; statement is missing from the schema.bib_vis_tester.sql upgrade script and will need to be added before this is merged.

I'll take another look at this next week.

Kathy Lussier (klussier) wrote :

We added this code to a server with a production database that previously displayed cross-references and were unable to get any cross-references to display. We will be reverting the code on that server to determine if it's an issue with this code or an existing bug in Evergreen.

Kathy Lussier (klussier) wrote :

After reverting the code, the cross-references display, so it looks like it is indeed an issue with this branch.

Changed in evergreen:
milestone: 3.0.8 → 3.1.3
Changed in evergreen:
milestone: 3.1.3 → 3.1.4
Changed in evergreen:
milestone: 3.1.4 → 3.1.5
Changed in evergreen:
milestone: 3.1.5 → 3.1.6
Changed in evergreen:
milestone: 3.1.6 → 3.2.1
Changed in evergreen:
milestone: 3.2.1 → 3.2.2
Changed in evergreen:
milestone: 3.2.2 → 3.2.3
Changed in evergreen:
status: Confirmed → New
milestone: 3.2.3 → 3.3-beta1
Bill Erickson (berick) wrote :

Commenting here since it's related to the search.bib_is_visible() function proposed in Galen's branch...

I found on a 3.2 system transcendent bibs (no copies, no call numbers, no URI's) were visible in patron browse results, but not staff browse results. After much head scratching, I ended up adding this near the top of metabib.staged_browse():

         row_number := -1;
     END IF;

- IF NOT staff THEN
+ SELECT asset.bib_source_default() || '|' INTO b_tests;
         SELECT x.c_attrs, x.b_attrs INTO c_tests, b_tests
           FROM asset.patron_default_visibility_mask() x;
     END IF;

I'm not confident this is the best solution, but presumably similar logic will be needed in search.bib_is_visible(). Maybe we need an asset.staff_default_visbility_mask() function?

Related, should we teach metabib.staged_browse() to call the proposed new search.bib_is_visible() function instead of repeating the logic internally?

Changed in evergreen:
milestone: 3.3-beta1 → 3.3-rc
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers