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

[#861] Change the logic for sorting dataset search #997

Merged
merged 3 commits into from Jul 1, 2013

Conversation

nigelbabu
Copy link
Contributor

Default to metadata_modified desc if no query and relevance if there is
a query.

This breaks a whole lot of tests and I'm not sure if I'm doing the right thing. @seanh, if you could take a look at a fix and tell me if I'm going in the right direction, that'd be great.

Default to metadata_modified desc if no query and relevance if there is
a query.
@seanh
Copy link
Contributor

seanh commented Jun 13, 2013

It looks like when there is a search query, your code sets the sort order to 'score desc', that should be 'score desc, metadata_modified desc'.

Also I'm not sure about adding 'metadata_modified' to the if condition. The question you're trying to ask here, is whether the user has chosen a sort ordering from the dropdown, or whether we're using the default. If they've chosen something we just go with that, no logic.

@ghost ghost assigned nigelbabu Jun 13, 2013
Also, modify the default sort order if there is a search query to be
"score desc, metadata_modified"
@ghost ghost assigned seanh Jun 30, 2013
@nigelbabu
Copy link
Contributor Author

@seanh YES, this is ready for review again! Sorry about not changing the assignment or pinging you.

@tobes
Copy link
Contributor

tobes commented Jul 1, 2013

merged and requesting for 2.1

@tobes tobes merged commit 067d430 into master Jul 1, 2013
@tobes tobes deleted the 861-order-by-dropdown branch July 1, 2013 11:54
@amercader
Copy link
Member

@seanh @nigelbabu @tobes @kindly
OK, a couple of problems with this PR and issue, which mean that it won't go into 2.1 (as it is now).

First of all, the merge went awry and basically we have the same behaviour as before in master.
https://github.com/okfn/ckan/blob/master/ckan/logic/action/get.py#L1324

Apart from that, the implementation on 861-order-by-dropdown is not right, because in the dataset search page you always get 'Last modified' as the default. That's because when submitting the form we are forcing the sort to be 'metadata_modified desc', which defeats the purpose of the original plan "sort by relevance if q otherwise metadata_modified".

Which brings me to the point that I don't like the original decision at all (sorry for not raising concerns on the issue, I missed the discussion). I think that the logic we are trying to fit is far more complicated than the supposed complexity for users. Relevance is the best sort in most cases, and if you don't have a query defined, like in the initial datasets page, the most relevant records are the most recent. It is important not to mistake Relevance with score, the actual numeric variable used to rank results. Relevance is what is more relevant to users, period.

My vote would go to revert the change in master and leave things as they were.

We take great care in providing relevant search results and this implementation means that this is no longer leveraged in the frontend.

@tobes
Copy link
Contributor

tobes commented Jul 1, 2013

ok I reverted this change in master but github doesn't allow reopening so will do the original issue

@seanh
Copy link
Contributor

seanh commented Jul 2, 2013

Relevance is the best sort in most cases, and if you don't have a query defined, like in the initial
datasets page, the most relevant records are the most recent.

IIRC the original problem (reported by @darwinp ) was that when there's no search query it says "Order by: Relevance" which (from the user's point of view) doesn't make any sense because relevance is meaningless when there's no search term - relevant to what? (Again, from the user's point of view) and the actual sort order is modification date. So it's more of a user interface problem, rather than the actual sorting logic being wrong.

So we needed some fix where (at least on the frontend) it would say order by: updated when there was no search term or order by: relevance when there is.

Maybe this can be done in the frontend instead of in the logic?

I guess it all goes back to the original discussion in #861.

Anyway happy for this pull request to be rejected and lets either try another fix for #861 in 2.2 or just close #861

Apart from that, the implementation on 861-order-by-dropdown is not right, because in the dataset search page you always get 'Last modified' as the default. That's because when submitting the form we are forcing the sort to be 'metadata_modified desc', which defeats the purpose of the original plan "sort by relevance if q otherwise metadata_modified".

D'oh!

We take great care in providing relevant search results and this implementation means that
this is no longer leveraged in the frontend.

Yeah, with any solution to #861 when definitely want to keep the relevance in there at least when there's a search query (this was always the plan but sounds like we messed up)

@tobes
Copy link
Contributor

tobes commented Jul 3, 2013

How about when we do the search for relevance with no query we silently sort by last_modified as that is the most relevant and keep the sort order showing as relevant then other searches are unaffected.

Would that give us an easy solution?

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.

None yet

4 participants