This project is archived and is in readonly mode.

#6227 ✓invalid
Andrea Franceschini

Rails3 - accepts_nested_attributes_for reject_if is evaluated but ignored.

Reported by Andrea Franceschini | December 27th, 2010 @ 04:05 PM

This may very well be a duplicate of #5579 but I'm reporting this for rails-3.0.3 (and activerecord-3.0.3) so opening a new ticket seems more appropriately (sorry if I first posted a follow up to the other bug).

FTR, here's my sources:

# app/models/invoice.rb
class Invoice < ActiveRecord::Base
    attr_accessible :tax, :discount, :modifier, :paid, :issue_date, :payment_date, :description, :items_attributes
    has_many :items, :class_name => 'InvoiceItem', :dependent => :destroy
    accepts_nested_attributes_for :items, :reject_if => proc { |a| a[:description].blank? }, :allow_destroy => true
end

# app/models/invoice_item.rb
class InvoiceItem < ActiveRecord::Base
    attr_accessible :price, :discount, :description
    belongs_to :invoice
end

I could verify that the :reject_if is honoured when creating a new Invoice, while it's not when updating an existing one.

Comments and changes to this ticket

  • Andrea Franceschini

    Andrea Franceschini December 30th, 2010 @ 11:16 PM

    I have put together a simpler example for this, and it still doesn't work (though the proc correctly works with the symbol hash).

    # app/models/user.rb
class User < ActiveRecord::Base
    attr_accessible :name, :contacts_attributes
    
    has_many :contacts
    accepts_nested_attributes_for :contacts,
        :reject_if => proc { |a| a[:content].blank? },
        :allow_destroy => true
end

# app/models/contact.rb
class Contact < ActiveRecord::Base
    attr_accessible :content
    
    belongs_to :user
end

# app/controllers/users_controller.rb
class UsersController < ApplicationController
    
    ## REST stuff
    
    def index
        @users = User.all
    end
    
    def show
        @user = User.find(params[:id])
    end
    
    def create
        @user = User.new(params[:id])
        if @user.save
            flash[:success] = "User created"
            redirect_to @user
        else
            render :new
        end
    end
    
    def update
        @user = User.find(params[:id])
        if @user.update_attributes(params[:user])
            flash[:success] = "User updated"
            redirect_to @user
        else
            render :edit
        end
    end
    
    def destroy
        User.find(params[:id]).destroy
        flash[:success] = "User destroyed"
    end
    
    ## forms
    
    def new
        @user = User.new
    end
    
    def edit
        @user = User.find(params[:id])
    end
end

# app/views/users/edit.html.erb
<!DOCTYPE html>
<html>
    <head>
        <title>Users</title>
    </head>
    
    <body>
        <h1>Users :: edit</h1>
        <%= form_for @user do |f| %>
            <p><%= f.label :name %>
            <%= f.text_field :name %></p>
            
            <h2>Contacts</h2>
            <%= f.fields_for :contacts do |b| %>
                <p><%= b.label :content %>
                <%= b.text_field :content %></p>
            <% end %>
            
            <%= f.submit %>
        <% end %>
    </body>
</html>

    Using this example I could verify that the proc is correctly run, but the result is somewhat ignored. I was expecting the corresponding Contact to be destroyed, and yet it survives, unchanged. That is, if I leave empty the "content" field in the form, the whole record is not updated, nor destroyed.

  • Neeraj Singh

    Neeraj Singh December 31st, 2010 @ 09:56 PM

    • Importance changed from “” to “Low”

    Hi Andrea :

    I worked on #5579 and would like to see that this ticket gets resolved.

    Thanks for providing all the details to recreate the problem. If you don't mind then can you create an app with everything that you provided and push that app to github. In this way I will able to quickly reproduce the problem.

    Thanks.

  • Andrea Franceschini

    Andrea Franceschini January 1st, 2011 @ 01:03 PM

    Hi Neeraj,
    here's the application that code comes from: https://github.com/Morpheu5/reject_if
    I hope it helps, thanks a lot.

  • Andrea Franceschini

    Andrea Franceschini January 12th, 2011 @ 07:00 PM

    Hi there, any news?

  • Neeraj Singh

    Neeraj Singh January 13th, 2011 @ 02:42 AM

    @Andrea I will take a look at it tomorrow. sorry about the delay.

  • Neeraj Singh

    Neeraj Singh January 13th, 2011 @ 03:17 PM

    I added a test case and the test case is passing for me.

    In order to reproduce the case can you add to the readme of the app (https://github.com/Morpheu5/reject_if/tree/master/app/models) what steps I need to follow and what I should see and what I will actually see.

    Thanks

  • Andrea Franceschini

    Andrea Franceschini January 13th, 2011 @ 06:37 PM

    Hi Neeraj,
    I just committed some changes which I left out in the first place. You should sync with the latest master branch before following the steps in the readme file.

    Thanks a lot for your help.

    PS. Could you please attach the test case you wrote?

  • Neeraj Singh

    Neeraj Singh January 14th, 2011 @ 04:40 AM

    Please read the readme at https://github.com/neerajdotname/t_6227 .

  • Andrea Franceschini

    Andrea Franceschini January 14th, 2011 @ 09:16 AM

    I tried to add some tests like those to my application, then watched them fail. So I added some rspec tests, then watched them fail too. You can find the latest updates at https://github.com/Morpheu5/reject_if .

  • Neeraj Singh

    Neeraj Singh January 14th, 2011 @ 01:39 PM

    I don't follow your assertion. In order to make that test pass you need to do

     user.update_attributes :contacts_attributes => [{:id => contact.id, '_destroy' => true}]

    If you want to destroy a record you need to pass '_destroy' with a truthy value.

  • Andrea Franceschini

    Andrea Franceschini January 14th, 2011 @ 02:02 PM

    Uhm. It turns out I misunderstood the function of reject_if, since I thought that if a nested attribute is rejected, it would be deleted from the database too. Like "leave this field(s) blank and you'll effectively delete the record".

    Thanks for your time, and sorry for wasting it :) feel free to close this bug.

  • Sergio Cambra

    Sergio Cambra January 14th, 2011 @ 02:36 PM

    I understood it like you did too. But I think it was working like that in a previous rails version, I can't remember which. I have a test which it's failing with 3.0.3 but I think it was working in a previous version.

  • Andrea Franceschini

    Andrea Franceschini January 14th, 2011 @ 02:39 PM

    I think in my case episodes 196 and 197 of Railscasts is where I got that idea, but now I don't really remember. Anyway, those episodes were for Rails 2, not 3.

  • Neeraj Singh

    Neeraj Singh January 14th, 2011 @ 03:24 PM

    • State changed from “new” to “invalid”
  • bingbing

    bingbing March 26th, 2011 @ 01:29 AM

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Pages