diff --git a/app/assets/javascripts/admin/products/services/option_value_namer.js.coffee b/app/assets/javascripts/admin/products/services/option_value_namer.js.coffee index 49a2f470be..8993e6a230 100644 --- a/app/assets/javascripts/admin/products/services/option_value_namer.js.coffee +++ b/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)] diff --git a/app/assets/javascripts/admin/products/services/variant_unit_manager.js.coffee b/app/assets/javascripts/admin/products/services/variant_unit_manager.js.coffee index ad16ae7cff..244d51cde6 100644 --- a/app/assets/javascripts/admin/products/services/variant_unit_manager.js.coffee +++ b/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 diff --git a/app/models/product_import/unit_converter.rb b/app/models/product_import/unit_converter.rb index e97cff43f9..c5321a4990 100644 --- a/app/models/product_import/unit_converter.rb +++ 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' }, diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index c58b072892..558b2efc59 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/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' diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb index 41b1a4eb76..737b2a8da4 100644 --- a/app/models/spree/variant_decorator.rb +++ b/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 diff --git a/lib/open_food_network/option_value_namer.rb b/app/services/variant_units/option_value_namer.rb similarity index 51% rename from lib/open_food_network/option_value_namer.rb rename to app/services/variant_units/option_value_namer.rb index 706a0b443d..7cb8fc8547 100644 --- a/lib/open_food_network/option_value_namer.rb +++ b/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 diff --git a/lib/open_food_network/variant_and_line_item_naming.rb b/app/services/variant_units/variant_and_line_item_naming.rb similarity index 77% rename from lib/open_food_network/variant_and_line_item_naming.rb rename to app/services/variant_units/variant_and_line_item_naming.rb index 7eed59b892..6e325c1fe5 100644 --- a/lib/open_food_network/variant_and_line_item_naming.rb +++ b/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 diff --git a/config/locales/en.yml b/config/locales/en.yml index c4088825fb..fa966a815b 100644 --- a/config/locales/en.yml +++ b/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" diff --git a/engines/order_management/app/services/order_management/reports/bulk_coop/bulk_coop_report.rb b/engines/order_management/app/services/order_management/reports/bulk_coop/bulk_coop_report.rb index bc44c2b857..c9c8b822f1 100644 --- a/engines/order_management/app/services/order_management/reports/bulk_coop/bulk_coop_report.rb +++ b/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) diff --git a/lib/open_food_network/lettuce_share_report.rb b/lib/open_food_network/lettuce_share_report.rb index c04a7b3adb..49fffbadac 100644 --- a/lib/open_food_network/lettuce_share_report.rb +++ b/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), diff --git a/spec/javascripts/unit/admin/services/variant_unit_manager_spec.js.coffee b/spec/javascripts/unit/admin/services/variant_unit_manager_spec.js.coffee index ff70e2a1d1..40f03cf8cb 100644 --- a/spec/javascripts/unit/admin/services/variant_unit_manager_spec.js.coffee +++ b/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"], diff --git a/spec/models/calculator/weight_spec.rb b/spec/models/calculator/weight_spec.rb index 1ca1759b40..311e09775e 100644 --- a/spec/models/calculator/weight_spec.rb +++ b/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 diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index 2836c2c51c..4410a00975 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/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 diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb index 7a3b2ac85c..86cbbf174c 100644 --- a/spec/models/spree/variant_spec.rb +++ b/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!") diff --git a/spec/lib/open_food_network/option_value_namer_spec.rb b/spec/services/variant_units/option_value_namer_spec.rb similarity index 88% rename from spec/lib/open_food_network/option_value_namer_spec.rb rename to spec/services/variant_units/option_value_namer_spec.rb index 1e3ec2944c..3e153896fd 100644 --- a/spec/lib/open_food_network/option_value_namer_spec.rb +++ b/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