search

leikind / wice_grid

A Rails grid plugin to create grids with sorting, pagination, and (automatically generated) filters
MIT License
537 stars 213 forks source link

Refactor. #240

Closed ryanfox1985 closed 9 years ago

ryanfox1985 commented 9 years ago

Hi I did my first refactor.

Can you check if fine this file?

lib/wice/helpers/wice_grid_view_helpers.rb https://github.com/leikind/wice_grid/compare/development...ryanfox1985:development#diff-a2fdf58bd2318a96538f29bab5343700L156

Regards

leikind commented 9 years ago

filter_for is a helper which renders external filters. http://wicegrid.herokuapp.com/detached_filters

A external filter is defined by a key. If no such filter exists, it throws an exception, but if a grid has no records to show, no filters should be shown and we relax this rule by NOT throwing exceptions in this case

leikind commented 9 years ago

if @return_empty_strings_for_nonexistent_filters then no exception should be thrown

leikind commented 9 years ago

You have changed the arity of #find_one_for from 1 to 2, but you haven't changed the method itself. There will be an error

ryanfox1985 commented 9 years ago

Ok I understant what means, but can you check if my refactor is fine?

ryanfox1985 commented 9 years ago

Ok I understand I rollback some changes.

leikind commented 9 years ago

merged. Running my test suite

ryanfox1985 commented 9 years ago

Fine.

2015-07-16 11:47 GMT+02:00 Yuri Leikind notifications@github.com:

merged. Running my test suite

— Reply to this email directly or view it on GitHub https://github.com/leikind/wice_grid/pull/240#issuecomment-121907954.