1

My problem is similar to: My cloning method is stealing the children from the original model

But I can't seem to get the solution for this to work with mine. I'm trying to create an order exchange form which involves populating the form with the old record details. So when I save the form it creates a new Order record but the children seem to get deleted from the old Order record and siphoned into the new one.

Here's the code:

def new
 @old_order = Order.includes(:line_items).find(params[:id])
 @order = Order.new @old_order.attributes 
 @order.line_items = []
 @old_order.line_items.each do |old|
   new = old.dup    # the line_item id is set before creation. 
   new.order_id = @order.id
   new.save!

   @order.line_items << new
   @old_order.line_items << old   # this was to see if the old line_items would reappend to the old order. Didn't help...
 end
end

def create
 @order = Order.new(exchange_order_params)
 if @order.save
   @order.update_attributes!(stage: 2, ordered_at: Date.today)
   redirect_to admin_returns_url, notice: "Order moved to 'accepted' for processing"
 else
   flash.now[:alert] = "Please try again"
   render :action => "new"
 end
end

private
  def exchange_order_params
  params.require(:order).permit(:id, :user_id,
                 line_items_attributes: [:id, :order_id, :cart_id, :quantity, :_destroy, 
                 product_attributes: [:id, :sku, :euro_price, :sterling_price, :product_group_id, :product_size_id, :product_waistband_id]])
end

Schema.rb

create_table "orders", force: :cascade do |t|
    t.datetime "created_at",                         null: false
    t.datetime "updated_at",                         null: false
    t.boolean  "returned",           default: false
    t.date     "date_sent"
    t.date     "ordered_at"
    t.integer  "user_id"
    t.boolean  "return_requested",   default: false
    t.integer  "stage",              default: 0
    t.decimal  "order_total",        default: 0.0
    t.string   "transaction_secret"
    t.string   "token"
    t.string   "uuid"
    t.string   "currency"
    t.float    "discounted_by",      default: 0.0
  end

  add_index "line_items", ["cart_id"], name: "index_line_items_on_cart_id", using: :btree
  add_index "line_items", ["order_id"], name: "index_line_items_on_order_id", using: :btree
  add_index "line_items", ["product_id"], name: "index_line_items_on_product_id", using: :btree

  create_table "line_items", force: :cascade do |t|
    t.integer  "quantity"
    t.integer  "order_id"
    t.integer  "cart_id"
    t.integer  "product_id"
    t.datetime "created_at", null: false
    t.datetime "updated_at", null: false
    t.float    "unit_price"
    t.string   "currency"
  end



  create_table "product_groups", force: :cascade do |t|
    t.string   "name"
    t.text     "description"
    t.datetime "created_at",  null: false
    t.datetime "updated_at",  null: false
  end

  create_table "product_sizes", force: :cascade do |t|
    t.string   "specification"
    t.datetime "created_at",    null: false
    t.datetime "updated_at",    null: false
  end

  create_table "product_waistbands", force: :cascade do |t|
    t.string   "specification"
    t.datetime "created_at",    null: false
    t.datetime "updated_at",    null: false
  end

  create_table "products", force: :cascade do |t|
    t.integer  "sku"
    t.integer  "product_group_id"
    t.integer  "product_size_id"
    t.integer  "product_waistband_id"
    t.decimal  "euro_price"
    t.decimal  "sterling_price"
    t.datetime "created_at",                       null: false
    t.datetime "updated_at",                       null: false
    t.integer  "stock_level",          default: 0
  end

  add_index "products", ["product_group_id"], name: "index_products_on_product_group_id", using: :btree
  add_index "products", ["product_size_id"], name: "index_products_on_product_size_id", using: :btree
  add_index "products", ["product_waistband_id"], name: "index_products_on_product_waistband_id", using: :btree

Also in the Order model I am randomising the id before_create so that when the user submits the form, it creates a dupe copy with a different Order id. This is the same for LineItems.

