diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index ee0bb1f77d..77ab2e6dd6 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -185,16 +185,10 @@ class CheckoutController < Spree::CheckoutController def before_address associate_user - lua = OpenFoodNetwork::LastUsedAddress.new(@order.email) - last_used_bill_address = lua.last_used_bill_address.andand.clone - last_used_ship_address = lua.last_used_ship_address.andand.clone + lua = OpenFoodNetwork::LastUsedAddress.new(@order.email, @order.customer, spree_current_user) - preferred_bill_address, preferred_ship_address = spree_current_user.bill_address, spree_current_user.ship_address if spree_current_user - - customer_preferred_bill_address, customer_preferred_ship_address = @order.customer.bill_address, @order.customer.ship_address if @order.customer - - @order.bill_address ||= customer_preferred_bill_address || preferred_bill_address || last_used_bill_address || Spree::Address.default - @order.ship_address ||= customer_preferred_ship_address || preferred_ship_address || last_used_ship_address || Spree::Address.default + @order.bill_address = lua.bill_address + @order.ship_address = lua.ship_address end # Overriding Spree's methods diff --git a/app/controllers/spree/checkout_controller_decorator.rb b/app/controllers/spree/checkout_controller_decorator.rb index e5146394d5..7ea930ef90 100644 --- a/app/controllers/spree/checkout_controller_decorator.rb +++ b/app/controllers/spree/checkout_controller_decorator.rb @@ -24,12 +24,8 @@ Spree::CheckoutController.class_eval do associate_user lua = OpenFoodNetwork::LastUsedAddress.new(@order.email) - last_used_bill_address = lua.last_used_bill_address.andand.clone - last_used_ship_address = lua.last_used_ship_address.andand.clone - preferred_bill_address, preferred_ship_address = spree_current_user.bill_address, spree_current_user.ship_address if spree_current_user.respond_to?(:bill_address) && spree_current_user.respond_to?(:ship_address) - - @order.bill_address ||= preferred_bill_address || last_used_bill_address || Spree::Address.default - @order.ship_address ||= preferred_ship_address || last_used_ship_address || nil + @order.bill_address = lua.bill_address + @order.ship_address = lua.ship_address end end diff --git a/app/serializers/api/admin/user_serializer.rb b/app/serializers/api/admin/user_serializer.rb index 50ed69433c..58a6fcc87e 100644 --- a/app/serializers/api/admin/user_serializer.rb +++ b/app/serializers/api/admin/user_serializer.rb @@ -7,13 +7,11 @@ class Api::Admin::UserSerializer < ActiveModel::Serializer has_one :bill_address, serializer: Api::AddressSerializer def ship_address - object.ship_address || - OpenFoodNetwork::LastUsedAddress.new(object.email).last_used_ship_address + OpenFoodNetwork::LastUsedAddress.new(object.email, object).ship_address end def bill_address - object.bill_address || - OpenFoodNetwork::LastUsedAddress.new(object.email).last_used_bill_address + OpenFoodNetwork::LastUsedAddress.new(object.email, object).bill_address end def confirmed diff --git a/lib/open_food_network/last_used_address.rb b/lib/open_food_network/last_used_address.rb index da987b4ab5..d04fcc15ec 100644 --- a/lib/open_food_network/last_used_address.rb +++ b/lib/open_food_network/last_used_address.rb @@ -1,28 +1,71 @@ +# Finds an address based on the data provided +# Can take any combination of an email String, Customer or Spree::User as args +# The #bill_address and #ship_address methods automatically return matched addresses +# according to this order: customer addresses, user addresses, addresses from +# completed orders with an email that matches the email string provided. module OpenFoodNetwork class LastUsedAddress - def initialize(email) - @email = email + attr_accessor :email, :user, :customer + + def initialize(*args) + args.each do |arg| + case arg + when String + @email = arg unless @email + when Customer + @customer = arg unless @customer + when Spree::User + @user = arg unless @user + end + end end - def last_used_bill_address - recent_orders.detect(&:bill_address).andand.bill_address + def bill_address + customer_preferred_bill_address || user_preferred_bill_address || fallback_bill_address end - def last_used_ship_address - recent_orders.detect { |order| - order.ship_address && order.shipping_method.andand.delivery? - }.andand.ship_address + def ship_address + customer_preferred_ship_address || user_preferred_ship_address || fallback_ship_address end - private - def recent_orders - Spree::Order. - order("id DESC"). - where(email: @email). - where("state != 'cart'"). - limit(8) + def customer_preferred_bill_address + customer.andand.bill_address + end + + def customer_preferred_ship_address + customer.andand.ship_address + end + + def user_preferred_bill_address + user.andand.bill_address + end + + def user_preferred_ship_address + user.andand.ship_address + end + + def fallback_bill_address + last_used_bill_address.andand.clone || Spree::Address.default + end + + def fallback_ship_address + last_used_ship_address.andand.clone || Spree::Address.default + end + + def last_used_bill_address + return nil unless email + Spree::Order.joins(:bill_address).order('id DESC') + .complete.where(email: email) + .first.andand.bill_address + end + + def last_used_ship_address + return nil unless email + Spree::Order.complete.joins(:ship_address, :shipping_method).order('id DESC') + .where(email: email, spree_shipping_methods: { require_ship_address: true }) + .first.andand.ship_address end end end diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index 1fbab2f2c1..ac2a50e411 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -100,7 +100,7 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do fill_out_form end - it "sets user's default billing address and shipping address" do + it "allows user to save default billing address and shipping address" do user.bill_address.should be_nil user.ship_address.should be_nil @@ -139,6 +139,26 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do end end + context "when the user has a preset shipping and billing address" do + before do + user.bill_address = build(:address) + user.ship_address = build(:address) + user.save! + end + + it "checks out successfully" do + visit checkout_path + choose sm2.name + toggle_payment + choose pm1.name + + expect do + place_order + page.should have_content "Your order has been processed successfully" + end.to enqueue_job ConfirmOrderJob + end + end + context "with Stripe" do let!(:stripe_pm) do create(:stripe_payment_method, @@ -477,30 +497,5 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do end end end - - context "when the customer has a pre-set shipping and billing address" do - before do - # Load up the customer's order and give them a shipping and billing address - # This is equivalent to when the customer has ordered before and their addresses - # are pre-populated. - o = Spree::Order.last - o.ship_address = build(:address) - o.bill_address = build(:address) - o.save! - end - - it "checks out successfully", retry: 3 do - visit checkout_path - checkout_as_guest - choose sm2.name - toggle_payment - choose pm1.name - - expect do - place_order - page.should have_content "Your order has been processed successfully" - end.to enqueue_job ConfirmOrderJob - end - end end end diff --git a/spec/lib/open_food_network/last_used_address_spec.rb b/spec/lib/open_food_network/last_used_address_spec.rb index 93e62c3d9f..44ff3ffb58 100644 --- a/spec/lib/open_food_network/last_used_address_spec.rb +++ b/spec/lib/open_food_network/last_used_address_spec.rb @@ -4,60 +4,189 @@ require 'open_food_network/last_used_address' module OpenFoodNetwork describe LastUsedAddress do let(:email) { 'test@example.com' } - let(:address) { 'address' } - describe "last used bill address" do - let(:lua) { LastUsedAddress.new(email) } - let(:order_with_bill_address) { double(:order, bill_address: address) } - let(:order_without_bill_address) { double(:order, bill_address: nil) } + describe "initialisation" do + let(:user) { create(:user) } + let(:customer) { create(:customer) } - it "returns the bill address when present" do - lua.stub(:recent_orders) { [order_with_bill_address] } - lua.last_used_bill_address.should == address + context "when passed any combination of instances of String, Customer or Spree::User" do + let(:lua1) { LastUsedAddress.new(email, customer, user) } + let(:lua2) { LastUsedAddress.new(customer, user, email) } + + it "stores arguments based on their class" do + expect(lua1.email).to eq email + expect(lua2.email).to eq email + expect(lua1.customer).to be customer + expect(lua2.customer).to be customer + expect(lua1.user).to be user + expect(lua2.user).to be user + end end - it "returns nil when there's no order with a bill address" do - lua.stub(:recent_orders) { [order_without_bill_address] } - lua.last_used_bill_address.should be_nil - end + context "when passed multiples instances of a class" do + let(:email2) { 'test2@example.com' } + let(:user2) { create(:user) } + let(:customer2) { create(:customer) } + let(:lua1) { LastUsedAddress.new(user2, email, email2, customer2, user, customer) } + let(:lua2) { LastUsedAddress.new(email2, customer, user, email, user2, customer2) } - it "returns nil when there are no recent orders" do - lua.stub(:recent_orders) { [] } - lua.last_used_bill_address.should be_nil + it "only stores the first encountered instance of a given class" do + expect(lua1.email).to eq email + expect(lua2.email).to eq email2 + expect(lua1.customer).to be customer2 + expect(lua2.customer).to be customer + expect(lua1.user).to be user2 + expect(lua2.user).to be user + end end end - describe "last used ship address" do + describe "fallback_bill_address" do let(:lua) { LastUsedAddress.new(email) } - let(:pickup) { double(:shipping_method, require_ship_address: false) } - let(:delivery) { double(:shipping_method, require_ship_address: true) } - let(:order_with_ship_address) { double(:order, ship_address: address, shipping_method: delivery) } - let(:order_with_unrequired_ship_address) { double(:order, ship_address: address, shipping_method: pickup) } - let(:order_without_ship_address) { double(:order, ship_address: nil) } + let(:address) { double(:address, clone: 'address_clone') } - it "returns the ship address when present" do - allow(delivery).to receive(:delivery?).and_return(true) - lua.stub(:recent_orders) { [order_with_ship_address] } - lua.last_used_ship_address.should == address + context "when a last_used_bill_address is found" do + before { allow(lua).to receive(:last_used_bill_address) { address } } + + it "returns a clone of the bill_address" do + expect(lua.send(:fallback_bill_address)).to eq "address_clone" + end end - it "returns nil when the order doesn't require a ship address" do - allow(order_with_unrequired_ship_address.shipping_method) - .to receive(:delivery?) - .and_return(false) + context "when no last_used_bill_address is found" do + before { allow(lua).to receive(:last_used_bill_address) { nil } } - lua.stub(:recent_orders) { [order_with_unrequired_ship_address] } - lua.last_used_ship_address.should be_nil + it "returns a new empty address" do + expect(lua.send(:fallback_bill_address)).to eq Spree::Address.default + end + end + end + + describe "fallback_ship_address" do + let(:lua) { LastUsedAddress.new(email) } + let(:address) { double(:address, clone: 'address_clone') } + + context "when a last_used_ship_address is found" do + before { allow(lua).to receive(:last_used_ship_address) { address } } + + it "returns a clone of the ship_address" do + expect(lua.send(:fallback_ship_address)).to eq "address_clone" + end end - it "returns nil when there's no order with a ship address" do - lua.stub(:recent_orders) { [order_without_ship_address] } - lua.last_used_ship_address.should be_nil + context "when no last_used_ship_address is found" do + before { allow(lua).to receive(:last_used_ship_address) { nil } } + + it "returns a new empty address" do + expect(lua.send(:fallback_ship_address)).to eq Spree::Address.default + end + end + end + + describe "last_used_bill_address" do + let(:distributor) { create(:distributor_enterprise) } + let(:address) { create(:address) } + let(:order) { create(:completed_order_with_totals, user: nil, email: email, distributor: distributor) } + + context "when an email has not been provided" do + let(:lua) { LastUsedAddress.new(nil) } + + context "and an order with a bill address exists" do + before do + order.update_attribute(:bill_address_id, address.id) + end + + it "returns nil" do + expect(lua.send(:last_used_bill_address)).to eq nil + end + end end - it "returns nil when there are no recent orders" do - lua.stub(:recent_orders) { [] } - lua.last_used_ship_address.should be_nil + context "when an email has been provided" do + let(:lua) { LastUsedAddress.new(email) } + + context "and an order with a bill address exists" do + before { order.update_attribute(:bill_address_id, address.id) } + + it "returns the bill_address" do + expect(lua.send(:last_used_bill_address)).to eq address + end + end + + context "and an order without a bill address exists" do + before { order } + + it "return nil" do + expect(lua.send(:last_used_bill_address)).to eq nil + end + end + + context "when no orders exist" do + it "returns nil" do + expect(lua.send(:last_used_bill_address)).to eq nil + end + end + end + end + + describe "last_used_ship_address" do + let(:address) { create(:address) } + let(:distributor) { create(:distributor_enterprise) } + let!(:pickup) { create(:shipping_method, require_ship_address: false, distributors: [distributor]) } + let!(:delivery) { create(:shipping_method, require_ship_address: true, distributors: [distributor]) } + let(:order) { create(:completed_order_with_totals, user: nil, email: email, distributor: distributor) } + + context "when an email has not been provided" do + let(:lua) { LastUsedAddress.new(nil) } + + context "and an order with a required ship address exists" do + before do + order.update_attribute(:ship_address_id, address.id) + order.update_attribute(:shipping_method_id, delivery.id) + end + + it "returns nil" do + expect(lua.send(:last_used_ship_address)).to eq nil + end + end + end + + context "when an email has been provided" do + let(:lua) { LastUsedAddress.new(email) } + + context "and an order with a ship address exists" do + before { order.update_attribute(:ship_address_id, address.id) } + + context "and the shipping method requires an address" do + before { order.update_attribute(:shipping_method_id, delivery.id) } + + it "returns the ship_address" do + expect(lua.send(:last_used_ship_address)).to eq address + end + end + + context "and the shipping method does not require an address" do + before { order.update_attribute(:shipping_method_id, pickup.id) } + + it "returns nil" do + expect(lua.send(:last_used_ship_address)).to eq nil + end + end + end + + context "and an order without a ship address exists" do + before { order } + + it "return nil" do + expect(lua.send(:last_used_ship_address)).to eq nil + end + end + + context "when no orders exist" do + it "returns nil" do + expect(lua.send(:last_used_ship_address)).to eq nil + end + end end end end