Pitfalls of using QueryObject pattern

[Galathius]

Hi folks! Thanks for your work on this gem.

QueryObject is a commonly used pattern in RoR community, and I noticed that everyone is re-implementing it similarly. All realizations I saw were mostly designed to be used in ActiveRecord scopes definition:

class User < ApplicationRecord
  scope :active, Users::ActiveQuery
end

Using query objects in this way has quite unpleasant and hidden behavior – if you try to create some subqueries on User model inside of query object – it will be scoped to the "current" scope.

I've briefly described the problem itself here.

Also there are some instructions how to reproduce the issue with this gem:

# rails new pattern-gem-test && cd pattern-gem-test
# bundle add rails-patterns

# rails g model User name:string terminated:boolean manager_id:integer

# rails db:migrate

# app/models/user.rb
class User < ApplicationRecord
  belongs_to :manager, class_name: 'User', optional: true

  scope :with_terminated_manager, Users::WithTerminatedManagerQuery
end

# app/models/users/with_terminated_manager_query.rb
class Users::WithTerminatedManagerQuery < Patterns::Query
  queries User

  def query
    relation.where(manager: User.where(terminated: true))
  end
end

# Then by executing the scope this query:
User.where(name: 'Galalthius').with_terminated_manager
# SELECT "users".*
# FROM "users"
# WHERE "users"."name" = 'Galathius'       <---------- name condition
#   AND "users"."manager_id" IN
#     (SELECT "users"."id"
#      FROM "users"
#      WHERE "users"."name" = 'Galathius'  <---------- name condition
#        AND "users"."terminated" = TRUE)

So far I found the only workaround for this, is simply define scopes using lambdas, although this reduces readability:

  scope :with_terminated_manager, -> { Users::WithTerminatedManagerQuery.new(self).call }

[stevo]

Personally I dislike using it this way altogether. I tend to always use those objects directly (without being triggered by scope) just to encapsulate specific filtering settings for a specific use-case(s).

[Galathius]

@stevo I personally agree, but I can also imagine that some people may want to use it exactly in this way. I would propose to add one more class method to Query pattern, something like ar_scope which will return lambda. Something like this:

class << self
    def ar_scope
      klass = self
      ->(*args) { klass.new(self, *args).resolve }
    end
  end

And then you can use it in your scope definition:

scope :with_terminated_manager, Users::WithTerminatedManagerQuery.ar_scope

If you don't mind I can create PR for this change.

[wojtodzio]

Just quickly looking at it, I think you could also undo whatever Rails' scoping does quite easily

> relation = Event.paid; relation.scoping { Event.all.count }
=> 4
> relation = Event.paid; relation.scoping { relation.unscoped.scoping { Event.all.count }}
=> 31

I think you could use it to do something like this:

class CoolQuery < Patterns::Query
  def call
    relation.unscoped.scoping { query }
  end

  def query
    # Your query goes here
  end

  # The rest of the class was omitted for brevity
end