Merge pull request #5888 from andrewpbrett/imperial-auto-units

Allow US units on products/variants

app/assets/javascripts/admin/products/services/option_value_namer.js.coffee

@@ -53,17 +53,18 @@ angular.module("admin.products").factory "OptionValueNamer", (VariantUnitManager
       [value, unit_name]
 
     scale_for_unit_value: ->
-      # Find the largest available unit where unit_value comes to >= 1 when expressed in it.
-      # If there is none available where this is true, use the smallest available unit.
-      unit = ([scale, unit_name] for scale, unit_name of VariantUnitManager.unitNames[@variant.product.variant_unit] when @variant.unit_value / scale >= 1).reduce (unit, [scale, unit_name]) ->
-        if (unit && scale > unit[0]) || !unit?
-          [scale, unit_name]
-        else
-          unit
-      , null
-      if !unit?
-        unit = ([scale, unit_name] for scale, unit_name of VariantUnitManager.unitNames[@variant.product.variant_unit]).reduce (unit, [scale, unit_name]) ->
-          if scale < unit[0] then [scale, unit_name] else unit
-        , [Infinity,""]
+      # Find the largest available and compatible unit where unit_value comes
+      # to >= 1 when expressed in it.
+      # If there is none available where this is true, use the smallest
+      # available unit.
+      product = @variant.product
+      scales = VariantUnitManager.compatibleUnitScales(product.variant_unit_scale, product.variant_unit)
+      variantUnitValue = @variant.unit_value
 
-      unit
+      # sets largestScale = last element in filtered scales array
+      [_, ..., largestScale] = (scales.filter (s) -> variantUnitValue / s >= 1)
+
+      if (largestScale)
+        [largestScale, VariantUnitManager.getUnitName(largestScale, product.variant_unit)]
+      else
+        [scales[0], VariantUnitManager.getUnitName(scales[0], product.variant_unit)]

app/assets/javascripts/admin/products/services/variant_unit_manager.js.coffee

@@ -1,17 +1,35 @@
 angular.module("admin.products").factory "VariantUnitManager", ->
   class VariantUnitManager
-    @unitNames:
+    @units:
       'weight':
-        1.0: 'g'
-        1000.0: 'kg'
-        1000000.0: 'T'
+        1.0:
+          name: 'g'
+          system: 'metric'
+        1000.0:
+          name: 'kg'
+          system: 'metric'
+        1000000.0:
+          name: 'T'
+          system: 'metric'
+        453.6:
+          name: 'lb'
+          system: 'imperial'
+        28.35:
+          name: 'oz'
+          system: 'imperial'
       'volume':
-        0.001: 'mL'
-        1.0: 'L'
-        1000.0: 'kL'
+        0.001:
+          name: 'mL'
+          system: 'metric'
+        1.0:
+          name: 'L'
+          system: 'metric'
+        1000.0:
+          name: 'kL'
+          system: 'metric'
 
     @variantUnitOptions: ->
-      options = for unit_type, scale_with_name of @unitNames
+      options = for unit_type, _ of @units
         for scale in @unitScales(unit_type)
           name = @getUnitName(scale, unit_type)
           ["#{I18n.t(unit_type)} (#{name})", "#{unit_type}_#{scale}"]
@@ -30,7 +48,16 @@ angular.module("admin.products").factory "VariantUnitManager", ->
         unitScales[0]
 
     @getUnitName: (scale, unitType) ->
-      @unitNames[unitType][scale]
+      if @units[unitType][scale]
+        @units[unitType][scale]['name']
+      else
+        ''
 
     @unitScales: (unitType) ->
-      (parseFloat(scale) for scale in Object.keys(@unitNames[unitType])).sort()
+      (parseFloat(scale) for scale in Object.keys(@units[unitType])).sort (a, b) ->
+         a - b
+
+    @compatibleUnitScales: (scale, unitType) ->
+      scaleSystem = @units[unitType][scale]['system']
+      (parseFloat(scale) for scale, scaleInfo of @units[unitType] when scaleInfo['system'] == scaleSystem).sort (a, b) ->
+         a - b

app/models/product_import/unit_converter.rb

@@ -32,6 +32,8 @@ module ProductImport
       {
         'g' => { scale: 1, unit: 'weight' },
         'kg' => { scale: 1000, unit: 'weight' },
+        'oz' => { scale: 28.35, unit: 'weight' },
+        'lb' => { scale: 453.6, unit: 'weight' },
         't' => { scale: 1_000_000, unit: 'weight' },
         'ml' => { scale: 0.001, unit: 'volume' },
         'l' => { scale: 1, unit: 'volume' },

app/models/spree/line_item_decorator.rb

@@ -1,8 +1,8 @@
 require 'open_food_network/scope_variant_to_hub'
-require 'open_food_network/variant_and_line_item_naming'
+require 'variant_units/variant_and_line_item_naming'
 
 Spree::LineItem.class_eval do
-  include OpenFoodNetwork::VariantAndLineItemNaming
+  include VariantUnits::VariantAndLineItemNaming
   include LineItemBasedAdjustmentHandling
   has_and_belongs_to_many :option_values, join_table: 'spree_option_values_line_items', class_name: 'Spree::OptionValue'
 

app/models/spree/variant_decorator.rb

@@ -1,5 +1,5 @@
 require 'open_food_network/enterprise_fee_calculator'
-require 'open_food_network/variant_and_line_item_naming'
+require 'variant_units/variant_and_line_item_naming'
 require 'concerns/variant_stock'
 
 Spree::Variant.class_eval do
@@ -8,7 +8,7 @@ Spree::Variant.class_eval do
   # This file may be double-loaded in delayed job environment, so we check before
   # removing the Spree method to prevent error.
   remove_method :options_text if instance_methods(false).include? :options_text
-  include OpenFoodNetwork::VariantAndLineItemNaming
+  include VariantUnits::VariantAndLineItemNaming
   include VariantStock
 
   has_many :exchange_variants

app/services/variant_units/option_value_namer.rb

@@ -2,7 +2,7 @@
 
 require "open_food_network/i18n_inflections"
 
-module OpenFoodNetwork
+module VariantUnits
   class OptionValueNamer
     def initialize(variant = nil)
       @variant = variant
@@ -39,7 +39,6 @@ module OpenFoodNetwork
       if @variant.unit_value.present?
         if %w(weight volume).include? @variant.product.variant_unit
           value, unit_name = option_value_value_unit_scaled
-
         else
           value = @variant.unit_value
           unit_name = pluralize(@variant.product.variant_unit_name, value)
@@ -63,21 +62,44 @@ module OpenFoodNetwork
     end
 
     def scale_for_unit_value
-      units = { 'weight' => { 1.0 => 'g', 1000.0 => 'kg', 1_000_000.0 => 'T' },
-                'volume' => { 0.001 => 'mL', 1.0 => 'L',  1000.0 => 'kL' } }
+      units = {
+        'weight' => {
+          1.0 => { 'name' => 'g', 'system' => 'metric' },
+          28.35 => { 'name' => 'oz', 'system' => 'imperial' },
+          453.6 => { 'name' => 'lb', 'system' => 'imperial' },
+          1000.0 => { 'name' => 'kg', 'system' => 'metric' },
+          1_000_000.0 => { 'name' => 'T', 'system' => 'metric' }
+        },
+        'volume' => {
+          0.001 => { 'name' => 'mL', 'system' => 'metric' },
+          1.0 => { 'name' => 'L', 'system' => 'metric' },
+          1000.0 => { 'name' => 'kL', 'system' => 'metric' }
+        }
+      }
 
-      # Find the largest available unit where unit_value comes to >= 1 when expressed in it.
-      # If there is none available where this is true, use the smallest available unit.
-      unit = units[@variant.product.variant_unit].select { |scale, _unit_name|
-        @variant.unit_value / scale >= 1
-      }.to_a.last
-      unit = units[@variant.product.variant_unit].first if unit.nil?
+      scales = units[@variant.product.variant_unit]
+      product_scale = @variant.product.variant_unit_scale
+      product_scale_system = scales[product_scale.to_f]['system']
 
-      unit
+      largest_unit = find_largest_unit(scales, product_scale_system)
+      [largest_unit[0], largest_unit[1]["name"]]
+    end
+
+    # Find the largest available and compatible unit where unit_value comes
+    #   to >= 1 when expressed in it.
+    # If there is none available where this is true, use the smallest available unit.
+    def find_largest_unit(scales, product_scale_system)
+      largest_unit = scales.select { |scale, unit_info|
+        unit_info['system'] == product_scale_system &&
+          @variant.unit_value / scale >= 1
+      }.max
+      return scales.first if largest_unit.nil?
+
+      largest_unit
     end
 
     def pluralize(unit_name, count)
-      I18nInflections.pluralize(unit_name, count)
+      OpenFoodNetwork::I18nInflections.pluralize(unit_name, count)
     end
   end
 end

app/services/variant_units/variant_and_line_item_naming.rb

@@ -2,9 +2,9 @@
 # It contains all of our logic for creating and naming option values (which are associated
 # with both models) and methods for printing human readable "names" for instances of these models.
 
-require 'open_food_network/option_value_namer'
+require 'variant_units/option_value_namer'
 
-module OpenFoodNetwork
+module VariantUnits
   module VariantAndLineItemNaming
     # Copied and modified from Spree::Variant
     def options_text
@@ -18,7 +18,24 @@ module OpenFoodNetwork
                    order("#{Spree::OptionType.table_name}.position asc")
                end
 
-      values.map(&:presentation).to_sentence(words_connector: ", ", two_words_connector: ", ")
+      values.map { |option_value|
+        presentation(option_value)
+      }.to_sentence(words_connector: ", ", two_words_connector: ", ")
+    end
+
+    def presentation(option_value)
+      return option_value.presentation unless option_value.option_type.name == "unit_weight"
+
+      return display_as if has_attribute?(:display_as) && display_as.present?
+
+      return variant.display_as if variant_display_as?
+
+      option_value.presentation
+    end
+
+    def variant_display_as?
+      respond_to?(:variant) && variant.present? &&
+        variant.respond_to?(:display_as) && variant.display_as.present?
     end
 
     def product_and_full_name
@@ -48,9 +65,11 @@ module OpenFoodNetwork
     end
 
     def unit_to_display
-      return options_text if !has_attribute?(:display_as) || display_as.blank?
+      return display_as if has_attribute?(:display_as) && display_as.present?
 
-      display_as
+      return variant.display_as if variant_display_as?
+
+      options_text
     end
 
     def update_units
@@ -84,7 +103,7 @@ module OpenFoodNetwork
       if has_attribute?(:display_as) && display_as.present?
         display_as
       else
-        option_value_namer = OpenFoodNetwork::OptionValueNamer.new self
+        option_value_namer = VariantUnits::OptionValueNamer.new self
         option_value_namer.name
       end
     end

config/locales/en.yml

@@ -656,7 +656,7 @@ en:
         order_date: "Completed at"
         max: "Max"
         product_unit: "Product: Unit"
-        weight_volume: "Weight/Volume"
+        weight_volume: "Weight/Volume (g)"
         ask: "Ask?"
         page_title: "Bulk Order Management"
         actions_delete: "Delete Selected"

engines/order_management/app/services/order_management/reports/bulk_coop/bulk_coop_report.rb

@@ -194,11 +194,11 @@ module OrderManagement
         end
 
         def option_value_value(line_items)
-          OpenFoodNetwork::OptionValueNamer.new(line_items.first).value
+          VariantUnits::OptionValueNamer.new(line_items.first).value
         end
 
         def option_value_unit(line_items)
-          OpenFoodNetwork::OptionValueNamer.new(line_items.first).unit
+          VariantUnits::OptionValueNamer.new(line_items.first).unit
         end
 
         def order_billing_address_name(line_items)

lib/open_food_network/lettuce_share_report.rb

@@ -1,4 +1,5 @@
 require 'open_food_network/products_and_inventory_report_base'
+require 'variant_units/option_value_namer'
 
 module OpenFoodNetwork
   class LettuceShareReport < ProductsAndInventoryReportBase
@@ -27,8 +28,8 @@ module OpenFoodNetwork
           variant.product.name,
           variant.full_name,
           '',
-          OptionValueNamer.new(variant).value,
-          OptionValueNamer.new(variant).unit,
+          VariantUnits::OptionValueNamer.new(variant).value,
+          VariantUnits::OptionValueNamer.new(variant).unit,
           variant.price,
           '',
           gst(variant),

spec/javascripts/unit/admin/services/variant_unit_manager_spec.js.coffee

@@ -28,16 +28,23 @@ describe "VariantUnitManager", ->
       expect(VariantUnitManager.getUnitName(1000, "volume")).toEqual "kL"
 
   describe "unitScales", ->
-    it "returns a set of scales for unit type weight", ->
-      expect(VariantUnitManager.unitScales('weight')).toEqual [1.0, 1000.0, 1000000.0]
+    it "returns a sorted set of scales for unit type weight", ->
+      expect(VariantUnitManager.unitScales('weight')).toEqual [1.0, 28.35, 453.6, 1000.0, 1000000.0]
 
-    it "returns a set of scales for unit type volume", ->
+    it "returns a sorted set of scales for unit type volume", ->
       expect(VariantUnitManager.unitScales('volume')).toEqual [0.001, 1.0, 1000.0]
 
+  describe "compatibleUnitScales", ->
+    it "returns a sorted set of compatible scales based on the scale and unit type provided", ->
+      expect(VariantUnitManager.compatibleUnitScales(1, "weight")).toEqual [1.0, 1000.0, 1000000.0]
+      expect(VariantUnitManager.compatibleUnitScales(453.6, "weight")).toEqual [28.35, 453.6]
+
   describe "variantUnitOptions", ->
     it "returns an array of options", ->
       expect(VariantUnitManager.variantUnitOptions()).toEqual [
         ["Weight (g)", "weight_1"],
+        ["Weight (oz)", "weight_28.35"],
+        ["Weight (lb)", "weight_453.6"],
         ["Weight (kg)", "weight_1000"],
         ["Weight (T)", "weight_1000000"],
         ["Volume (mL)", "volume_0.001"],

spec/models/calculator/weight_spec.rb

@@ -103,6 +103,28 @@ describe Calculator::Weight do
           expect(calculator.compute(line_item)).to eq(42_000) # 7000 * 6
         end
       end
+
+      context "when the product is in lb (1lb)" do
+        let!(:product_attributes) { { variant_unit: "weight", variant_unit_scale: 453.6 } }
+        let!(:variant_attributes) { { unit_value: 453.6, weight: 453.6 } }
+
+        it "is correct" do
+          expect(line_item.final_weight_volume).to eq(907.2) # 2lb
+          line_item.final_weight_volume = 680.4 # 1.5lb
+          expect(calculator.compute(line_item)).to eq(4.08) # 0.6804 * 6
+        end
+      end
+
+      context "when the product is in oz (8oz)" do
+        let!(:product_attributes) { { variant_unit: "weight", variant_unit_scale: 28.35 } }
+        let!(:variant_attributes) { { unit_value: 226.8, weight: 226.8 } }
+
+        it "is correct" do
+          expect(line_item.final_weight_volume).to eq(453.6) # 2 * 8oz == 1lb
+          line_item.final_weight_volume = 680.4 # 1.5lb
+          expect(calculator.compute(line_item)).to eq(4.08) # 0.6804 * 6
+        end
+      end
     end
 
     context "when the product uses volume unit" do

spec/models/spree/line_item_spec.rb

@@ -545,11 +545,24 @@ module Spree
       end
 
       describe "getting unit for display" do
+        let(:o) { create(:order) }
+        let(:p1) { create(:product, name: 'Clear Honey', variant_unit_scale: 1) }
+        let(:v1) { create(:variant, product: p1, unit_value: 500) }
+        let(:li1) { create(:line_item, order: o, product: p1, variant: v1) }
+        let(:p2) { create(:product, name: 'Clear United States Honey', variant_unit_scale: 453.6) }
+        let(:v2) { create(:variant, product: p2, unit_value: 453.6) }
+        let(:li2) { create(:line_item, order: o, product: p2, variant: v2) }
+
         it "returns options_text" do
           li = create(:line_item)
           allow(li).to receive(:options_text).and_return "ponies"
           expect(li.unit_to_display).to eq("ponies")
         end
+
+        it "returns options_text based on units" do
+          expect(li1.options_text).to eq("500g")
+          expect(li2.options_text).to eq("1lb")
+        end
       end
 
       context "when the line_item already has a final_weight_volume set (and all required option values do not exist)" do

spec/models/spree/variant_spec.rb

@@ -1,5 +1,5 @@
 require 'spec_helper'
-require 'open_food_network/option_value_namer'
+require 'variant_units/option_value_namer'
 
 module Spree
   describe Variant do
@@ -436,7 +436,7 @@ module Spree
         let!(:v) { create(:variant, product: p, unit_value: 5, unit_description: 'bar', display_as: '') }
 
         it "requests the name of the new option_value from OptionValueName" do
-          expect_any_instance_of(OpenFoodNetwork::OptionValueNamer).to receive(:name).exactly(1).times.and_call_original
+          expect_any_instance_of(VariantUnits::OptionValueNamer).to receive(:name).exactly(1).times.and_call_original
           v.update(unit_value: 10, unit_description: 'foo')
           ov = v.option_values.last
           expect(ov.name).to eq("10g foo")
@@ -448,7 +448,7 @@ module Spree
         let!(:v) { create(:variant, product: p, unit_value: 5, unit_description: 'bar', display_as: 'FOOS!') }
 
         it "does not request the name of the new option_value from OptionValueName" do
-          expect_any_instance_of(OpenFoodNetwork::OptionValueNamer).not_to receive(:name)
+          expect_any_instance_of(VariantUnits::OptionValueNamer).not_to receive(:name)
           v.update!(unit_value: 10, unit_description: 'foo')
           ov = v.option_values.last
           expect(ov.name).to eq("FOOS!")

spec/services/variant_units/option_value_namer_spec.rb

@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-module OpenFoodNetwork
+module VariantUnits
   describe OptionValueNamer do
     describe "generating option value name" do
       let(:v) { Spree::Variant.new }
@@ -83,12 +83,21 @@ module OpenFoodNetwork
         expect(subject.send(:option_value_value_unit)).to eq [1, 'kg']
       end
 
+      it "returns only values that are in the same measurement systems" do
+        p = double(:product, variant_unit: 'weight', variant_unit_scale: 1.0)
+        allow(v).to receive(:product) { p }
+        allow(v).to receive(:unit_value) { 500 }
+        # 500g would convert to > 1 pound, but we don't want the namer to use
+        # pounds since it's in a different measurement system.
+        expect(subject.send(:option_value_value_unit)).to eq [500, 'g']
+      end
+
       it "generates values for all weight scales" do
-        [[1.0, 'g'], [1000.0, 'kg'], [1_000_000.0, 'T']].each do |scale, unit|
+        [[1.0, 'g'], [28.35, 'oz'], [453.6, 'lb'], [1000.0, 'kg'], [1_000_000.0, 'T']].each do |scale, unit|
           p = double(:product, variant_unit: 'weight', variant_unit_scale: scale)
           allow(v).to receive(:product) { p }
-          allow(v).to receive(:unit_value) { 100 * scale }
-          expect(subject.send(:option_value_value_unit)).to eq [100, unit]
+          allow(v).to receive(:unit_value) { 10.0 * scale }
+          expect(subject.send(:option_value_value_unit)).to eq [10, unit]
         end
       end