Add preservation of decorator options on QueryMethods

[DouglasLutz]

References

• GitHub Issue #864

Description

The context given to a Collection Decorator was being lost when an ActiveRecord::QueryMethod was being called on a Draper::CollectionDecorator.

E.g.

decorated_collection = PostDecorator.decorate_collection(Post.all, context: {user: "foo"}).order("created_at asc")
decorated_collection.context #{}
decorated_collection.decorator_class #nil

This was happening because method_missing on draper/lib/draper/query_methods.rb is invoking ActiveRecord method with send and creating another instance of decorator to decorate the result.

 def method_missing(method, *args, &block) 
   return super unless strategy.allowed? method 
  
   object.send(method, *args, &block).decorate 
 end 

The problem is that the object context and decorator_class were being lost on creation of another instance.
The solution to this was to pass the options alongside the decorate call, it creates the instance of CollectionDecorator with the old options.

Testing

The new behaviour of the Collection Decorator is the following.

decorated_collection = PostDecorator.decorate_collection(Post.all, context: {user: "foo"}).order("created_at asc")
decorated_collection.context #{:user=>"foo"}
decorated_collection.decorator_class #PostDecorator

Also added a test to cover this alteration.

[brunohkbx]

Hey, @DouglasLutz @synth. Good catch! 👍
Sorry, I didn't realize that on #845

@codebycliff could you please take a look?

[codebycliff]

This looks good. Now that Draper::QueryMethods depends upon decorator_class and context, I'm not sure it makes sense for this code to be extracted to its own module. The module won't work unless it's included in CollectionDecorator, which I think makes a strong case for just moving the code there. That being said, it's not a big deal the way it is. Just a thought. What do you guys think?

If we don't care to move it in, we can merge this once the formatting suggestion is applied.

[DouglasLutz]

@codebycliff I agree with you with the dependency of decorator_class and context, Draper::QueryMethods is modeled to CollectionDecorator.

I will apply the formatting suggestion and depending on your thoughts I can move the code there.

[codebycliff]

Yeah, I think it makes sense to move it into CollectionDecorator, but I think that change is bigger than I thought it would be. We'll go with this for now. Thanks for the contribution!