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

First shot at an automatic ransack implementation #36

Merged
merged 3 commits into from Apr 29, 2014

Conversation

jvdp
Copy link

@jvdp jvdp commented Apr 15, 2014

I forgot what else I wanted to add before I went on vacation so I'll just post this now.

This PR adds the ransack gem and a filter_fields directive to Brightcontent controllers. By default, this gives you a dropdown with all the (unique) values in the database but you can also override this with a partial named _filter_field_* as is customary.


<%= render_if_exists "index_top" %>
<%= render "list_filters" %>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved some things around to make naming more consistent:

  • _index_top.html is replaced by _index_header.html and contains the resource name and "new resource" button (you can add eg. CSV buttons here without replacing _index.html.) For backwards compatibility, _list_header.html loads _index_top.html
  • list filters are added below the header

class_eval <<-RUBY
included do
%w[list_fields filter_fields form_fields default_fields].each do |name|
class_eval <<-RUBY, __FILE__, __LINE__ + 1
def self.#{name}(*fields)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did something like this recently, not sure if I like it better. Although this implementation is a bit cleaner, the interface is a bit less clear. I think here we'd want a nice DSL

class << self
  attr_accessor :list_fields, :form_fields, :default_fields
end

@list_fields = [:title, :body]

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain what the advantages or disadvantages are? I personally prefer the class method approach because it gives much better DSL then defining class instance methods. Imagine defining ActiveRecord relations like that...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said, the DSL is a lot nicer as it is. I think the other implementation is a tad cleaner though, without class_eval. But I think the nicer DSL is what counts here.

Conflicts:
	core/app/views/brightcontent/base/index.html.erb
jvdp added a commit that referenced this pull request Apr 29, 2014
First shot at an automatic ransack implementation
@jvdp jvdp merged commit 8a981ef into stefanroex:master Apr 29, 2014
@jvdp jvdp deleted the ransack-filtering branch June 16, 2014 14:26
jvdp added a commit to jvdp/brightcontent that referenced this pull request Apr 20, 2015
…lter-fields

Allow arbitrary filter field names (fixes stefanroex#36)
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

3 participants