Rails5：事务+如何测试它

Question

我有一堂课有以下交易：

# frozen_string_literal: true

class InactivateEmployee
  include ServiceResult

  def call(id)
    begin
      ActiveRecord::Base.transaction do
        employee = Employee.find(id)
        employee.update(is_active: false)

        if employee.tasks.any?
          employee.tasks.delete_all
        end

        response(code: 204, value: employee)
      rescue ActiveRecord::ActiveRecordError
        raise ActiveRecord::Rollback
      end
    rescue ActiveRecord::Rollback => e
      response(code: 422, errors: e)
    end
  end
end

ServiceResult 在哪里：

# frozen_string_literal: true

# ServiceResult should be included in each Service Class to have a unified returned object from each service

ServiceResultResponse = Struct.new(:success?, :response_code, :errors, :value, keyword_init: true)

module ServiceResult
  def response(code:, errors: nil, value: nil )
    ServiceResultResponse.new(
      success?: code.to_s[0] == '2',
      response_code: code,
      errors: errors,
      value: value
    )
  end
end

问题 1： 这段代码可以吗？有什么可以改进的？

问题 2 如何使用 Rspec 测试此交易？如何在我的测试中模拟 destroy_all 引发和错误？我尝试过这样的事情 - 但它不起作用....

   before do
        allow(ActiveRecord::Associations::CollectionAssociation).to receive(:delete_all).and_return(ActiveRecord::ActiveRecordError.new)
      end

Answer 1

问题 1：这段代码可以吗？有什么可以改进的？

首先，call 不应确定响应代码。这结合了在特定上下文中进行调用。那是别人的责任。例如，422 似乎不合适，这里唯一可能的错误是找不到员工 (404) 或内部错误 (500)。一般来说，如果你正在拯救 ActiveRecordError，你可能正在拯救更具体的东西。

这需要是一个完整的服务对象吗？它没有使用服务。它只对员工起作用。如果它是 Employee 的方法，则可以在任何现有的 Employee 对象上使用。

class Employee
  def deactivate!
    # There's no need for the find to be inside the transaction.
    transaction do
      # Use update! so it will throw an exception if it fails.
      update!(is_active: false)

      # Don't check first, it's an extra query and a race condition.
      tasks.delete_all
    end
  end
end

其他东西负责捕获错误并确定响应代码。可能是controller。像数据库故障这样的一般错误应该由更高层处理，可能由a default template 处理。

begin
  employee = Employee.find(id)
  employee.deactivate!
rescue ActiveRecord::RecordNotFound
  render status: :not_found
end

render status: :no_content

---

在 ServiceResult 中，您使用 code.to_s[0] == '2' 检查是否成功，请改用数学或范围。调用者根本不应该这样做，但这样做是因为您有一个模块返回一个不能为自己做任何事情的 Struct。

ServiceResult 应该是一个带有success? 方法的类。它更灵活，发生的事情更明显，并且不会污染调用者的命名空间。

class ServiceResult
  # This makes it act like a Model.
  include ActiveModel::Model

  # These will be accepted by `new`
  # You had "errors" but it takes a single error.
  attr_accessor :code, :error, :value

  def success?
    (200...300).include?(code)
  end
end

result = ServiceResult.new(code: 204, error: e)
puts "Huzzah!" if result.success?

我怀疑它是否需要。它似乎篡夺了render 的功能。它是 InactivateEmployee 试图做太多事情并不得不通过它对周围发生的事情的解释的产物吗？

---

问题 2 如何使用 Rspec 测试此事务？如何在我的测试中模拟 destroy_all 引发和错误？

既然您没有在单一方法中做太多事情，它就简单多了。

describe '#deactivate!' do
  context 'with an active employee' do
    # I'm assuming you're using FactoryBot.
    let(:employee) { create(:employee, is_active: true) }

    context 'when there is an error deleting tasks' do
      before do
        allow(employee.tasks).to receive(:delete_all)
          # Exceptions are raised, not returned.
          .and_raise(ActiveRecord::ActiveRecordError)
      end

      # I'm assuming there's an Employee#active?
      it 'remains active' do
        # same as `expect(employee.active?).to be true` with better diagnostics.
        expect(employee).to be_active
      end
    end
  end
end