Rails Ruby on Rails

Comments and changes to this ticket

• You flagged this item as spam.
  

  Aaron Aberg September 14th, 2010 @ 09:45 AM

  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

• 

  David Trasbo September 23rd, 2010 @ 09:53 AM

  • Importance changed from “” to “Low”

  What do you mean by this? "I have a Customer which is a User which is a Person." Are you using STI or have you set up belongs_to associations between the models? Please paste extracts of the models' code, if you don't mind.

• You flagged this item as spam.
  

  Aaron Aberg September 24th, 2010 @ 02:13 AM

  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.

• You flagged this item as spam.
  

  Aaron Aberg September 24th, 2010 @ 02:17 AM

  • Title changed from “validates_associated should be allowed to not create an error” to “validates_associated should be allowed to filter out the generic error for nested models.”

• You flagged this item as spam.
  

  Aaron Aberg September 25th, 2010 @ 11:02 AM

  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
  

• 

  David Trasbo September 25th, 2010 @ 05:55 PM

  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.

• You flagged this item as spam.
  

  Aaron Aberg September 25th, 2010 @ 06:45 PM

  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.

• 

  Jeff Kreeftmeijer November 7th, 2010 @ 04:56 PM

  • Tag cleared.

  Automatic cleanup of spam.

• 

  Santiago Pastorino February 9th, 2011 @ 12:32 AM

  • State changed from “new” to “open”

  This issue has been automatically marked as stale because it has not been commented on for at least three months.

  The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

  Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

• 

  Santiago Pastorino February 9th, 2011 @ 12:32 AM

  • State changed from “open” to “stale”

• You flagged this item as spam.
  

  bingbing March 26th, 2011 @ 02:15 AM

  louis vuitton watches