Refactoring LastUsedAddress to take customers or users

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

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

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

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

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

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