感谢你的分享

这里有一些问题我想指出来，希望可以帮到大家

1. OrderID建议加个 index, 能加快 query 的速度

2. after_destroy :destroy_all_same_orderID 没有必要，你只需要在Order里加一个OrderItem的关联，然后设置 destroy 的关系就可以了

   class Order < ApplicationRecord
     has_many :order_items, dependent: :destroy
   end
   
   class OrderItem < ApplicationRecord
     belongs_to :order, foreign_key: "OrderID"
   end
   

3. 读取数据的时候不需要加self ，只有在写数据的时候需要

4. after_create :create_order_items 里的 OrderItem 的创建没有检测到底成功了没有，如果出错了会导致 OrderItem 的遗失。而且因为你没有用 bang method, 所以出错了也不会有异常抛出，也就不会有回滚了

我的一些建议

1. 不要用 callbacks, 可以网上搜一下 rails callbacks hell，就能了解到 callbacks 带来的问题，而这些问题会随着业务逻辑的复杂而越来越难修正
2. 不要把业务逻辑放在 model 里。业务逻辑可以放在 service objects 里，keep them isolated, easily testable, reusable, SRP

非常感谢@mengqing 的指点。 第 1 点，加 index 是个不错的主意！ 第 2 点。这里并不是 has_many 的关系，orderID 是生成后插入的，2 张表都有的，相同的。 其他几点我再考虑下，怎么调整。

对于第 4 点，测试一下回滚。

在 each 循环里面，抛出异常，测试是否能回滚。

def create_order_items
      current_user = self.user
      current_user.carts.each do |cart|
        OrderItem.new(user_id: current_user.id, orderID: self.orderID, product_id: cart.product_id, price: cart.product.price, amount: cart.amount).save
        # 抛出异常
        raise "crash for test"
      end
      # raise "crash for test"
      Cart.where(user_id: current_user.id).destroy_all
    end

日志如下，可以看到，已经 ROLLBACK

