diff --git a/.rubocop_styleguide.yml b/.rubocop_styleguide.yml index 7f80e7d866..ead7f299be 100644 --- a/.rubocop_styleguide.yml +++ b/.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 diff --git a/app/controllers/concerns/checkout_callbacks.rb b/app/controllers/concerns/checkout_callbacks.rb index b6dbbfd6c1..bf6a23b860 100644 --- a/app/controllers/concerns/checkout_callbacks.rb +++ b/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 diff --git a/app/serializers/api/admin/subscription_customer_serializer.rb b/app/serializers/api/admin/subscription_customer_serializer.rb index 66e676aee4..16243067b2 100644 --- a/app/serializers/api/admin/subscription_customer_serializer.rb +++ b/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 diff --git a/app/serializers/api/admin/user_serializer.rb b/app/serializers/api/admin/user_serializer.rb index 462515d112..116397e245 100644 --- a/app/serializers/api/admin/user_serializer.rb +++ b/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 diff --git a/lib/open_food_network/address_finder.rb b/lib/open_food_network/address_finder.rb index 32975f9428..0e681bcb6f 100644 --- a/lib/open_food_network/address_finder.rb +++ b/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 diff --git a/spec/lib/open_food_network/address_finder_spec.rb b/spec/lib/open_food_network/address_finder_spec.rb index c7068eefe9..79462bb28b 100644 --- a/spec/lib/open_food_network/address_finder_spec.rb +++ b/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