From d78563769a7e6cef23e2dbbe20ab2014376fbeca Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Fri, 10 Jun 2016 13:36:08 -0700 Subject: [PATCH 1/3] Add failing, pending spec for Store.current When Store.current is called without args, it is expected to return the default store. Instead it returns an arbitrary Store (probably the default) --- core/spec/models/spree/store_spec.rb | 33 +++++++++++++++++++--------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/core/spec/models/spree/store_spec.rb b/core/spec/models/spree/store_spec.rb index 628ca49b3a2..e760a5c6d3b 100644 --- a/core/spec/models/spree/store_spec.rb +++ b/core/spec/models/spree/store_spec.rb @@ -16,23 +16,36 @@ end describe '.current' do - # there is a default store created with the test_app rake task. - let!(:store_1) { Spree::Store.first || create(:store) } - + let!(:store_1) { create(:store) } + let!(:store_default) { create(:store, name: 'default', default: true) } let!(:store_2) { create(:store, default: false, url: 'www.subdomain.com') } let!(:store_3) { create(:store, default: false, url: 'www.another.com', code: 'CODE') } - it 'should return default when no domain' do - expect(subject.class.current).to eql(store_1) + delegate :current, to: :described_class + + context "with no argument" do + it 'should return default' do + pending "This returns an arbitrary store" + expect(current).to eql(store_default) + end + end + + context "with no match" do + it 'should return the default domain' do + expect(current('foobar.com')).to eql(store_default) + end end - it 'should return store for domain' do - expect(subject.class.current('spreecommerce.com')).to eql(store_1) - expect(subject.class.current('www.subdomain.com')).to eql(store_2) + context "with matching url" do + it 'should return matching store' do + expect(current('www.subdomain.com')).to eql(store_2) + end end - it 'should return store by code' do - expect(subject.class.current('CODE')).to eql(store_3) + context "with matching code" do + it 'should return matching store' do + expect(current('CODE')).to eql(store_3) + end end end From ddf24b7d0aefae3a9b0e6ad1bf6b88480c20131a Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Fri, 10 Jun 2016 14:42:19 -0700 Subject: [PATCH 2/3] Fix store.current with no arguments Previously this was returning an arbitrary store (likely the first), since Store.by_url(nil) returns an SQL scope of "url LIKE '%%'" --- core/app/models/spree/store.rb | 2 +- core/spec/models/spree/store_spec.rb | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/core/app/models/spree/store.rb b/core/app/models/spree/store.rb index c263884b596..8e3563a43d8 100644 --- a/core/app/models/spree/store.rb +++ b/core/app/models/spree/store.rb @@ -15,7 +15,7 @@ class Store < Spree::Base scope :by_url, lambda { |url| where("url like ?", "%#{url}%") } def self.current(store_key = nil) - current_store = Store.find_by(code: store_key) || Store.by_url(store_key).first + current_store = Store.find_by(code: store_key) || Store.by_url(store_key).first if store_key current_store || Store.default end diff --git a/core/spec/models/spree/store_spec.rb b/core/spec/models/spree/store_spec.rb index e760a5c6d3b..dd961a20b65 100644 --- a/core/spec/models/spree/store_spec.rb +++ b/core/spec/models/spree/store_spec.rb @@ -25,7 +25,6 @@ context "with no argument" do it 'should return default' do - pending "This returns an arbitrary store" expect(current).to eql(store_default) end end From a682147000c7797a5ca35178e5816026aaca80cf Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Fri, 10 Jun 2016 14:42:56 -0700 Subject: [PATCH 3/3] Add deprecation on Store.current without args --- core/app/models/spree/store.rb | 1 + core/lib/spree/core/current_store.rb | 6 +++++- core/spec/models/spree/store_spec.rb | 4 +++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/core/app/models/spree/store.rb b/core/app/models/spree/store.rb index 8e3563a43d8..c3cc9af2a83 100644 --- a/core/app/models/spree/store.rb +++ b/core/app/models/spree/store.rb @@ -15,6 +15,7 @@ class Store < Spree::Base scope :by_url, lambda { |url| where("url like ?", "%#{url}%") } def self.current(store_key = nil) + Spree::Deprecation.warn "Spree::Store.current needs a code or URL as an argument. If you want the default store use Spree::Store.default", caller if !store_key current_store = Store.find_by(code: store_key) || Store.by_url(store_key).first if store_key current_store || Store.default end diff --git a/core/lib/spree/core/current_store.rb b/core/lib/spree/core/current_store.rb index 3b9552d607c..f836ebce035 100644 --- a/core/lib/spree/core/current_store.rb +++ b/core/lib/spree/core/current_store.rb @@ -13,7 +13,11 @@ def initialize(request) # looking up by the requesting server's name. # @return [Spree::Store] def store - Spree::Store.current(store_key) + if store_key + Spree::Store.current(store_key) + else + Spree::Store.default + end end private diff --git a/core/spec/models/spree/store_spec.rb b/core/spec/models/spree/store_spec.rb index dd961a20b65..2155a880e7b 100644 --- a/core/spec/models/spree/store_spec.rb +++ b/core/spec/models/spree/store_spec.rb @@ -25,7 +25,9 @@ context "with no argument" do it 'should return default' do - expect(current).to eql(store_default) + Spree::Deprecation.silence do + expect(current).to eql(store_default) + end end end