Skip to content

Commit 620ff14

Browse files
committed
Change spree_countries.iso column to be unique
We're using `spree_countries.iso` as a primary key between prices and countries. In order to add a foreign key constraint, primary keys have to be unique. This adds the necessary uniqueness constraint. A few tests had to be amended to not create the same country many times.
1 parent 700b0c9 commit 620ff14

9 files changed

Lines changed: 34 additions & 20 deletions

File tree

core/app/models/spree/country.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ class Country < Spree::Base
1616
dependent: :restrict_with_error,
1717
inverse_of: :country
1818

19-
validates :name, :iso_name, presence: true
19+
validates :name, :iso_name, :iso, presence: true
20+
validates :iso, uniqueness: true
2021

2122
self.allowed_ransackable_attributes = %w[name]
2223

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
class ChangeCountriesIsoToUnique < ActiveRecord::Migration[8.0]
2+
def change
3+
remove_index :spree_countries, :iso
4+
5+
add_index :spree_countries, :iso, unique: true
6+
end
7+
end

core/spec/helpers/base_helper_spec.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111
let(:country) { create(:country) }
1212

1313
before do
14-
3.times { create(:country) }
14+
create(:country, iso: "BR")
15+
create(:country, iso: "DE")
16+
create(:country, iso: "FR")
1517
end
1618

1719
context "with no checkout zone defined" do
@@ -24,7 +26,7 @@
2426
end
2527

2628
it "uses locales for country names" do
27-
expect(available_countries).to include(having_attributes(name: "United States of America"))
29+
expect(available_countries).to include(having_attributes(name: "Brazil"))
2830
end
2931
end
3032

core/spec/lib/spree/core/importer/order_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ module Core
226226
}
227227
end
228228

229-
let(:other_state) { create(:state, name: "Uhuhuh", country: create(:country)) }
229+
let(:other_state) { create(:state, name: "Uhuhuh", country: create(:country, iso: "BR")) }
230230

231231
before do
232232
ship_address.delete(:state_id)

core/spec/models/spree/country_spec.rb

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,16 +104,15 @@
104104
end
105105

106106
describe '#prices' do
107-
let(:country) { create(:country) }
107+
let(:country) { create(:country, iso: "BR") }
108108
subject { country.prices }
109109

110110
it { is_expected.to be_a(ActiveRecord::Associations::CollectionProxy) }
111111

112112
context "if the country has associated prices" do
113-
let!(:price_one) { create(:price) }
114-
let!(:price_two) { create(:price) }
113+
let!(:price_one) { create(:price, country:) }
114+
let!(:price_two) { create(:price, country:) }
115115
let!(:price_three) { create(:price) }
116-
let(:country) { create(:country, prices: [price_one, price_two]) }
117116

118117
it { is_expected.to contain_exactly(price_one, price_two) }
119118
end

core/spec/models/spree/credit_card_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ def self.payment_states
8888
end
8989

9090
let!(:persisted_card) { Spree::CreditCard.find(credit_card.id) }
91-
let(:country) { create(:country, states_required: true) }
91+
let(:country) { create(:country, iso: "BR", states_required: true) }
9292
let(:state) { create(:state, country:) }
9393
let(:valid_address_attributes) do
9494
{

core/spec/models/spree/price_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@
169169

170170
describe 'scopes' do
171171
describe '.for_any_country' do
172-
let(:country) { create(:country) }
172+
let(:country) { create(:country, iso: "BR") }
173173
let!(:fallback_price) { create(:price, country_iso: nil) }
174174
let!(:country_price) { create(:price, country:) }
175175

core/spec/models/spree/tax_rate_spec.rb

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,14 @@
6464
end
6565

6666
context "when no rate zones match the tax zone" do
67-
let(:rate_zone) { create(:zone, :with_country) }
67+
let(:usa) { create(:country) }
68+
let(:rate_zone) { create(:zone, countries: [usa]) }
6869
let!(:rate) { create :tax_rate, zone: rate_zone }
6970

7071
context "when there is no default tax zone" do
7172
context "and the zone has no shared members with the rate zone" do
72-
let(:zone) { create(:zone, :with_country) }
73+
let(:canada) { create(:country, iso: "CA")}
74+
let(:zone) { create(:zone, countries: [canada]) }
7375

7476
it "should return an empty array" do
7577
expect(subject).to eq([])
@@ -94,8 +96,8 @@
9496
end
9597

9698
context "when the tax_zone is contained within a rate zone" do
97-
let(:country1) { create :country }
98-
let(:country2) { create :country }
99+
let(:country1) { create :country, iso: "FR" }
100+
let(:country2) { create :country, iso: "BR" }
99101
let(:rate_zone) { create(:zone, countries: [country1, country2]) }
100102
let(:zone) { create(:zone, countries: [country1]) }
101103

@@ -127,7 +129,8 @@
127129
end
128130

129131
context "when the zone is outside the default zone" do
130-
let(:zone) { create(:zone, :with_country) }
132+
let(:brazil) { create(:country, iso: "BR") }
133+
let(:zone) { create(:zone, countries: [brazil]) }
131134

132135
it { is_expected.to be_empty }
133136
end

core/spec/models/spree/zone_spec.rb

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@
44

55
RSpec.describe Spree::Zone, type: :model do
66
describe 'for_address' do
7-
let(:new_york_address) { create(:address, state_code: "NY") }
7+
let(:canada) { create(:country, iso: "CA") }
8+
let(:usa) { create(:country, iso: "US") }
9+
let(:new_york_address) { create(:address, state_code: "NY", country: usa) }
810
let(:alabama_address) { create(:address) }
9-
let(:canada_address) { create(:address, country_iso_code: "CA") }
11+
let(:canada_address) { create(:address, country: canada) }
1012

1113
let!(:new_york_zone) { create(:zone, states: [new_york_address.state]) }
1214
let!(:alabama_zone) { create(:zone, states: [alabama_address.state]) }
@@ -101,7 +103,7 @@
101103
it "should remove existing state members" do
102104
zone = create(:zone, name: 'foo', zone_members: [])
103105
state = create(:state)
104-
country = create(:country)
106+
country = create(:country, iso: "BR")
105107
zone.members.create(zoneable: state)
106108
country_member = zone.members.create(zoneable: country)
107109
zone.save
@@ -159,8 +161,8 @@
159161

160162
context ".with_shared_members" do
161163
let!(:country) { create(:country) }
162-
let!(:country2) { create(:country, name: 'OtherCountry') }
163-
let!(:country3) { create(:country, name: 'TaxCountry') }
164+
let!(:country2) { create(:country, iso: "MX", name: 'OtherCountry') }
165+
let!(:country3) { create(:country, iso: "CA", name: 'TaxCountry') }
164166

165167
subject(:zones_with_shared_members) { Spree::Zone.with_shared_members(zone) }
166168

0 commit comments

Comments
 (0)