Order.rb (same in LineItem.rb)

before_create :randomize_id

private
  def randomize_id
    begin
      self.id = SecureRandom.random_number(1_000_000)
    end while Order.where(id: self.id).exists?
  end
Community
  • 1
  • 1
jellybean_232
  • 189
  • 1
  • 4
  • 16

1 Answers1

0

My approach would be to override the ActiveRecord::Base#dup method in the Order model so that it's recursive, meaning that it also duplicates the LineItem collection:

class Order < ActiveRecord::Base
  def dup
    duped_order = super
    duped_order.line_items = line_items.map(&:dup)
    duped_order
  end
end

doing it this way makes it easily testable. Now the controller becomes:

class OrderController < ApplicationController
  def new
    @order = Order.find(params[:id]).dup
  end

  def create
    # not sure how your form populates the params hash
    # here you need to new-up and then save the order and the line items
    # with the attributes from the form
  end
end

Do write tests to confirm that this is doing what you intend. This is the perfect example of where the old "fat model skinny controller" paradigm should be applied.

Les Nightingill
  • 5,662
  • 1
  • 29
  • 32
  • I've attempted this but the problem arises when I'm in "create". From the code I pass "exchange_order_params" (which I have added to my snippet) but the product ID is looking for the LineItem id (Error: Couldn't find Product with ID=2 for LineItem with ID= ). I've noticed that the LineItem id and Order id is now nil for some strange reason (probably from the dup). I need the old LineItem and Order id in order to pass the old record into the new (with different Ids of course). Sorry if that's a bit hard to understand but I'm trying my best to explain my problem. – jellybean_232 Oct 05 '15 at 09:21
  • Also just to mention, I need to keep the IDs. I cannot take them out for reasons. I understand that if I remove the ids it would work but then It would raise another problem which had been solved by keeping the ids. So is there a way to pass the old LineItem Id whilst duping it? – jellybean_232 Oct 05 '15 at 09:58
  • If you need to keep id's then you're not 'cloning' the objects, but instead you are using the actual original objects, and so I'm not understanding what you are trying to do. Yes, 'dup' removes the id, and this is generally the desired behaviour. The "other problem" that you mention probably can be solved without keeping the ids. From my understanding of your question, you shouldn't be copying the ids, but maybe I don't understand fully your question. If you want to elaborate on the "other problem" maybe we can solve that one. – Les Nightingill Oct 05 '15 at 13:40
  • Yeah sorry, quite a hard one to explain, but basically I have a products table which lists all the products and the stock levels. So where the ids come in is where I create that new Order record but it needs to update the stock levels in the products table. I know cloning a record but keeping the ID sounds really odd, but I'm basically making an exchange order form where it takes the attributes of the old order and populates an Order new form (because I want to create a new record from the old ones). – jellybean_232 Oct 05 '15 at 13:43
  • So keeping the ids in the strong params means that I update the products table, and not creating new records. I found this solution here: http://stackoverflow.com/questions/18946479/ror-nested-attributes-produces-duplicates-when-edit – jellybean_232 Oct 05 '15 at 13:44
  • You're not dup'ing the product objects. When you dup the line items, they keep the same product_id but set their own id to nil. Updating product stock levels should probably be done with an after_create callback on the LineItem model. I think an ill-advised solution to the stock-level management has caused the problem you posted here. When you say something like "xxx sounds really odd" it's a hint that you might not be doing it the best way. Suggest you solve this problem as I proposed and revisit the stock management problem. Your test suite will confirm that both problems are solved. – Les Nightingill Oct 05 '15 at 16:08
  • Yeah I've had a good think about it and what you said is right. I will definitely have to find a new way. It's obvious that not a lot of people have asked my kind of questions for a reason =) Thanks so much for what you've suggested it does work very well. I will mark as accepted =) – jellybean_232 Oct 05 '15 at 17:13