Made OrderItemInvoicingProcess more readable

- Used Infra::ProcessManager
- Slightly changed the way we call MoneySplitter (it was too generic)
- the process is now mutation tested (apart from MoneySplitter)
- removed left over DetermineVatRates include (why mutant doesnt catch it?)
- reduced defensive checks in MoneySplitter - if we control how we call it and we mutation test it, its safe enough

Author: andrzejkrzywda

ecommerce/processes/.mutant.yml

@@ -12,7 +12,8 @@ matcher:
     - Processes::OrderConfirmation#stream_name
     - Processes::Test*
     - Processes::ReleasePaymentProcess*
-    - Processes::OrderItemInvoicingProcess*
+    - Processes::MoneySplitter*
+    - Processes::OrderItemInvoicingProcess#fetch_id
     - Processes::SyncShipmentFromPricing*
     - Processes::SyncInventoryFromOrdering*
     - Processes::NotifyPaymentsAboutOrderValue*

ecommerce/processes/lib/processes/determine_vat_rates_on_order_placed.rb

@@ -1,6 +1,5 @@
 module Processes
   class DetermineVatRatesOnOrderPlaced
-    include Infra::Retry
 
     ProcessState = Data.define(:offer_accepted, :order_placed, :order_id, :order_lines) do
       def initialize(offer_accepted: false, order_placed: false, order_id: nil, order_lines: [])

ecommerce/processes/lib/processes/order_item_invoicing_process.rb

@@ -1,17 +1,43 @@
 module Processes
   class OrderItemInvoicingProcess
-    include Infra::Retry
 
-    def initialize(event_store, command_bus)
-      @event_store = event_store
-      @command_bus = command_bus
+    ProcessState = Data.define(:order_id, :product_id, :quantity, :vat_rate, :discounted_amount) do
+      def initialize(order_id: nil, product_id: nil, quantity: nil, vat_rate: nil, discounted_amount: nil)
+        super(order_id:, product_id:, quantity:, vat_rate:, discounted_amount:)
+      end
+
+      def can_create_invoice_item?
+        order_id && product_id && quantity && vat_rate && discounted_amount
+      end
     end
 
-    def call(event)
-      state = build_state(event)
-      return unless state.create_invoice_item?
+    include Infra::ProcessManager.with_state(ProcessState)
 
