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

Iqss/4977 - Reorder Publication Year Facet Chronologically #6958

Merged
merged 13 commits into from
Jun 22, 2020

Conversation

qqmyers
Copy link
Member

@qqmyers qqmyers commented Jun 3, 2020

What this PR does / why we need it: This reorders the results for the Publication Year facet to be chronological

Which issue(s) this PR closes:

Closes #4977

Special notes for your reviewer: FWIW: This PR doesn't resolve the underlying issue that the results for all facets are coming back form solr in order of # of counts. I'm simply reordering the results for this one facet. Since the number of years is not large, sorting should be reasonable. The other shortcut I made is to just use alphabetical sorting since the years are strings. That could be changed too, either in solr, or in the sorting compareTo method I wrote. I'd be happy to do the latter if desired but I don't have the time/expertise to dig into any changes requiring updates to solr itself.

Suggestions on how to test this: The Publication Year facet should show years in chronological descending order. I don't have a repo with more than 5 years available so if there is one, it would be good to test that this works even with the limit of displaying 5 and then having to ask for more. (It should).

Does this PR introduce a user interface change? If mockups are available, please link/include them here: Just the change in order of the entries which is what the issue is about.

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

Additional documentation: None

@coveralls
Copy link

coveralls commented Jun 3, 2020

Coverage Status

Coverage decreased (-0.009%) to 19.557% when pulling e25e2aa on QualitativeDataRepository:IQSS/4977 into 676da97 on IQSS:develop.

@qqmyers qqmyers changed the title Iqss/4977 Iqss/4977 - Reorder Publication Year Facet Chronologically Jun 4, 2020
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.

I didn't run the code but it makes sense and seems like a nice feature, being able to have the Publication Year sorted by year instead of count. I don't want to just chuck this into QA though. Design meeting regulars like @TaniaSchlatter @mheppler @scolapasta and @djbrooke should think about if we want this one facet to behave differently than the others.

For what it's worth, it's a small change we could easily back out. Or we could ask @qqmyers to add a config option to turn it off.

@djbrooke
Copy link
Contributor

djbrooke commented Jun 5, 2020

Thanks @pdurbin for flagging. I'm fine with this change. Sounds like it was an issue for DataverseNO based on the original issue report and an issue for QDR since @qqmyers put up the PR.

I'll let it sit until early next week in case others have differing opinions.

@mheppler
Copy link
Contributor

mheppler commented Jun 5, 2020

@qqmyers, @scolapasta it appears this change does not apply to any date facets, other than Publication Year, correct? I am asking in regards specifically to the Time Period Covered facet also referenced in issue #4977 which this PR closes. Is the solution scalable to cover other date facets that would benefit from this change?

@scolapasta
Copy link
Contributor

scolapasta commented Jun 5, 2020

Looked at code - it os only for publication year.

My 2 cents, I think this isn't really how facets are meant to work. i.e. they are meant to guide users towards what there is the most of. By showing chronologically, you will not necessarily see a large group, without showing "more". e.g:

2020 (1)
2019 (2)
2018 (1)
2017 (2)
2016 (2)

and behind more is 2015 with 100 results.

I completely understand how this is a useful need, just wondering if changing the order of the facet for just this one (or for just date related) may end up being more confusing.

