diff --git a/app/assets/javascripts/admin/components/screened-ip-address-form.js.es6 b/app/assets/javascripts/admin/components/screened-ip-address-form.js.es6 index 0f8a29ba8c3..ef6e7596d11 100644 --- a/app/assets/javascripts/admin/components/screened-ip-address-form.js.es6 +++ b/app/assets/javascripts/admin/components/screened-ip-address-form.js.es6 @@ -48,13 +48,17 @@ export default Ember.Component.extend({ action_name: this.get('actionName') }); screenedIpAddress.save().then(result => { - this.setProperties({ ip_address: '', formSubmitted: false }); - this.sendAction('action', ScreenedIpAddress.create(result.screened_ip_address)); - Ember.run.schedule('afterRender', () => this.$('.ip-address-input').focus()); + if (result.success) { + this.setProperties({ ip_address: '', formSubmitted: false }); + this.sendAction('action', ScreenedIpAddress.create(result.screened_ip_address)); + Ember.run.schedule('afterRender', () => this.$('.ip-address-input').focus()); + } else { + bootbox.alert(result.errors); + } }).catch(e => { this.set('formSubmitted', false); - const msg = (e.responseJSON && e.responseJSON.errors) ? - I18n.t("generic_error_with_reason", {error: e.responseJSON.errors.join('. ')}) : + const msg = (e.jqXHR.responseJSON && e.jqXHR.responseJSON.errors) ? + I18n.t("generic_error_with_reason", {error: e.jqXHR.responseJSON.errors.join('. ')}) : I18n.t("generic_error"); bootbox.alert(msg, () => this.$('.ip-address-input').focus()); }); diff --git a/app/models/screened_ip_address.rb b/app/models/screened_ip_address.rb index f0e51ab0456..60871eaba43 100644 --- a/app/models/screened_ip_address.rb +++ b/app/models/screened_ip_address.rb @@ -10,11 +10,21 @@ class ScreenedIpAddress < ActiveRecord::Base default_action :block validates :ip_address, ip_address_format: true, presence: true + after_validation :check_for_match def self.watch(ip_address, opts = {}) match_for_ip_address(ip_address) || create(opts.slice(:action_type).merge(ip_address: ip_address)) end + def check_for_match + unless self.errors[:ip_address].present? + matched = self.class.match_for_ip_address(self.ip_address) + if matched && matched.action_type == self.action_type + self.errors.add(:ip_address, :ip_address_already_screened) + end + end + end + # In Rails 4.0.0, validators are run to handle invalid assignments to inet columns (as they should). # In Rails 4.0.1, an exception is raised before validation happens, so we need this hack for # inet/cidr columns: diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 47135628c88..52fd114f69d 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -132,6 +132,7 @@ en: odd: must be odd record_invalid: ! 'Validation failed: %{errors}' max_emojis: "can't have more than %{max_emojis_count} emoji" + ip_address_already_screened: "is already included in an existing rule" restrict_dependent_destroy: one: "Cannot delete record because a dependent %{record} exists" many: "Cannot delete record because dependent %{record} exist" diff --git a/spec/models/screened_ip_address_spec.rb b/spec/models/screened_ip_address_spec.rb index 6210f832b0d..2b8a8d82f80 100644 --- a/spec/models/screened_ip_address_spec.rb +++ b/spec/models/screened_ip_address_spec.rb @@ -31,6 +31,13 @@ describe ScreenedIpAddress do described_class.new(valid_params.merge(action_name: nil)) }.to raise_error(ArgumentError) end + + it 'returns a useful error if ip address matches an existing record' do + ScreenedIpAddress.create(ip_address: '2600:387:b:f::7a/128', action_name: :block) + r = ScreenedIpAddress.new(ip_address: '2600:387:b:f::7a', action_name: :block) + expect(r.save).to eq(false) + expect(r.errors[:ip_address]).to be_present + end end describe "ip_address_with_mask" do