Rails Ruby on Rails

Comments and changes to this ticket

• You flagged this item as spam.
  

  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)

• You flagged this item as spam.
  

  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.

• You flagged this item as spam.
  

  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?

• You flagged this item as spam.
  

  duncanbeevers May 3rd, 2008 @ 12:52 AM

  Perhaps

  :scope_method
  

• You flagged this item as spam.
  

  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...

• You flagged this item as spam.
  

  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.

• You flagged this item as spam.
  

  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.

• You flagged this item as spam.
  

  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 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.