diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index 689cabcc6c..4852e17a5c 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -1,6 +1,14 @@ +require 'open_food_network/variant_and_line_item_naming' + Spree::LineItem.class_eval do + include OpenFoodNetwork::VariantAndLineItemNaming + has_and_belongs_to_many :option_values, join_table: 'spree_option_values_line_items', class_name: 'Spree::OptionValue' + attr_accessible :max_quantity, :final_weight_volume attr_accessible :final_weight_volume, :price, :as => :api + after_save :update_units + + delegate :unit_description, to: :variant # -- Scopes scope :managed_by, lambda { |user| @@ -57,4 +65,9 @@ Spree::LineItem.class_eval do def display_amount_with_adjustments Spree::Money.new(amount_with_adjustments, { :currency => currency }) end + + def unit_value + return 0 if quantity == 0 + final_weight_volume / quantity unless final_weight_volume.nil? + end end diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 4ca328a5b1..4e1ae0736c 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -130,6 +130,7 @@ Spree::Order.class_eval do else current_item = Spree::LineItem.new(:quantity => quantity, max_quantity: max_quantity) current_item.variant = variant + current_item.option_values = variant.option_values if variant.unit_value current_item.final_weight_volume = variant.unit_value * quantity else diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb index 5ac89c1430..09e917b8a7 100644 --- a/app/models/spree/variant_decorator.rb +++ b/app/models/spree/variant_decorator.rb @@ -1,7 +1,9 @@ require 'open_food_network/enterprise_fee_calculator' -require 'open_food_network/option_value_namer' +require 'open_food_network/variant_and_line_item_naming' Spree::Variant.class_eval do + include OpenFoodNetwork::VariantAndLineItemNaming + has_many :exchange_variants, dependent: :destroy has_many :exchanges, through: :exchange_variants has_many :variant_overrides @@ -62,44 +64,7 @@ Spree::Variant.class_eval do OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle).fees_by_type_for self end - - # Copied and modified from Spree::Variant - def options_text - values = self.option_values.joins(:option_type).order("#{Spree::OptionType.table_name}.position asc") - - values.map! &:presentation # This line changed - - values.to_sentence({ :words_connector => ", ", :two_words_connector => ", " }) - end - - def delete_unit_option_values - ovs = self.option_values.where(option_type_id: Spree::Product.all_variant_unit_option_types) - self.option_values.destroy ovs - end - - # Used like "product.name - full_name". If called like this, a product with - # name "Bread" would be displayed as one of these: - # Bread - 1kg # if display_name blank - # Bread - Spelt Sourdough, 1kg # if display_name is "Spelt Sourdough, 1kg" - # Bread - 1kg Spelt Sourdough # if unit_to_display is "1kg Spelt Sourdough" - # Bread - Spelt Sourdough (1kg) # if display_name is "Spelt Sourdough" and unit_to_display is "1kg" - def full_name - return unit_to_display if display_name.blank? - return display_name if display_name.downcase.include? unit_to_display.downcase - return unit_to_display if unit_to_display.downcase.include? display_name.downcase - "#{display_name} (#{unit_to_display})" - end - - def name_to_display - return product.name if display_name.blank? - display_name - end - - def unit_to_display - return options_text if display_as.blank? - display_as - end - + # TODO: Should this be moved into VariantAndLineItemNaming? def product_and_variant_name name = product.name @@ -109,17 +74,6 @@ Spree::Variant.class_eval do name end - def update_units - delete_unit_option_values - - option_type = self.product.variant_unit_option_type - if option_type - name = option_value_name - ov = Spree::OptionValue.where(option_type_id: option_type, name: name, presentation: name).first || Spree::OptionValue.create!({option_type: option_type, name: name, presentation: name}, without_protection: true) - option_values << ov - end - end - def delete if product.variants == [self] # Only variant left on product errors.add :product, "must have at least one variant" @@ -133,19 +87,9 @@ Spree::Variant.class_eval do end end - private def update_weight_from_unit_value self.weight = unit_value / 1000 if self.product.variant_unit == 'weight' && unit_value.present? end - - def option_value_name - if display_as.present? - display_as - else - option_value_namer = OpenFoodNetwork::OptionValueNamer.new self - option_value_namer.name - end - end end diff --git a/db/migrate/20150924054538_add_option_values_line_items_join_table.rb b/db/migrate/20150924054538_add_option_values_line_items_join_table.rb new file mode 100644 index 0000000000..7a51c851ec --- /dev/null +++ b/db/migrate/20150924054538_add_option_values_line_items_join_table.rb @@ -0,0 +1,14 @@ +class AddOptionValuesLineItemsJoinTable < ActiveRecord::Migration + def change + create_table :spree_option_values_line_items, :id => false, :force => true do |t| + t.integer :line_item_id + t.integer :option_value_id + end + + Spree::LineItem.all.each do |line_item| + line_item.update_units + end + + add_index :spree_option_values_line_items, :line_item_id, :name => 'index_option_values_line_items_on_line_item_id' + end +end diff --git a/db/schema.rb b/db/schema.rb index 436d84303c..8f164fb66c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -635,6 +635,13 @@ ActiveRecord::Schema.define(:version => 20151002020537) do t.datetime "updated_at", :null => false end + create_table "spree_option_values_line_items", :id => false, :force => true do |t| + t.integer "line_item_id" + t.integer "option_value_id" + end + + add_index "spree_option_values_line_items", ["line_item_id"], :name => "index_option_values_line_items_on_line_item_id" + create_table "spree_option_values_variants", :id => false, :force => true do |t| t.integer "variant_id" t.integer "option_value_id" diff --git a/lib/open_food_network/variant_and_line_item_naming.rb b/lib/open_food_network/variant_and_line_item_naming.rb new file mode 100644 index 0000000000..47ffb007bb --- /dev/null +++ b/lib/open_food_network/variant_and_line_item_naming.rb @@ -0,0 +1,70 @@ +# This module is included in both the Spree::Variant and Spree::LineItem model decorators +# 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' + +module OpenFoodNetwork + module VariantAndLineItemNaming + + # Copied and modified from Spree::Variant + def options_text + values = self.option_values.joins(:option_type).order("#{Spree::OptionType.table_name}.position asc") + + values.map! &:presentation # This line changed + + values.to_sentence({ :words_connector => ", ", :two_words_connector => ", " }) + end + + # Used like "product.name - full_name". If called like this, a product with + # name "Bread" would be displayed as one of these: + # Bread - 1kg # if display_name blank + # Bread - Spelt Sourdough, 1kg # if display_name is "Spelt Sourdough, 1kg" + # Bread - 1kg Spelt Sourdough # if unit_to_display is "1kg Spelt Sourdough" + # Bread - Spelt Sourdough (1kg) # if display_name is "Spelt Sourdough" and unit_to_display is "1kg" + def full_name + return unit_to_display if !self.has_attribute?(:display_name) || display_name.blank? + return display_name if display_name.downcase.include? unit_to_display.downcase + return unit_to_display if unit_to_display.downcase.include? display_name.downcase + "#{display_name} (#{unit_to_display})" + end + + def name_to_display + return product.name if !self.has_attribute?(:display_name) || display_name.blank? + display_name + end + + def unit_to_display + return options_text if !self.has_attribute?(:display_as) || display_as.blank? + display_as + end + + + def update_units + delete_unit_option_values + + option_type = self.product.variant_unit_option_type + if option_type + name = option_value_name + ov = Spree::OptionValue.where(option_type_id: option_type, name: name, presentation: name).first || Spree::OptionValue.create!({option_type: option_type, name: name, presentation: name}, without_protection: true) + option_values << ov + end + end + + def delete_unit_option_values + ovs = self.option_values.where(option_type_id: Spree::Product.all_variant_unit_option_types) + self.option_values.destroy ovs + end + + private + + def option_value_name + if self.has_attribute?(:display_as) && display_as.present? + display_as + else + option_value_namer = OpenFoodNetwork::OptionValueNamer.new self + option_value_namer.name + end + end + end +end diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index f9c0e97f20..fcc45e79ec 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -78,5 +78,96 @@ module Spree li_no_tax.should_not have_tax end end + + describe "unit value/description" do + describe "generating the full name" do + let(:li) { LineItem.new } + + before do + li.stub(:unit_to_display) { 'unit_to_display' } + end + + it "returns unit_to_display" do + li.full_name.should == 'unit_to_display' + end + end + + describe "getting name for display" do + it "returns product name" do + li = create(:variant, product: create(:product)) + li.name_to_display.should == li.product.name + end + end + + describe "getting unit for display" do + it "returns options_text" do + li = create(:line_item) + li.stub(:options_text).and_return "ponies" + li.unit_to_display.should == "ponies" + end + end + + context "when the line_item already has a final_weight_volume set (and all required option values do not exist)" do + let!(:p0) { create(:simple_product, variant_unit: 'weight', variant_unit_scale: 1) } + let!(:v) { create(:variant, product: p0, unit_value: 10, unit_description: 'bar') } + + let!(:p) { create(:simple_product, variant_unit: 'weight', variant_unit_scale: 1) } + let!(:li) { create(:line_item, product: p, final_weight_volume: 5) } + + it "removes the old option value and assigns the new one" do + ov_orig = li.option_values.last + ov_var = v.option_values.last + allow(li).to receive(:unit_description) { 'foo' } + + expect { + li.update_attributes!(final_weight_volume: 10) + }.to change(Spree::OptionValue, :count).by(1) + + li.option_values.should_not include ov_orig + li.option_values.should_not include ov_var + ov = li.option_values.last + ov.name.should == "10g foo" + end + end + + context "when the variant already has a value set (and all required option values exist)" do + let!(:p0) { create(:simple_product, variant_unit: 'weight', variant_unit_scale: 1) } + let!(:v) { create(:variant, product: p0, unit_value: 10, unit_description: 'bar') } + + let!(:p) { create(:simple_product, variant_unit: 'weight', variant_unit_scale: 1) } + let!(:li) { create(:line_item, product: p, final_weight_volume: 5) } + + it "removes the old option value and assigns the new one" do + ov_orig = li.option_values.last + ov_new = v.option_values.last + allow(li).to receive(:unit_description) { 'bar' } + + expect { + li.update_attributes!(final_weight_volume: 10) + }.to change(Spree::OptionValue, :count).by(0) + + li.option_values.should_not include ov_orig + li.option_values.should include ov_new + end + end + end + + describe "deleting unit option values" do + let!(:p) { create(:simple_product, variant_unit: 'weight', variant_unit_scale: 1) } + let!(:ot) { Spree::OptionType.find_by_name 'unit_weight' } + let!(:li) { create(:line_item, product: p) } + + it "removes option value associations for unit option types" do + expect { + li.delete_unit_option_values + }.to change(li.option_values, :count).by(-1) + end + + it "does not delete option values" do + expect { + li.delete_unit_option_values + }.to change(Spree::OptionValue, :count).by(0) + end + end end end diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb index 69dda85ff5..283655e627 100644 --- a/spec/models/spree/variant_spec.rb +++ b/spec/models/spree/variant_spec.rb @@ -115,44 +115,6 @@ module Spree end end - describe "generating the full name" do - let(:v) { Variant.new } - - before do - v.stub(:display_name) { 'display_name' } - v.stub(:unit_to_display) { 'unit_to_display' } - end - - it "returns unit_to_display when display_name is blank" do - v.stub(:display_name) { '' } - v.full_name.should == 'unit_to_display' - end - - it "returns display_name when it contains unit_to_display" do - v.stub(:display_name) { 'DiSpLaY_name' } - v.stub(:unit_to_display) { 'name' } - v.full_name.should == 'DiSpLaY_name' - end - - it "returns unit_to_display when it contains display_name" do - v.stub(:display_name) { '_to_' } - v.stub(:unit_to_display) { 'unit_TO_display' } - v.full_name.should == 'unit_TO_display' - end - - it "returns a combination otherwise" do - v.stub(:display_name) { 'display_name' } - v.stub(:unit_to_display) { 'unit_to_display' } - v.full_name.should == 'display_name (unit_to_display)' - end - - it "is resilient to regex chars" do - v = Variant.new display_name: ")))" - v.stub(:unit_to_display) { ")))" } - v.full_name.should == ")))" - end - end - describe "generating the product and variant name" do let(:v) { Variant.new } let(:p) { double(:product, name: 'product') } @@ -282,6 +244,44 @@ module Spree end describe "unit value/description" do + describe "generating the full name" do + let(:v) { Variant.new } + + before do + v.stub(:display_name) { 'display_name' } + v.stub(:unit_to_display) { 'unit_to_display' } + end + + it "returns unit_to_display when display_name is blank" do + v.stub(:display_name) { '' } + v.full_name.should == 'unit_to_display' + end + + it "returns display_name when it contains unit_to_display" do + v.stub(:display_name) { 'DiSpLaY_name' } + v.stub(:unit_to_display) { 'name' } + v.full_name.should == 'DiSpLaY_name' + end + + it "returns unit_to_display when it contains display_name" do + v.stub(:display_name) { '_to_' } + v.stub(:unit_to_display) { 'unit_TO_display' } + v.full_name.should == 'unit_TO_display' + end + + it "returns a combination otherwise" do + v.stub(:display_name) { 'display_name' } + v.stub(:unit_to_display) { 'unit_to_display' } + v.full_name.should == 'display_name (unit_to_display)' + end + + it "is resilient to regex chars" do + v = Variant.new display_name: ")))" + v.stub(:unit_to_display) { ")))" } + v.full_name.should == ")))" + end + end + describe "getting name for display" do it "returns display_name if present" do v = create(:variant, display_name: "foo") @@ -347,23 +347,18 @@ module Spree end end - context "when the variant already has a value set (and all required option values exist)" do - let!(:p0) { create(:simple_product, variant_unit: 'weight', variant_unit_scale: 1) } - let!(:v0) { create(:variant, product: p0, unit_value: 10, unit_description: 'foo') } - + context "when the variant already has a value set (and all required option values do not exist)" do let!(:p) { create(:simple_product, variant_unit: 'weight', variant_unit_scale: 1) } let!(:v) { create(:variant, product: p, unit_value: 5, unit_description: 'bar') } it "removes the old option value and assigns the new one" do ov_orig = v.option_values.last - ov_new = v0.option_values.last expect { v.update_attributes!(unit_value: 10, unit_description: 'foo') - }.to change(Spree::OptionValue, :count).by(0) + }.to change(Spree::OptionValue, :count).by(1) v.option_values.should_not include ov_orig - v.option_values.should include ov_new end end