Refactor VoucherAdjustmentsService

It's more inline with existing coding style

app/controllers/spree/orders_controller.rb

@@ -70,7 +70,7 @@ module Spree
         @order.recreate_all_fees! # Enterprise fees on line items and on the order itself
 
         # Re apply the voucher
-        VoucherAdjustmentsService.calculate(@order)
+        VoucherAdjustmentsService.new(@order).calculate
         @order.update_totals_and_states
 
         if @order.complete?

app/models/spree/order/checkout.rb

@@ -83,7 +83,7 @@ module Spree
               end
 
               after_transition to: :confirmation do |order|
-                VoucherAdjustmentsService.calculate(order)
+                VoucherAdjustmentsService.new(order).calculate
                 order.update_totals_and_states
               end
 

app/models/voucher.rb

@@ -22,7 +22,7 @@ class Voucher < ApplicationRecord
   # the same method to stay as consistent as possible.
   #
   # Creates a new voucher adjustment for the given order with an amount of 0
-  # The amount will be calculated via VoucherAdjustmentsService.calculate
+  # The amount will be calculated via VoucherAdjustmentsService#calculate
   def create_adjustment(label, order)
     adjustment_attributes = {
       amount: 0,

app/services/voucher_adjustments_service.rb

@@ -1,43 +1,49 @@
 # frozen_string_literal: true
 
 class VoucherAdjustmentsService
-  def self.calculate(order)
-    return if order.nil?
+  def initialize(order)
+    @order = order
+  end
+
+  def calculate
+    return if @order.nil?
 
     # Find open Voucher Adjustment
-    return if order.voucher_adjustments.empty?
+    return if @order.voucher_adjustments.empty?
 
     # We only support one voucher per order right now, we could just loop on voucher_adjustments
-    adjustment = order.voucher_adjustments.first
+    adjustment = @order.voucher_adjustments.first
 
     # Calculate value
-    amount = adjustment.originator.compute_amount(order)
+    amount = adjustment.originator.compute_amount(@order)
 
     # It is quite possible to have an order with both tax included in and tax excluded from price.
     # We should be able to caculate the relevant amount apply the current calculation.
     #
     # For now we just assume it is either all tax included in price or all tax excluded from price.
-    if order.additional_tax_total.positive?
-      handle_tax_excluded_from_price(order, amount)
-    elsif order.included_tax_total.positive?
-      handle_tax_included_in_price(order, amount)
+    if @order.additional_tax_total.positive?
+      handle_tax_excluded_from_price(amount)
+    elsif @order.included_tax_total.positive?
+      handle_tax_included_in_price(amount)
     else
       adjustment.amount = amount
       adjustment.save
     end
   end
 
-  def self.handle_tax_excluded_from_price(order, amount)
-    voucher_rate = amount / order.pre_discount_total
+  private
 
-    adjustment = order.voucher_adjustments.first
+  def handle_tax_excluded_from_price(amount)
+    voucher_rate = amount / @order.pre_discount_total
+
+    adjustment = @order.voucher_adjustments.first
 
     # Adding the voucher tax part
-    tax_amount = voucher_rate * order.additional_tax_total
+    tax_amount = voucher_rate * @order.additional_tax_total
 
     adjustment_attributes = {
       originator: adjustment.originator,
-      order: order,
+      order: @order,
       label: "Tax #{adjustment.label}",
       mandatory: false,
       state: 'open',
@@ -46,12 +52,12 @@ class VoucherAdjustmentsService
     }
 
     # Update the amount if tax adjustment already exist, create if not
-    tax_adjustment = order.adjustments.find_or_initialize_by(adjustment_attributes)
+    tax_adjustment = @order.adjustments.find_or_initialize_by(adjustment_attributes)
     tax_adjustment.amount = tax_amount
     tax_adjustment.save
 
     # Update the adjustment amount
-    adjustment_amount = voucher_rate * (order.pre_discount_total - order.additional_tax_total)
+    adjustment_amount = voucher_rate * (@order.pre_discount_total - @order.additional_tax_total)
 
     adjustment.update_columns(
       amount: adjustment_amount,
@@ -59,12 +65,12 @@ class VoucherAdjustmentsService
     )
   end
 
-  def self.handle_tax_included_in_price(order, amount)
-    voucher_rate = amount / order.pre_discount_total
-    included_tax = voucher_rate * order.included_tax_total
+  def handle_tax_included_in_price(amount)
+    voucher_rate = amount / @order.pre_discount_total
+    included_tax = voucher_rate * @order.included_tax_total
 
     # Update Adjustment
-    adjustment = order.voucher_adjustments.first
+    adjustment = @order.voucher_adjustments.first
 
     return unless amount != adjustment.amount || included_tax != 0
 
@@ -74,6 +80,4 @@ class VoucherAdjustmentsService
       updated_at: Time.zone.now
     )
   end
