Rails Ruby on Rails → 3.0.2

Comments and changes to this ticket

• You flagged this item as spam.
  

  Gaël Deest April 4th, 2010 @ 01:55 PM

  • no changes were found...

  • scope_support_for_activerecord_uniqueness_validator.diff 1.5 KB

• You flagged this item as spam.
  

  Elliot Winkler April 6th, 2010 @ 03:22 PM

  So if I understand correctly, you want to be able to say something like

  class Foo < ActiveRecord::Base
    named_scope :enabled, :conditions => {:enabled => true}
    validates_uniqueness_of :guid, :scope => :enabled
  end
  

  and so in that case the validates_uniqueness_of would actually run something like

  foo = Foo.new(:guid => "dk93dkd", :enabled => true)
  Foo.enabled.exists?(:guid => foo.guid)
  

  instead of something like

  Foo.exists?(:guid => foo.guid, :enabled => true)
  

  (I think I got what v_u_o does right, ha. Sorry for the 2.x syntax but you get the idea)

  Yeah, you are going to break a lot of people's code if you change the meaning of :scope ;) It is unfortunate that :scope is named what it is but it may be set in stone now... I'm not sure.

  I wonder if it would be a good idea to mix the two meanings -- so :scope would include any class methods that happen to match as well as any attributes that happen to match? Maybe that would be confusing though.

  Anyway, I can't say I've ever needed this ability; usually attributes work fine for me (thinks like :user_id or :enabled or whatever). Maybe you can provide us with an example?

• You flagged this item as spam.
  

  Gaël Deest April 6th, 2010 @ 03:58 PM

  Here is a (trimmed down, but real life) example. I'm working on a project where my customers will have their own subdomains. Subdomains are represented by a state machine, based on their activation state. They can go through four states: 'reserved', 'available', 'unavailable' and 'removed'. Active subdomains should be unique, but I'd also like to make "removed" subdomains available again to new customers. So basically I'd like to do something like this:

      class Subdomain < ActiveRecord::Base
        scope :active, where(:state => ['reserved', 'available', 'unavailable'])
        validates :name, :unique => {:scope => :active}
        
        state_machine :initial => :reserved do
          state :reserved
          state :available
          state :unavailable
          state :removed
          ...
        end
      end
  

  I may of course create a new boolean field called 'enabled' or something similar and that's probably what I'll end up doing. Yet I find it pretty redundant.

  I thought about your mixed solution. I can see two problems with it:
  - An attribute may have the same name as a class method. What should be do then ? - :scope currently accepts several attributes, while it isn't possible to mix several scopes (unless I'm missing something). That may not be a big problem though.

• You flagged this item as spam.
  

  Gaël Deest April 6th, 2010 @ 04:06 PM

  The 'validates' line should have read:

      validates :name, :uniqueness => {:scope => :active}
  

  Also, I'm actually using class methods and not true 'scopes', but that's another issue.

• 

  José Valim April 10th, 2010 @ 12:48 PM

  • Milestone cleared.
  • Assigned user set to “José Valim”

  I like the idea, but I don't like the fact it will be completely backward incompatible. Maybe we could allow the following to work:

  class Subdomain < ActiveRecord::Base
    scope :active, where(:state => ['reserved', 'available', 'unavailable'])
    validates :name, :uniqueness => { :scope => Subdomain.active }
  end
  

  So when the :scope is a Symbol, it should use the previous behavior, otherwise use the one you propose. What do you think?

• You flagged this item as spam.
  

  Gaël Deest April 10th, 2010 @ 02:23 PM

  Really nice AND flexible idea (thanks arel !). Coming back soon with an updated patch with tests and documentation.

• You flagged this item as spam.
  

  Gaël Deest April 10th, 2010 @ 05:53 PM

  It turns out to be much more difficult to implement than I first thought, and my first attempt was actually broken.

  The problem is that when saving a new record, there is no way that I know of to determine whether it matches the specified relation, and therefore to decide whether the unicity scope applies. I must say I'm stuck here. Does anyone have any idea ?

• 

  José Valim April 12th, 2010 @ 10:04 AM

  If a :scope is provided and it's not a symbol or array, you should use it for the querying. So I guess you want to do this:

  if options[:scope].is_a?(ActiveRecord::Relation)
    scopes = []
    table  = options[:scope]
  else
    scopes = Array(options[:scope])
    table  = finder_class.unscoped
  end
  

• You flagged this item as spam.
  

  Gaël Deest April 12th, 2010 @ 10:26 AM

  Yes, I had done something like that and was writing some test cases when I realized it did not work.

  Here is a model:

  class Employee < ActiveRecord::Base
    scope :active, where(:status => ["trainee", "veteran"])
    validates_uniqueness_of :office_id, :scope => self.active
  end
  

  And here is a test case:

    def test_validate_uniqueness_with_scope_relation
      paul = Employee.create(:office_id => 1, :status => "trainee")
      puts paul.id
      assert paul.valid?, "Saving paul"
      
      fred = Employee.create(:office_id => 1, :status => "veteran")
      assert !fred.valid?, "Saving fred. Duplicate office_id."
      
      fred.status = "retired"
      assert fred.save, "Saving fred. Duplicate (but allowed) office_id."
    end
  

  The first two assertions will pass, but the third wont, the reason being that office_id already exists within the specified scope. We should not care, because the model does not match: the employee being saved is neither a "trainee" nor a "veteran". The problem is, there is no way to know whether a new record matches a given scope !

  To make it work, one would have to write:

  class Employee < ActiveRecord::Base
    scope :active, where(:status => ["trainee", "veteran"])
    validates_uniqueness_of :office_id, :scope => self.active, :if => {|e| ["trainee", "veteran"].include?(e.status)}
  end
  

  I don't know about you, but I find it awkward. Yet it does work, and allows something impossible before, so if it seems acceptable to you I will submit my patch, but I had dreamed of something simpler :(

• 

  José Valim April 25th, 2010 @ 09:25 AM

  Yes, I agree it's awkward and had this issue in mind because we had a similar discussion in another ticket. Someone thought that the behavior you just described as awkward, is completely normal.

  So, in your opinion, is the patch still worth? Do see an use case for it?

• 

  José Valim June 7th, 2010 @ 09:36 AM

  • State changed from “new” to “wontfix”

  I'm marking as won't fix. If you are still in mood to work on a patch, please let me know and I will reopen it.

• 

  Jeremy Kemper October 15th, 2010 @ 11:01 PM

  • Milestone set to “3.0.2”
  • Importance changed from “” to “Medium”

  [bulk edit]