Blacklight 8, empty range facet shown even for zero results page

[jrochkind]

I wanted to reproduce this on a fresh empty app, but I've been unable to create a fresh empty Blacklight 8 app with blacklight_range_limit, per #236

But I do somehow have a branch of my actual app working with blacklight_range_limit 8.3.0, blacklight 8.0.1, and rails 7.0.5.1. (I am not sure how I got this to work, and have not been able to reproduce in a new blank app!)

Here I have only one problem. In cases of zero results, when the Blacklight "zero results" message is shown, with Blacklight 7, you get no facets in sidebar -- this is expected.

In Blacklight 8, my single blacklight_range_limit facet is still showing up in sidebar on "zero results" page, and with facet body content that does not make any sense.

I believe that this is not about anything otherwise custom to my app, and could be reproducible in a stock app -- if anyone can make one! But these screenshots do show CSS customizations, and the custom "other search tools" static content we've added to our sidebar.

Blacklight 7

As expected, no facet content in "zero results" page. ("Other search tools" static content in sidebar is an unrelated customization). As there are no available facets to display at all, we don't even get the "Limit your search" facet section header.

Blacklight 8

"Date" is our range facet -- it is showing up in sidebar even in "zero results" page, but with a body that doesn't make any sense. This should not be showing up in a zero results page, as it was not in BL 7.

[jrochkind]

This is related to projectblacklight/blacklight#3054 , but is also it's own problem.

If the problem were solved so blacklight_range_limit were not rendering a box for an "empty" result set, then you would still get "Limit your search" header because of projectblacklight/blacklight#3054 . But there's a separate pre-requisite problem here, to make it stop rendering the nonsensical facet box for empty results.

[jrochkind]

I think I have a solution, which would be implementing BlacklightRangeLimit::RangeFacetComponent#render? as so:

    # Don't render if we have no values at all -- most commonly on a zero results page.
    #
    # Normally we'll have at least a min and a max (of values in result set, solr returns),
    # OR a count of objects missing a value -- if we don't have ANY of that, there is literally
    # nothing we can display, and we're probably in a zero results situation.
    def render?
      (@facet_field.min && @facet_field.max) || @facet_field.missing_facet_item
    end

But I'm having trouble putting together a PR, because I am having so much trouble with existing tests.

Trying to run bundle exec rake ci I always get

Port 8983 is already being used by another process (pid: 26116)

Even though there is in fact no process at 26116. I feel like I remember there's an unsolved race condition here maybe in solr_wrapper, but now I can't find an issue on it. Or maybe it's an unrelated problem.

If I try to run ONLY the range_facet_component_spec.rb specs, to at least get somewhere locally... they ALL fail with:

     Failure/Error:
       instance_double(
         BlacklightRangeLimit::FacetFieldPresenter,
         key: 'key',
         html_id: 'id',
         active?: false,
         collapsed?: false,
         in_modal?: false,
         label: 'My facet field',
         selected_range: nil,
         selected_range_facet_item: nil,

       the BlacklightRangeLimit::FacetFieldPresenter class does not implement the instance method: html_id
     # ./spec/components/range_facet_component_spec.rb:15:in `block (2 levels) in <top (required)>'

If I remove html_id: 'id' from instance_double... I still get other test failures.

I wonder if tests are actually failing on main in latest Blacklight, maybe this just doesn't really work with BL8 at all currently, and has a bunch of things that need to be fixed?

[seanaery]

@jrochkind I tried out your proposed solution here (adding render? method to RangeFacetComponent) and it works great. So if those test issues you encountered last year are resolved and you're able to get this into a PR, I think it'd be a nice addition for version 9.

[jrochkind]

Oh interesting! if you already tried it out, any interest in doing the PR @seanaery ? I guess it would need a spec too.

I've got a couple other bugs I'd like to attack still too.

[seanaery]

Sure, I'm glad to do it -- just didn't want to take credit for your work. I'll likely be able to get to this tomorrow.