Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extra settings for limiting search facets 10570 #10590

Merged
merged 7 commits into from
Jul 1, 2024

Conversation

landreev
Copy link
Contributor

What this PR does / why we need it:

This gives an instance admin some extra flexibility in limiting the search facets, as shown on the pages, to users and/or bots. Could be another useful tool to have in case of a search engine disaster. Explained in the guide.

Which issue(s) this PR closes:

Closes #10570

Special notes for your reviewer:

Suggestions on how to test this:

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

Is there a release notes update needed for this change?:

Additional documentation:

landreev added 2 commits May 20, 2024 10:42
…n limiting

the search-related bells and whistles luxuries - such as the facets and the object
counts for the types left unchecked in the object types facets - more granularly.
The default behavior is left intact in all cases. #10570
@landreev landreev added Size: 10 A percentage of a sprint. 7 hours. Size: 3 A percentage of a sprint. 2.1 hours. and removed Size: 10 A percentage of a sprint. 7 hours. labels May 23, 2024
@coveralls
Copy link

coveralls commented May 23, 2024

Coverage Status

coverage: 20.628% (-0.001%) from 20.629%
when pulling 9d2a14a on 10570-search-improvements
into 2917dbe on develop.

This comment has been minimized.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, this seems great. I left a comment about naming and docs. I could simply be confused.

doc/sphinx-guides/source/installation/config.rst Outdated Show resolved Hide resolved

Similar to the above, but will disable the facets for Guest users only.

:DisableSolrFacetsForAnonymousUsers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about :DisableSolrFacetsForInitialRequests instead?

Perhaps I'm misunderstanding the paragraph below, but when set to true, people would be affected by this (and not see the facets) the first time they visit a Dataverse installation, right? Unfortunately, these people would get a less rich, facetless experience, if I understand correctly, when this is set to true for the purpose of mostly avoiding loads from bots.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for human UI users the page will only be facet-less on the first load.
I could rename it :DisableSolrFacetsWithoutJSESSION, to just literally spell out what it does.
It's important to keep in mind that these are simply extra tools to have around for desperate situations.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, :DisableSolrFacetsWithoutJsession (or whatever cApiTaLization) sounds good to me. 😄

This comment has been minimized.

@qqmyers
Copy link
Member

qqmyers commented Jun 18, 2024

@landreev - a couple things to get this through review:

  • I see @pdurbin had one requested name change for a flag that needs to be done (looks like you are OK with the suggestion)
  • the guides build is reporting a bad hyperlink
  • looks like the IT tests didn't run last time - hopefully resolved with a merge with develop and/or your commits for other fixes.

This comment has been minimized.

This comment has been minimized.

Copy link

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:10570-search-improvements
ghcr.io/gdcc/configbaker:10570-search-improvements

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

Copy link
Member

@qqmyers qqmyers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Build fails due to solr changes, so should be merged/tests rerun as part of QA once the solr PR is in.

@qqmyers qqmyers removed their assignment Jun 20, 2024
@landreev landreev added this to the 6.3 milestone Jun 20, 2024
@landreev landreev removed their assignment Jun 26, 2024
@sekmiller sekmiller self-assigned this Jun 28, 2024
@sekmiller sekmiller merged commit 72a2e89 into develop Jul 1, 2024
15 checks passed
@landreev
Copy link
Contributor Author

landreev commented Jul 1, 2024

I was about to repeat that if there was any doubt whatsoever about this pr, it was entirely ok to leave it out of 6.3... but happy to see that it's been merged!

@pdurbin pdurbin deleted the 10570-search-improvements branch July 8, 2024 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: 3 A percentage of a sprint. 2.1 hours.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

More indexing and searching improvements
5 participants