#3218 default_scope is having a side affecct on create - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#3218 ✓committed

default_scope is having a side affecct on create

Reported by johnf | September 16th, 2009 @ 03:59 PM | in 3.0.2

If I have the following

class Foo
  default_scope :conditions => {:bar => 1}
end

If I try to create a new object

>> Foo.new(:bar => 5)
=> #<Foo id: nil, name: nil, bar: 1, created_at: nil, updated_at: nil>

The object now has bar set to one when I've explicitly asked for it to be set to 5.

It makes sense to set the default if I don't pass it in but not otherwise

At a rough guess in ActiveRecord::Base initialize

2438         self.attributes = attributes unless attributes.nil?
2439         self.class.send(:scope, :create).each { |att,value| self.send("#{att}=", value) } if self.class.send(:scoped?, :create)

should be swapped around

Comments and changes to this ticket

CancelProfileIsBroken September 25th, 2009 @ 12:05 PM
- Tag changed from “activerecord, default_scope” to “activerecord, bugmash, default_scope”
You flagged this item as spam.
hsume2 (Henry) September 25th, 2009 @ 06:05 PM
verified on 2-3-stable

I've attached a failing test.
- active_record_default_scope_overwriting_attributes.diff 979 Bytes
You flagged this item as spam.
Elad Meidar September 25th, 2009 @ 10:56 PM
+1 verified on 2-3-stable, i guess i am lucky for not running into it up until now.

Attached patches for 2-3-stable and master

I generally allowed the scoped attributes to override existing attributes only:

if the attribute value is #blank?

if the attribute is not blank, but the value was initiated via a column default definition.

otherwise the value is not overridden.
- LH3218-default_scope_override_values_master.diff 2.4 KB
- LH3218-default_scope_override_values_2-3-stable.diff 5.9 KB
You flagged this item as spam.
Elad Meidar September 25th, 2009 @ 10:56 PM
heh, silly Lighthouse textile :)
You flagged this item as spam.
hsume2 (Henry) September 26th, 2009 @ 01:47 AM
I think I get what you've done. However, I think your implementation conficts with #test_default_values_on_empty_strings in base_test.rb. I've attached a patch (with an additional fail test) that fixes it. (Should apply cleanly to master and 2-3-stable)

Also, note: expecting 50000, as opposed to the default value of 70000 defined in the schema.
- LH3218-default_scope_overwrite_values_master.diff 2.7 KB
You flagged this item as spam.
hsume2 (Henry) September 26th, 2009 @ 01:48 AM
Sorry, I don't mean conflict.. exactly. More like, the behavior is different from that in #test_default_values_on_empty_strings.
You flagged this item as spam.
Elad Meidar September 26th, 2009 @ 01:53 AM
- Tag changed from “activerecord, bugmash, default_scope” to “activerecord, bugmash, bugmash-review, default_scope”

Elad Meidar September 26th, 2009 @ 02:02 AM

Mmmm,
by doing

-        self.attributes = attributes unless attributes.nil?

     self.class.send(:scope, :create).each { |att,value| self.send(&quot;#{att}=&quot;, value) } if self.class.send(:scoped?, :create)

 self.attributes = attributes unless attributes.nil?</code>






you are overriding the attributes from the scope.... the order of steps i did was to ensure that the last value that the user specified, takes affect.

default db column value
scoped :conditions
initialize/find parameters

You flagged this item as spam.
Elad Meidar September 26th, 2009 @ 02:03 AM
Mmmm,
by doing
```
- self.attributes = attributes unless attributes.nil?
  self.class.send(:scope, :create).each { |att,value| self.send("#{att}=", value) } if self.class.send(:scoped?, :create)
+ self.attributes = attributes unless attributes.nil?
```
you are overriding the attributes from the scope.... the order of steps i did was to ensure that the last value that the user specified, takes affect.
1. default db column value
2. scoped :conditions
3. initialize/find parameters
You flagged this item as spam.
Elad Meidar September 26th, 2009 @ 03:05 AM
+1 on @hsume2' patch, solves it in a much cleaner way than my patches
David Trasbo September 26th, 2009 @ 05:20 PM
+1 though the patch no longer applies to 2-3-stable.
You flagged this item as spam.
Elad Meidar September 26th, 2009 @ 05:59 PM
@david, which one did you try?
You flagged this item as spam.
Josh Sharpe September 26th, 2009 @ 06:08 PM
+1 to hsume2's master patch

Here's the patch for 2-3 which is basically identical to the master patch.
- LH3218-default_scope_overwrite_values_2-3.diff 2.7 KB
You flagged this item as spam.
sr.iniv.t September 27th, 2009 @ 04:55 AM
+1 verified.

Both patches (hsume2's on master and Josh's on 2-3) apply cleanly and all tests pass.
CancelProfileIsBroken September 27th, 2009 @ 12:06 PM
- Tag changed from “activerecord, bugmash, bugmash-review, default_scope” to “activerecord, bugmash-review, default_scope”
José Valim February 21st, 2010 @ 11:52 AM
- Assigned user set to “José Valim”
- Milestone cleared.
José Valim February 26th, 2010 @ 10:10 AM
- State changed from “new” to “committed”
Applied on master. Can someone please rebase the patch for 2-3-stable?
Repository February 26th, 2010 @ 11:15 AM
(from [bf9a0ae12b701cba7a8aac2955ce243866ac7bf6]) Fix a bug where default_scope was overriding attributes given on model initialization [#3218 status:resolved]

Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/bf9a0ae12b701cba7a8aac2955ce24...
Rizwan Reza May 15th, 2010 @ 06:40 PM
- Tag changed from “activerecord, bugmash-review, default_scope” to “activerecord, default_scope”
Jeremy Kemper October 15th, 2010 @ 11:01 PM
- Milestone set to “3.0.2”
- Importance changed from “” to “”
[bulk edit]