Rails core - Oct 2010 - Patch for #1812 -- allow default_scope to take a proc

Can I get someone to review this very small patch (only 5 lines, plus
tests of course) to allow default_scope to accept a proc?

It applies to 3-0-stable and to master, and passes tests on both
branches.

-Tim

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

Sorry I forgot to link to the ticket:
https://rails.lighthouseapp.com/projects/8994/tickets/1812-default_scope-cant-take-procs

And the patch is here:
https://rails.lighthouseapp.com/projects/8994/tickets/1812/a/721344/allow-default_scope-to-accept-a-proc.patch

Or you can pull from my Rails fork here: http://github.com/seven1m/rails

-Tim

On Oct 19, 9:47 am, Tim Morgan <t...@timmorgan.org>
wrote:
> Can I get someone to review this very small patch (only 5 lines, plus
> tests of course) to allow default_scope to accept a proc?
>
> It applies to 3-0-stable and to master, and passes tests on both
> branches.
>
> -Tim
-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

On Tue, Oct 19, 2010 at 08:57:02AM -0700, Tim Morgan
wrote:
> Sorry I forgot to link to the ticket:
>
https://rails.lighthouseapp.com/projects/8994/tickets/1812-default_scope-cant-take-procs
> 
> And the patch is here:
>
https://rails.lighthouseapp.com/projects/8994/tickets/1812/a/721344/allow-default_scope-to-accept-a-proc.patch
> 
> Or you can pull from my Rails fork here: http://github.com/seven1m/rails
Two things:

1. Can you change it so that it uses "&&" rather than
"and"

2. Change from m.is_a?(Proc) to m.respond_to?(:call)

#1 is for style.  #2 is because we don''t care that this is a Proc
object, we only care that it responds to "call".

-- 
Aaron Patterson
http://tenderlovemaking.com/

Sure!

Here is the updated patch:
https://rails.lighthouseapp.com/projects/8994/tickets/1812/a/727956/allow-default_scope-to-accept-a-proc2.patch

Also, the updated commit on GitHub:
http://github.com/seven1m/rails/commit/e0b73b64c30e81e5b4279ce55dd2b7d65cfae00a

<http://github.com/seven1m/rails/commit/e0b73b64c30e81e5b4279ce55dd2b7d65cfae00a>
-Tim

On Tue, Oct 19, 2010 at 3:35 PM, Aaron Patterson
<aaron@tenderlovemaking.com
> wrote:
> On Tue, Oct 19, 2010 at 08:57:02AM -0700, Tim Morgan wrote:
> > Sorry I forgot to link to the ticket:
> >
>
https://rails.lighthouseapp.com/projects/8994/tickets/1812-default_scope-cant-take-procs
> >
> > And the patch is here:
> >
>
https://rails.lighthouseapp.com/projects/8994/tickets/1812/a/721344/allow-default_scope-to-accept-a-proc.patch
> >
> > Or you can pull from my Rails fork here:
http://github.com/seven1m/rails
>
> Two things:
>
> 1. Can you change it so that it uses "&&" rather than
"and"
>
> 2. Change from m.is_a?(Proc) to m.respond_to?(:call)
>
> #1 is for style.  #2 is because we don''t care that this is a Proc
> object, we only care that it responds to "call".
>
> --
> Aaron Patterson
> http://tenderlovemaking.com/
>
-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

On Tue, Oct 19, 2010 at 04:16:33PM -0500, Tim Morgan
wrote:
> Sure!
> 
> Here is the updated patch:
>
https://rails.lighthouseapp.com/projects/8994/tickets/1812/a/727956/allow-default_scope-to-accept-a-proc2.patch
Thanks!  I''ve applied it and extended it a little bit so that
default_scope will take any object that responds to "call".

I''ll show some code, then try to explain the benefits.  This is now
possible:

  class AuthorFilter < Struct.new(:klass, :author_id)
    def call
      klass.where(:author_id => author_id)
    end
  end

  class AmazingPost < ActiveRecord::Base
    self.table_name = ''posts''
    default_scope AuthorFilter.new(self, 2)
  end

The Object form does not need to save binding like the lambda form does.  This
can lead to less memory usage and help curb "leaking objects".

Because we can use objects, it allows our default scopes to contain more
logic:  we can actually break up functionality to other methods.  For
example:

  class AuthorFilter < Struct.new(:klass, :some_value)
    def calculate_id
      ... do som complex logic to figure out the id ...
    end

    def call
      klass.where(:author_id => calculate_id)
    end
  end

We can even use modules or subclasses to extend our logic.  For example:

  module BodyFilter
    def call
      super.where(:body => ''hello'')
    end
  end

  class AuthorFilter < Struct.new(:klass, :author_id)
    def call
      klass.where(:author_id => author_id)
    end
  end

  class AmazingPost < ActiveRecord::Base
    self.table_name = ''posts''
    # Produces: WHERE author_id = 2 AND body = ''hello''
    default_scope AuthorFilter.new(self, 2).extend(BodyFilter)
  end

