From e75b595b971f9d41f18d199bfe767454c1479240 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 5 Aug 2015 14:02:41 +1000 Subject: [PATCH 1/4] Tidy syntax --- .../products_and_inventory_report.rb | 61 ++++++++++--------- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/lib/open_food_network/products_and_inventory_report.rb b/lib/open_food_network/products_and_inventory_report.rb index 2b797bd944..7d695f4345 100644 --- a/lib/open_food_network/products_and_inventory_report.rb +++ b/lib/open_food_network/products_and_inventory_report.rb @@ -2,6 +2,7 @@ module OpenFoodNetwork class ProductsAndInventoryReport attr_reader :params + def initialize(user, params = {}) @user = user @params = params @@ -9,41 +10,40 @@ module OpenFoodNetwork def header [ - "Supplier", - "Producer Suburb", - "Product", - "Product Properties", - "Taxons", - "Variant Value", - "Price", - "Group Buy Unit Quantity", - "Amount" - ] + "Supplier", + "Producer Suburb", + "Product", + "Product Properties", + "Taxons", + "Variant Value", + "Price", + "Group Buy Unit Quantity", + "Amount" + ] end def table variants.map do |variant| - [variant.product.supplier.name, - variant.product.supplier.address.city, - variant.product.name, - variant.product.properties.map(&:name).join(", "), - variant.product.taxons.map(&:name).join(", "), - variant.full_name, - variant.price, - variant.product.group_buy_unit_size, - "" + [ + variant.product.supplier.name, + variant.product.supplier.address.city, + variant.product.name, + variant.product.properties.map(&:name).join(", "), + variant.product.taxons.map(&:name).join(", "), + variant.full_name, + variant.price, + variant.product.group_buy_unit_size, + "" ] end end def permissions - return @permissions unless @permissions.nil? - @permissions = OpenFoodNetwork::Permissions.new(@user) + @permissions ||= OpenFoodNetwork::Permissions.new(@user) end def visible_products - return @visible_products unless @visible_products.nil? - @visible_products = permissions.visible_products + @visible_products ||= permissions.visible_products end def variants @@ -51,15 +51,16 @@ module OpenFoodNetwork end def child_variants - Spree::Variant.where(:is_master => false) - .joins(:product) - .merge(visible_products) - .order("spree_products.name") + Spree::Variant. + where(is_master: false). + joins(:product). + merge(visible_products). + order('spree_products.name') end def filter(variants) # NOTE: Ordering matters. - # filter_to_order_cycle and filter_to_distributor return Arrays not Arel + # filter_to_order_cycle and filter_to_distributor return arrays not relations filter_to_distributor filter_to_order_cycle filter_on_hand filter_to_supplier filter_not_deleted variants end @@ -68,8 +69,8 @@ module OpenFoodNetwork end def filter_on_hand(variants) - if params[:report_type] == "inventory" - variants.where("spree_variants.count_on_hand > 0") + if params[:report_type] == 'inventory' + variants.where('spree_variants.count_on_hand > 0') else variants end From 6f4dc6943ebd5b6e0b047d9945bc6e48fed0e17f Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 5 Aug 2015 16:58:54 +1000 Subject: [PATCH 2/4] Add first cut of LettuceShare report --- .../admin/reports_controller_decorator.rb | 10 ++- lib/open_food_network/lettuce_share_report.rb | 68 ++++++++++++++++ .../products_and_inventory_report.rb | 77 +------------------ .../products_and_inventory_report_base.rb | 76 ++++++++++++++++++ spec/features/admin/reports_spec.rb | 41 ++++++---- 5 files changed, 182 insertions(+), 90 deletions(-) create mode 100644 lib/open_food_network/lettuce_share_report.rb create mode 100644 lib/open_food_network/products_and_inventory_report_base.rb diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index a597bce934..50762a97e1 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -1,6 +1,7 @@ require 'csv' require 'open_food_network/order_and_distributor_report' require 'open_food_network/products_and_inventory_report' +require 'open_food_network/lettuce_share_report' require 'open_food_network/group_buy_report' require 'open_food_network/order_grouper' require 'open_food_network/customers_report' @@ -26,7 +27,8 @@ Spree::Admin::ReportsController.class_eval do ], products_and_inventory: [ ['All products', :all_products], - ['Inventory (on hand)', :inventory] + ['Inventory (on hand)', :inventory], + ['LettuceShare', :lettuce_share] ], customers: [ ["Mailing List", :mailing_list], @@ -212,7 +214,11 @@ Spree::Admin::ReportsController.class_eval do def products_and_inventory @report_types = REPORT_TYPES[:products_and_inventory] - @report = OpenFoodNetwork::ProductsAndInventoryReport.new spree_current_user, params + if params[:report_type] != 'lettuce_share' + @report = OpenFoodNetwork::ProductsAndInventoryReport.new spree_current_user, params + else + @report = OpenFoodNetwork::LettuceShareReport.new spree_current_user, params + end render_report(@report.header, @report.table, params[:csv], "products_and_inventory_#{timestamp}.csv") end diff --git a/lib/open_food_network/lettuce_share_report.rb b/lib/open_food_network/lettuce_share_report.rb new file mode 100644 index 0000000000..b847245f47 --- /dev/null +++ b/lib/open_food_network/lettuce_share_report.rb @@ -0,0 +1,68 @@ +require 'open_food_network/products_and_inventory_report_base' + +module OpenFoodNetwork + class LettuceShareReport < ProductsAndInventoryReportBase + def header + [ + "PRODUCT", + "Description", + "Qty", + "Pack Size", + "Unit", + "Unit Price", + "Total", + "GST incl.", + "Grower and growing method", + '', + '' + ] + end + + def table + variants.map do |variant| + [ + variant.product.name, + variant.full_name, + '', + OptionValueNamer.new(variant).value, + OptionValueNamer.new(variant).unit, + variant.price - gst(variant), + variant.price, + gst(variant), + variant.product.supplier.name, + certification(variant), + variant.product.primary_taxon.name + ] + end + end + + + private + + def gst(variant) + tax_category = variant.product.tax_category + if tax_category + tax_rate = tax_category.tax_rates.first + line_item = mock_line_item(variant, tax_category) + tax_rate.calculator.compute line_item + else + 0 + end + end + + def mock_line_item(variant, tax_category) + product = OpenStruct.new tax_category: tax_category + line_item = Spree::LineItem.new quantity: 1 + line_item.define_singleton_method(:product) { variant.product } + line_item.define_singleton_method(:price) { variant.price } + line_item + end + + def certification(variant) + variant.product.properties_including_inherited.map do |p| + "#{p[:name]} (#{p[:value]})" + end.join(', ') + end + + end +end diff --git a/lib/open_food_network/products_and_inventory_report.rb b/lib/open_food_network/products_and_inventory_report.rb index 7d695f4345..39109cb104 100644 --- a/lib/open_food_network/products_and_inventory_report.rb +++ b/lib/open_food_network/products_and_inventory_report.rb @@ -1,13 +1,7 @@ +require 'open_food_network/products_and_inventory_report_base' + module OpenFoodNetwork - - class ProductsAndInventoryReport - attr_reader :params - - def initialize(user, params = {}) - @user = user - @params = params - end - + class ProductsAndInventoryReport < ProductsAndInventoryReportBase def header [ "Supplier", @@ -38,70 +32,5 @@ module OpenFoodNetwork end end - def permissions - @permissions ||= OpenFoodNetwork::Permissions.new(@user) - end - - def visible_products - @visible_products ||= permissions.visible_products - end - - def variants - filter(child_variants) - end - - def child_variants - Spree::Variant. - where(is_master: false). - joins(:product). - merge(visible_products). - order('spree_products.name') - end - - def filter(variants) - # NOTE: Ordering matters. - # filter_to_order_cycle and filter_to_distributor return arrays not relations - filter_to_distributor filter_to_order_cycle filter_on_hand filter_to_supplier filter_not_deleted variants - end - - def filter_not_deleted(variants) - variants.not_deleted - end - - def filter_on_hand(variants) - if params[:report_type] == 'inventory' - variants.where('spree_variants.count_on_hand > 0') - else - variants - end - end - - def filter_to_supplier(variants) - if params[:supplier_id].to_i > 0 - variants.where("spree_products.supplier_id = ?", params[:supplier_id]) - else - variants - end - end - - def filter_to_distributor(variants) - if params[:distributor_id].to_i > 0 - distributor = Enterprise.find params[:distributor_id] - variants.select do |v| - Enterprise.distributing_product(v.product_id).include? distributor - end - else - variants - end - end - - def filter_to_order_cycle(variants) - if params[:order_cycle_id].to_i > 0 - order_cycle = OrderCycle.find params[:order_cycle_id] - variants.select { |v| order_cycle.variants.include? v } - else - variants - end - end end end diff --git a/lib/open_food_network/products_and_inventory_report_base.rb b/lib/open_food_network/products_and_inventory_report_base.rb new file mode 100644 index 0000000000..df5b656848 --- /dev/null +++ b/lib/open_food_network/products_and_inventory_report_base.rb @@ -0,0 +1,76 @@ +module OpenFoodNetwork + class ProductsAndInventoryReportBase + attr_reader :params + + def initialize(user, params = {}) + @user = user + @params = params + end + + def permissions + @permissions ||= OpenFoodNetwork::Permissions.new(@user) + end + + def visible_products + @visible_products ||= permissions.visible_products + end + + def variants + filter(child_variants) + end + + def child_variants + Spree::Variant. + where(is_master: false). + joins(:product). + merge(visible_products). + order('spree_products.name') + end + + def filter(variants) + # NOTE: Ordering matters. + # filter_to_order_cycle and filter_to_distributor return arrays not relations + filter_to_distributor filter_to_order_cycle filter_on_hand filter_to_supplier filter_not_deleted variants + end + + def filter_not_deleted(variants) + variants.not_deleted + end + + def filter_on_hand(variants) + if params[:report_type] == 'inventory' + variants.where('spree_variants.count_on_hand > 0') + else + variants + end + end + + def filter_to_supplier(variants) + if params[:supplier_id].to_i > 0 + variants.where("spree_products.supplier_id = ?", params[:supplier_id]) + else + variants + end + end + + def filter_to_distributor(variants) + if params[:distributor_id].to_i > 0 + distributor = Enterprise.find params[:distributor_id] + variants.select do |v| + Enterprise.distributing_product(v.product_id).include? distributor + end + else + variants + end + end + + def filter_to_order_cycle(variants) + if params[:order_cycle_id].to_i > 0 + order_cycle = OrderCycle.find params[:order_cycle_id] + variants.select { |v| order_cycle.variants.include? v } + else + variants + end + end + end +end diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index 503a52762e..006544ca87 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -283,18 +283,27 @@ feature %q{ end describe "products and inventory report" do - it "shows products and inventory report" do - product1 = create(:simple_product, name: "Product Name", price: 100) - variant1 = product1.variants.first - variant2 = create(:variant, product: product1, price: 80.0) - product2 = create(:simple_product, name: "Product 2", price: 99.0, variant_unit: 'weight', variant_unit_scale: 1, unit_value: '100') - variant3 = product2.variants.first + let(:supplier) { create(:supplier_enterprise, name: 'Supplier Name') } + let(:taxon) { create(:taxon, name: 'Taxon Name') } + let(:product1) { create(:simple_product, name: "Product Name", price: 100, supplier: supplier, primary_taxon: taxon) } + let(:product2) { create(:simple_product, name: "Product 2", price: 99.0, variant_unit: 'weight', variant_unit_scale: 1, unit_value: '100', supplier: supplier, primary_taxon: taxon) } + let(:variant1) { product1.variants.first } + let(:variant2) { create(:variant, product: product1, price: 80.0) } + let(:variant3) { product2.variants.first } + + before do + product1.set_property 'Organic', 'NASAA 12345' + product2.set_property 'Organic', 'NASAA 12345' + product1.taxons = [taxon] + product2.taxons = [taxon] variant1.update_column(:count_on_hand, 10) variant2.update_column(:count_on_hand, 20) variant3.update_column(:count_on_hand, 9) variant1.option_values = [create(:option_value, :presentation => "Test")] variant2.option_values = [create(:option_value, :presentation => "Something")] + end + it "shows products and inventory report" do login_to_admin_section click_link 'Reports' @@ -303,15 +312,19 @@ feature %q{ click_link 'Products & Inventory' page.should have_content "Supplier" - rows = find("table#listing_products").all("tr") - table = rows.map { |r| r.all("th,td").map { |c| c.text.strip } } + page.should have_table_row ["Supplier", "Producer Suburb", "Product", "Product Properties", "Taxons", "Variant Value", "Price", "Group Buy Unit Quantity", "Amount"] + page.should have_table_row [product1.supplier.name, product1.supplier.address.city, "Product Name", product1.properties.map(&:presentation).join(", "), product1.primary_taxon.name, "Test", "100.0", product1.group_buy_unit_size.to_s, ""] + page.should have_table_row [product1.supplier.name, product1.supplier.address.city, "Product Name", product1.properties.map(&:presentation).join(", "), product1.primary_taxon.name, "Something", "80.0", product1.group_buy_unit_size.to_s, ""] + page.should have_table_row [product2.supplier.name, product1.supplier.address.city, "Product 2", product1.properties.map(&:presentation).join(", "), product2.primary_taxon.name, "100g", "99.0", product1.group_buy_unit_size.to_s, ""] + end - table.sort.should == [ - ["Supplier", "Producer Suburb", "Product", "Product Properties", "Taxons", "Variant Value", "Price", "Group Buy Unit Quantity", "Amount"], - [product1.supplier.name, product1.supplier.address.city, "Product Name", product1.properties.join(", "), product1.primary_taxon.name, "Test", "100.0", product1.group_buy_unit_size.to_s, ""], - [product1.supplier.name, product1.supplier.address.city, "Product Name", product1.properties.join(", "), product1.primary_taxon.name, "Something", "80.0", product1.group_buy_unit_size.to_s, ""], - [product2.supplier.name, product1.supplier.address.city, "Product 2", product1.properties.join(", "), product2.primary_taxon.name, "100g", "99.0", product1.group_buy_unit_size.to_s, ""] - ].sort + it "shows the LettuceShare report" do + login_to_admin_section + click_link 'Reports' + click_link 'LettuceShare' + + page.should have_table_row ['PRODUCT', 'Description', 'Qty', 'Pack Size', 'Unit', 'Unit Price', 'Total', 'GST incl.', 'Grower and growing method', '', ''] + page.should have_table_row ['Product 2', '100g', '', '100', 'g', '99.0', '99.0', '0', 'Supplier Name', 'Organic (NASAA 12345)', 'Taxon Name'] end end From 3eea002a0c929ca54d8a0df7defce81cfe9263c6 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 7 Aug 2015 11:06:16 +1000 Subject: [PATCH 3/4] Put rspec-retry on flaky specs --- spec/features/consumer/authentication_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/consumer/authentication_spec.rb b/spec/features/consumer/authentication_spec.rb index bfa9f141fe..b01b04fe84 100644 --- a/spec/features/consumer/authentication_spec.rb +++ b/spec/features/consumer/authentication_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature "Authentication", js: true do +feature "Authentication", js: true, retry: 3 do include UIComponentHelper # Attempt to address intermittent failures in these specs From 3e5028b6b9e60e741044ed70a3046bd28875f2eb Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 16 Aug 2015 11:06:52 +0800 Subject: [PATCH 4/4] Revise lettuceshare report - combine grower and cert cols, add column headings --- lib/open_food_network/lettuce_share_report.rb | 20 +++++++++++---- spec/features/admin/reports_spec.rb | 4 +-- .../lettuce_share_report_spec.rb | 25 +++++++++++++++++++ 3 files changed, 42 insertions(+), 7 deletions(-) create mode 100644 spec/lib/open_food_network/lettuce_share_report_spec.rb diff --git a/lib/open_food_network/lettuce_share_report.rb b/lib/open_food_network/lettuce_share_report.rb index b847245f47..b990d67866 100644 --- a/lib/open_food_network/lettuce_share_report.rb +++ b/lib/open_food_network/lettuce_share_report.rb @@ -13,8 +13,7 @@ module OpenFoodNetwork "Total", "GST incl.", "Grower and growing method", - '', - '' + "Taxon" ] end @@ -29,8 +28,7 @@ module OpenFoodNetwork variant.price - gst(variant), variant.price, gst(variant), - variant.product.supplier.name, - certification(variant), + grower_and_method(variant), variant.product.primary_taxon.name ] end @@ -58,9 +56,21 @@ module OpenFoodNetwork line_item end + def grower_and_method(variant) + cert = certification(variant) + + result = producer_name(variant) + result += " (#{cert})" if cert.present? + result + end + + def producer_name(variant) + variant.product.supplier.name + end + def certification(variant) variant.product.properties_including_inherited.map do |p| - "#{p[:name]} (#{p[:value]})" + "#{p[:name]} - #{p[:value]}" end.join(', ') end diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index 006544ca87..b4b5a2318f 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -323,8 +323,8 @@ feature %q{ click_link 'Reports' click_link 'LettuceShare' - page.should have_table_row ['PRODUCT', 'Description', 'Qty', 'Pack Size', 'Unit', 'Unit Price', 'Total', 'GST incl.', 'Grower and growing method', '', ''] - page.should have_table_row ['Product 2', '100g', '', '100', 'g', '99.0', '99.0', '0', 'Supplier Name', 'Organic (NASAA 12345)', 'Taxon Name'] + page.should have_table_row ['PRODUCT', 'Description', 'Qty', 'Pack Size', 'Unit', 'Unit Price', 'Total', 'GST incl.', 'Grower and growing method', 'Taxon'] + page.should have_table_row ['Product 2', '100g', '', '100', 'g', '99.0', '99.0', '0', 'Supplier Name (Organic - NASAA 12345)', 'Taxon Name'] end end diff --git a/spec/lib/open_food_network/lettuce_share_report_spec.rb b/spec/lib/open_food_network/lettuce_share_report_spec.rb new file mode 100644 index 0000000000..121cbf9e9a --- /dev/null +++ b/spec/lib/open_food_network/lettuce_share_report_spec.rb @@ -0,0 +1,25 @@ +require 'open_food_network/lettuce_share_report' + +module OpenFoodNetwork + describe LettuceShareReport do + let(:user) { create(:user) } + let(:report) { LettuceShareReport.new user } + let(:v) { create(:variant) } + + describe "grower and method" do + it "shows just the producer when there is no certification" do + report.stub(:producer_name) { "Producer" } + report.stub(:certification) { "" } + + report.send(:grower_and_method, v).should == "Producer" + end + + it "shows producer and certification when a certification is present" do + report.stub(:producer_name) { "Producer" } + report.stub(:certification) { "Method" } + + report.send(:grower_and_method, v).should == "Producer (Method)" + end + end + end +end