From 4968e0a6c095f535a90a91a887e6c42b2dba7357 Mon Sep 17 00:00:00 2001
From: Elia Schito <elia@schito.me>
Date: Tue, 19 Nov 2019 01:22:43 +0100
Subject: [PATCH] Improve the extendability of Rules::ItemTotal

- Make operators extendable without reopening the method
- Allow to overwrite/extend the total
- Allow to overwrite/extend the threshold
- Simplify #eligible? making it more readable
- Migrate the OPERATORS constant to an extendible method
- Fix the default preferred operator
---
 .../promotions/rules/_item_total.html.erb     |  2 +-
 .../spree/promotion/rules/item_total.rb       | 46 +++++++++++++++++--
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/backend/app/views/spree/admin/promotions/rules/_item_total.html.erb b/backend/app/views/spree/admin/promotions/rules/_item_total.html.erb
index d0060ac4418..1b0767b2ea9 100644
--- a/backend/app/views/spree/admin/promotions/rules/_item_total.html.erb
+++ b/backend/app/views/spree/admin/promotions/rules/_item_total.html.erb
@@ -1,7 +1,7 @@
 <div class="row">
   <div class="col-6">
     <div class="field">
-      <%= select_tag "#{param_prefix}[preferred_operator]", options_for_select(Spree::Promotion::Rules::ItemTotal::OPERATORS.map{|o| [t(o, scope: 'spree.item_total_rule.operators'),o]}, promotion_rule.preferred_operator), {class: 'custom-select select_item_total fullwidth'} %>
+      <%= select_tag "#{param_prefix}[preferred_operator]", options_for_select(promotion_rule.class.operator_options, promotion_rule.preferred_operator), {class: 'custom-select select_item_total fullwidth'} %>
     </div>
   </div>
   <div class="col-6">
diff --git a/core/app/models/spree/promotion/rules/item_total.rb b/core/app/models/spree/promotion/rules/item_total.rb
index 6e5dc6d693e..522291ad912 100644
--- a/core/app/models/spree/promotion/rules/item_total.rb
+++ b/core/app/models/spree/promotion/rules/item_total.rb
@@ -6,11 +6,34 @@ module Rules
       # A rule to apply to an order greater than (or greater than or equal to)
       # a specific amount
       class ItemTotal < PromotionRule
+        include ActiveSupport::Deprecation::DeprecatedConstantAccessor
+
         preference :amount, :decimal, default: 100.00
         preference :currency, :string, default: ->{ Spree::Config[:currency] }
         preference :operator, :string, default: 'gt'
 
-        OPERATORS = ['gt', 'gte']
+        # The list of allowed operators names mapped to their symbols.
+        def self.operators_map
+          {
+            gte: :>=,
+            gt: :>,
+          }
+        end
+
+        def self.operator_options
+          operators_map.map do |name, _method|
+            [I18n.t(name, scope: 'spree.item_total_rule.operators'), name]
+          end
+        end
+
+        # @deprecated
+        OPERATORS = operators_map.keys.map(&:to_s)
+        deprecate_constant(
+          :OPERATORS,
+          :operators_map,
+          message: "OPERATORS is deprecated! Use `operators_map.keys.map(&:to_s)` instead.",
+          deprecator: Spree::Deprecation,
+        )
 
         def applicable?(promotable)
           promotable.is_a?(Spree::Order)
@@ -18,8 +41,8 @@ def applicable?(promotable)
 
         def eligible?(order, _options = {})
           return false unless order.currency == preferred_currency
-          item_total = order.item_total
-          unless item_total.send(preferred_operator == 'gte' ? :>= : :>, BigDecimal(preferred_amount.to_s))
+
+          unless total_for_order(order).send(operator, threshold)
             eligibility_errors.add(:base, ineligible_message, error_code: ineligible_error_code)
           end
 
@@ -28,12 +51,27 @@ def eligible?(order, _options = {})
 
         private
 
+        def operator
+          self.class.operators_map.fetch(
+            preferred_operator.to_sym,
+            preferred_operator_default,
+          )
+        end
+
+        def total_for_order(order)
+          order.item_total
+        end
+
+        def threshold
+          BigDecimal(preferred_amount.to_s)
+        end
+
         def formatted_amount
           Spree::Money.new(preferred_amount, currency: preferred_currency).to_s
         end
 
         def ineligible_message
-          if preferred_operator == 'gte'
+          if preferred_operator.to_s == 'gte'
             eligibility_error_message(:item_total_less_than, amount: formatted_amount)
           else
             eligibility_error_message(:item_total_less_than_or_equal, amount: formatted_amount)