-
Notifications
You must be signed in to change notification settings - Fork 494
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
Make licenses searchable and facetable #10204
Make licenses searchable and facetable #10204
Conversation
…searchable in API and filterable in GUI facets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't run the code but it looks good and the video is fantastic! I feel vindicated that very little code was required to add this feature! 🎉
I left a few comments. Thanks, @luddaniel! ❤️
doc/release-notes/9060-7482-make-licenses-searchable-faceatable.md
Outdated
Show resolved
Hide resolved
doc/release-notes/9060-7482-make-licenses-searchable-faceatable.md
Outdated
Show resolved
Hide resolved
@@ -239,7 +239,7 @@ private List<String> getSolrFilterQueries(boolean totalCountsOnly){ | |||
//fq=publicationStatus:"Unpublished"&fq=publicationStatus:"Draft" | |||
|
|||
// ----------------------------------------------------------------- | |||
// (4) FQ by dataset metadata vlidity | |||
// (4) FQ by dataset metadata validity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha, good catch.
@@ -741,23 +744,23 @@ public SolrQueryResponse search( | |||
// logger.info("field: " + facetField.getName() + " " + facetFieldCount.getName() + " (" + facetFieldCount.getCount() + ")"); | |||
String localefriendlyName = null; | |||
if (facetFieldCount.getCount() > 0) { | |||
if(metadataBlockName.length() > 0 ) { | |||
localefriendlyName = getLocaleTitle(datasetFieldName,facetFieldCount.getName(), metadataBlockName); | |||
if(metadataBlockName.length() > 0 ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and below it looks like we're fixing some whitespace problems. Only three spaces or something.
Just added Custom Terms and internationalization on license facet. Also suggestions of first review by @pdurbin. demo_license_facet_i18n.mp4 |
FWIW: QDR is running this in production. |
Very cool to see this at https://data.qdr.syr.edu ! @philippconzett et al., check it out! |
2024/01/29
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like requested changes have been made, so approving so it can move on.
QA: works great. Also edited licenses after publishing and was very happy. |
What this PR does / why we need it:
This PR allow user to filter by License via GUI Facets and via Search API
fq
parameter.Which issue(s) this PR closes:
Special notes for your reviewer:
There is 2 ideas that I decided to discard :
Feel free to give your thoughts.
Suggestions on how to test this:
This has been tested with 2 datasets of different licenses and 1 file.
Ex. of search API queries :
http://localhost:8080/api/search?q=*&fq=license%3A%22CC+BY+4.0%22
{"status":"OK","data":{"q":"*","total_count":2
http://localhost:8080/api/search?q=*&fq=license%3A%22CC0+1.0%22
{"status":"OK","data":{"q":"*","total_count":1
Does this PR introduce a user interface change?
GUI demo :
demo_license_facet_search_solr.mp4
Is there a release notes update needed for this change?
Yes