From ab7bfd10c5db070799a13ea7bba3fa6d422ea3d9 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 8 Oct 2015 14:50:41 +1100 Subject: [PATCH] Revert "Renaming options_text to unit_text so that we can use method from included VariantAndLineItemNaming module" This reverts commit e86e08b72e939fee61eddd41d56081395666a877. Conflicts: lib/open_food_network/order_and_distributor_report.rb spec/features/admin/variants_spec.rb spec/lib/open_food_network/order_and_distributor_report_spec.rb --- ...t_autocomplete_with_distribution_filter.js.coffee | 2 +- .../javascripts/darkswarm/services/cart.js.coffee | 2 +- app/helpers/spree/base_helper_decorator.rb | 2 +- app/models/spree/variant_decorator.rb | 8 +++----- app/serializers/api/admin/variant_serializer.rb | 2 +- app/serializers/api/variant_serializer.rb | 2 +- app/views/spree/admin/variants/search.rabl | 2 +- app/views/spree/api/variants/bulk_show.v1.rabl | 4 ++-- .../spree/order_mailer/_order_summary.html.haml | 4 ++-- app/views/spree/orders/_line_item.html.haml | 4 ++-- lib/open_food_network/group_buy_report.rb | 4 ++-- .../order_and_distributor_report.rb | 2 +- .../variant_and_line_item_naming.rb | 4 ++-- .../spree/api/variants_controller_spec.rb | 2 +- spec/features/admin/bulk_order_management_spec.rb | 4 ++-- spec/features/admin/products_spec.rb | 2 +- spec/features/admin/variants_spec.rb | 2 ++ .../consumer/shopping/variant_overrides_spec.rb | 6 +++--- .../unit/darkswarm/services/cart_spec.js.coffee | 4 ++-- spec/lib/open_food_network/group_buy_report_spec.rb | 4 ++-- .../order_and_distributor_report_spec.rb | 2 +- spec/models/spree/line_item_spec.rb | 4 ++-- spec/models/spree/variant_spec.rb | 12 ++++++------ spec/requests/shop_spec.rb | 4 ++-- spec/serializers/spree/variant_serializer_spec.rb | 2 +- 25 files changed, 45 insertions(+), 45 deletions(-) diff --git a/app/assets/javascripts/admin/variant_autocomplete_with_distribution_filter.js.coffee b/app/assets/javascripts/admin/variant_autocomplete_with_distribution_filter.js.coffee index 5bfcfb74fc..e3d66d5cd3 100644 --- a/app/assets/javascripts/admin/variant_autocomplete_with_distribution_filter.js.coffee +++ b/app/assets/javascripts/admin/variant_autocomplete_with_distribution_filter.js.coffee @@ -26,5 +26,5 @@ $.fn.variantAutocomplete = -> formatResult: formatVariantResult formatSelection: (variant) -> - $(@element).parent().children(".options_placeholder").html variant.unit_text + $(@element).parent().children(".options_placeholder").html variant.options_text variant.name diff --git a/app/assets/javascripts/darkswarm/services/cart.js.coffee b/app/assets/javascripts/darkswarm/services/cart.js.coffee index 66723e7bee..7550b151f0 100644 --- a/app/assets/javascripts/darkswarm/services/cart.js.coffee +++ b/app/assets/javascripts/darkswarm/services/cart.js.coffee @@ -86,5 +86,5 @@ Darkswarm.factory 'Cart', (CurrentOrder, Variants, $timeout, $http, storage)-> variant.product_name else name = "#{variant.product_name} - #{variant.name_to_display}" - name += " (#{variant.unit_text})" if variant.unit_text + name += " (#{variant.options_text})" if variant.options_text name diff --git a/app/helpers/spree/base_helper_decorator.rb b/app/helpers/spree/base_helper_decorator.rb index 1ce9316885..fa058f9518 100644 --- a/app/helpers/spree/base_helper_decorator.rb +++ b/app/helpers/spree/base_helper_decorator.rb @@ -3,7 +3,7 @@ module Spree # human readable list of variant options # Override: Do not show out of stock text def variant_options(v, options={}) - v.unit_text + v.options_text end end end diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb index 01145eb24f..b49cb6df13 100644 --- a/app/models/spree/variant_decorator.rb +++ b/app/models/spree/variant_decorator.rb @@ -2,8 +2,10 @@ require 'open_food_network/enterprise_fee_calculator' require 'open_food_network/variant_and_line_item_naming' Spree::Variant.class_eval do + remove_method :options_text # Remove method From Spree, so method from the naming module is used instead include OpenFoodNetwork::VariantAndLineItemNaming + has_many :exchange_variants, dependent: :destroy has_many :exchanges, through: :exchange_variants has_many :variant_overrides @@ -69,7 +71,7 @@ Spree::Variant.class_eval do name = product.name name += " - #{name_to_display}" if name_to_display != product.name - name += " (#{unit_text})" if unit_text + name += " (#{options_text})" if options_text name end @@ -87,10 +89,6 @@ Spree::Variant.class_eval do end end - def options_text - # Use unit_text instead - end - private def update_weight_from_unit_value diff --git a/app/serializers/api/admin/variant_serializer.rb b/app/serializers/api/admin/variant_serializer.rb index 81b527fbfd..510f7af333 100644 --- a/app/serializers/api/admin/variant_serializer.rb +++ b/app/serializers/api/admin/variant_serializer.rb @@ -1,5 +1,5 @@ class Api::Admin::VariantSerializer < ActiveModel::Serializer - attributes :id, :unit_text, :unit_value, :unit_description, :unit_to_display, :on_demand, :display_as, :display_name, :name_to_display + attributes :id, :options_text, :unit_value, :unit_description, :unit_to_display, :on_demand, :display_as, :display_name, :name_to_display attributes :on_hand, :price has_many :variant_overrides diff --git a/app/serializers/api/variant_serializer.rb b/app/serializers/api/variant_serializer.rb index 6224c33463..6908eaf84f 100644 --- a/app/serializers/api/variant_serializer.rb +++ b/app/serializers/api/variant_serializer.rb @@ -1,6 +1,6 @@ class Api::VariantSerializer < ActiveModel::Serializer attributes :id, :is_master, :count_on_hand, :name_to_display, :unit_to_display, - :unit_text, :on_demand, :price, :fees, :price_with_fees, :product_name + :options_text, :on_demand, :price, :fees, :price_with_fees, :product_name def price object.price diff --git a/app/views/spree/admin/variants/search.rabl b/app/views/spree/admin/variants/search.rabl index 9a44f7f545..afd3f39ce6 100644 --- a/app/views/spree/admin/variants/search.rabl +++ b/app/views/spree/admin/variants/search.rabl @@ -2,7 +2,7 @@ # overriding spree/core/app/views/spree/admin/variants/search.rabl # collection @variants -attributes :sku, :unit_text, :count_on_hand, :id, :cost_price +attributes :sku, :options_text, :count_on_hand, :id, :cost_price node(:name) do |v| # TODO: when products must have a unit, full_name will always be present diff --git a/app/views/spree/api/variants/bulk_show.v1.rabl b/app/views/spree/api/variants/bulk_show.v1.rabl index 3305a53ac7..4a2a6bae9c 100644 --- a/app/views/spree/api/variants/bulk_show.v1.rabl +++ b/app/views/spree/api/variants/bulk_show.v1.rabl @@ -1,7 +1,7 @@ object @variant -attributes :id, :unit_text, :unit_value, :unit_description, :on_demand, :display_as, :display_name +attributes :id, :options_text, :unit_value, :unit_description, :on_demand, :display_as, :display_name # Infinity is not a valid JSON object, but Rails encodes it anyway node( :on_hand ) { |v| v.on_hand.nil? ? 0 : ( v.on_hand.to_f.finite? ? v.on_hand : "On demand" ) } -node( :price ) { |v| v.price.nil? ? 0.to_f : v.price } +node( :price ) { |v| v.price.nil? ? 0.to_f : v.price } \ No newline at end of file diff --git a/app/views/spree/order_mailer/_order_summary.html.haml b/app/views/spree/order_mailer/_order_summary.html.haml index 777ecc440a..86b1af1f3b 100644 --- a/app/views/spree/order_mailer/_order_summary.html.haml +++ b/app/views/spree/order_mailer/_order_summary.html.haml @@ -17,8 +17,8 @@ %strong %span= "#{raw(item.variant.product.name)}" %span= "- " + "#{raw(item.variant.name_to_display)}" - - if item.variant.unit_text - = "(" + "#{raw(item.variant.unit_text)}" + ")" + - if item.variant.options_text + = "(" + "#{raw(item.variant.options_text)}" + ")" %br %small %em= raw(item.variant.product.supplier.name) diff --git a/app/views/spree/orders/_line_item.html.haml b/app/views/spree/orders/_line_item.html.haml index 18d3fb2e03..d687290561 100644 --- a/app/views/spree/orders/_line_item.html.haml +++ b/app/views/spree/orders/_line_item.html.haml @@ -18,12 +18,12 @@ - if variant.product.name == variant.name_to_display %h5 %span= variant.product.name - %span.text-small.text-skinny= " (" + variant.unit_text + ")" unless variant.unit_text.empty? + %span.text-small.text-skinny= " (" + variant.options_text + ")" unless variant.options_text.empty? - else %h5 %span= variant.product.name %span= "- " + variant.name_to_display - %span.text-small.text-skinny= " (" + variant.unit_text + ")" unless variant.unit_text.empty? + %span.text-small.text-skinny= " (" + variant.options_text + ")" unless variant.options_text.empty? - if @order.insufficient_stock_lines.include? line_item %span.out-of-stock diff --git a/lib/open_food_network/group_buy_report.rb b/lib/open_food_network/group_buy_report.rb index f0dfd2ce1e..b4875b49a5 100644 --- a/lib/open_food_network/group_buy_report.rb +++ b/lib/open_food_network/group_buy_report.rb @@ -2,7 +2,7 @@ module OpenFoodNetwork GroupBuyVariantRow = Struct.new(:variant, :sum_quantities, :sum_max_quantities) do def to_row - [variant.product.supplier.name, variant.product.name, "UNITSIZE", variant.unit_text, variant.weight, sum_quantities, sum_max_quantities] + [variant.product.supplier.name, variant.product.name, "UNITSIZE", variant.options_text, variant.weight, sum_quantities, sum_max_quantities] end end @@ -33,7 +33,7 @@ module OpenFoodNetwork variant_groups = line_items_by_product.group_by { |li| li.variant } variant_groups.each do |variant, line_items_by_variant| sum_quantities = line_items_by_variant.sum { |li| li.quantity } - sum_max_quantities = line_items_by_variant.sum { |li| li.max_quantity || 0 } + sum_max_quantities = line_items_by_variant.sum { |li| li.max_quantity || 0 } variants_and_quantities << GroupBuyVariantRow.new(variant, sum_quantities, sum_max_quantities) end diff --git a/lib/open_food_network/order_and_distributor_report.rb b/lib/open_food_network/order_and_distributor_report.rb index 8c10ea31f2..570087e493 100644 --- a/lib/open_food_network/order_and_distributor_report.rb +++ b/lib/open_food_network/order_and_distributor_report.rb @@ -20,7 +20,7 @@ module OpenFoodNetwork order.line_items.each do |line_item| order_and_distributor_details << [order.created_at, order.id, order.bill_address.full_name, order.email, order.bill_address.phone, order.bill_address.city, - line_item.product.sku, line_item.product.name, line_item.unit_text, line_item.quantity, line_item.max_quantity, line_item.price * line_item.quantity, line_item.distribution_fee, + line_item.product.sku, line_item.product.name, line_item.options_text, line_item.quantity, line_item.max_quantity, line_item.price * line_item.quantity, line_item.distribution_fee, order.payments.first.andand.payment_method.andand.name, order.distributor.andand.name, order.distributor.address.address1, order.distributor.address.city, order.distributor.address.zipcode, order.special_instructions ] end diff --git a/lib/open_food_network/variant_and_line_item_naming.rb b/lib/open_food_network/variant_and_line_item_naming.rb index b430515943..62ff91a4c3 100644 --- a/lib/open_food_network/variant_and_line_item_naming.rb +++ b/lib/open_food_network/variant_and_line_item_naming.rb @@ -8,7 +8,7 @@ module OpenFoodNetwork module VariantAndLineItemNaming # Copied and modified from Spree::Variant - def unit_text + def options_text values = self.option_values.joins(:option_type).order("#{Spree::OptionType.table_name}.position asc") values.map! &:presentation # This line changed @@ -35,7 +35,7 @@ module OpenFoodNetwork end def unit_to_display - return unit_text if !self.has_attribute?(:display_as) || display_as.blank? + return options_text if !self.has_attribute?(:display_as) || display_as.blank? display_as end diff --git a/spec/controllers/spree/api/variants_controller_spec.rb b/spec/controllers/spree/api/variants_controller_spec.rb index 921d28a24c..5fb9f2f2a0 100644 --- a/spec/controllers/spree/api/variants_controller_spec.rb +++ b/spec/controllers/spree/api/variants_controller_spec.rb @@ -8,7 +8,7 @@ module Spree let!(:variant1) { FactoryGirl.create(:variant) } let!(:variant2) { FactoryGirl.create(:variant) } let!(:variant3) { FactoryGirl.create(:variant) } - let(:attributes) { [:id, :unit_text, :price, :on_hand, :unit_value, :unit_description, :on_demand, :display_as, :display_name] } + let(:attributes) { [:id, :options_text, :price, :on_hand, :unit_value, :unit_description, :on_demand, :display_as, :display_name] } before do stub_authentication! diff --git a/spec/features/admin/bulk_order_management_spec.rb b/spec/features/admin/bulk_order_management_spec.rb index dba95af4ea..5bcca783bd 100644 --- a/spec/features/admin/bulk_order_management_spec.rb +++ b/spec/features/admin/bulk_order_management_spec.rb @@ -67,7 +67,7 @@ feature %q{ it "displays a column for variant description, which shows only product name when options text is blank" do expect(page).to have_selector "th.variant", text: "PRODUCT: UNIT", :visible => true expect(page).to have_selector "td.variant", text: li1.product.name, :visible => true - expect(page).to have_selector "td.variant", text: (li2.product.name + ": " + li2.variant.unit_text), :visible => true + expect(page).to have_selector "td.variant", text: (li2.product.name + ": " + li2.variant.options_text), :visible => true end it "displays a field for quantity" do @@ -589,7 +589,7 @@ feature %q{ before :each do visit '/admin/orders/bulk_management' within "tr#li_#{li3.id}" do - find("a", text: li3.product.name + ": " + li3.variant.unit_text).click + find("a", text: li3.product.name + ": " + li3.variant.options_text).click end end diff --git a/spec/features/admin/products_spec.rb b/spec/features/admin/products_spec.rb index 9241b9f9c9..2aafc82b02 100644 --- a/spec/features/admin/products_spec.rb +++ b/spec/features/admin/products_spec.rb @@ -55,7 +55,7 @@ feature %q{ product.description.should == "A description..." product.group_buy.should be_false product.master.option_values.map(&:name).should == ['5kg'] - product.master.unit_text.should == "5kg" + product.master.options_text.should == "5kg" # Distributors visit spree.product_distributions_admin_product_path(product) diff --git a/spec/features/admin/variants_spec.rb b/spec/features/admin/variants_spec.rb index 528481b32b..7303e81766 100644 --- a/spec/features/admin/variants_spec.rb +++ b/spec/features/admin/variants_spec.rb @@ -68,8 +68,10 @@ feature %q{ within "tr#spree_variant_#{v.id}" do page.find('a.delete-resource').click end + page.should_not have_selector "tr#spree_variant_#{v.id}" + v.reload v.deleted_at.should_not be_nil end diff --git a/spec/features/consumer/shopping/variant_overrides_spec.rb b/spec/features/consumer/shopping/variant_overrides_spec.rb index 0137053271..f850436e52 100644 --- a/spec/features/consumer/shopping/variant_overrides_spec.rb +++ b/spec/features/consumer/shopping/variant_overrides_spec.rb @@ -46,14 +46,14 @@ feature "shopping with variant overrides defined", js: true do it "looks up stock from the override" do # Product should appear but one of the variants is out of stock - page.should_not have_content v2.unit_text + page.should_not have_content v2.options_text # Entire product should not appear - no stock page.should_not have_content p2.name - page.should_not have_content v3.unit_text + page.should_not have_content v3.options_text # On-demand product with VO of no stock should NOT appear - page.should_not have_content v5.unit_text + page.should_not have_content v5.options_text end it "calculates fees correctly" do diff --git a/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee index ed43f988c8..0519b59763 100644 --- a/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee @@ -92,6 +92,6 @@ describe 'Cart service', -> variant = product_name: 'product_name' name_to_display: 'name_to_display' - unit_text: 'unit_text' + options_text: 'options_text' - expect(Cart.extendedVariantName(variant)).toEqual "product_name - name_to_display (unit_text)" + expect(Cart.extendedVariantName(variant)).toEqual "product_name - name_to_display (options_text)" diff --git a/spec/lib/open_food_network/group_buy_report_spec.rb b/spec/lib/open_food_network/group_buy_report_spec.rb index 101d84ae1a..252c352c31 100644 --- a/spec/lib/open_food_network/group_buy_report_spec.rb +++ b/spec/lib/open_food_network/group_buy_report_spec.rb @@ -62,7 +62,7 @@ module OpenFoodNetwork sum_quantities = line_items.map { |li| li.quantity }.sum sum_max_quantities = line_items.map { |li| li.max_quantity || 0 }.sum - table[0].should == [@variant1.product.supplier.name,@variant1.product.name,"UNITSIZE",@variant1.unit_text,@variant1.weight,sum_quantities,sum_max_quantities] + table[0].should == [@variant1.product.supplier.name,@variant1.product.name,"UNITSIZE",@variant1.options_text,@variant1.weight,sum_quantities,sum_max_quantities] end it "should return a table wherein each rows contains the same number of columns as the heading" do @@ -76,7 +76,7 @@ module OpenFoodNetwork end end - it "should split and group line items from multiple suppliers and of multiple variants" do + it "should split and group line items from multiple suppliers and of multiple variants" do subject = GroupBuyReport.new @orders table_row_objects = subject.variants_and_quantities diff --git a/spec/lib/open_food_network/order_and_distributor_report_spec.rb b/spec/lib/open_food_network/order_and_distributor_report_spec.rb index 154e697ae0..e8759823db 100644 --- a/spec/lib/open_food_network/order_and_distributor_report_spec.rb +++ b/spec/lib/open_food_network/order_and_distributor_report_spec.rb @@ -40,7 +40,7 @@ module OpenFoodNetwork table[0].should == [@order.created_at, @order.id, @bill_address.full_name, @order.email, @bill_address.phone, @bill_address.city, - @line_item.product.sku, @line_item.product.name, @line_item.unit_text, @line_item.quantity, @line_item.max_quantity, @line_item.price * @line_item.quantity, @line_item.distribution_fee, + @line_item.product.sku, @line_item.product.name, @line_item.options_text, @line_item.quantity, @line_item.max_quantity, @line_item.price * @line_item.quantity, @line_item.distribution_fee, @payment_method.name, @distributor.name, @distributor.address.address1, @distributor.address.city, @distributor.address.zipcode, @shipping_instructions ] end diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index a52ebafecf..fed4287eb4 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -126,9 +126,9 @@ module Spree end describe "getting unit for display" do - it "returns unit_text" do + it "returns options_text" do li = create(:line_item) - li.stub(:unit_text).and_return "ponies" + li.stub(:options_text).and_return "ponies" li.unit_to_display.should == "ponies" end end diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb index 55018828a0..283655e627 100644 --- a/spec/models/spree/variant_spec.rb +++ b/spec/models/spree/variant_spec.rb @@ -122,7 +122,7 @@ module Spree before do v.stub(:product) { p } v.stub(:name_to_display) { p.name } - v.stub(:unit_text) { nil } + v.stub(:options_text) { nil } end it "returns the product name only when there's no extra info" do @@ -135,13 +135,13 @@ module Spree end it "shows the options text when present" do - v.stub(:unit_text) { 'OT' } + v.stub(:options_text) { 'OT' } v.product_and_variant_name.should == 'product (OT)' end it "displays all attributes" do v.stub(:name_to_display) { 'NTD' } - v.stub(:unit_text) { 'OT' } + v.stub(:options_text) { 'OT' } v.product_and_variant_name.should == 'product - NTD (OT)' end end @@ -302,11 +302,11 @@ module Spree v.unit_to_display.should == "foo" end - it "returns unit_text if display_as is blank" do + it "returns options_text if display_as is blank" do v = create(:variant) v1 = create(:variant, display_as: "") - v.stub(:unit_text).and_return "ponies" - v1.stub(:unit_text).and_return "ponies" + v.stub(:options_text).and_return "ponies" + v1.stub(:options_text).and_return "ponies" v.unit_to_display.should == "ponies" v1.unit_to_display.should == "ponies" end diff --git a/spec/requests/shop_spec.rb b/spec/requests/shop_spec.rb index 5f3651a1d5..cc8838220c 100644 --- a/spec/requests/shop_spec.rb +++ b/spec/requests/shop_spec.rb @@ -49,9 +49,9 @@ describe "Shop API" do it "filters products based on availability" do # It shows on demand variants - response.body.should include v43.unit_text + response.body.should include v43.options_text # It does not show variants that are neither on hand or on demand - response.body.should_not include v42.unit_text + response.body.should_not include v42.options_text # It does not show products that have no available variants in this distribution response.body.should_not include p5.name # It does not show deleted products diff --git a/spec/serializers/spree/variant_serializer_spec.rb b/spec/serializers/spree/variant_serializer_spec.rb index 38dc0519a9..8f6b1e0bf5 100644 --- a/spec/serializers/spree/variant_serializer_spec.rb +++ b/spec/serializers/spree/variant_serializer_spec.rb @@ -2,6 +2,6 @@ describe Api::Admin::VariantSerializer do let(:variant) { create(:variant) } it "serializes a variant" do serializer = Api::Admin::VariantSerializer.new variant - serializer.to_json.should match variant.unit_text + serializer.to_json.should match variant.options_text end end