#2365 add :new_if_blank option to fields_for for nested associations - 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.

#2365 ✓wontfix

add :new_if_blank option to fields_for for nested associations

Reported by Mike Breen | March 27th, 2009 @ 11:30 PM | in 2.3.6

This patch will allow you to initialize any blank associations for your models that are using nested_attributes_for.

# for one-to-one association
form_for(@post) do |f|
  f.text_field(:title)

  f.fields_for(:author, :new_if_blank => true) do |af|
    af.text_field(:name)
  end
end

# for collection associations
# this will create 3 blank comments
form_for(@post) do |f|
  f.text_field(:title)

  f.fields_for(:comments, :new_if_blank => 3) do |cf|
    cf.text_field(:name)
  end
end

new_if_blank.diff 4.4 KB

Comments and changes to this ticket

Eloy Duran March 28th, 2009 @ 01:03 AM
- Assigned user set to “Eloy Duran”
Eloy Duran March 29th, 2009 @ 12:34 PM
The patch looks good! Although I only had time to look at the patch atm, so disregard any notes if I didn't understand it well enough ;-)

Instead of using:
```
(args[:new_if_blank].to_i rescue 1).times do
   nested_association << klass.new
end
```
I'd prefer:
```
(args[:new_if_blank].is_a?(Numeric) ? args[:new_if_blank] : 1).times do
   nested_association << klass.new
end
```
As there is always the chance of masking real exceptions when using a inline rescue. Plus the fact that args[:new_if_blank] can be, for instance, 'true' which is not exceptional.

Personally I'd refactor that area somewhat like the following. But I will have a more thorough pass at this once I apply the patch anyways, so it doesn't really matter for now.
```
def build_nested_association(association_name, times)
  klass = association_name.to_s.classify.constantize
  if @object.send(association_name).is_a?(Array)
    Array.new(times.is_a?(Numeric) ? times : 1) { klass.new }
  else
    klass.new
  end
end

...
if association.blank? && args.last.is_a?(Hash) && (times = args.last[:new_if_blank])
  association = build_nested_association(association_name, times)
end
```
Also, as I understand it there will still be a NoMethodError raised if there's no single association build right?

I think that in this case we should not yield a new form builder at all, as is the case with an empty collection association. This was already being discussed on #2172. Thoughts?
Eloy Duran March 29th, 2009 @ 12:40 PM
Oh btw, normally I'd refrain from theorizing. But I wonder if it might be a good idea to have ":new => 3" as an option for a collection association as well, which would always add 3 new objects.
You flagged this item as spam.
Mike Breen March 29th, 2009 @ 03:09 PM
I think the refactorings are spot on. The fact that :new_is_blank can be true is not really exceptional in this case. Also build_nested_association is now a little easier on the eyes.
You flagged this item as spam.
Mike Breen March 29th, 2009 @ 03:24 PM
Here's an updated patch based on Eloy's suggestions for refactoring.
- new_if_blank_2.diff 4.2 KB
You flagged this item as spam.
Mike Breen March 29th, 2009 @ 03:32 PM
I'm not sure I like the idea of the option ":new => 3" for collection associations. I'd have to play around with it to get a real feel for it.
José Valim April 11th, 2009 @ 07:21 PM
Guys, wouldn't be better to use :build_if_blank? We are building an object in such cases...
You flagged this item as spam.
Mike Breen April 11th, 2009 @ 07:38 PM
I was originally calling the param create_blank but I can't for the life of me remember why we renamed it. Eloy do you recall?
You flagged this item as spam.
Mike Breen April 12th, 2009 @ 04:01 AM
@Jose

I believe the reason we went with "new" over "build" (or "create") is because Eloy wants to stay ORM agnostic. "build" is an AR convention so we went with "new". If you look at the patch it doesn't actually call build it calls new.

(hopefully Eloy will correct me if I got it wrong)
Eloy Duran April 12th, 2009 @ 11:55 AM
(hopefully Eloy will correct me if I got it wrong)

You are spot on good sir :)
José Valim April 12th, 2009 @ 02:13 PM
agreed and understood! :)
Eloy Duran April 21st, 2009 @ 03:07 PM
- State changed from “new” to “verified”
+1
Pratik April 21st, 2009 @ 03:26 PM
- Assigned user changed from “Eloy Duran” to “Michael Koziarski”
Michael Koziarski June 9th, 2009 @ 09:20 AM
- Milestone changed from “2.x” to “2.3.4”
As I mentioned to mike in irc, the only line here which concerns me is:
```
+          if @object.send(association_name).is_a?(Array)
```
is that a reasonable way to test this condition? I assume the is_a? test won't fetch the association?

If that still looks right to you guys, I'm happy to merge it it.
You flagged this item as spam.
Andrew France June 9th, 2009 @ 11:42 AM
Association array determination is a problem I looked at when putting together my patch #2648. As far as I can tell the is_a? will not be intercepted by the collection proxy so it will instantiate the association. I found it hard to find a neat way around this while still keeping the view code ORM agnostic. I did consider checking for "#{association_name}_ids=" method or adding is_a? to the proxy collection but neither seem like great ideas.
Jeremy Kemper September 11th, 2009 @ 11:04 PM
- Milestone changed from “2.3.4” to “2.3.6”
[milestone:id#50064 bulk edit command]
Eloy Duran December 30th, 2009 @ 09:52 PM
- Assigned user changed from “Michael Koziarski” to “Eloy Duran”
Jeremy Kemper January 4th, 2010 @ 10:27 PM
This option is hard to understand and harder to discover. I'm worried it's making the API muddier not clearer.

When would you want :build_if_blank => false?
Eloy Duran January 5th, 2010 @ 02:48 PM
In fact, iirc, I only needed that once. And it was quite the edge case. So am I correct in understanding that you mean; ”default to build if there's no associated record“ ?

If so, I think you're right. I would still like to be able to disable it though. And for a collection association you might want build more than 1 record. Although in retrospect, we could default this to just one as well. At least for now.
Rizwan Reza May 16th, 2010 @ 02:41 AM
- Tag changed from “2.3.x, accepts_nested_attributes_for, fields_for, patch” to “2.3.x, accepts_nested_attributes_for, bugmash, fields_for, patch”
[bulk edit]
Rizwan Reza May 16th, 2010 @ 02:46 AM
- Tag changed from “2.3.x, accepts_nested_attributes_for, bugmash, fields_for, patch” to “2.3.x, accepts_nested_attributes_for, fields_for, patch”
- State changed from “verified” to “wontfix”
I am closing this, feel free to reopen if needed.
You flagged this item as spam.
af001 May 5th, 2011 @ 02:55 AM
私の中で、総合評価のとっても低いアバアバクロホリスタークロ銀座店。アバクロは大好きなんですけどね。一昨日の東京駅付近での打ち合わせの後、散歩がてら久々に行ってきました。そしたらビックリ！相変わらアバクロず、踊っているだけの店員さんとかもいましたが、
You flagged this item as spam.
klkk May 23rd, 2011 @ 02:54 AM
www.louisvuittonwarehouse.com