Quería limpiar y refactorizar un poco el código de mi método de creación dentro de mi controlador de pedidos, y leí que es una buena práctica para usar objetos de servicio. A partir de este código horrible aquí:

def create
  if current_user.orders.where(paid: false).present?
    order = current_user.orders.last
    order_id = order.id
    product_id = @product.id
    @product.ordinable = false
    @product.save
    order_amount = order.amount
    if order.products << @product
      order.products.each do |x|
        @order_amountnew = order_amount + x.price
      end
      order.amount = @order_amountnew
      order.save
      respond_to do |format|
        format.html { redirect_to products_path, notice: 'Product added to the cart!' }
      end
    else
      respond_to do |format|
        format.html { redirect_to products_path, notice: 'There was a problem while adding the product to the cart!' }
      end
    end
  else
    product_id = @product.id
    order = current_user.orders.new
    order.save
    order_id = order.id
    @product.ordinable = false
    @product.save
    order_amount = order.amount
    if order.products << @product
      order.products.each do |x|
        @order_amountnew = order_amount + x.price
      end
      order.amount = @order_amountnew
      order.save
      respond_to do |format|
        format.html { redirect_to products_path, notice: 'Product added to the cart!' }
      end
      OrderPaidCheckJob.set(wait: 3.minutes).perform_later(order_id)
    else
      respond_to do |format|
        format.html { redirect_to products_path, notice: 'There was a problem while adding the product to the cart!' }
      end
    end
  end
end

Quería dividir el método en dos segmentos básicamente, así que creé dos módulos en la carpeta de servicios de la siguiente manera. Llamé al primero order_present_create_service

module OrderPresentCreateService
  class << self
    def create(params)
      order = current_user.orders.last
      order_id = order.id
      product_id = @product.id
      @product.ordinable = false
      @product.save
      order_amount = order.amount
      if order.products << @product
        order.products.each do |x|
          @order_amountnew = order_amount + x.price
        end
        order.amount = @order_amountnew
        order.save
        respond_to do |format|
          format.html { redirect_to products_path, notice: 'Product added to the cart!' }
        end
      else
        respond_to do |format|
          format.html { redirect_to products_path, notice: 'There was a problem while adding the product to the cart!' }
        end
      end
    end
  end
end

Y llamé a la segunda order_new_create_service

module OrderNewCreateService
  class << self
    def create(params)
      product_id = params[:id]
      order = current_user.orders.new
      order.save
      order_id = order.id
      @product.ordinable = false
      @product.save
      order_amount = order.amount
      if order.products << @product
        order.products.each do |x|
          @order_amountnew = order_amount + x.price
        end
        order.amount = @order_amountnew
        order.save
        respond_to do |format|
          format.html { redirect_to products_path, notice: 'Product added to the cart!' }
        end
        OrderPaidCheckJob.set(wait: 3.minutes).perform_later(order_id)
      else
        respond_to do |format|
          format.html { redirect_to products_path, notice: 'There was a problem while adding the product to the cart!' }
        end
      end
    end
  end
end

Aquí mi nueva controladora:

def create
  if current_user.orders.where(paid: false).present?
    OrderPresentCreateService.create(params)
  else
    OrderNewCreateService.create(params)
  end
end

Simplemente estaba siguiendo este artículo aquí para que funcione. Cuando intento crear un pedido, aparece este error:

