patricklindsay / wice_grid

A Rails grid plugin to create grids with sorting, pagination, and (automatically generated) filters
http://wicegrid.herokuapp.com/
MIT License
33 stars 29 forks source link

Add custom_sort option for model-based sorting #3

Closed JasonBarnabe closed 6 years ago

JasonBarnabe commented 6 years ago

(Retry of #2 - I messed up the branching)

This adds the ability to sort the grid by an arbitrary model attribute or calculation. To enable this feature, add an attribute option to the column definition (does not need to be a DB column name - will just be used as a parameter name) and a custom_sort option. custom_sort should be a lambda that accepts the model and returns the value to be sorted on.

For example, if you are writing blogging software and wish to display the number of published posts an author has, it is not easy to be able to sort by this column if it is not an attribute in the authors table. With custom_sort, you can do:

  g.column name: 'Published Posts', attribute: 'public_posts', custom_sort: ->(author) { author.posts.published.count } do |author|
    author.posts.published.count
  end

and sorting will be enabled on that column.

custom_sort has the same kind of drawback as custom_filter. When the sort is active, all results (not just those on the current page) will be loaded by ActiveRecord, and the calculation will be performed on all results. As such, it is not recommended if you have lots of results or if the calculation is expensive.

I can add documentation if the code changes look acceptable.

patricklindsay commented 6 years ago

Thanks for the contribution. Sounds like a useful feature however I'm not really happy merging new features unless I can be confident that what I'm merging isn't going to break anything.

Ideally everything that is merged in should have release notes, docs & test coverage. A first step would be to move into this repo all the tests from WiceGrid testbed then setup a minimum coverage CI build.

In terms of the PR itself... If we're going to add any new features I'd like to tidy up these ginormous files, or at least not make them any larger. lib/wice_grid.rb alone has 690 LOC. Some housekeeping wouldn't go amiss.

JasonBarnabe commented 6 years ago

I am fine with adding docs and tests for this feature. I wanted to get an opinion on the general concept before I went that far.

While adding the testbed and refactoring may be useful and good, I don't see any reason to do so in this PR. If you want those things done first, that's fine, but I'm not currently planning to do them.

JasonBarnabe commented 6 years ago

For what it's worth, I'm able to run the testbed as is with a dozen failures, but the testbed needs to be updated to work with Rails 5.

patricklindsay commented 6 years ago

No we won't add the test suite in this PR but it will have to go in before anything else does.

Checkout Setup CI Build project for more information.

JasonBarnabe commented 6 years ago

@patricklindsay I know there are probably higher priorities than new feature development, but this is the last change my company needs to get off our fork and back on to this repo's version of wice_grid.

I have:

patricklindsay commented 6 years ago

Thanks for the hard work @JasonBarnabe !