This project is archived and is in readonly mode.
Build model from association with conditions
Reported by Alexis Toulotte | September 6th, 2010 @ 09:24 AM
Building a model from an association does not set attributes from conditions.:
>> Post.first.comments.where(:author => "John").build.author
=> nil
But from model directly it does:
>> Comment.where(:author => "John").build.author
=> "John"
Comments and changes to this ticket
-
Santiago Pastorino September 8th, 2010 @ 12:38 PM
- Importance changed from “” to “Low”
Can you provide a failing test case and/or a patch?
Thanks. -
Marcelo Giorgi September 8th, 2010 @ 01:32 PM
Hi,
I've been working on this and came up with this test and patch.
-
Marcelo Giorgi September 8th, 2010 @ 01:44 PM
- Tag changed from “associations, scope” to “associations, patch, scope”
-
xds2000 September 14th, 2010 @ 11:32 AM
+1 i have test Marcelo Giorgi patch,pass
my env:
rvm 1.0.7
ruby 1.8.7 (2009-12-24 patchlevel 248) [i686-linux], MBARI 0x8770, Ruby Enterprise Edition 2010.01 -
xds2000 September 14th, 2010 @ 11:50 AM
more test result:
[xds2000@localhost activerecord] (build_model_from_association %)$ rake test_mysql TEST=test/cases/method_scoping_test.rb (in /home/xds2000/workspace/rails/activerecord) /home/xds2000/.rvm/rubies/ree-1.8.7-2010.01/bin/ruby -w -I"lib:test:test/connections/native_mysql" "/home/xds2000/.rvm/gems/ree-1.8.7-2010.01/gems/rake-0.8.7/lib/rake/rake_test_loader.rb" "test/cases/method_scoping_test.rb" /home/xds2000/.rvm/rubies/ree-1.8.7-2010.01/lib/ruby/1.8/pathname.rb:263: warning:*' interpreted as argument prefix Using native MySQL<br/> Loaded suite /home/xds2000/.rvm/gems/ree-1.8.7-2010.01/gems/rake-0.8.7/lib/rake/rake_test_loader<br/> Started<br/> ..................................................... Finished in 1.12336 seconds.
53 tests, 131 assertions, 0 failures, 0 errors
-
Lake September 15th, 2010 @ 06:42 AM
Hey, nice addition. I'm on 1.9.2 and the tests pass for me as well.
However, I notice that there two test method definitions with typos. The word "through" should be used, but "thorugh" is used instead. You can find the typos in "activerecord/test/cases/associations/has_many_through_associations_test.rb"
Also, I noticed that this block of code is duplicated.
if scope = self.class.send(:current_scoped_methods) create_with = scope.scope_for_create create_with.each { |att,value| self.respond_to?(:"#{att}=") && self.send("#{att}=", value) } if create_with end
It is written once in the initialize method and once in the initialize_copy method. What steps can be taken to reduce the duplication? I attempted to pull out the code into an instance method. That seemed to work, though I had difficulties coming up with a proper name and home for the method. Maybe you have a better idea?
-
Marcelo Giorgi September 15th, 2010 @ 01:41 PM
- Assigned user set to “Santiago Pastorino”
You see, that's what I love about our community, great feedback!
I agree with your recommendation, so I've created a method called populate_with_scope_attributes, to avoid that duplication. I've updated the patch with this fix, and made the correspondent adjustments for the test typos.
Please let me know if this changes are appropriate for what you suggested.
BTW: Patch still applies cleanly on master
Thank you very much
-
Marcelo Giorgi September 15th, 2010 @ 04:38 PM
Just noticed that Lake was right and the entire block can be wrapped into a separated method.
This is the updated patch
-
Lake September 15th, 2010 @ 11:57 PM
Thanks for updating the patch! Your modifications look great.
This patch applies cleanly to master and the "rake test_mysql" suite is passing.
Nice work! +1
-
Alvaro Campodonico September 16th, 2010 @ 10:52 PM
+1
This patch applies cleanly on master now and test are passing.
I like the last patch as it is more DRY (as Lake mentioned).
About my environment, I'm using ree-1.8.7-2010.01
-
Lake September 16th, 2010 @ 10:58 PM
- Tag changed from “associations, patch, scope” to “associations, patch, scope, verified”
-
Repository September 28th, 2010 @ 07:23 PM
- State changed from “new” to “resolved”
(from [cdfd013dd7953fc87038c73ecddbaa8cfe29a301]) Set attributes properly for model built from association with conditions [#5562 state:resolved]
Signed-off-by: Santiago Pastorino santiago@wyeworks.com
http://github.com/rails/rails/commit/cdfd013dd7953fc87038c73ecddbaa... -
Repository September 28th, 2010 @ 07:23 PM
(from [9e5545cd662a7ffdf93eb176c25f83170a6a388e]) Set attributes properly for model built from association with conditions [#5562 state:resolved]
Signed-off-by: Santiago Pastorino santiago@wyeworks.com
http://github.com/rails/rails/commit/9e5545cd662a7ffdf93eb176c25f83...
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Tags
Referenced by
- 5562 Build model from association with conditions (from [cdfd013dd7953fc87038c73ecddbaa8cfe29a301]) Set att...
- 5562 Build model from association with conditions (from [9e5545cd662a7ffdf93eb176c25f83170a6a388e]) Set att...