Recently I was working on refactoring some unwieldy code for filtering data. Here's a short story of how it went 🙂.

The code I was refactoring was large, but one repetition particularly stood out:

# app/models/user.rb

def self.filter(filters)
  if filters[:status].present?
    User.where(status: filters[:status]
  end
  if filters[:location_id].present?
    User.where(location_id: filters[:location_id])
  end
  # and so on...
end

The UI had a large set of filters for filtering through a list - which is what this code does - filters through records from database.

My first thoughts were:

  • Why is this code so verbose?

  • Why do we need those if conditions? Without them, we can just chain those where clauses and this code would read so much better!

Let us try removing the conditions. Ohh wait, this code does not have tests! 😱

Time to flex rspec muscles 💪. Yeah TDD!

# spec/models/user_spec.rb

it "filters users by status" do
  # Create some users - Arrange
  filtered_users = User.filter(status: "pending").pluck(:id) # Act
  expect(filtered_users).to match_array(users.map(&:id)) # Assert
end

Run tests - passes ✅

Remove "if" - passes ✅ As expected.

Now for the real test.. We naturally expect all records to be returned if no filter is applied.

# spec/models/user_spec.rb

it "returns all users if no status filter applied" do
  # Create some users - Arrange
  filtered_users = User.filter(status: nil).pluck(:id) # Act
  expect(filtered_users).to match_array(User.pluck(:id)) # Assert
end

Run tests - passes ✅

Remove "if" - fails ❌ There you go. That is why we need the condition.

To see what exactly is causing this: ** Hops into REPL **

irb(main)> User.count
=> 10
irb(main)> User.where(status: "pending").count
   (1.0ms)  SELECT COUNT(*) FROM "users" WHERE "users"."status" = $1  [["status", "pending"]]
=> 4
irb(main)> User.where(status: nil).count
   (0.9ms)  SELECT COUNT(*) FROM "users" WHERE "users"."status" IS NULL
=> 0

Fair enough. ActiveRecord gets nil, it filters by NULL. Let us see if we can find a way to somehow still chain those methods.

ActiveRecord expects a Relation object for chaining to work. Try creating a generic method:

# app/models/user.rb

def where_if(filters, attribute:)
  data = filters[attribute]
  return all unless data

  where(attribute => data)
end

The secret sauce here is ActiveRecord's all method which returns scope (Relation) object of all queries applied until this point. And we know that if we have a Relation object, we can chain methods.

Refactoring User.filter method:

# app/models/user.rb

def self.filter(filters)
  where_if(filters, :status)
    .where_if(filters, :location_id)
    .where_if(filters, :is_active)
    .where_if(filters, :locale)
    # and some more...
end

Much better! 🎉

Taking it a step further, the application had few more listing pages which means other models with similar .filter method. Our .where_if method is generic enough to be used by all of them.

# app/models/concerns/filterable.rb

module Filterable
  def where_if(filters, attribute:, parameter: nil)
    data = parameter ? filters[parameter] : filters[attribute]
    return all unless data

    where(attribute => data)
  end
end

And now we can include Filterable in all those models having filter. I also went ahead and separated attribute and parameter which helps in cases where parameter name from frontend does not match the database attribute name in backend.