diff --git a/lib/identity_validations/identity_validator.rb b/lib/identity_validations/identity_validator.rb index 780a4df..8873413 100644 --- a/lib/identity_validations/identity_validator.rb +++ b/lib/identity_validations/identity_validator.rb @@ -3,7 +3,7 @@ class IdentityValidator < ActiveModel::Validator attr_writer :attribute def validate(record) - raise NotImplementedError, "IdentityValidator#validator is an abstract method and needs to be overridden" + raise NotImplementedError, "IdentityValidator#validate is an abstract method that needs to be overridden" end def attribute @@ -13,7 +13,9 @@ def attribute # I was tempted to make `record` an instance variable with an accessor, but that could # cause problems with how Rails instantiates and reuses validator instances def get_attribute(record) - return unless attribute.present? + if attribute.blank? + raise ArgumentError, "IdentityValidator: can't validate #{record.class}, no attribute specified" + end if !record.respond_to?(attribute.to_sym) raise ArgumentError, "IdentityValidator: attribute '#{attribute}' not found in class #{record.class}" end diff --git a/spec/identity_validations/identity_validator_spec.rb b/spec/identity_validations/identity_validator_spec.rb index 70a78a1..3115cae 100644 --- a/spec/identity_validations/identity_validator_spec.rb +++ b/spec/identity_validations/identity_validator_spec.rb @@ -10,8 +10,8 @@ class TestModel let(:model) { TestModel.new } it 'throws an error due to being abstract' do - expect {model.validates_with IdentityValidations::IdentityValidator}. - to raise_error(NotImplementedError, 'IdentityValidator#validator is an abstract method and needs to be overridden') + expect { model.validates_with IdentityValidations::IdentityValidator }. + to raise_error(NotImplementedError, 'IdentityValidator#validate is an abstract method that needs to be overridden') end describe 'a simple inheriting validator' do @@ -21,17 +21,39 @@ def validate(record) end end - it 'throws an error when an invalid attribute is specified' do - model.an_attribute = "something" - expect {model.validates_with SimpleValidator, attribute: :invalid_attribute}. - to raise_error(ArgumentError, "IdentityValidator: attribute 'invalid_attribute' not found in class TestModel") + describe "invoked via ActiveModel's #validates_with" do + it 'throws an error when an invalid attribute is specified' do + model.an_attribute = "something" + expect {model.validates_with SimpleValidator, attribute: :invalid_attribute}. + to raise_error(ArgumentError, "IdentityValidator: attribute 'invalid_attribute' not found in class TestModel") + end + + it 'will validate when passed an attribute' do + model.an_attribute = "something" + model.validates_with SimpleValidator, attribute: :an_attribute + expect(model.errors).to be_blank + expect(model.errors.messages).to be_blank + end + + it 'throws an error when not given an attribute' do + model.an_attribute = "something" + expect { model.validates_with SimpleValidator }. + to raise_error(ArgumentError, "IdentityValidator: can't validate TestModel, no attribute specified") + end end - it 'will validate when passed an attribute' do - model.an_attribute = "something" - model.validates_with SimpleValidator, attribute: :an_attribute - expect(model.errors).to be_blank - expect(model.errors.messages).to be_blank + describe "#validate" do + it 'throws an error when not given an attribute' do + model.an_attribute = "something" + + # In theory, we can call SimpleValidator.new with arguments to pass in an option. + # However, this is an ability inherited from the Rails parent class. + # This inherited option argument is more likely to be changed by Rails than the `validates_with` + # option argument, so ideally our usage and tests shouldn't rely on that Rails implementation detail + + expect {SimpleValidator.new.validate(model)}. + to raise_error(ArgumentError, "IdentityValidator: can't validate TestModel, no attribute specified") + end end end end