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

custom dropdown filter with fake column #162

Open ghost opened 10 years ago

ghost commented 10 years ago

I'm trying to create a single column header that allows the user to select, via multiple selection, from a list of boolean columns. They are sort of like tags in my application so I don't really want to display an entire column for each one.

My conditions generator looks like this:

class ConditionsGeneratorMultipleBooleansFilter < Wice::Columns::ConditionsGeneratorColumn
  def generate_conditions(table_alias, opts)
    conditions = opts.map do |value|
      "#{@column_wrapper.alias_or_table_name(table_alias)}.#{value} = ?"
    end
    [conditions.join(" AND "), *[true] * conditions.length]
  end
end

Because I need a dropdown, I had to implement my own version of the custom dropdown that hard-coded the values I wanted (because the custom_filter option triggers the custom dropdown automatically).

class ViewColumnMultipleBooleansDropdown < Wice::Columns::ViewColumnCustomDropdown
  def initialize(*)
    super
    self.custom_filter = <array of booleans>
    self.allow_multiple_selection = true
  end
end

Finally, in my haml view, I needed to pick some column or it wouldn't work at all, even though the column has little to do with my query:

  - grid.column(name: 'Attributes', attribute: 'some_column', filter_type: :multiple_booleans) { |product| capture do
    ... show all the boolean attributes here ...
  - }

So I have two questions:

1) Is there a way to pass arguments to a custom-build column filter? If not, do you think there should be? 2) Is there any way that I can avoid picking some arbitrary column to filter on?

Finally, if you have some suggestion for a better way to implement this, I'd love to hear about it. It feels pretty cludgy to me. Thanks!

leikind commented 10 years ago

If "They are sort of like tags in my application so I don't really want to display an entire column for each one." then why implement it as part of WiceGrid at all? Why don't you code your own dropdown outside of WiceGrid the way you want? You could use dump_filter_parameters_as_hidden_fields(grid_object) like it is done http://wicegrid.herokuapp.com/integration_with_forms , or just place the grid(){} helper together with your dropdown inside a form. And then, depending on the state of your dropdown, build the necessary conditions for initialize_grid.

As for your questions: 1) Unfortunately, no, there isn't. You are the first user of the plugin who has redefined a conditions generator :) 2) Could you please elaborate, I am not sure I get the question.

ashanbrown commented 10 years ago

Meant to submit that with my personal account... anyhow, thanks for the response.

The reason I wanted to define custom-built column filter is that I really wanted a column to display the tags and I like the way that I can make the filter appear in the header for the column. I have used detached dropdowns for somethings, although that's still been within the context of WiceGrid. I also want whatever styling wice_grid is giving me for the controls. Question 2 was just about this: do I have to link a column to an active record column?

Since we're on the topic of conditions generators, I've noticed a couple of things. First, I defined a version of the string handler that supports a fulltext-based search on mysql (MATCH ... AGAINST instead of LIKE). It would be handy to make this an option on the existing string handler, but I'd need to be able to let the condition generator know that I've selected to use the fulltext index. Ideally it would determine the value based on the schema, but for now it seemed easiest to make it an option. Currently, I'm just specifying a custom filter_type (ie. I call it 'string_fulltext' and can make a pull-request to add it if you're interested).

The other thing I've noticed trying to build custom filters is that they are loaded at initialization time and, as far as I've observed, they fail very quietly if there is a syntax error. This means I can't use the rails dynamic class loader to load them (I have to restart my app each time I'm testing a change) and that when I do restart I don't really know why it isn't working (I just see that the filter couldn't be loaded). Just wondering if you've noticed those things and if there's some reason they can't be loaded more dynamically.

Thanks (in advance ;) for your thoughts!

leikind commented 10 years ago

1 do I have to link a column to an active record column? Yes.

2. I see your point. A custom filter_type is actually two things: the UI, class Foo < ViewColumn, and the sql conditions, class Baz < ConditionsGeneratorColumn. And in your case it's not about the view, it's only about the SQL query, so having a separate filter_type doesn't seem to be right. It has one advantage though: the simplicity of the API. You just declare a filter type and you are done.

The second option is a global column option. The plugin already has a few global column options which only make sense for a certain filter type, and I wouldn't say I like it a lot. But it works.

The third option is to implement a way of passing arguments to ViewColumn and ConditionsGeneratorColumn. Even though it seems like most correct thing to, this means making the API even more complex.

I am a bit hesitant as to what the way to go should be . Which of these options is your favorite?

3. My initial thinking was that plugins are not reloaded anyway, and these classes are part of the plugin, but then I made it possible to load custom ViewColumns and ConditionsGeneratorColumns from outside of the plugin, and yes, one would expect them to be reloaded in the development environment. It can definitely be improved.

ashanbrown commented 10 years ago

Having a way to pass arguments would definitely be my preference, because that would also solve the other problem I have where I want to give a list of columns to my fake column filter. I do in fact, like you suggested, use the normal view for my fulltext string filter. My config file currently looks like this:

  Wice::Defaults::ADDITIONAL_COLUMN_PROCESSORS = {
    multiple_booleans: %w{
      ViewColumnMultipleBooleansDropdown
      ConditionsGeneratorColumnMultipleBooleansFilter
    },
    string_fulltext: %w{
      Wice::Columns::ViewColumnString
      ConditionsGeneratorColumnStringFulltext
    }
  }

Having a filter_options key that I can use to pass a hash down through everything to my view would do it. I'm not sure how you'd communicate options to the conditions generator though, since it looks like it just takes request parameters. I suppose you could store the filter parameters in the column wrapper. I'd be happy to take a pass at this if you think it's something you might pull. At this point, though, I'd have to start running your test suite and I'm not sure if you were planning on merging it back into the main tree because that would certainly make contributing easier. I imagine you had a good reason to pull it out but separating it does make properly tested pull requests a little more complicated.

Regarding reloading, I just wanted to make sure that the behavior was expected. While I do like automatic reloading during development, the thing I found difficult was that it didn't appear to simply fail to start rails when the class didn't parse, so I'd only find out what was going on later when my filter wasn't being applied.

Anyhow, thanks again for walking through this all with me. I've certainly been able to accomplish what I needed with the gem as it is.

leikind commented 10 years ago

Then I'd agree that filter_options is the way to go.

The test suite is in the demo application which serves its purpose as a standalone application: http://wicegrid.herokuapp.com/ . Capybara tests for an app with turbolinks are a big PITA, by the way.