6641 search ui bug

[sekmiller]

What this PR does / why we need it:
This PR prevents the display of duplicate filter query facets on the Dataverse Page. It also removes the subtree and dvobject type facets from the selected filter queries display.

Which issue(s) this PR closes:

Closes #6641 Search UI Bug

Special notes for your reviewer:

Suggestions on how to test this:
From a sub-dataverse select multiple facets and verify that search works as expected.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Bug fix

Is there a release notes update needed for this change?:
No
Additional documentation:
None

[coveralls]

Coverage decreased (-0.005%) to 19.539% when pulling be40d35 on 6641-search-ui-bug into d002459 on develop.

[pdurbin]

I'm a little confused about the fix. I thought we wanted to go back to how things were. If I look at ADA running 4.6.1 for example (screenshot below), we only see "Publication Date" and nothing about subtreePaths or Type. Sort of a clean look, in my opinion.

4.6.1

This pull request

[mheppler]

@sekmiller, the suggestion from @pdurbin above is correct. The intention of opening the issue and reporting the bug was to return those selected facet components to their original functionality, as outlined in the 4.6.1 screenshot.

Comparing the screenshot above, to the screenshot provided in the original issue, I can see the duplicates were fixed, but there are still the "strange" subTreePaths and Type selected facets displayed. As stated in the original issue, "we do not want to display the strange ID facet, but also I believe that displaying the Type: (files) is a bug."

Removing those still needs to be done.

[pdurbin]

I haven't actually run the code but I left a few questions and comments in my review.

The primary one I'm wondering about is if we can simply revert setFilterQueries to what it was two releases ago.

If not, that's fine. Whatever we end up with, we should update "What this PR does / why we need it" so it's clear to QA what has changed since the initial pull request.

There's also an if that would benefit from some curlies.

[pdurbin]

Is this setFilterQueries the crux of the bug? Of #6641? I see some conversation at cbad027#r360054910 about it was switched from filterQueries to filterQueriesFinal in cbad027 (part of pull request #6473). I guess I'm wondering if a one line change to switch it back from filterQueriesFinal to filterQueries would fix it the bug.

[scolapasta]

A one line fix, of course would be nice (and preferred) though we should make sure we understand why that line was changed in the first place. @sekmiller can you look at this and try to understand? Maybe build a branch with the one line change to see if it fixes it, then we can decide if how to get both issues to work?

[sekmiller]

More than a one line fix but fairly simple. The addition of the DVobject types and subtree to the list of filter queries was necessary for the fix in #6268. I added a method to the SearchIncludeFragment getTypeFromFilterQuery in order to not render those filter queries in the search-include-fragment (see lines 341-342)

[pdurbin]

I see. I kept wondering why we're adding more code in this pull request when I thought we'd be reverting something. Because the new types were added recently (DvObject and subtree), now we need to hide them, it sounds like.

[sekmiller]

Right and also during development I noticed that the filter queries were being added multiple times hence the isFilterQueryAlreadyInMap method.