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

What's the plan for which filters are supported? #37

Open
riyengar8 opened this issue May 3, 2019 · 3 comments
Open

What's the plan for which filters are supported? #37

riyengar8 opened this issue May 3, 2019 · 3 comments

Comments

@riyengar8
Copy link
Collaborator

@fractaledmind - Can you tell me what your vision is for the filter API in this gem? At the moment, on ActiveRecord sets, all the arel predicates are supported as filters. For Enumerable sets, I believe you can use anything as a filter as long as the type you are filtering against supports your filter as a method call.

Do you have any particular plan for trying to get Enumerable sets to support the arel filters for the sake of ease of use?

Here's the situation from which my question arises: I recently had a filter in an application that was using eq_any on a "status" field such that the app would support queries where users wanted to return records that matched some number of slected statuses. This "status" field was originally a DB attribute of the model being filtered, but for unrelated reasons, I changed it to be a computed attribute on model instances. That broke my filter, as instead of using arel, suddenly I was calling "string".eq_any(search_criteria) on every instance in my set.

After looking into the gem to see what was going on, I quickly realized my error, but it has me wondering if it wouldn't be a good idea to try to standardize the filters here in a way that allows some common core group of filters to be used across both ActiveRecord and Enumerable sets. I think that could be a big win for ease-of-use.

FWIW, I ended up solving my problem by monkeypatching String:

class String
  def eq_any(collection)
    collection.include? self
  end
end

That's obviously a hack that is very specific to the particular filter and datatype that I happened to be working with.

One thought I had was that we may be able to use method_missing here to define some of the arel filters on native datatypes such that they will work when passed as filters on enumerabe sets. But, before I did anything like that I thought I'd get your thoughts on what sorts of filters should be supported Enumerable sets or if this gem should do anything to try to have a standard set of filtering signals that work regardless of the set type.

@fractaledmind
Copy link
Owner

Yes. My plan is to have feature parity across both ActiveRecord and Enumerable filters. It may be easier at some point to walk thru the details of my thinking over a call, but I will try to lay out my basic thoughts here.

First, a tad of history/context. Originally, all filter instructions were essentially just dot-separated paths to get to a scalar value followed by a signal that could be passed to that value along with a value attribute. So, for Enumerable, you were actually calling methods on objects and receiving boolean responses; for ActiveRecord, you were essentially string concatenating SQL methods with SQL columns and values. This approach for ActiveRecord was problematic for a few reasons, but also limiting, so I wanted to make things more like Enumerable by actually staying in Ruby and passing signals. Thus, now filtering instructions for ActiveRecord are dot-separated paths to get to an ARel column, which we then pass the filter operation signal to.

This is less problematic than before (fewer edge case bugs) and also less restrictive (more operations you can use); however, your problem is a central issue with this set up. Another problem is that this still restricts filtering to the ARel predication methods. So if, for example, you want to filter a string column to find rows where the cell starts with some substring, you will have to ensure that you add the appropriate wildcards to the value passed to the filter instruction (e.g. path.to.column(matches): 'substring%'). I would prefer having path.to.column(matches_start): 'substring'. What this immediately shows me is that we need a layer for the filtering operations themselves, instead of just passing them directly to the ARel column or Ruby object. Having this layer will then also allow us to implement the same semantics for filtering across both ActiveRecord and Enumerable fields. Organizing, shaping, and implementing this layer is where I have been spending most of my time lately. I have focused on ActiveRecord, because I could move a bit faster and I wanted to find the right patterns as quickly as possible; however, I do have basic implementation plans for Enumerable as well.

So, the plan is to have ActionSet specific filtering operations (the names that go inside the parentheses in the instructions), which are converted into the appropriate commands in whatever context they are being used in. And, this organization will then also allow applications to setup and register their own custom filtering operations as well. I'd say I'm currently like 75% of the way to defining the architecture for this, at which point actually defining each filtering operation should be quite simple.

I'm happy to dig into more detail about any particular aspect of this, so just hit me with whatever follow-up questions. I'm currently working on getting the monorepo version of ActionSet released, at which point I can return to this architectural work. When I do, I'll be sure to let you know what branch I'm working in.

@fractaledmind
Copy link
Owner

While I'm here, just a note that I pushed out a few point releases today to ActiveSet to get the codebase as squared-away as possible before I merge it with ActionSet. The latest release is 0.8.5, and you can read the CHANGELOG and view the PRs to see the changes (mostly in spec, but a couple of bug fixes, including porting your Enumerable sorting will nils fix)

@fractaledmind
Copy link
Owner

Note: I just published version 0.8.6 with some edge case bug fixes found as I refactor the test-suite

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