From 9fe775a90a9b5189bb901929bc3314083972e168 Mon Sep 17 00:00:00 2001 From: Jordan Brough Date: Tue, 27 Jun 2017 08:59:34 -0600 Subject: [PATCH 1/2] Make Address.find_all_by_name_or_abbr case-insensitive This is helpful when importing data from third parties. E.g. PayPal. --- core/app/models/spree/state.rb | 6 ++++- core/spec/models/spree/state_spec.rb | 35 ++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/core/app/models/spree/state.rb b/core/app/models/spree/state.rb index 384b4f20a72..3b79e6509cb 100644 --- a/core/app/models/spree/state.rb +++ b/core/app/models/spree/state.rb @@ -6,7 +6,11 @@ class State < Spree::Base validates :country, :name, presence: true def self.find_all_by_name_or_abbr(name_or_abbr) - where('name = ? OR abbr = ?', name_or_abbr, name_or_abbr) + where( + arel_table[:name].matches(name_or_abbr).or( + arel_table[:abbr].matches(name_or_abbr) + ) + ) end # table of { country.id => [ state.id , state.name ] }, arrays sorted by name diff --git a/core/spec/models/spree/state_spec.rb b/core/spec/models/spree/state_spec.rb index 42c9eb552d3..0bb0f70a3ed 100644 --- a/core/spec/models/spree/state_spec.rb +++ b/core/spec/models/spree/state_spec.rb @@ -1,10 +1,37 @@ require 'spec_helper' describe Spree::State, type: :model do - it "can find a state by name or abbr" do - state = create(:state, name: "California", abbr: "CA") - expect(Spree::State.find_all_by_name_or_abbr("California")).to include(state) - expect(Spree::State.find_all_by_name_or_abbr("CA")).to include(state) + describe '.find_all_by_name_or_abbr' do + subject do + Spree::State.find_all_by_name_or_abbr(search_term) + end + + let!(:state) { create(:state, name: "California", abbr: "CA") } + + context 'by invalid term' do + let(:search_term) { 'NonExistent' } + it { is_expected.to be_empty } + end + + context 'by name' do + let(:search_term) { 'California' } + it { is_expected.to include(state) } + end + + context 'by abbr' do + let(:search_term) { 'CA' } + it { is_expected.to include(state) } + end + + context 'by case-insensitive abbr' do + let(:search_term) { 'CaLiFoRnIa' } + it { is_expected.to include(state) } + end + + context 'by case-insensitive abbr' do + let(:search_term) { 'cA' } + it { is_expected.to include(state) } + end end it "can find all states group by country id" do From 67f775260969503ca97f96b8d59a4836704c5d7f Mon Sep 17 00:00:00 2001 From: Jordan Brough Date: Wed, 28 Jun 2017 12:46:23 -0600 Subject: [PATCH 2/2] Add `State.with_name_or_abbr` to replace `find_all_by_name_or_abbr` --- core/app/models/spree/address.rb | 2 +- core/app/models/spree/state.rb | 6 +++++- core/spec/models/spree/address_spec.rb | 2 +- core/spec/models/spree/state_spec.rb | 4 ++-- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/core/app/models/spree/address.rb b/core/app/models/spree/address.rb index 69927794fe2..34af690a790 100644 --- a/core/app/models/spree/address.rb +++ b/core/app/models/spree/address.rb @@ -193,7 +193,7 @@ def state_validate # ensure state_name belongs to country without states, or that it matches a predefined state name/abbr if state_name.present? if country.states.present? - states = country.states.find_all_by_name_or_abbr(state_name) + states = country.states.with_name_or_abbr(state_name) if states.size == 1 self.state = states.first diff --git a/core/app/models/spree/state.rb b/core/app/models/spree/state.rb index 3b79e6509cb..aed1b3967a5 100644 --- a/core/app/models/spree/state.rb +++ b/core/app/models/spree/state.rb @@ -5,13 +5,17 @@ class State < Spree::Base validates :country, :name, presence: true - def self.find_all_by_name_or_abbr(name_or_abbr) + scope :with_name_or_abbr, ->(name_or_abbr) do where( arel_table[:name].matches(name_or_abbr).or( arel_table[:abbr].matches(name_or_abbr) ) ) end + class << self + alias_method :find_all_by_name_or_abbr, :with_name_or_abbr + deprecate find_all_by_name_or_abbr: :with_name_or_abbr, deprecator: Spree::Deprecation + end # table of { country.id => [ state.id , state.name ] }, arrays sorted by name # blank is added elsewhere, if needed diff --git a/core/spec/models/spree/address_spec.rb b/core/spec/models/spree/address_spec.rb index e274821557f..c562966d9ab 100644 --- a/core/spec/models/spree/address_spec.rb +++ b/core/spec/models/spree/address_spec.rb @@ -21,7 +21,7 @@ let(:address) { build(:address, country: country) } before do - allow(country.states).to receive_messages find_all_by_name_or_abbr: [state] + allow(country.states).to receive_messages with_name_or_abbr: [state] end context 'address does not require state' do diff --git a/core/spec/models/spree/state_spec.rb b/core/spec/models/spree/state_spec.rb index 0bb0f70a3ed..664e9463d47 100644 --- a/core/spec/models/spree/state_spec.rb +++ b/core/spec/models/spree/state_spec.rb @@ -1,9 +1,9 @@ require 'spec_helper' describe Spree::State, type: :model do - describe '.find_all_by_name_or_abbr' do + describe '.with_name_or_abbr' do subject do - Spree::State.find_all_by_name_or_abbr(search_term) + Spree::State.with_name_or_abbr(search_term) end let!(:state) { create(:state, name: "California", abbr: "CA") }