Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Build model from association with conditions

#5562

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"

Reported by Alexis Toulotte · September 6th, 2010 @ 09:24 AM

State: resolved
Milestone: none
Assigned to: Santiago Pastorino Santiago Pastorino
Importance: Low

Activity

  1. Santiago Pastorino
    Santiago Pastorino
    • Importance changed from to Low

    Can you provide a failing test case and/or a patch?
    Thanks.

    September 8th, 2010 @ 12:38 PM

  2. Alexis Toulotte
  3. Marcelo Giorgi
    Marcelo Giorgi

    Hi,

    I've been working on this and came up with this test and patch.

    September 8th, 2010 @ 01:32 PM

  4. Marcelo Giorgi
    Marcelo Giorgi
    • Tag changed from associations, scope to associations, patch, scope

    September 8th, 2010 @ 01:44 PM

  5. xds2000
    xds2000

    +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

    September 14th, 2010 @ 11:32 AM

  6. xds2000
    xds2000

    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

    September 14th, 2010 @ 11:50 AM

  7. Lake
    Lake

    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?

    September 15th, 2010 @ 06:42 AM

  8. Marcelo Giorgi
    Marcelo Giorgi
    • 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

    September 15th, 2010 @ 01:41 PM

  9. Marcelo Giorgi
    Marcelo Giorgi

    Just noticed that Lake was right and the entire block can be wrapped into a separated method.

    This is the updated patch

    September 15th, 2010 @ 04:38 PM

  10. Lake
    Lake

    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

    September 15th, 2010 @ 11:57 PM

  11. Alvaro Campodonico
    Alvaro Campodonico

    +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

    September 16th, 2010 @ 10:52 PM

  12. Lake
    Lake
    • Tag changed from associations, patch, scope to associations, patch, scope, verified

    September 16th, 2010 @ 10:58 PM

  13. Repository
    Repository
    • State changed from new to resolved

    (from [cdfd013dd7953fc87038c73ecddbaa8cfe29a301]) Set attributes properly for model built from association with conditions [#5562 Build model from association with conditions state:resolved]

    Signed-off-by: Santiago Pastorino santiago@wyeworks.com
    http://github.com/rails/rails/commit/cdfd013dd7953fc87038c73ecddbaa...

    September 28th, 2010 @ 07:23 PM

  14. Repository