Merge pull request #13298 from dacook/cleanup-address-finder

Cleanup AddressFinder

.rubocop_styleguide.yml

@@ -86,10 +86,6 @@ Metrics/MethodLength:
   Enabled: true
   Max: 25 # default 10
 
-Naming/MemoizedInstanceVariableName:
-  Exclude:
-    - 'lib/open_food_network/address_finder.rb'
-
 Metrics/ParameterLists:
   CountKeywordArgs: false
 

app/controllers/concerns/checkout_callbacks.rb

@@ -48,7 +48,8 @@ module CheckoutCallbacks
   end
 
   def load_saved_addresses
-    finder = OpenFoodNetwork::AddressFinder.new(@order.email, @order.customer, spree_current_user)
+    finder = OpenFoodNetwork::AddressFinder.new(email: @order.email, customer: @order.customer,
+                                                user: spree_current_user)
 
     @order.bill_address ||= finder.bill_address
     @order.ship_address ||= finder.ship_address

app/serializers/api/admin/subscription_customer_serializer.rb

@@ -10,7 +10,7 @@ module Api
       delegate :ship_address, to: :finder
 
       def finder
-        @finder ||= OpenFoodNetwork::AddressFinder.new(object, object.email)
+        @finder ||= OpenFoodNetwork::AddressFinder.new(customer: object, email: object.email)
       end
     end
   end

app/serializers/api/admin/user_serializer.rb

@@ -11,11 +11,11 @@ module Api
       has_one :bill_address, serializer: Api::AddressSerializer
 
       def ship_address
-        OpenFoodNetwork::AddressFinder.new(object.email, object).ship_address
+        OpenFoodNetwork::AddressFinder.new(email: object.email, user: object).ship_address
       end
 
       def bill_address
-        OpenFoodNetwork::AddressFinder.new(object.email, object).bill_address
+        OpenFoodNetwork::AddressFinder.new(email: object.email, user: object).bill_address
       end
 
       def confirmed

lib/open_food_network/address_finder.rb

@@ -10,13 +10,10 @@ module OpenFoodNetwork
   class AddressFinder
     attr_reader :email, :user, :customer
 
-    def initialize(*args)
-      args.each do |arg|
-        type = types[arg.class]
-        next unless type
-
-        public_send("#{type}=", arg)
-      end
+    def initialize(email: nil, user: nil, customer: nil)
+      @email = email
+      @user = user
+      @customer = customer
     end
 
     def bill_address
@@ -27,28 +24,8 @@ module OpenFoodNetwork
       customer_preferred_ship_address || user_preferred_ship_address || fallback_ship_address
     end
 
-    def email=(arg)
-      @email ||= arg
-    end
-
-    def customer=(arg)
-      @customer ||= arg
-    end
-
-    def user=(arg)
-      @user ||= arg
-    end
-
     private
 
-    def types
-      {
-        String => "email",
-        Customer => "customer",
-        Spree::User => "user"
-      }
-    end
-
     def customer_preferred_bill_address
       customer&.bill_address
     end

spec/lib/open_food_network/address_finder_spec.rb

@@ -12,39 +12,18 @@ module OpenFoodNetwork
       let(:customer) { create(:customer) }
 
       context "when passed any combination of instances of String, Customer or Spree::User" do
-        let(:finder1) { AddressFinder.new(email, customer, user) }
-        let(:finder2) { AddressFinder.new(customer, user, email) }
+        let(:finder1) { AddressFinder.new(email:, customer:, user:) }
 
-        it "stores arguments based on their class" do
+        it "stores arguments" do
           expect(finder1.email).to eq email
-          expect(finder2.email).to eq email
           expect(finder1.customer).to be customer
-          expect(finder2.customer).to be customer
           expect(finder1.user).to be user
-          expect(finder2.user).to be user
-        end
-      end
-
-      context "when passed multiples instances of a class" do
-        let(:email2) { 'test2@example.com' }
-        let(:user2) { create(:user) }
-        let(:customer2) { create(:customer) }
-        let(:finder1) { AddressFinder.new(user2, email, email2, customer2, user, customer) }
-        let(:finder2) { AddressFinder.new(email2, customer, user, email, user2, customer2) }
-
-        it "only stores the first encountered instance of a given class" do
-          expect(finder1.email).to eq email
-          expect(finder2.email).to eq email2
-          expect(finder1.customer).to be customer2
-          expect(finder2.customer).to be customer
-          expect(finder1.user).to be user2
-          expect(finder2.user).to be user
         end
       end
     end
 
     describe "fallback_bill_address" do
-      let(:finder) { AddressFinder.new(email) }
+      let(:finder) { AddressFinder.new(email:) }
       let(:address) { double(:address, clone: 'address_clone') }
 
       context "when a last_used_bill_address is found" do
@@ -65,7 +44,7 @@ module OpenFoodNetwork
     end
 
     describe "fallback_ship_address" do
-      let(:finder) { AddressFinder.new(email) }
+      let(:finder) { AddressFinder.new(email:) }
       let(:address) { double(:address, clone: 'address_clone') }
 
       context "when a last_used_ship_address is found" do
@@ -92,7 +71,7 @@ module OpenFoodNetwork
         create(:completed_order_with_totals, user: nil, email:, distributor:,
                                              bill_address: nil)
       }
-      let(:finder) { AddressFinder.new(email) }
+      let(:finder) { AddressFinder.new(email:) }
 
       context "when searching by email is not allowed" do
         before do
@@ -142,7 +121,7 @@ module OpenFoodNetwork
     describe "last_used_ship_address" do
       let(:address) { create(:address) }
       let(:distributor) { create(:distributor_enterprise) }
-      let(:finder) { AddressFinder.new(email) }
+      let(:finder) { AddressFinder.new(email:) }
 
       context "when searching by email is not allowed" do
         before do