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

Optional filters and allow_nil #283

Open
tmikoss opened this issue Feb 27, 2020 · 5 comments
Open

Optional filters and allow_nil #283

tmikoss opened this issue Feb 27, 2020 · 5 comments

Comments

@tmikoss
Copy link

tmikoss commented Feb 27, 2020

I've ran into a specific case - I'd like to allow for one of the filter values to be nil, but also for that filter to not be ran.

My use case is running dynamically constructed filters through datagrid instances to fetch data. For example, let's assume we have UsersGrid that allows filtering by referrer_id.

Given following params, I'd like to select:

  1. {} - all users, as referrer is of no concern here;
  2. { referrer_id: nil } - only those users that are not referred by anyone;
  3. { referrer_id: 123 } - only those with specific referrer value.

Currently, datagrid can't seem to distinguish between 1 & 2, because the attribute reader would return nil in both cases.

My current workaround is to set the default to some invalid value, and then skip filter when encountering that value:

    filter :referrer_id, allow_nil: true, default: DO_NOT_FILTER do |value|
      if value == DO_NOT_FILTER
        self
      else
        where(referrer_id: value)
      end
    end

Am I missing some better way to handle this case?

@bogdan
Copy link
Owner

bogdan commented Feb 27, 2020

You are doing it right as far as I know ruby.

Fortunately, ruby is not javascript and doesn't distinguish between null and undefined.
And you met the only 1 of a million case when this is a problem.

I don't know any better solution than you have because only Hash can distinuish between having a key and not having a key, ruby object can not and Hash is only used to construct the grid object - it is disposed after Grid#initialize finishes.

@tmikoss
Copy link
Author

tmikoss commented Feb 28, 2020

Would you be open to change to this? I could try implementing something that keeps track of which filters are set from params / defaults, and which are not.

@bogdan
Copy link
Owner

bogdan commented Feb 28, 2020

I am open to a good change. It would be ideal if you form your proposal here before "implementing something" that may not look good at the end.

@tmikoss
Copy link
Author

tmikoss commented Feb 28, 2020

I've had just a cursory glance of the source, so it's only a vague idea at the moment.

I'm thinking along the lines of modifying BaseFilter#unapplicable_value? to reject values that were not set by user. Set values would be tracked from calls to Core#[]=, and however the default values are set.

Since it's a breaking change, and possibly not very obvious one at that, you'd have to enable the behaviour on a per-filter basis with a new option, or a top-level opt-in (my preference).

If this sounds about right, I'll spend some more time getting to know the existing codebase, and then formulate a more concrete proposal.

@bogdan
Copy link
Owner

bogdan commented Mar 3, 2020

I don't see a way to make it work without some conventions: you need to define a special value that you will treat as no value. In your case DO_NOT_FILTER whatever it is.
Also there is an issue that not all values are assignable to all data types.

There is a default filter data type which acts like - no type cast for filter value. Others will typecast filter value to something, which is bad because DO_NOT_FILTER will also be typecasted.

The idea that filter value may return something different from the type specified in filter definition is hard to understand for general user who is familiar with datagrid internals.

The thing we can do in you case is apply a cosmetics like in your BaseGrid class that you can inherit in every subclass:

def self.filter(*args, **options, &block) 
  filter(name, allow_nil: true, default: DO_NOT_FILTER, **options ) do |value|
    if value === DO_NOT_FILTER
      where(name => value)
    end
end

Overall I don't see a generalization of this for each filter type yet.

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