This project is archived and is in readonly mode.

#90 ✓ invalid
duncanbeevers

Enhancement: Add support for named scopes with optional arguments

Reported by duncanbeevers | May 1st, 2008 @ 09:57 PM

The current declaration of dynamic named scopes requires use of the lambda syntax which has no support for optional arguments or arguments with default values.

This patch adds the ability to specify a class method name to use for generating the scope options.

It also provides a simpler fallback allowing you to rely on a class method whose name is the scopes name, prefixed with "scope_"

class Topic < ActiveRecord::Base
  named_scope :written_before
  def self.scope_written_before time
    { :conditions => [ 'written_on < ?', time ] }
  end
end

Comments and changes to this ticket

  • MatthewRudy

    MatthewRudy May 3rd, 2008 @ 12:37 AM

    I don't know if I quite get the reason for this.

    and it seems less clear what's going on

    class Topic < ActiveRecord::Base
      named_scope :written_before, Proc.new{|time|
        time ||= Time.now
        {:conditions => ["written_on < ?", time]}
      end
    end
    

    accepts an optional argument, if that's what you're looking for.

    Alternatively it might be alright if you gave the name of the method in the Hash,

    class Topic < ActiveRecord::Base
      named_scope :written_before, :scope_name => :scope_written_before
      def scope_written_before(time=Time.now)
        {:conditions => ["written_on < ?", time]}
      end
    end
    

    but it just doesnt feel like it should be the default behaviour...

    prefer it like it is now...

    default behaviour does nothing, rather than raising an exception

    (lighthouse needs a "preview" function, so I can see if my ruby gets formatted proper)

  • duncanbeevers

    duncanbeevers May 3rd, 2008 @ 12:49 AM

    The provided patch allows for providing a method name as the scope using a symbol as the method name.

    def Topic < ActiveRecord::Base
      named_scope :written_before, :scope_written_before
    end
    

    Additionally, using Procs this way doesn't allow for mandatory arguments on named scopes.

    The primary motivation for this patch though is to ease unit testing of complex named scopes by allowing you to simply call the class method and examine the output hash as opposed to traversing a bunch of associations.

  • MatthewRudy

    MatthewRudy May 3rd, 2008 @ 12:50 AM

    can't think what the right name for the key to pass

    :scope_name doesnt feel right,

    as the "scope name" is the name we just gave it...

    so maybe :scope_options

    but that's no good either.

    is there a precedent for such an option?

  • duncanbeevers

    duncanbeevers May 3rd, 2008 @ 12:52 AM

    Perhaps

    :scope_method
    
  • MatthewRudy

    MatthewRudy May 3rd, 2008 @ 12:58 AM

    I agree that the patch allows the (2nd argument == :symbol)

    but my feeling is that it isn't intuitive.

    (also, shouldn't it accept symbols and strings)

    if all you want to do is extract the options being used by the current scope,

    wouldn't it be better to add a method to look at the @proxy_options variable inside the Scope.

    eg:

    assert_equal(
      Topic.written_before(time).scope_options,
      {:conditions => ["written_on < ?", time}
      )
    

    think that would work pretty much out of the box (line 200ish named_scope.rb)

    although it may want to return a recurse array of all the chained up scopes...

  • MatthewRudy

    MatthewRudy May 3rd, 2008 @ 12:59 AM

    forgot to point out as well...

    Proc.new doesn't enforce the number of arguments,

    ends up the same as a lambda, but lambda will raise if it doesnt get its required arguments.

  • MatthewRudy

    MatthewRudy May 3rd, 2008 @ 01:00 AM

    btw...

    apologies if I seem harsh...

    I just love "named_scope"

    was playing with a similar idea a year ago,

    made my day seeing it in edge.

  • duncanbeevers

    duncanbeevers May 3rd, 2008 @ 02:06 AM

    No offense taken.

    I quite like the idea of having the scope itself emit its options, and I've realy come around on using class methods as scopes. Could be useful, but having all the behavior in the named_scope declaration is nicer, (though it's a little painful to see these nests of squiggly brackets in Ruby)

    I took your suggestion of examining the scope_options on the proxy directly and submitted the patch here.

    http://rails.lighthouseapp.com/p...

    I think I'll modify this patch to use the :scope_method option instead of a naked symbol name, as I think the ability to have a mix of mandatory and optional arguments on a named scope is still valuable.

  • Pratik

    Pratik May 11th, 2008 @ 09:45 PM

    • State changed from “new” to “invalid”

    I don't like the idea of the patch because the proposed functionality is dependent on return value of a class method, which doesn't feel intuitive for anything other than booleans.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Tickets have moved to Github

The new ticket tracker is available at https://github.com/rails/rails/issues

Shared Ticket Bins

People watching this ticket

Attachments

Referenced by

Pages