-
-  private_class_method :handle_tax_included_in_price, :handle_tax_excluded_from_price
 end

spec/services/voucher_adjustments_service_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe VoucherAdjustmentsService do
-  describe '.calculate' do
+  describe '#calculate' do
     let(:enterprise) { build(:enterprise) }
     let(:voucher) { create(:voucher, code: 'new_code', enterprise: enterprise, amount: 10) }
 
@@ -16,7 +16,7 @@ describe VoucherAdjustmentsService do
         voucher.create_adjustment(voucher.code, order)
         order.update_columns(item_total: 6)
 
-        VoucherAdjustmentsService.calculate(order)
+        VoucherAdjustmentsService.new(order).calculate
 
         expect(subject.amount.to_f).to eq(-6.0)
       end
@@ -46,7 +46,7 @@ describe VoucherAdjustmentsService do
         order.update_shipping_fees!
         order.update_order!
 
-        VoucherAdjustmentsService.calculate(order)
+        VoucherAdjustmentsService.new(order).calculate
       end
 
       it 'updates the adjustment included_tax' do
@@ -60,13 +60,13 @@ describe VoucherAdjustmentsService do
       context "when re calculating" do
         it "does not update the adjustment amount" do
           expect do
-            VoucherAdjustmentsService.calculate(order)
+            VoucherAdjustmentsService.new(order).calculate
           end.not_to change { subject.reload.amount }
         end
 
         it "does not update the tax adjustment" do
           expect do
-            VoucherAdjustmentsService.calculate(order)
+            VoucherAdjustmentsService.new(order).calculate()
           end.not_to change { subject.reload.included_tax }
         end
 
@@ -77,13 +77,13 @@ describe VoucherAdjustmentsService do
 
           it "does update the adjustment amount" do
             expect do
-              VoucherAdjustmentsService.calculate(order)
+              VoucherAdjustmentsService.new(order).calculate
             end.not_to change { subject.reload.amount }
           end
 
           it "updates the tax adjustment" do
             expect do
-              VoucherAdjustmentsService.calculate(order)
+              VoucherAdjustmentsService.new(order).calculate
             end.to change { subject.reload.included_tax }
           end
         end
@@ -114,7 +114,7 @@ describe VoucherAdjustmentsService do
         order.update_shipping_fees!
         order.update_order!
 
-        VoucherAdjustmentsService.calculate(order)
+        VoucherAdjustmentsService.new(order).calculate
       end
 
       it 'includes amount without tax' do
@@ -137,13 +137,13 @@ describe VoucherAdjustmentsService do
       context "when re calculating" do
         it "does not update the adjustment amount" do
           expect do
-            VoucherAdjustmentsService.calculate(order)
+            VoucherAdjustmentsService.new(order).calculate
           end.not_to change { adjustment.reload.amount }
         end
 
         it "does not update the tax adjustment" do
           expect do
-            VoucherAdjustmentsService.calculate(order)
+            VoucherAdjustmentsService.new(order).calculate
           end.not_to change { tax_adjustment.reload.amount }
         end
 
@@ -154,13 +154,13 @@ describe VoucherAdjustmentsService do
 
           it "updates the adjustment amount" do
             expect do
-              VoucherAdjustmentsService.calculate(order)
+              VoucherAdjustmentsService.new(order).calculate
             end.to change { adjustment.reload.amount }
           end
 
           it "updates the tax adjustment" do
             expect do
-              VoucherAdjustmentsService.calculate(order)
+              VoucherAdjustmentsService.new(order).calculate
             end.to change { tax_adjustment.reload.amount }
           end
         end
@@ -169,7 +169,7 @@ describe VoucherAdjustmentsService do
 
     context 'when no order given' do
       it "doesn't blow up" do
-        expect { VoucherAdjustmentsService.calculate(nil) }.to_not raise_error
+        expect { VoucherAdjustmentsService.new(nil).calculate }.to_not raise_error
       end
     end
 
@@ -177,7 +177,7 @@ describe VoucherAdjustmentsService do
       let(:order) { create(:order_with_line_items, line_items_count: 1, distributor: enterprise) }
 
       it "doesn't blow up" do
-        expect { VoucherAdjustmentsService.calculate(order) }.to_not raise_error
+        expect { VoucherAdjustmentsService.new(order).calculate }.to_not raise_error
       end
     end
   end

spec/system/consumer/split_checkout_spec.rb

@@ -1119,7 +1119,7 @@ describe "As a consumer, I want to checkout my order" do
         before do
           voucher.create_adjustment(voucher.code, order)
           order.update_totals
-          VoucherAdjustmentsService.calculate(order)
+          VoucherAdjustmentsService.new(order).calculate
 
           visit checkout_step_path(:summary)
         end