[rspec-users] Controller spec: testing that scope is set

[Zach Dennis]

On Mon, Apr 20, 2009 at 1:35 PM, Zach Dennis <zach.dennis at gmail.com> wrote:
> On Sun, Apr 19, 2009 at 6:41 PM, Michael Schuerig <michael at schuerig.de> wrote:
>> On Sunday 19 April 2009, Zach Dennis wrote:
>>> On Sun, Apr 19, 2009 at 2:09 PM, Michael Schuerig
>> <michael at schuerig.de> wrote:
>>> > On Sunday 19 April 2009, Zach Dennis wrote:
>>> >> On Sun, Apr 19, 2009 at 12:27 PM, Michael Schuerig
>>> >
>>> > <michael at schuerig.de> wrote:
>>> >> > In a Rails controller I set the scope on a model class in an
>>> >> > around filter. I have defined expectations on the model classes,
>>> >> > and ideally, I would add a further expectation for the scope. Is
>>> >> > this already possible in some way? How would I go about adding
>>> >> > support a scope expectation?
>>> >>
>>> >> How are you setting the said scope?
>>> >
>>> > In an around filter. However, I don't want to test the around
>>> > filter mechanism, it might as well be rack middleware instead.
>>>
>>> Sorry, I don't know what scope means to you in your app. Can you
>>> share your around_filter?
>>
>> Oops, sorry, I assumed the concept from ActiveRecord would be familiar.
>> If you know ActiveRecord::Base#with_scope that's really all there is. A
>> scope, within a block or through a proxy, defines options that are
>> merged with the arguments to #find et al. This merging happens behind
>> the scenes, therefore the scoped options are effective, but don't show
>> up as arguments anywhere.
>>
>> I'm using this in conjunction with a generic query representation
>> (inspired by JSON Query) that is map through a combination of Rack
>> middleware and generated around_filters, see below for a glimpse.
>>
>> Michael
>>
>>
>> class PeopleController < ApplicationController
>>  include QueryScope
>>
>>  query_scope :only => :index do
>>    # Only allow to filter and order by the
>>    # virtual name attribute.
>>    # This attribute is mapped onto the real
>>    # firstname and lastname attributes.
>>    allow     :name
>>    condition :name =>
>>      "LOWER(firstname || ' ' || lastname) :op LOWER(?)"
>>    order     :name => "lastname :dir, firstname :dir"
>>  end
>>  ...
>>
>> Somewhere in QueryScope
>>
>> def query_scope(options = {}, &config_block)
>>  model_class = extract_resource!(options)
>>  builder = QueryScopeBuilder.new(config_block)
>>  around_filter(options) do |controller, action|
>>    req = builder.build_request_conditioner(controller.request)
>>    controller.instance_variable_set(:@offset_limit, req.offset_limit)
>>    model_class.send(:with_scope, :find => req.find_options, &action)
>>  end
>> end
>
> I think I am starting to understand what you're after. You want to
> ensure the scope defined in your query_scope configuration block in
> the controller is used to set the scope on the controller's model.
> Right?
>
> With the assumption that that is correct, I would probably refactor
> how your #query_scope method works. Right now you're implicitly going
> through a QueryScopeBuilder to get a RequestConditioner, in order to
> access the #find_options and #offset_limit behaviour on that
> RequestConditioner. I would make your controller deal with one object,
> perhaps a RequestToQueryTranslator. Your #query_scope method would
> come out looking like:
>
>   def query_scope(options = {}, &config_block)
>    model_class = extract_resource!(options)
>    query = RequestToQueryTranslator.translate(controller.request,
> &config_block)
>    around_filter(options) do |controller, action|
>      controller.instance_variable_set(:@offset_limit, query.offset_limit)
>      model_class.send(:with_scope, :find => query.find_options, &action)
>    end
>   end
>
> This simplifies the #query_scope method and gives you more
> implementation freedom how your query is constructed. This still
> leaves something difficult to spec though, you are passing your
> query_scope config_block through, and I'm guessing it is
> instance_eval'd. You can't be sure of what's going on in that
> config_block unless you actually instance_eval inside of the
> appropriate object. This limits your ability to write a clean
> object-level example expecting the right query is constructed because
> it requires your controller to work with real dependent objects.
>
> Two approaches that come to mind for dealing with this are to change
> how your config_block works. Rather than:
>
>  query_scope :only => :index do
>    allow     :name
>    condition :name =>
>      "LOWER(firstname || ' ' || lastname) :op LOWER(?)"
>    order     :name => "lastname :dir, firstname :dir"
>  end
>
> You could do:
>
>  query_scope :only => :index do |query|
>    query.allow     :name
>    query.condition :name =>
>      "LOWER(firstname || ' ' || lastname) :op LOWER(?)"
>    query.order     :name => "lastname :dir, firstname :dir"
>  end
>
> Now in your spec, you can write a spec against the query_scope, by
> ensuring the passed in object receives #allow, #condition and #order
> with appropriate arguments. Now you don't have instance eval the block
> in some dependent object somewhere, you can simply pass the query your
> RequestToQueryTranslator.translate method returns to the config_block.
> This gives you the advantage of ensuring that the controller sets up
> the proper query, and it allows you to spec your
> RequestToQueryTranslator in isolation to ensure that given a certain
> set of method calls that it builds the right find options.
>

In addition to this I would still have an example that expected
with_scope to be set on the appropriate model based on the results of
query.find_options, and that @offset_limit was assigned based on the
results of query.offest_limit.


> The code is not as pretty I agree because you have to call methods on
> the query object passed to your config block. However, the advantage
> is that its much easier to spec out, and you're able to right examples
> that are clearer than the alternative (ones that ensure magic happens
> with a set dependent objects).
>
> WDYT?
>
>
>
> --
> Zach Dennis
> http://www.continuousthinking.com
> http://www.mutuallyhuman.com
>



-- 
Zach Dennis
http://www.continuousthinking.com
http://www.mutuallyhuman.com