variable local indefinida o método `current_user 'para OrderNewCreateService: Module

Al principio recibía un error similar con product_id = @ product.id, así que lo cambié en product_id = params [: id] y lo hago funcionar de alguna manera. ¿Dónde lo estoy haciendo mal?

0
Kaido 16 oct. 2018 a las 17:56

2 respuestas

La mejor respuesta

Me parece que puedes refactorizar tu acción create sin usar servicios. (Soy un gran admirador de los servicios. No creo que los necesites aquí).

Primero, agregaría un par de métodos a su modelo de usuario, algo como esto:

class User < ApplicationRecord

  def unpaid_orders
    orders.where(paid: false)
  end

  def unpaid_orders?
    unpaid_orders.any?
  end

end

Entonces, haría amount un método en lugar de un atributo, algo como:

class Order < ApplicationRecord 

  def amount
    products.sum(&:price)
  end

end

Luego, en su controlador, puede hacer algo como:

delegate *%w(
  unpaid_orders?
  orders
), to: :current_user

def create
  order = unpaid_orders? ? orders.last : orders.create!
  @product.update(ordinable: false)
  if order.products << @product 
    @notice = 'Product added to the Cart!'
    OrderPaidCheckJob.set(wait: 3.minutes).perform_later(order.id) unless unpaid_orders?
  else 
    @notice = 'There was a problem while adding the product to the cart!'
  end
  redirect_to products_path, notice: @notice
end

Si no desea que amount sea un método en Order, puede hacer lo siguiente:

delegate *%w(
  unpaid_orders?
  orders
), to: :current_user

def create
  order = unpaid_orders? ? orders.last : orders.create!
  @product.update(ordinable: false)
  if order.products << @product 
    order.update(amount: order.products.sum(&:price))
    @notice = 'Product added to the Cart!'
    OrderPaidCheckJob.set(wait: 3.minutes).perform_later(order.id) unless unpaid_orders?
  else 
    @notice = 'There was a problem while adding the product to the cart!'
  end
  redirect_to products_path, notice: @notice
end
1
jvillian 16 oct. 2018 a las 18:19

Si lo entiendo correctamente, la tarea es refactorizar la acción original del controlador. Su código refactorizado tiene una lista de problemas, así que para que la respuesta sea más corta, permítame omitir la revisión y centrarme en la acción original.

Los problemas que veo son:

  • La variable product_id se inicializa pero no se utiliza
  • La variable order_id en la 1ra rama de la condición if se inicializa pero no se usa
  • El bloque respond_to tiene una condición para el formato html únicamente. Dado que no hay código para otros formatos, se puede simplificar
  • El método OrderPaidCheckJob.set se coloca después de redirect_to, por lo que nunca se llamará. Para que funcione, colóquelo antes de redirect_to
  • A pesar de las condiciones de if, la acción termina con redirect_to en la misma ruta, solo cambia notice. Así que podría configurar correctamente notice y mover redirect_to de if s
  • @product.ordinable = false; @product.save se puede simplificar como @product.update(ordinable: false). Dado que está en ambas ramas de if, se puede mover fuera de ese bloque.
  • No estoy seguro de por qué if order.products << @product. ¿Cuándo cree que esta condición es falsa? En caso de problemas para agregarlo a la base de datos, lanzará una excepción.
  • order.products.each do |x| etc bloque se puede simplificar como order.amount = @order_amountnew = order_amount + order.products.last.price (sospecho que tiene un error aquí porque itera sobre products pero guarda el resultado solo para el último producto)
  • Inicializa la variable de instancia @order_amountnew pero no la usa más tarde debido a la redirección. Se puede quitar, creo

Resultado preliminar:

def create
  @product.update(ordinable: false)

  if current_user.orders.where(paid: false).present?
    order = current_user.orders.last
    order_amount = order.amount
    if order.products << @product
      order.amount = @order_amountnew = order_amount + order.products.last.price
      order.save

      notice = 'Product added to the cart!'
    else
      notice = 'There was a problem while adding the product to the cart!'
    end

  else
    order = current_user.orders.new
    order.save
    order_id = order.id
    order_amount = order.amount
    if order.products << @product
      order.amount = @order_amountnew = order_amount + order.products.last.price
      order.save

      OrderPaidCheckJob.set(wait: 3.minutes).perform_later(order_id)

      notice = 'Product added to the cart!'
    else
      notice = 'There was a problem while adding the product to the cart!'
    end
  end

  redirect_to products_path, notice: notice
end

¿Se ve mejor y más legible? Como todavía tiene el código repetido, se puede SECAR aún más. Pero recomendaría manejar los problemas que señalé antes de refactorizarlo más.

1
Ilya Konyukhov 16 oct. 2018 a las 16:01