-      unit_prices = MoneySplitter.new(state.discounted_amount, Array.new(state.quantity, 1)).call
+    subscribes_to(
+      Pricing::PriceItemValueCalculated,
+      Taxes::VatRateDetermined
+    )
+
+    def apply(event)
+      case event
+      when Pricing::PriceItemValueCalculated
+        state.with(
+          order_id: event.data.fetch(:order_id),
+          product_id: event.data.fetch(:product_id),
+          quantity: event.data.fetch(:quantity),
+          discounted_amount: event.data.fetch(:discounted_amount)
+        )
+      when Taxes::VatRateDetermined
+        state.with(
+          vat_rate: event.data.fetch(:vat_rate)
+        )
+      end
+    end
+
+    def act
+      return unless state.can_create_invoice_item?
+
+      unit_prices = MoneySplitter.new(state.discounted_amount, state.quantity).call
       unit_prices.tally.each do |unit_price, quantity|
         command_bus.call(Invoicing::AddInvoiceItem.new(
           invoice_id: state.order_id,
@@ -25,48 +51,15 @@ def call(event)
 
     private
 
-    attr_reader :event_store, :command_bus
-
-    def build_state(event)
-      with_retry do
-        stream_name = "OrderInvoicingProcess$#{event.data.fetch(:order_id)}$#{event.data.fetch(:product_id)}"
-        past = event_store.read.stream(stream_name).to_a
-        last_stored = past.size - 1
-        event_store.link(event.event_id, stream_name: stream_name, expected_version: last_stored)
-        ProcessState.new.tap do |state|
-          past.each { |ev| state.call(ev) }
-          state.call(event)
-        end
-      end
-    end
-
-    class ProcessState
-      attr_reader :order_id, :product_id, :quantity, :vat_rate, :discounted_amount
-
-      def call(event)
-        @order_id ||= event.data.fetch(:order_id)
-        @product_id ||= event.data.fetch(:product_id)
-        case event
-        when Pricing::PriceItemValueCalculated
-          @quantity = event.data.fetch(:quantity)
-          @discounted_amount = event.data.fetch(:discounted_amount)
-        when Taxes::VatRateDetermined
-          @vat_rate = event.data.fetch(:vat_rate).symbolize_keys
-        end
-      end
-
-      def create_invoice_item?
-        [order_id, product_id, quantity, vat_rate, discounted_amount].all?
-      end
+    def fetch_id(event)
+      "#{event.data.fetch(:order_id)}$#{event.data.fetch(:product_id)}"
     end
   end
 
   class MoneySplitter
-    def initialize(amount, weights)
-      raise ArgumentError unless weights.instance_of? Array
-      raise ArgumentError if weights.empty?
+    def initialize(amount, quantity)
       @amount = amount
-      @weights = weights
+      @weights = Array.new(quantity, 1)
     end
 
     def call

ecommerce/processes/test/money_splitter_test.rb

@@ -5,14 +5,9 @@ class MoneySplitterTest < Minitest::Test
     cover "Processes::MoneySplitter"
 
     def test_splitting_money_without_losing_cents
-      assert_equal([0.01, 0.01, 0.01], MoneySplitter.new(0.03, [1, 1, 1]).call)
-      assert_equal([0.01, 0.02], MoneySplitter.new(0.03, [1, 1]).call.sort)
-      assert_equal([0, 0, 0.01, 0.01, 0.01], MoneySplitter.new(0.03, [1, 1, 1, 1, 1]).call.sort)
-      assert_equal([0, 0, 0.03], MoneySplitter.new(0.03, [1, 0, 0]).call.sort)
-
-      assert_raises(ArgumentError) { MoneySplitter.new(0.03, nil).call }
-      assert_raises(ArgumentError) { MoneySplitter.new(0.03, 'not nil nor array').call }
-      assert_raises(ArgumentError) { MoneySplitter.new(0.03, []).call }
+      assert_equal([0.01, 0.01, 0.01], MoneySplitter.new(0.03, 3).call)
+      assert_equal([0.01, 0.02], MoneySplitter.new(0.03, 2).call.sort)
+      assert_equal([0, 0, 0.01, 0.01, 0.01], MoneySplitter.new(0.03, 5).call.sort)
     end
   end
 end

ecommerce/processes/test/order_item_invoicing_process_test.rb

@@ -4,40 +4,86 @@ module Processes
   class OrderItemInvoicingProcessTest < Test
     cover "Processes::OrderItemInvoicingProcess*"
 
-    def test_invoice_item_being_created
-      product_id = SecureRandom.uuid
-      amount = 100.to_d
-      discounted_amount = 90.to_d
-      quantity = 5
-      vat_rate = Infra::Types::VatRate.new(rate: 20, code: "20")
+    def setup
+      super
+      @product_id = SecureRandom.uuid
+      @amount = 100.to_d
+      @discounted_amount = 90.to_d
+      @quantity = 5
+      @vat_rate = Infra::Types::VatRate.new(rate: 20, code: "20")
+    end
 
-      item_value_calculated = Pricing::PriceItemValueCalculated.new(
-        data: {
-          order_id: order_id,
-          product_id: product_id,
-          quantity: quantity,
-          amount: amount,
-          discounted_amount: discounted_amount
-        }
-      )
-      vat_rate_determined = Taxes::VatRateDetermined.new(
-        data: {
-          order_id: order_id,
-          product_id: product_id,
-          vat_rate: vat_rate
-        }
-      )
+    def test_invoice_item_being_created
       process = OrderItemInvoicingProcess.new(event_store, command_bus)
-      given([item_value_calculated, vat_rate_determined]).each do |event|
+      given([
+              Pricing::PriceItemValueCalculated.new(
+                data: {
+                  order_id: order_id,
+                  product_id: @product_id,
+                  quantity: @quantity,
+                  amount: @amount,
+                  discounted_amount: @discounted_amount
+                }
+              ),
+              Taxes::VatRateDetermined.new(
+                data: {
+                  order_id: order_id,
+                  product_id: @product_id,
+                  vat_rate: @vat_rate
+                }
+              )]).each do |event|
         process.call(event)
       end
       assert_command(Invoicing::AddInvoiceItem.new(
         invoice_id: order_id,
-        product_id: product_id,
-        quantity: quantity,
-        vat_rate: vat_rate,
+        product_id: @product_id,
+        quantity: @quantity,
+        vat_rate: @vat_rate,
         unit_price: 18.to_d
       ))
     end
   end
+
+  def test_vat_rate_comes_first
+    process = OrderItemInvoicingProcess.new(event_store, command_bus)
+    given([
+            Taxes::VatRateDetermined.new(
+              data: {
+                order_id: order_id,
+                product_id: @product_id,
+                vat_rate: @vat_rate
+              }
+            ),
+            Pricing::PriceItemValueCalculated.new(
+              data: {
+                order_id: order_id,
+                product_id: @product_id,
+                quantity: @quantity,
+                amount: @amount,
+                discounted_amount: @discounted_amount
+              }
+            )
+          ]).each do |event|
+      process.call(event)
+    end
+    assert_command(Invoicing::AddInvoiceItem.new(
+      invoice_id: order_id,
+      product_id: @product_id,
+      quantity: @quantity,
+      vat_rate: @vat_rate,
+      unit_price: 18.to_d
+    ))
+  end
+
+  def test_stream_name
+    process = OrderItemInvoicingProcess.new(event_store, command_bus)
+    given([Pricing::PriceItemValueCalculated.new(data: { order_id: order_id,
+                                                         product_id: product_id,
+                                                         quantity: quantity,
+                                                         amount: amount,
+                                                         discounted_amount: discounted_amount })]).each do |event|
+      process.call(event)
+    end
+    assert_equal "Processes::OrderItemInvoicingProcess$#{order_id}", process.send(:stream_name)
+  end
 end