在Rails中如何从请求高效且符合规范地创建模型关联？

Great job getting the basic functionality working! You're right that we can make this more efficient and aligned with Rails best practices. Let's break down the key improvements and refactor the code step by step.

Key Optimization Areas #

• Database Transactions: Wrap all operations in a transaction to ensure data consistency (if one creation fails, nothing gets saved).
• Batch Operations: Reduce round-trips to the database by creating records in batches where possible.
• Nested Attributes: Leverage Rails' accepts_nested_attributes_for to let your models handle the association logic, which is cleaner than manually linking records.
• Clean Parameter Handling: Move parameter sanitization into private methods for better readability and reusability.
• Remove Hardcoded Values: Replace user_id: 1 with a dynamic value (like current_user.id once authentication is set up).
• Error Handling: Add validation checks and error handling to avoid silent failures.

Step 1: Update Your Models #

First, add accepts_nested_attributes_for to your Purchase model to handle reimbursement creation directly:

# app/models/purchase.rb
class Purchase < ApplicationRecord
  belongs_to :user
  belongs_to :credit_card
  has_many :reimbursements, dependent: :destroy
  accepts_nested_attributes_for :reimbursements, allow_destroy: true

  # Add validations as needed
  validates :description, :amount, :transaction_date, presence: true
end

# app/models/reimbursement.rb
class Reimbursement < ApplicationRecord
  belongs_to :purchase
  belongs_to :user

  # Add validations as needed
  validates :pay_from_pot_id, :total_amount, presence: true
end

Step 2: Refactor the Controller Action #

Here's the optimized controller code that uses transactions, nested attributes, and cleaner parameter handling:

def import
  card = CreditCard.find_by(id: params[:card])
  pay_to_pot_id = params[:cardPot]
  user = User.find(1) # Replace with current_user in production

  ActiveRecord::Base.transaction do
    purchases_attributes = params[:purchases].map do |p|
      # Sanitize purchase params and add nested reimbursement attributes
      purchase_params = p.permit(:description, :amount, :transaction_date, :statement_reference, :statement_description)
                         .merge(user_id: user.id, credit_card_id: card.id)

      # Build reimbursement attributes for each purchase
      reimbursement_attrs = p[:reimbursements].map do |r|
        start_date = p[:start_date] || Date.today
        instalments = p[:spread] ? card.free_months_remaining : 1

        {
          user_id: user.id,
          pay_to_pot_id: pay_to_pot_id,
          pay_from_pot_id: r[:pot],
          total_amount: p[:amount],
          instalments: instalments,
          start_date: start_date
        }
      end

      purchase_params.merge(reimbursements_attributes: reimbursement_attrs)
    end

    # Batch create all purchases with their reimbursements
    Purchase.create!(purchases_attributes)
  end

  # Handle success case
  render json: { message: "Purchases and reimbursements imported successfully" }, status: :created
rescue ActiveRecord::RecordInvalid => e
  # Handle validation errors
  render json: { error: e.record.errors.full_messages }, status: :unprocessable_entity
rescue ActiveRecord::RecordNotFound => e
  # Handle missing credit card or user
  render json: { error: e.message }, status: :not_found
end

private

# Optional: Formalize parameter sanitization
def import_purchase_params
  params.require(:purchases).each do |p|
    p.permit(
      :description, :amount, :transaction_date, :statement_reference, :statement_description,
      reimbursements: [:pot]
    )
  end
end

Why This Works Better #

1. Transaction Safety: If any purchase or reimbursement fails validation, the entire import rolls back, so you don't end up with partial data in your database.
2. Batch Creation: Purchase.create!(purchases_attributes) creates all purchases and their associated reimbursements in fewer database queries compared to looping and creating each one individually.
3. Rails Convention: Using accepts_nested_attributes_for keeps association logic where it belongs—in the models—instead of cluttering the controller.
4. Readability: Breaking the logic into mapping attributes first makes the code easier to follow and maintain.
5. Error Handling: Explicit rescue blocks let you return meaningful error responses instead of letting failures bubble up silently.

Alternative: Batch Purchases First, Then Reimbursements #

If you prefer to create purchases in a batch first (without nested attributes), you can map the original purchase data to track which reimbursements belong to which purchase:

def import
  card = CreditCard.find_by(id: params[:card])
  pay_to_pot_id = params[:cardPot]
  user = User.find(1)

  ActiveRecord::Base.transaction do
    # Prepare purchase data and keep track of reimbursement mappings
    purchase_data = params[:purchases].map do |p|
      purchase_attrs = p.permit(:description, :amount, :transaction_date, :statement_reference, :statement_description)
                        .merge(user_id: user.id, credit_card_id: card.id)
      { attrs: purchase_attrs, reimbursements: p[:reimbursements], spread: p[:spread], start_date: p[:start_date] }
    end

    # Batch create purchases
    purchases = Purchase.create!(purchase_data.map { |pd| pd[:attrs] })

    # Batch create reimbursements by linking to the new purchase IDs
    reimbursement_attrs = purchases.zip(purchase_data).flat_map do |purchase, pd|
      pd[:reimbursements].map do |r|
        start_date = pd[:start_date] || Date.today
        instalments = pd[:spread] ? card.free_months_remaining : 1

        {
          purchase_id: purchase.id,
          user_id: user.id,
          pay_to_pot_id: pay_to_pot_id,
          pay_from_pot_id: r[:pot],
          total_amount: pd[:attrs][:amount],
          instalments: instalments,
          start_date: start_date
        }
      end
    end

    Reimbursement.create!(reimbursement_attrs)
  end

  render json: { message: "Import successful" }, status: :created
rescue ActiveRecord::RecordInvalid => e
  render json: { error: e.record.errors.full_messages }, status: :unprocessable_entity
rescue ActiveRecord::RecordNotFound => e
  render json: { error: e.message }, status: :not_found
end

This approach is also valid and might be preferable if you need more control over the order of operations.

内容的提问来源于stack exchange，提问作者Chris A