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

Changing facet sort is broken in pagination view when default sort is set #462

Closed
devn opened this issue Oct 4, 2012 · 5 comments
Closed
Assignees
Milestone

Comments

@devn
Copy link

devn commented Oct 4, 2012

If you set:

config.default_solr_params = {
  ...
  :'f.foo.facet.sort' => 'index',
  ...
}

You will be unable to switch to Numeric sorting in the paginated facet view. You can click it, but the sort order does not change.

@ghost ghost assigned cbeer Oct 5, 2012
@cbeer
Copy link
Member

cbeer commented Oct 5, 2012

I see it. We're using the generic facet.sort parameter in Blacklight::SolrHelper#solr_facet_params (https://github.com/projectblacklight/blacklight/blob/master/lib/blacklight/solr_helper.rb#L450), which is overridden by the more specific f.field.facet.sort. I don't see why we shouldn't change #solr_facet_params to use the more specific form.

Here's the offending section:

    solr_params['facet.offset'] = input[  Blacklight::Solr::FacetPaginator.request_keys[:offset]  ].to_i # will default to 0 if nil
    solr_params['facet.sort'] = input[  Blacklight::Solr::FacetPaginator.request_keys[:sort] ]     
    solr_params[:rows] = 0

    return solr_params
  end

I think we can turn that into:

    solr_params["f.#{facet_field}.facet.offset"] = input[  Blacklight::Solr::FacetPaginator.request_keys[:offset]  ].to_i # will default to 0 if nil
    solr_params["f.#{facet_field}.facet.sort"] = input[  Blacklight::Solr::FacetPaginator.request_keys[:sort] ]     
    solr_params[:rows] = 0

    return solr_params
  end

If you (or another committer) want to pull together a patch for master with some tests (and, let me know if you want this backported to some other version.. it should be easy enough to apply elsewhere), that might speed things up. Else, I'll try to get to it in the next week.

devn added a commit to devn/blacklight that referenced this issue Oct 6, 2012
@devn
Copy link
Author

devn commented Oct 6, 2012

I submitted a patch. I cannot get tests to run so there's no test included in my patch. If you want to backport to 3.5.0 that'd be cool.

@devn
Copy link
Author

devn commented Oct 6, 2012

Tried the above suggestion in the patch I submitted, but it is now actually more broken than before: When you click the button to sort by count in the pagination view the buttons are actually removed from the page and the sort order does not change.

devn added a commit to devn/blacklight that referenced this issue Oct 6, 2012
@devn
Copy link
Author

devn commented Oct 6, 2012

I based the above commit off of the release-3.5 branch. Again this is untested, but it looks like this will fix it.

@cbeer cbeer closed this as completed in ed66fd9 Oct 6, 2012
@cbeer
Copy link
Member

cbeer commented Oct 6, 2012

I got my symbols and strings mixed up in my example code. ed66fd9 should be right.

devn added a commit to devn/blacklight that referenced this issue Oct 6, 2012
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

No branches or pull requests

2 participants