(all that said, I'm fine if others decide in favor of this change; if it's more challenging we can always revert back - I do like Phil's suggestion of having this be a configuration setting)

@qqmyers
Copy link
Member Author

qqmyers commented Jun 5, 2020

FWIW: This is what QDR wants and I've deployed there. For things like this, the PR to IQSS is definitely meant to be up-to-you whether it's generally useful or not. That said, I was surprised when I went to create an issue and found that this same thing had already been requested by other groups (#6945 most recently), so maybe its worth making it an option at least.

@jggautier
Copy link
Contributor

My 2 cents, I think this isn't really how facets are meant to work. i.e. they are meant to guide users towards what there is the most of. By showing chronologically, you will not necessarily see a large group, without showing "more"

Agreed that filters can guide users towards what there's most of. What if the searcher is interested in datasets published in a certain year or in the most recent year? The current ordering can make that more difficult, especially when there are more than five years. How often do people want to know the year in which the most datasets were published, or see those datasets? How often do they want to see all datasets published in a particular year?

QDR's repo has fewer than five years, but would they reconsider the ordering when they have more than five listed?

@qqmyers
Copy link
Member Author

qqmyers commented Jun 5, 2020

@mheppler - this could be applied to other 'date' fields. That said, I'm not sure there's a 'date' category (they're all strings), so it might have to be per name, e.g. if it's configurable you'd list all the fields that should be sorted.

@mheppler
Copy link
Contributor

mheppler commented Jun 5, 2020

@qqmyers, thank you for the clarification. When I saw this PR come in, I too looked at the older issues that were opened suggesting this same change. There appears enough community and stakeholder support for this change that we wouldn't need a configuration setting. User testing this change can confirm assumptions on the benefits to users browsing dataverse results by date.

You can find plenty of examples of facets not uniformly following the highest number of results first pattern. If you have facets that filter results by size, or by price, those break from the other facets and display them 1-10 or 10-1 order.

@scolapasta
Copy link
Contributor

@jggautier technically for finding all datasets that were published in a specific year, you would search for that specifically (using advanced search). One idea (independent of whether we accept this or not, would be to have the advanced search features more easily available, i.e. not as a separate page but as something you can open on this page (to add search criteria to an existing search - which I don't think is currently possible in an easy way - you can always manually type in the query syntax)

@pdurbin
Copy link
Member

pdurbin commented Jun 5, 2020

I'm not sure there's a 'date' category (they're all strings)

In DatasetFieldType there's an enum called FieldType with "DATE". Here's the full list: TEXT, TEXTBOX, DATE, EMAIL, URL, FLOAT, INT, NONE

As an aside, it would also be be nice to have some special handling when in comes to floats and integers, at least for searching (ranges).

@adam3smith
Copy link
Contributor

QDR's repo has fewer than five years, but would they reconsider the ordering when they have more than five listed?

We'd definitely leave this sorted. I (and our testers) find the frequency sorting for years confusing (and untidy ;), but also, I'm not sure that "let me look at the data from the year with most publications" is a particularly meaningful query, whereas "let me look at data from the most recent year" or "last year" seems pretty standard.

@TaniaSchlatter
Copy link
Member

I am in favor of this change. We have user feedback that the quantity-based ranking of dates is non-intuitive. Ideally Deposit Date would display in chronological order (most recent first) as well.

@TaniaSchlatter TaniaSchlatter removed their assignment Jun 5, 2020
@pdurbin
Copy link
Member

pdurbin commented Jun 8, 2020

The other enum that might help is called SolrType in SolrField. Here are the values:

STRING("string"), TEXT_EN("text_en"), INTEGER("int"), LONG("long"), DATE("text_en"), EMAIL("text_en");

You can do getSolrField from DatasetFieldType.

@djbrooke djbrooke self-assigned this Jun 8, 2020
@djbrooke
Copy link
Contributor

djbrooke commented Jun 9, 2020

Hi @qqmyers, we took 5 to discuss this in the design meeting earlier today. There were two requests:

  • Can you add a setting for this? Out of the box, the default sort would be the way that you have it implemented in this PR. The setting would allow installations to change back to the current method.
  • Can you apply this sort to all facets that have dates?

Thanks, and if you have further thoughts or don't want to make these changes for whatever reason, let us know.

@djbrooke djbrooke assigned qqmyers and unassigned djbrooke Jun 9, 2020
@qqmyers
Copy link
Member Author

qqmyers commented Jun 12, 2020

@djbrooke - should be good to go. New :ChronologicalDateFacets setting that is applied to pulicationDate all DatasetFileTypes with FieldType.DATE that are in facets (e.g. when chosen as a facet for a Dataverse). I tested briefly with depositDate at QDR - looks like it works.

@djbrooke
Copy link
Contributor

Thanks @qqmyers !

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.

Looks good. I'm glad to see it's configurable and that all the date facets are included.

@kcondon kcondon self-assigned this Jun 22, 2020
@kcondon kcondon merged commit b97185a into IQSS:develop Jun 22, 2020
@djbrooke djbrooke added this to the Dataverse 5 milestone Jun 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Publication year sort in search facets
10 participants