Refactoring Filters
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 thosewhere
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.