diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 659745078fd..937d2ff90a7 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -317,7 +317,6 @@ Metrics/LineLength: - spec/lib/open_food_network/order_cycle_form_applicator_spec.rb - spec/lib/open_food_network/order_cycle_permissions_spec.rb - spec/lib/open_food_network/order_grouper_spec.rb - - spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb - spec/lib/open_food_network/packing_report_spec.rb - spec/lib/open_food_network/permissions_spec.rb - spec/lib/open_food_network/products_and_inventory_report_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index b0c7a074571..3df9808e333 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -736,7 +736,6 @@ Layout/SpaceInsideBlockBraces: - 'spec/jobs/update_billable_periods_spec.rb' - 'spec/lib/open_food_network/order_cycle_form_applicator_spec.rb' - 'spec/lib/open_food_network/order_grouper_spec.rb' - - 'spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb' - 'spec/lib/open_food_network/products_renderer_spec.rb' - 'spec/lib/open_food_network/tag_rule_applicator_spec.rb' - 'spec/models/column_preference_spec.rb' @@ -1077,7 +1076,6 @@ Lint/Void: - 'spec/lib/open_food_network/enterprise_fee_calculator_spec.rb' - 'spec/lib/open_food_network/enterprise_issue_validator_spec.rb' - 'spec/lib/open_food_network/group_buy_report_spec.rb' - - 'spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb' - 'spec/lib/open_food_network/packing_report_spec.rb' - 'spec/lib/open_food_network/reports/report_spec.rb' - 'spec/lib/open_food_network/reports/rule_spec.rb' @@ -2000,7 +1998,6 @@ Style/MixinUsage: - 'spec/features/admin/orders_spec.rb' - 'spec/lib/open_food_network/bulk_coop_report_spec.rb' - 'spec/lib/open_food_network/order_cycle_management_report_spec.rb' - - 'spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb' - 'spec/lib/open_food_network/packing_report_spec.rb' # Offense count: 4 diff --git a/app/models/concerns/address_display.rb b/app/models/concerns/address_display.rb new file mode 100644 index 00000000000..c585087e18d --- /dev/null +++ b/app/models/concerns/address_display.rb @@ -0,0 +1,5 @@ +module AddressDisplay + def full_name_reverse + [lastname, firstname].reject(&:blank?).join(" ") + end +end diff --git a/app/models/spree/address_decorator.rb b/app/models/spree/address_decorator.rb index ff7c1f06659..603423e8d65 100644 --- a/app/models/spree/address_decorator.rb +++ b/app/models/spree/address_decorator.rb @@ -1,4 +1,6 @@ Spree::Address.class_eval do + include AddressDisplay + has_one :enterprise, dependent: :restrict belongs_to :country, class_name: "Spree::Country" diff --git a/lib/open_food_network/orders_and_fulfillments_report.rb b/lib/open_food_network/orders_and_fulfillments_report.rb index eabdbc6c5f9..2f766bd640d 100644 --- a/lib/open_food_network/orders_and_fulfillments_report.rb +++ b/lib/open_food_network/orders_and_fulfillments_report.rb @@ -102,55 +102,57 @@ def rules sort_by: proc { |full_name| full_name } } ] when "order_cycle_customer_totals" [ { group_by: proc { |line_item| line_item.order.distributor }, - sort_by: proc { |distributor| distributor.name } }, + sort_by: proc { |distributor| distributor.name } }, { group_by: proc { |line_item| line_item.order }, - sort_by: proc { |order| order.bill_address.lastname + " " + order.bill_address.firstname }, - summary_columns: [ - proc { |line_items| line_items.first.order.distributor.name }, - proc { |line_items| line_items.first.order.bill_address.firstname + " " + line_items.first.order.bill_address.lastname }, - proc { |line_items| "" }, - proc { |line_items| "" }, - proc { |line_items| "" }, - proc { |line_items| I18n.t('admin.reports.total') }, - proc { |line_items| "" }, + sort_by: proc { |order| order.bill_address.full_name_reverse }, + summary_columns: [ + proc { |line_items| line_items.first.order.distributor.name }, + proc { |line_items| line_items.first.order.bill_address.full_name }, + proc { |line_items| "" }, + proc { |line_items| "" }, + proc { |line_items| "" }, + proc { |line_items| I18n.t('admin.reports.total') }, + proc { |line_items| "" }, - proc { |line_items| "" }, - proc { |line_items| line_items.sum { |li| li.amount } }, - proc { |line_items| line_items.sum { |li| li.amount_with_adjustments } }, - proc { |line_items| line_items.map { |li| li.order }.uniq.sum { |o| o.admin_and_handling_total } }, - proc { |line_items| line_items.map { |li| li.order }.uniq.sum { |o| o.ship_total } }, - proc { |line_items| line_items.map { |li| li.order }.uniq.sum { |o| o.payment_fee } }, - proc { |line_items| line_items.map { |li| li.order }.uniq.sum { |o| o.total } }, - proc { |line_items| line_items.all? { |li| li.order.paid? } ? I18n.t(:yes) : I18n.t(:no) }, + proc { |line_items| "" }, + proc { |line_items| line_items.sum(&:amount) }, + proc { |line_items| line_items.sum(&:amount_with_adjustments) }, + proc { |line_items| line_items.first.order.admin_and_handling_total }, + proc { |line_items| line_items.first.order.ship_total }, + proc { |line_items| line_items.first.order.payment_fee }, + proc { |line_items| line_items.first.order.total }, + proc { |line_items| line_items.first.order.paid? ? I18n.t(:yes) : I18n.t(:no) }, - proc { |line_items| "" }, - proc { |line_items| "" }, + proc { |line_items| "" }, + proc { |line_items| "" }, - proc { |line_items| "" }, - proc { |line_items| "" }, - proc { |line_items| "" }, - proc { |line_items| "" }, - proc { |line_items| "" }, - - proc { |line_items| line_items.first.order.special_instructions } , - proc { |line_items| "" }, - - proc { |line_items| line_items.first.order.order_cycle.andand.name }, - proc { |line_items| line_items.first.order.payments.first.andand.payment_method.andand.name }, - proc { |line_items| "" }, - proc { |line_items| "" }, + proc { |line_items| "" }, + proc { |line_items| "" }, + proc { |line_items| "" }, + proc { |line_items| "" }, + proc { |line_items| "" }, - proc { |line_items| "" }, - proc { |line_items| "" }, - proc { |line_items| "" }, - proc { |line_items| "" }, - proc { |line_items| "" } - ] }, + proc { |line_items| line_items.first.order.special_instructions } , + proc { |line_items| "" }, + proc { |line_items| line_items.first.order.order_cycle.andand.name }, + proc { |line_items| + line_items.first.order.payments.first.andand.payment_method.andand.name + }, + proc { |line_items| "" }, + proc { |line_items| "" }, + proc { |line_items| "" }, + proc { |line_items| "" }, + proc { |line_items| "" }, + proc { |line_items| "" }, + proc { |line_items| "" } + ] }, { group_by: proc { |line_item| line_item.product }, - sort_by: proc { |product| product.name } }, + sort_by: proc { |product| product.name } }, + { group_by: proc { |line_item| line_item.variant }, + sort_by: proc { |variant| variant.full_name } }, { group_by: proc { |line_item| line_item.full_name }, - sort_by: proc { |full_name| full_name } } ] + sort_by: proc { |full_name| full_name } } ] else [ { group_by: proc { |line_item| line_item.product.supplier }, sort_by: proc { |supplier| supplier.name } }, @@ -222,7 +224,7 @@ def columns proc { |line_items| line_items.first.order.ship_address.andand.state if rsa.call(line_items) }, proc { |line_items| "" }, - proc { |line_items| line_items.first.product.sku }, + proc { |line_items| line_items.first.variant.sku }, proc { |line_items| line_items.first.order.order_cycle.andand.name }, proc { |line_items| line_items.first.order.payments.first.andand.payment_method.andand.name }, diff --git a/spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb b/spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb index a33722859cd..e8ab9270bc2 100644 --- a/spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb +++ b/spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb @@ -1,116 +1,242 @@ require 'spec_helper' -include AuthenticationWorkflow - -module OpenFoodNetwork - describe OrdersAndFulfillmentsReport do - describe "fetching orders" do - let(:d1) { create(:distributor_enterprise) } - let(:oc1) { create(:simple_order_cycle) } - let(:o1) { create(:order, completed_at: 1.day.ago, order_cycle: oc1, distributor: d1) } - let(:li1) { build(:line_item) } - - before { o1.line_items << li1 } - - context "as a site admin" do - let(:user) { create(:admin_user) } - subject { PackingReport.new user, {}, true } - - it "fetches completed orders" do - o2 = create(:order) - o2.line_items << build(:line_item) - subject.table_items.should == [li1] - end +describe OpenFoodNetwork::OrdersAndFulfillmentsReport do + include AuthenticationWorkflow + + let(:distributor) { create(:distributor_enterprise) } + let(:order_cycle) { create(:simple_order_cycle) } + let(:address) { create(:address) } + let(:order) { + create( + :order, + completed_at: 1.day.ago, + order_cycle: order_cycle, + distributor: distributor, + bill_address: address + ) + } + let(:line_item) { build(:line_item) } + let(:user) { create(:user) } + let(:admin_user) { create(:admin_user) } + + describe "fetching orders" do + before { order.line_items << line_item } + + context "as a site admin" do + subject { described_class.new admin_user, {}, true } + + it "fetches completed orders" do + o2 = create(:order) + o2.line_items << build(:line_item) + expect(subject.table_items).to eq([line_item]) + end - it "does not show cancelled orders" do - o2 = create(:order, state: "canceled", completed_at: 1.day.ago) - o2.line_items << build(:line_item) - subject.table_items.should == [li1] - end + it "does not show cancelled orders" do + o2 = create(:order, state: "canceled", completed_at: 1.day.ago) + o2.line_items << build(:line_item) + expect(subject.table_items).to eq([line_item]) end + end - context "as a manager of a supplier" do - let!(:user) { create(:user) } - subject { OrdersAndFulfillmentsReport.new user, {}, true } + context "as a manager of a supplier" do + subject { described_class.new user, {}, true } - let(:s1) { create(:supplier_enterprise) } + let(:s1) { create(:supplier_enterprise) } - before do - s1.enterprise_roles.create!(user: user) - end - - context "that has granted P-OC to the distributor" do - let(:o2) { create(:order, distributor: d1, completed_at: 1.day.ago, bill_address: create(:address), ship_address: create(:address)) } - let(:li2) { build(:line_item, product: create(:simple_product, supplier: s1)) } + before do + s1.enterprise_roles.create!(user: user) + end - before do - o2.line_items << li2 - create(:enterprise_relationship, parent: s1, child: d1, permissions_list: [:add_to_order_cycle]) - end + context "that has granted P-OC to the distributor" do + let(:o2) { + create( + :order, + distributor: distributor, + completed_at: 1.day.ago, + bill_address: create(:address), + ship_address: create(:address) + ) + } + let(:li2) { build(:line_item, product: create(:simple_product, supplier: s1)) } - it "shows line items supplied by my producers, with names hidden" do - subject.table_items.should == [li2] - subject.table_items.first.order.bill_address.firstname.should == "HIDDEN" - end + before do + o2.line_items << li2 + create( + :enterprise_relationship, + parent: s1, + child: distributor, + permissions_list: [:add_to_order_cycle] + ) end - context "that has not granted P-OC to the distributor" do - let(:o2) { create(:order, distributor: d1, completed_at: 1.day.ago, bill_address: create(:address), ship_address: create(:address)) } - let(:li2) { build(:line_item, product: create(:simple_product, supplier: s1)) } - - before do - o2.line_items << li2 - end - - it "shows line items supplied by my producers, with names hidden" do - subject.table_items.should == [] - end + it "shows line items supplied by my producers, with names hidden" do + expect(subject.table_items).to eq([li2]) + expect(subject.table_items.first.order.bill_address.firstname).to eq("HIDDEN") end end - context "as a manager of a distributor" do - let!(:user) { create(:user) } - subject { OrdersAndFulfillmentsReport.new user, {}, true } + context "that has not granted P-OC to the distributor" do + let(:o2) { + create( + :order, + distributor: distributor, + completed_at: 1.day.ago, + bill_address: create(:address), + ship_address: create(:address) + ) + } + let(:li2) { build(:line_item, product: create(:simple_product, supplier: s1)) } before do - d1.enterprise_roles.create!(user: user) + o2.line_items << li2 end - it "only shows line items distributed by enterprises managed by the current user" do - d2 = create(:distributor_enterprise) - d2.enterprise_roles.create!(user: create(:user)) - o2 = create(:order, distributor: d2, completed_at: 1.day.ago) - o2.line_items << build(:line_item) - subject.table_items.should == [li1] + it "shows line items supplied by my producers, with names hidden" do + expect(subject.table_items).to eq([]) end + end + end - it "only shows the selected order cycle" do - oc2 = create(:simple_order_cycle) - o2 = create(:order, distributor: d1, order_cycle: oc2) - o2.line_items << build(:line_item) - subject.stub(:params).and_return(order_cycle_id_in: oc1.id) - subject.table_items.should == [li1] - end + context "as a manager of a distributor" do + subject { described_class.new user, {}, true } + + before do + distributor.enterprise_roles.create!(user: user) + end + + it "only shows line items distributed by enterprises managed by the current user" do + d2 = create(:distributor_enterprise) + d2.enterprise_roles.create!(user: create(:user)) + o2 = create(:order, distributor: d2, completed_at: 1.day.ago) + o2.line_items << build(:line_item) + expect(subject.table_items).to eq([line_item]) + end + + it "only shows the selected order cycle" do + oc2 = create(:simple_order_cycle) + o2 = create(:order, distributor: distributor, order_cycle: oc2) + o2.line_items << build(:line_item) + allow(subject).to receive(:params).and_return(order_cycle_id_in: order_cycle.id) + expect(subject.table_items).to eq([line_item]) end end + end - describe "columns are aligned" do - let(:d1) { create(:distributor_enterprise) } - let(:oc1) { create(:simple_order_cycle) } - let(:o1) { create(:order, completed_at: 1.day.ago, order_cycle: oc1, distributor: d1) } - let(:li1) { build(:line_item) } - let(:user) { create(:admin_user)} + describe "columns are aligned" do + it 'has aligned columsn' do + report_types = [ + "", + "order_cycle_supplier_totals", + "order_cycle_supplier_totals_by_distributor", + "order_cycle_distributor_totals_by_supplier", + "order_cycle_customer_totals" + ] + + report_types.each do |report_type| + report = described_class.new admin_user, report_type: report_type + expect(report.header.size).to eq(report.columns.size) + end + end + end - before { o1.line_items << li1 } + describe "order_cycle_customer_totals" do + let!(:product) { line_item.product } + let!(:fuji) { build(:variant, product: product, display_name: "Fuji", sku: "FUJI", on_hand: 100) } + let!(:gala) { build(:variant, product: product, display_name: "Gala", sku: "GALA", on_hand: 100) } - it 'has aligned columsn' do - report_types = ["", "order_cycle_supplier_totals", "order_cycle_supplier_totals_by_distributor", "order_cycle_distributor_totals_by_supplier", "order_cycle_customer_totals"] + let(:items) { + report = described_class.new(admin_user, { report_type: "order_cycle_customer_totals" }, true) + OpenFoodNetwork::OrderGrouper.new(report.rules, report.columns).table(report.table_items) + } - report_types.each do |report_type| - report = OrdersAndFulfillmentsReport.new user, report_type: report_type - report.header.size.should == report.columns.size - end + before do + # Clear price so it will be computed based on quantity and variant price. + order.line_items << build(:line_item, variant: fuji, price: nil, quantity: 1) + order.line_items << build(:line_item, variant: gala, price: nil, quantity: 2) + end + + it "has a product row" do + product_name_field = items.first[5] + expect(product_name_field).to eq product.name + end + + it "has a summary row" do + product_name_field = items.last[5] + expect(product_name_field).to eq "TOTAL" + end + + # Expected Report for Scenario: + # + # Row 1: Armstrong Amari, Fuji Apple, price: 8 + # Row 2: SUMMARY + # Row 3: Bartoletti Brooklyn, Fuji Apple, price: 1 + 4 + # Row 4: Bartoletti Brooklyn, Gala Apple, price: 2 + # Row 5: SUMMARY + describe "grouping of line items" do + let!(:address) { create(:address, last_name: "Bartoletti", first_name: "Brooklyn") } + + let!(:second_address) { create(:address, last_name: "Armstrong", first_name: "Amari") } + let!(:second_order) do + create(:order, completed_at: 1.day.ago, order_cycle: order_cycle, distributor: distributor, + bill_address: second_address) + end + + before do + # Add a second line item for Fuji variant to the order, to test grouping in this edge case. + order.line_items << build(:line_item, variant: fuji, price: nil, quantity: 4) + + second_order.line_items << build(:line_item, variant: fuji, price: nil, quantity: 8) end + + it "groups line items by variant and order" do + expect(items.length).to eq(5) + + # Row 1: Armstrong Amari, Fuji Apple, price: 8 + row_data = items[0] + expect(customer_name(row_data)).to eq(second_address.full_name) + expect(amount(row_data)).to eq(fuji.price * 8) + expect(variant_sku(row_data)).to eq(fuji.sku) + + # Row 2: SUMMARY + row_data = items[1] + expect(totals_row?(row_data)).to eq(true) + expect(customer_name(row_data)).to eq(second_address.full_name) + expect(amount(row_data)).to eq(fuji.price * 8) + + # Row 3: Bartoletti Brooklyn, Fuji Apple, price: 1 + 4 + row_data = items[2] + expect(customer_name(row_data)).to eq(address.full_name) + expect(amount(row_data)).to eq(fuji.price * 5) + expect(variant_sku(row_data)).to eq(fuji.sku) + + # Row 4: Bartoletti Brooklyn, Gala Apple, price: 2 + row_data = items[3] + expect(customer_name(row_data)).to eq(address.full_name) + expect(amount(row_data)).to eq(gala.price * 2) + expect(variant_sku(row_data)).to eq(gala.sku) + + # Row 5: SUMMARY + row_data = items[4] + expect(totals_row?(row_data)).to eq(true) + expect(customer_name(row_data)).to eq(address.full_name) + expect(amount(row_data)).to eq(fuji.price * 5 + gala.price * 2) + end + end + + def totals_row?(row_data) + row_data[5] == I18n.t("admin.reports.total") + end + + def customer_name(row_data) + row_data[1] + end + + def amount(row_data) + row_data[8] + end + + def variant_sku(row_data) + row_data[23] end end end diff --git a/spec/models/spree/addresses_spec.rb b/spec/models/spree/addresses_spec.rb index 5d78cadcf2b..f895db91c70 100644 --- a/spec/models/spree/addresses_spec.rb +++ b/spec/models/spree/addresses_spec.rb @@ -24,6 +24,26 @@ end end + describe "#full_name_reverse" do + it "joins last name and first name" do + address.firstname = "Jane" + address.lastname = "Doe" + expect(address.full_name_reverse).to eq("Doe Jane") + end + + it "is last name when first name is blank" do + address.firstname = "" + address.lastname = "Doe" + expect(address.full_name_reverse).to eq("Doe") + end + + it "is first name when last name is blank" do + address.firstname = "Jane" + address.lastname = "" + expect(address.full_name_reverse).to eq("Jane") + end + end + describe "geocode address" do it "should include address1, address2, zipcode, city, state and country" do expect(address.geocode_address).to include(address.address1)