Finally, we can more easily test any complex logic we need in this
filter:

  class FilterTest < Test::Unit::TestCase
    class DreamCatcher
      attr_reader :wheres
      def initialize; @wheres = []; end
      def where(arg); @wheres << arg; end
    end

    def test_appropriate_where_clause
      dc = DreamCatcher.new
      AuthorFilter.new(dc, 2).call
      assert_equal([{:author_id => 2}], dc.wheres)
    end
  end

I''d like to see the rest of our scope methods to allow arbitrary
objects
that respond to `call` for the above reasons.  :-)

-- 
Aaron Patterson
http://tenderlovemaking.com/

This is great, and I wouldn''t have seen the possibilities if you
hadn''t
explained it in code!

Thanks!

-Tim

On Tue, Oct 19, 2010 at 5:28 PM, Aaron Patterson
<aaron@tenderlovemaking.com
> wrote:
> On Tue, Oct 19, 2010 at 04:16:33PM -0500, Tim Morgan wrote:
> > Sure!
> >
> > Here is the updated patch:
> >
>
https://rails.lighthouseapp.com/projects/8994/tickets/1812/a/727956/allow-default_scope-to-accept-a-proc2.patch
>
> Thanks!  I''ve applied it and extended it a little bit so that
> default_scope will take any object that responds to "call".
>
> I''ll show some code, then try to explain the benefits.  This is
now
> possible:
>
>  class AuthorFilter < Struct.new(:klass, :author_id)
>    def call
>      klass.where(:author_id => author_id)
>    end
>  end
>
>  class AmazingPost < ActiveRecord::Base
>    self.table_name = ''posts''
>    default_scope AuthorFilter.new(self, 2)
>  end
>
> The Object form does not need to save binding like the lambda form does.
>  This
> can lead to less memory usage and help curb "leaking objects".
>
> Because we can use objects, it allows our default scopes to contain more
> logic:  we can actually break up functionality to other methods.  For
> example:
>
>  class AuthorFilter < Struct.new(:klass, :some_value)
>    def calculate_id
>      ... do som complex logic to figure out the id ...
>    end
>
>    def call
>      klass.where(:author_id => calculate_id)
>    end
>  end
>
> We can even use modules or subclasses to extend our logic.  For example:
>
>  module BodyFilter
>    def call
>      super.where(:body => ''hello'')
>    end
>  end
>
>  class AuthorFilter < Struct.new(:klass, :author_id)
>    def call
>      klass.where(:author_id => author_id)
>    end
>  end
>
>  class AmazingPost < ActiveRecord::Base
>    self.table_name = ''posts''
>    # Produces: WHERE author_id = 2 AND body = ''hello''
>    default_scope AuthorFilter.new(self, 2).extend(BodyFilter)
>  end
>
> Finally, we can more easily test any complex logic we need in this
> filter:
>
>  class FilterTest < Test::Unit::TestCase
>    class DreamCatcher
>      attr_reader :wheres
>      def initialize; @wheres = []; end
>      def where(arg); @wheres << arg; end
>    end
>
>    def test_appropriate_where_clause
>      dc = DreamCatcher.new
>      AuthorFilter.new(dc, 2).call
>      assert_equal([{:author_id => 2}], dc.wheres)
>    end
>  end
>
> I''d like to see the rest of our scope methods to allow arbitrary
objects
> that respond to `call` for the above reasons.  :-)
>
> --
> Aaron Patterson
> http://tenderlovemaking.com/
>
-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

On Tue, Oct 19, 2010 at 06:06:15PM -0500, Tim Morgan
wrote:
> This is great, and I wouldn''t have seen the possibilities if you
hadn''t
> explained it in code!
> 
> Thanks!
No problem.  I''ve updated the `scope` method to accept objects that
respond to `call` as well:

 
http://github.com/rails/rails/blob/master/activerecord/test/models/topic.rb#L22-26
 
http://github.com/rails/rails/blob/master/activerecord/test/cases/named_scope_test.rb#L128-132

Now we can more easily reuse our scope code!
-- 
Aaron Patterson
http://tenderlovemaking.com/

Wheeeeeeeeee :D

// sorry for the spammy message, but really, this is fantastic in many levels :D

On Tue, Oct 19, 2010 at 10:31 PM, Aaron Patterson
<aaron@tenderlovemaking.com> wrote:
> On Tue, Oct 19, 2010 at 06:06:15PM -0500, Tim Morgan wrote:
>> This is great, and I wouldn''t have seen the possibilities if
you hadn''t
>> explained it in code!
>>
>> Thanks!
>
> No problem.  I''ve updated the `scope` method to accept objects
that
> respond to `call` as well:
>
>
 http://github.com/rails/rails/blob/master/activerecord/test/models/topic.rb#L22-26
>
 http://github.com/rails/rails/blob/master/activerecord/test/cases/named_scope_test.rb#L128-132
>
> Now we can more easily reuse our scope code!
> --
> Aaron Patterson
> http://tenderlovemaking.com/
>
-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.