There are some small issues with the current nested attributes and autosave implementation that I would like to address until Rails 3.
1) The first one was already dissused on the mailing list, which is the use of "_delete" in accepts_nested_attributes_for when both the DSL (:allow_destroy) and the behavior are destroy. I've created a patch that just checks for _destroy instead of _delete with backwards compatibility.
https://rails.lighthouseapp.com/projects/8994/tickets/2889
This is to avoid confusion and open room for more improvements in the future.
2) How messages are created with nested attributes are hard to translate and does not work for other languages. For instance:
model_name + " " + attribute_name + " " + message
Is not grammatically correct in portuguese and japanese. There is a ticket about it on Lighthouse:
https://rails.lighthouseapp.com/projects/8994/tickets/2210
Although the ticket does not solve the whole problem. It only deals with attributes at the beginning of the message, but not with the errors messages. For instance, when you have a project which has many tasks, it will search for messages at:
+ activerecord.errors.messages.project.tasks.name
And it does not fallback to:
+ activerecord.errors.messages.task.name
So if I customize the error message, I will have to do that in two places.
3) Another problem that has bitten me frequently when using autosave is that I don't want error messages to be copied to the parent when I'm giving both parent and child to error_messages_for. For that, I usually need to create an after_validate callback that delete the messages in the parent:
def after_validate self.errors.each do |key, value| next unless key.to_s =~ /child_/ self.errors.instance_variable_get('@errors').delete(key) end end
Right now, we deal with parent-child errors in two ways:
+ We copy the child errors to the parent (default for autosave); + We add an error to the parent saying that the child is invalid (default for validate => true);
I would like to add a third one:
+ Do nothing with the error messages;
The patch would be simple, the tricky is how the DSL is going to be. A suggestion (which definitely needs improvement):
has_many :tasks, :validate => :copy_errors # copy all messages to the parent has_many :account, :validate => true # add one message that the parent is invalid has_one :account, :validate => :skip_errors # validate but skip the error messages has_one :account, :validate => false # does not validate
I already have a patch for (1), working on another patch for (2) and would like to have some discussion on the DSL for (3) before working on it.
What are your thoughts?