Started POST "/orders" for ::1 at 2017-02-28 21:28:36 +0800
Processing by OrdersController#create as HTML
  Parameters: {"address"=>"8", "pay_method"=>"alipay"}
  User Load (0.3ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1 ORDER BY "users"."id" ASC LIMIT $2  [["id", 4], ["LIMIT", 1]]
  Cart Load (0.6ms)  SELECT "carts".* FROM "carts" WHERE "carts"."user_id" = $1  [["user_id", 4]]
  Product Load (0.2ms)  SELECT  "products".* FROM "products" WHERE "products"."id" = $1 LIMIT $2  [["id", 4], ["LIMIT", 1]]
  Address Load (0.2ms)  SELECT  "addresses".* FROM "addresses" WHERE "addresses"."id" = $1 LIMIT $2  [["id", 8], ["LIMIT", 1]]
   (0.1ms)  BEGIN
  SQL (0.4ms)  INSERT INTO "orders" ("user_id", "orderID", "created_at", "updated_at", "fixed_address", "total_price") VALUES ($1, $2, $3, $4, $5, $6) RETURNING "id"  [["user_id", 4], ["orderID", "1488288516220"], ["created_at", 2017-02-28 13:28:36 UTC], ["updated_at", 2017-02-28 13:28:36 UTC], ["fixed_address", "李明 17030006000 北京 北京 海淀区 青松国际 36203"], ["total_price", #<BigDecimal:7f98c1e6e680,'0.25E1',18(27)>]]
  User Load (0.4ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1 LIMIT $2  [["id", 4], ["LIMIT", 1]]
  Cart Load (0.3ms)  SELECT "carts".* FROM "carts" WHERE "carts"."user_id" = $1  [["user_id", 4]]
  Product Load (0.2ms)  SELECT  "products".* FROM "products" WHERE "products"."id" = $1 LIMIT $2  [["id", 4], ["LIMIT", 1]]
  SQL (0.5ms)  INSERT INTO "order_items" ("orderID", "product_id", "price", "amount", "created_at", "updated_at", "user_id") VALUES ($1, $2, $3, $4, $5, $6, $7) RETURNING "id"  [["orderID", "1488288516220"], ["product_id", 4], ["price", #<BigDecimal:7f98c2152d60,'0.25E1',18(27)>], ["amount", #<BigDecimal:7f98c2152950,'0.1E1',9(18)>], ["created_at", 2017-02-28 13:28:36 UTC], ["updated_at", 2017-02-28 13:28:36 UTC], ["user_id", 4]]
   (0.2ms)  ROLLBACK
Completed 500 Internal Server Error in 43ms (ActiveRecord: 3.5ms)

结果是能够回滚，通过界面测试也没问题，订单和购物车都还是原样不变，只是我这里还没有加异常判断。

你这里回滚了是因为你手动抛出了异常，save命令是不会抛出异常的，只会返回 true 或者 false。如果你需要抛出异常就要用save!

@mengqing 这个明白了，多谢！

Any exception that is not ActiveRecord::Rollback or ActiveRecord::RecordInvalid will be re-raised by Rails after the callback chain is halted. Raising an exception other than ActiveRecord::Rollback or ActiveRecord::RecordInvalid may break code that does not expect methods like save and update_attributes (which normally try to return true or false) to raise an exception.

我的经验教训，订单和订单项的创建是主业务逻辑，不建议放到 callbacks 中，放到 callbacks 里代码不直观，可读性差很多。 至于你提到的作为一个事务处理，很多处理的方法，比如：

• 1 最简单的方法建立 Order 和 OrderItem 关联，order has_many order_items ，通过 order.order_items.build(...) 构造 order_items，order.save! 是放在一个数据库事务中处理。
• 2 另外一种方法显式使用 Order. transaction {} 。

而 destory 使用 关联的 dependent: :destory 选项处理最简单。

@vincent , @mengqing 改造了一下，如果使用ActiveRecord::Base.transaction的话

将 create 逻辑放到了 controller 中核心代码如下：

def create
    ActiveRecord::Base.transaction do
      @order = Order.new(user_id: current_user.id, orderID: @orderID , message:params[:message], fixed_address: Address.find(address_id).full_address, total_price: @total)
      @order_saved = @order.save!
      current_user.carts.each do |cart|
        OrderItem.new(user_id: current_user.id, orderID: @orderID, product_id: cart.product_id, price: cart.product.price, amount: cart.amount).save!
        # raise "crash for test"
      end
      #raise "crash for test"
      Cart.where(user_id: current_user.id).destroy_all
    end
end

将 delete 逻辑放到了 controller 中核心代码如下：

def destroy
    ActiveRecord::Base.transaction do
      OrderItem.where(orderID: @order.orderID).destroy_all
      @order.destroy!
    end
end

一楼说的挺详细了，你可以看下 service 的用法。 你 controller 加事物，反模式了。 至于删除，楼上也指出了，关联一下就好了。不需要额外的加一个事物了呀

以前遇到过关于 build 的问题，如果关联的表的 外键设置了必输验证，那么保存会失败。之后就很少用 build 的方式了，transaction 就挺好的感觉

class Order < ApplicationRecord
  has_many :order_items, dependent: :destroy
end

class OrderItem < ApplicationRecord
  belongs_to :order, foreign_key: "OrderID"
end

order = Order.new(user_id: current_user.id, orderID: @orderID , message:params[:message], fixed_address: Address.find(address_id).full_address, total_price: @total)

current_user.carts.each do |cart|
    order.order_items.build(user_id: current_user.id, product_id: cart.product.id, price: cart.product.price, amount: cart.amount)
end
# 保存作为一次事务处理
order.save!

# order 删除会级联删除 order_items
order.destroy

感谢。

如果按照您这样 create order 和 create order_items 解决了，还有最后一步逻辑，下单后，清空购物车。

Cart.where(user_id: current_user.id).destroy_all

这个应该包含在事务中？这个逻辑该放在哪里呢？

嗯，controller 加ActiveRecord::Base.transaction do，觉得有些不对味。

这里的 order_item 应该不会让用户直接操作，所有没有验证的问题。失败是正常，如果保存失败，能保证回滚和处理异常就好。创建之前都要执行validate的。

这个可以直接用 ActiveRecord::Base.transaction 把它们包在一个事务里，到底是放在 controller 还是抽离到一个 service 看个人喜好了。

@vincent 好的，非常感谢您的建议！