Fixed CodeClimate warnings on services/cart_services.rb

app/controllers/cart_controller.rb

@@ -62,18 +62,21 @@ class CartController < BaseController
   def populate_variant_attributes
     order = current_order.reload
 
-    if params.key? :variant_attributes
-      params[:variant_attributes].each do |variant_id, attributes|
-        order.set_variant_attributes(Spree::Variant.find(variant_id), attributes)
-      end
-    end
+    populate_variant_attributes_from_variant(order) if params.key? :variant_attributes
+    populate_variant_attributes_from_product(order) if params.key? :quantity
+  end
 
-    if params.key? :quantity
-      params[:products].each do |_product_id, variant_id|
-        max_quantity = params[:max_quantity].to_i
-        order.set_variant_attributes(Spree::Variant.find(variant_id),
-                                     max_quantity: max_quantity)
-      end
+  def populate_variant_attributes_from_variant(order)
+    params[:variant_attributes].each do |variant_id, attributes|
+      order.set_variant_attributes(Spree::Variant.find(variant_id), attributes)
+    end
+  end
+
+  def populate_variant_attributes_from_product(order)
+    params[:products].each do |_product_id, variant_id|
+      max_quantity = params[:max_quantity].to_i
+      order.set_variant_attributes(Spree::Variant.find(variant_id),
+                                   max_quantity: max_quantity)
     end
   end
 

app/services/cart_service.rb

@@ -17,27 +17,23 @@ class CartService
     # this validation probably can't fail
     if !distribution_can_supply_products_in_cart(@distributor, @order_cycle)
       errors.add(:base, I18n.t(:spree_order_populator_error))
+      return false
     end
 
-    if valid?
-      @order.with_lock do
-        variants = read_variants from_hash
+    @order.with_lock do
+      variants = read_variants from_hash
+      attempt_cart_add_variants variants
+      overwrite_variants variants unless !overwrite
+    end
+    valid?
+  end
 
-        variants.each do |v|
-          if varies_from_cart(v)
-            attempt_cart_add(v[:variant_id], v[:quantity], v[:max_quantity])
-          end
-        end
-
-        if overwrite
-          variants_removed(variants).each do |id|
-            cart_remove(id)
-          end
-        end
+  def attempt_cart_add_variants(variants)
+    variants.each do |v|
+      if varies_from_cart(v)
+        attempt_cart_add(v[:variant_id], v[:quantity], v[:max_quantity])
       end
     end
-
-    valid?
   end
 
   def attempt_cart_add(variant_id, quantity, max_quantity = nil)
@@ -45,17 +41,18 @@ class CartService
     max_quantity = max_quantity.to_i if max_quantity
     variant = Spree::Variant.find(variant_id)
     OpenFoodNetwork::ScopeVariantToHub.new(@distributor).scope(variant)
-    if quantity > 0 &&
-       check_order_cycle_provided_for(variant) &&
-       check_variant_available_under_distribution(variant)
 
-      quantity_to_add, max_quantity_to_add = quantities_to_add(variant, quantity, max_quantity)
+    return unless quantity > 0 && valid_variant?(variant)
 
-      if quantity_to_add > 0
-        @order.add_variant(variant, quantity_to_add, max_quantity_to_add, currency)
-      else
-        @order.remove_variant variant
-      end
+    cart_add(variant, quantity, max_quantity)
+  end
+
+  def cart_add(variant, quantity, max_quantity)
+    quantity_to_add, max_quantity_to_add = quantities_to_add(variant, quantity, max_quantity)
+    if quantity_to_add > 0
+      @order.add_variant(variant, quantity_to_add, max_quantity_to_add, currency)
+    else
+      @order.remove_variant variant
     end
   end
 
@@ -69,6 +66,12 @@ class CartService
     [quantity_to_add, max_quantity_to_add]
   end
 
+  def overwrite_variants(variants)
+    variants_removed(variants).each do |id|
+      cart_remove(id)
+    end
+  end
+
   private
 
   def read_variants(data)
@@ -77,17 +80,17 @@ class CartService
   end
 
   def read_products_hash(data)
-    (data[:products] || []).map do |product_id, variant_id|
-      {variant_id: variant_id, quantity: data[:quantity]}
+    (data[:products] || []).map do |_product_id, variant_id|
+      { variant_id: variant_id, quantity: data[:quantity] }
     end
   end
 
   def read_variants_hash(data)
     (data[:variants] || []).map do |variant_id, quantity|
       if quantity.is_a?(Hash)
-        {variant_id: variant_id, quantity: quantity[:quantity], max_quantity: quantity[:max_quantity]}
+        { variant_id: variant_id, quantity: quantity[:quantity], max_quantity: quantity[:max_quantity] }
       else
-        {variant_id: variant_id, quantity: quantity}
+        { variant_id: variant_id, quantity: quantity }
       end
     end
   end
@@ -125,6 +128,10 @@ class CartService
     (variant_ids_in_cart - variant_ids_given).uniq
   end
 
+  def valid_variant?(variant)
+    check_order_cycle_provided_for(variant) && check_variant_available_under_distribution(variant)
+  end
+
   def check_order_cycle_provided_for(variant)
     order_cycle_provided = (!order_cycle_required_for(variant) || @order_cycle.present?)
     errors.add(:base, "Please choose an order cycle for this order.") unless order_cycle_provided
@@ -132,12 +139,10 @@ class CartService
   end
 
   def check_variant_available_under_distribution(variant)
-    if DistributionChangeValidator.new(@order).variants_available_for_distribution(@distributor, @order_cycle).include? variant
-      return true
-    else
-      errors.add(:base, I18n.t(:spree_order_populator_availability_error))
-      return false
-    end
+    return true if DistributionChangeValidator.new(@order).variants_available_for_distribution(@distributor, @order_cycle).include? variant
+
+    errors.add(:base, I18n.t(:spree_order_populator_availability_error))
+    false
   end
 
   def order_cycle_required_for(variant)