2

I have a model named UserPrice which has the attribute :purchase_date(a date_select) in its table. With my form I can create multiple user_prices at once but for user convenience I made a virtual attribute inside of my UserPrice model called :all_dates that's also a date_select field and its job is to be the replacement of the :purchase_dates so users only have to select the :all_dates field for the date.


Problem & Question

The :all_dates field is not updating the :purchase_date fields of my user_prices that are being created. What do I need to do in order to get my :all_dates field to update the :purchase_date fields of my new UserPrices?

Does anyone have any tips on how to do this?


Parameters

Parameters: 
"user_price"=> { 
"all_dates(2i)"=>"10", 
"all_dates(3i)"=>"27", 
"all_dates(1i)"=>"2011"
}, 
"user_prices"=>
{
"0"=>{"product_name"=>"Item1", "store"=>"Apple Store","price"=>"6"}, 
"1"=>{"product_name"=>"Item2", "store"=>"Apple Store", "price"=>"7"}
}, 
"commit"=>"Submit"}

Code

  class CreateUserPrices < ActiveRecord::Migration
    def self.up
       create_table :user_prices do |t|
          t.decimal :price
          t.integer :product_id
          t.date :purchase_date
          t.timestamps
        end
     end
  end

I took out the :purchase_date field so it isn't inside of the user_price loop.

<%= form_tag create_multiple_user_prices_path, :method => :post do %>
 <%= date_select("user_price", "all_dates" )  %>
   <% @user_prices.each_with_index do |user_price, index| %>
      <%= fields_for "user_prices[#{index}]", user_price do |up| %>
          <%= render "user_price_fields", :f => up %>
      <% end %>
   <% end %>
<% end %>




class UserPrice < ActiveRecord::Base
  attr_accessible :price, :product_name, :purchase_date, :all_dates, :store
  attr_accessor :all_dates
  after_save :save_all_dates_to_user_prices
  composed_of :all_dates, :class_name => "DateTime",
    :mapping => %w(Time to_s),
    :constructor => Proc.new { |item| item },
    :converter => Proc.new { |item| item }

  def user_prices
    @user_prices = Array.new() { UserPrice.new }
  end

  protected

  def save_all_dates_to_user_prices 
     if !self.all_dates.nil?       
      self.user_prices.each {|up| up.purchase_date = self.all_dates if up.new_record?}
     end
  end


class UserPricesController < ApplicationController

 def new
    @user_prices = Array.new(5) { UserPrice.new }
 end

 def create_multiple
   @user_prices = params[:user_prices].values.collect { |up| UserPrice.new(up) }
   if @user_prices.all?(&:valid?)
     @user_prices.each(&:save!)
     redirect_to :back, :notice => "Successfully added prices."
   else
     redirect_to :back, :notice => "Error, please try again."
   end
end
2
  • Where is all_dates being populated? In the form? You should post your form.
    – bricker
    Oct 18, 2011 at 10:29
  • @bricker :all_dates is posted in my form, that is correct. Oct 20, 2011 at 21:34

2 Answers 2

2
+100

This is a case of trying to do in a model what is better left to the controller. All you're trying to do here is to auto-assign a certain attribute on creation from a parameter not directly tied to your model. But you're not even passing that extra parameter to the model anywhere - you're creating your model instances from the user_prices parts of the parameter hash, but the user_price sub-hash is not used anywhere. In any case, this is behavior that is more closely related to the view and action taken than the model, so keep it in the controller.

Try this:

  1. Throw out the virtual attribute, and get rid of the whole after_save callback stuff
  2. Throw away the user_prices method in your model
  3. Change the all_dates attribute name back to purchase_date in the form

Then your parameter hash should look like this:

{"user_price"=> { 
  "purchase_date(2i)"=>"10", 
  "purchase_date(3i)"=>"27", 
  "purchase_date(1i)"=>"2011"
}, 
"user_prices"=>
{
  "0"=>{"product_name"=>"Item1", "store"=>"Apple Store","price"=>"6"}, 
  "1"=>{"product_name"=>"Item2", "store"=>"Apple Store", "price"=>"7"}
}}

All that's left to do is to merge the single user_price attributeS into each user_prices sub-hash in your create_multiple action. Replace the first line in that action with this:

@user_prices = params[:user_prices].values.collect do |attributes| 
  UserPrice.new(attributes.merge(params[:user_price])) 
end
1
  • Yes, Finally, thank you so much Thilo. To achieve want I wanted, yours was a far better and understandable way. Thank you, everything is working how I wanted it to! Oct 28, 2011 at 16:32
1

I'm not sure why you are even using that virtual attribute is there more to this implementation? If you are just trying to save an associated model, you might simply want a accepts_nested_attributes_for :user_prices in your User model

This works great and many developers use this method, so it's nice to know for working on other projects as well as for the people who might end up maintaining yours.

http://railscasts.com/episodes/196-nested-model-form-part-1

http://railscasts.com/episodes/197-nested-model-form-part-2

10
  • The virtual attribute is for updating all of my :purchase_dates of my UserPrice's and the rails casts way of doing it different from how I did it. I will include more code so the picture is clearer. Oct 19, 2011 at 19:22
  • I am just trying to figure out if their is a method like update_all that would update only those particular resources in my create action. Oct 19, 2011 at 19:24
  • 1
    Okay I think I understand the "why" behind it now. I would probably do it the railscasts way and just use hidden fields on the form to let the "magic" of rails do it's thing. But you could self.user_prices.each {|up| up.purchase_date = self.all_dates if up.new_record?}
    – shaheenery
    Oct 19, 2011 at 20:25
  • I get a mass assignment error when I try that following above code. Take a look at my updated section. Oct 20, 2011 at 21:35
  • 1
    @JustinRoR, if you are okay with setting these attributes via mass-assignment you just need to add attr_accessible :attr1, :attr2, :attr3 to the model that they are on.
    – shaheenery
    Oct 20, 2011 at 22:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.