validates_associated should be allowed to filter out the generic error for nested models.

Currently, I am using a work around to get the behavior I want:

class SomeModel < ActiveRecord::Base
  has_one :singular_model
  has_many :plural_models

  validates_associated :singular_model
  validates_associated :plural_models

  def after_validation
    # Skip errors that won't be useful to the end user
    filtered_errors = self.errors.reject{ |err| %{ singular_model plural_models }.include?(err.first) }
    # this changes the field name from customer.user.person.first_name
    # to: First name
    filtered_errors.collect{ |err|
      if err[0] =~ /(.+\.)?(.+)$/
        err[0] = $2.titleize
      end
      err
    }
    self.errors.clear
    filtered_errors.each { |err| self.errors.add(*err) }
  end
end

The problem with this though, is that you have to put this method in every class that uses validates_associated

Hi David,
Here are the steps to produce this (from prompt):

rails nested
cd nested
script/generate scaffold user login:string password:string
script/generate model person first_name:string last_name:string person_role_id:integer person_role_type:string
script/generate model phone_number person_id:integer number:string

edit the migrations:

class CreateUsers < ActiveRecord::Migration
  def self.up
    create_table :users do |t|
      t.string :login, :null => false
      t.string :password, :null => false
      t.timestamps
    end
  end
  def self.down
    drop_table :users
  end
end

class CreatePeople < ActiveRecord::Migration
  def self.up
    create_table :people do |t|
      t.string :first_name, :null => false
      t.string :last_name, :null => false
      t.integer :person_role_id, :null => false 
      t.string :person_role_type, :null => false 
      t.timestamps
    end
  end
  def self.down
    drop_table :people
  end
end

class CreatePhoneNumbers < ActiveRecord::Migration
  def self.up
    create_table :phone_numbers do |t|
      t.integer :person_id, :null => false
      t.string :number, :null => false
      t.timestamps
    end
  end
  def self.down
    drop_table :phone_numbers
  end
end

migrate:

rake db:migrate

edit your models:

class User < ActiveRecord::Base
  has_one :person, :as => :person_role, :dependent => :destroy
  accepts_nested_attributes_for :person, :allow_destroy => :true
  validates_associated :person # here it would be great to have something like , :ignore_generic_error => true
  validates_presence_of :login
  validates_presence_of :password
end
class Person < ActiveRecord::Base
  belongs_to :person_role, :polymorphic => true
  has_many :phone_numbers
  accepts_nested_attributes_for :phone_numbers, :allow_destroy => :true
  validates_associated :phone_numbers # here it would be great to have something like , :ignore_generic_error => true
  validates_presence_of :first_name
  validates_presence_of :last_name
end
class PhoneNumber < ActiveRecord::Base
  belongs_to :person
  validates_presence_of :person_id
  validates_presence_of :number
end

edit your users_controller.rb#new:

class UsersController < ApplicationController
  #ignore all other methods and just edit new.
  def new
    @user = User.new
    @user.person = Person.new
    @user.person.phone_numbers << PhoneNumber.new
    
    respond_to do |format|
      format.html # new.html.erb
      format.xml  { render :xml => @user }
    end
  end
end

edit your users/new.html.erb:

<h1>New user</h1>
<% form_for(@user) do |f| %>
  <%= f.error_messages %>
  <% f.fields_for :person do |p| %>
    <p>
      <%= p.label :first_name %><br />
      <%= p.text_field :first_name %>
    </p>
    <p>
      <%= p.label :last_name %><br />
      <%= p.text_field :last_name %>
    </p>
    <% p.fields_for :phone_numbers do |ph| %>
      <p>
        <%= ph.label :number %><br />
        <%= ph.text_field :number %>
      </p>
    <% end %>
  <% end %>
  <p>
    <%= f.label :login %><br />
    <%= f.text_field :login %>
  </p>
  <p>
    <%= f.label :password %><br />
    <%= f.text_field :password %>
  </p>
  <p>
    <%= f.submit 'Create' %>
  </p>
<% end %>
<%= link_to 'Back', users_path %>

Start up the server and go to: http://localhost:3000/users/new. Click the 'Create' button without entering anything in the fields. You will get the following list of errors:

• Person can't be blank
• Number can't be blank
• Phone numbers is invalid
• First name can't be blank
• Last name can't be blank
• Person is invalid
• Login can't be blank
• Password can't be blank

You'll notice that there are two generic errors: 'Person is invalid' and 'Phone numbers is invalid'. These aren't very useful and they are generated when you use the validates_associated method. I think by default, they SHOULDN'T show up but that's my opinion. I think validates_associated needs a parameter that can turn off these extra errors.

Hopefully this explains what I'm talking about.

Ok, some additional information:

I did some research. The generic error gets added in AssociatedValidator.validate_each method.
/activerecord/lib/active_record/validations/associated.rb:

def validate_each(record, attribute, value)
  return if (value.is_a?(Array) ? value : [value]).collect{ |r| r.nil? || r.valid? }.all?
  record.errors.add(attribute, :invalid, options.merge(:value => value))
end

This method is passed the associated records and determines if there were any errors in those records. If there weren't any it returns right then and there. If there was at least one error, a generic X is invalid error is added. I figured out that this error is important because its the ONLY way that you can determine if the parent record is valid or not based on its nested associations.

To fix this, we need to do something like:

/activerecord/lib/active_record/validations/associated.rb:

# TODO: expose the @associated_are_valid variable somehow so the record.valid? method
# so that it can determine whether its valid or not based on its nested associations.
def validate_each(record, attribute, value)
  # lets assume everything is ok for now.
  @associated_are_valid = true

  return if (value.is_a?(Array) ? value : [value]).collect{ |r| r.nil? || r.valid? }.all?
        
  # ok, there was an error so regardless whether we want a generic error or not
  # we'll set this to false and check it later to determine if this record
  # and its nested associations are valid.
  @associated_are_valid = false
  
  if !options[:ignore_generic_error]
    record.errors.add(attribute, :invalid, options.merge(:value => value))
  end
end

/activerecord/lib/active_record/validations.rb:

def valid?(context = nil)
  context ||= (new_record? ? :create : :update)
  output = super(context)

  # TODO: Find out if any associated models have errors. If they do,
  # this method should return false even if errors.empty? IS true.
  # errors.empty? COULD be true if we told this record to not add a
  # generic error when one of its nested associations become invalid.

  errors.empty? && output
end

Aaron,

I think you should submit a patch for this, according to the guidelines outlined here: https://rails.lighthouseapp.com/projects/8994/sending-patches

As it's not a bug per se, and certainly not very critical I don't think you'll be able to get anyone else to do it. It's a "fix it yourself or leave it" ticket, as with many others in here.

Hi David,

Yeah I saw the "we don't do feature requests" part under contributing to rails. I realized last night that this was a feature request and not a bug. I've already been working on a patch. Thanks for your help.

louis vuitton watches