Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Admin address fixes and extra address validation #2371

Merged
merged 3 commits into from
Nov 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ Spree.Views.StateSelect = Backbone.View.extend({
},

render: function() {
this.$state_select.empty().hide();
this.$state_input.hide();
this.$state_select.empty().hide().prop('disabled', true);
this.$state_input.hide().prop('disabled', true);

if (!this.states.fetched) {
this.$state_select.show().prop("disabled", true);
this.$state_select.show();
} else if (this.states.length) {
var $state_select = this.$state_select;
this.states.each(function(state) {
Expand All @@ -54,7 +54,7 @@ Spree.Views.StateSelect = Backbone.View.extend({
this.$state_select.val(this.model.get("state_id"))
this.$state_select.show().prop("disabled", false);
} else {
this.$state_input.prop('disabled', false).show();
this.$state_input.show().prop('disabled', false);
}
}
})
Expand Down
2 changes: 2 additions & 0 deletions backend/app/views/spree/admin/shared/_address_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,11 @@
<div class="field <%= "#{type}-row" %>">
<%= f.label :state_id, Spree::State.model_name.human %>
<span id="<%= s_or_b %>state">
<%= f.hidden_field :state_name, value: nil %>
<%= f.text_field :state_name,
style: "display: #{f.object.country.states.empty? ? 'block' : 'none' };",
disabled: !f.object.country.states.empty?, class: 'fullwidth state_name js-state_name' %>
<%= f.hidden_field :state_id, value: nil %>
<%= f.collection_select :state_id, f.object.country.states.sort, :id, :name, {include_blank: true}, {class: 'custom-select fullwidth js-state_id', style: "display: #{f.object.country.states.empty? ? 'none' : 'block' };", disabled: f.object.country.states.empty?} %>
</span>
</div>
Expand Down
7 changes: 7 additions & 0 deletions core/app/models/spree/address.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class Address < Spree::Base
validates :phone, presence: true, if: :require_phone?

validate :state_validate
validate :validate_state_matches_country

alias_attribute :first_name, :firstname
alias_attribute :last_name, :lastname
Expand Down Expand Up @@ -209,5 +210,11 @@ def state_validate
# ensure at least one state field is populated
errors.add :state, :blank if state.blank? && state_name.blank?
end

def validate_state_matches_country
if state && state.country != country
errors.add(:state, :does_not_match_country)
end
end
end
end
4 changes: 4 additions & 0 deletions core/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,10 @@ en:
other: Zones
errors:
models:
spree/address:
attributes:
state:
does_not_match_country: does not match the country
spree/calculator/tiered_flat_rate:
attributes:
base:
Expand Down
23 changes: 18 additions & 5 deletions core/spec/models/spree/address_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,24 @@
expect(address.state_name).to be_nil
end

it "state is entered but country does not contain that state" do
address.state = state
address.country = Spree::Country.new(states_required: true)
address.valid?
expect(address.errors["state"]).to eq(['is invalid'])
context 'when the country does not match the state' do
context 'when the country requires states' do
it 'is invalid' do
address.state = state
address.country = Spree::Country.new(states_required: true)
address.valid?
expect(address.errors["state"]).to eq(['is invalid', 'does not match the country'])
end
end

context 'when the country does not require states' do
it 'is invalid' do
address.state = state
address.country = Spree::Country.new(states_required: false)
address.valid?
expect(address.errors["state"]).to eq(['does not match the country'])
end
end
end

it "both state and state_name are entered but country does not contain the state" do
Expand Down