Shoulda-matchers: Clear up confusion w/ presence around belongs_to associations. (#1214) breaks existing behaviors.

Created on 11 Jun 2019  路  7Comments  路  Source: thoughtbot/shoulda-matchers

v4.1.0 includes a change Clear up confusion w/ presence around belongs_to associations. (#1214 ) . This breaks existing behaviors.

My application code is below.

class HogeForm
  include ActiveModel::Model
  include ActiveModel::Attributes

  attribute :user
  attribute :note, :string

  validates :user, presence: true
  validates :note, presence: true, if: %i[user]

  def save
    return false if invalid?

    # do something
  end
end

And test code is below.

RSpec.describe HogeForm do
  context 'Validations' do
    it { is_expected.to validate_presence_of(:user) }
    ...
    ...
    ...
  end
end

When execute it { is_expected.to validate_presence_of(:user) }, then user attribute is inserted nil until v4.0.1, but in v.4.1.0 user attribute is inserted "" (empty string) and validates :note, presence: true, if: %i[user] is run unintentional.

Why did you determine to change the behavior? and why not restore the behavior?

Bug

Most helpful comment

The presence validation calls .present? on your attribute to determine whether it's "present". Presence is defined as either an empty string or nil: https://github.com/rails/rails/blob/master/activesupport/lib/active_support/core_ext/object/blank.rb#L6. Therefore, to truly test the presence validation, you need to see what happens if your attribute is an empty string. The matcher really should have been doing that all along, so that's why I added it.

That said, I can change it so that it only acts on an ActiveRecord model so that your tests pass again.

All 7 comments

The presence validation calls .present? on your attribute to determine whether it's "present". Presence is defined as either an empty string or nil: https://github.com/rails/rails/blob/master/activesupport/lib/active_support/core_ext/object/blank.rb#L6. Therefore, to truly test the presence validation, you need to see what happens if your attribute is an empty string. The matcher really should have been doing that all along, so that's why I added it.

That said, I can change it so that it only acts on an ActiveRecord model so that your tests pass again.

@kamillle Looking back at this, I'm having trouble writing a failing test for this based on your example. Are your tests breaking currently, and if so, which ones?

@mcmire oops...
Sorry for providing wrong code.
Correct code is below, plz check it.

### application code ###
class HogeForm
  include ActiveModel::Model
  include ActiveModel::Attributes

  attribute :user

  validates :user, presence: true
  validate :validate_something, if: %i[user]

  def save
    return false if invalid?

    # do something
  end

  private.
  def validate_something
    return if user.email

    errors.add(:base, 'Error!!!')
  end
end

### test code ###
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe HogeForm, type: :model do
  context 'Validations' do
    it { is_expected.to validate_presence_of(:user) }
  end
end

it { is_expected.to validate_presence_of(:user) } was passed until v4.0.1, but the test cat not be passed in v4.1.0.

  $ bin/rspec spec/forms/hoge_form_spec.rb
Running via Spring preloader in process 34469

Randomized with seed 26709
F

Failures:

  1) HogeForm Validations should validate that :user cannot be empty/falsy
     Failure/Error: return if user.email

     NoMethodError:
       undefined method `email' for "":String
     # ./app/forms/hoge_form.rb:19:in `validate_something'
     # ./.bundle/ruby/2.5.0/gems/activesupport-5.2.2.1/lib/active_support/callbacks.rb:426:in `block in make_lambda'
     # ./.bundle/ruby/2.5.0/gems/activesupport-5.2.2.1/lib/active_support/callbacks.rb:179:in `block (2 levels) in halting_and_conditional'
     # ./.bundle/ruby/2.5.0/gems/activesupport-5.2.2.1/lib/active_support/callbacks.rb:606:in `block (2 levels) in default_terminator'
     # ./.bundle/ruby/2.5.0/gems/activesupport-5.2.2.1/lib/active_support/callbacks.rb:605:in `catch'
     # ./.bundle/ruby/2.5.0/gems/activesupport-5.2.2.1/lib/active_support/callbacks.rb:605:in `block in default_terminator'
     # ./.bundle/ruby/2.5.0/gems/activesupport-5.2.2.1/lib/active_support/callbacks.rb:180:in `block in halting_and_conditional'
     # ./.bundle/ruby/2.5.0/gems/activesupport-5.2.2.1/lib/active_support/callbacks.rb:513:in `block in invoke_before'
     # ./.bundle/ruby/2.5.0/gems/activesupport-5.2.2.1/lib/active_support/callbacks.rb:513:in `each'
     # ./.bundle/ruby/2.5.0/gems/activesupport-5.2.2.1/lib/active_support/callbacks.rb:513:in `invoke_before'
     # ./.bundle/ruby/2.5.0/gems/activesupport-5.2.2.1/lib/active_support/callbacks.rb:131:in `run_callbacks'
     # ./.bundle/ruby/2.5.0/gems/activesupport-5.2.2.1/lib/active_support/callbacks.rb:816:in `_run_validate_callbacks'
     # ./.bundle/ruby/2.5.0/gems/activemodel-5.2.2.1/lib/active_model/validations.rb:409:in `run_validations!'
     # ./.bundle/ruby/2.5.0/gems/activemodel-5.2.2.1/lib/active_model/validations.rb:339:in `valid?'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/validator.rb:96:in `perform_validation'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/validator.rb:89:in `validation_result'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/validator.rb:85:in `validation_error_messages'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/validator.rb:64:in `messages'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/validator.rb:25:in `has_messages?'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/validator.rb:55:in `messages_match?'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/validator.rb:21:in `call'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/allow_value_matcher/attribute_setters_and_validators.rb:38:in `matches?'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/allow_value_matcher/attribute_setters_and_validators.rb:24:in `each'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/allow_value_matcher/attribute_setters_and_validators.rb:24:in `detect'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/allow_value_matcher/attribute_setters_and_validators.rb:24:in `first_passing'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/allow_value_matcher.rb:534:in `public_send'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/allow_value_matcher.rb:534:in `run'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/allow_value_matcher.rb:401:in `does_not_match?'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/disallow_value_matcher.rb:32:in `matches?'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/validation_matcher.rb:162:in `run_allow_or_disallow_matcher'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/validation_matcher.rb:98:in `disallows_value_of'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb:233:in `disallows_original_or_typecast_value?'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb:156:in `block in matches?'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb:155:in `each'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb:155:in `all?'
     # ./.bundle/ruby/2.5.0/gems/shoulda-matchers-4.1.0/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb:155:in `matches?'
     # ./spec/forms/hoge_form_spec.rb:9:in `block (3 levels) in <top (required)>'
     # ./.bundle/ruby/2.5.0/gems/bootsnap-1.4.4/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:54:in `load'
     # ./.bundle/ruby/2.5.0/gems/bootsnap-1.4.4/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:54:in `load'
     # ./.bundle/ruby/2.5.0/gems/activesupport-5.2.2.1/lib/active_support/dependencies.rb:285:in `block in load'
     # ./.bundle/ruby/2.5.0/gems/activesupport-5.2.2.1/lib/active_support/dependencies.rb:257:in `load_dependency'
     # ./.bundle/ruby/2.5.0/gems/activesupport-5.2.2.1/lib/active_support/dependencies.rb:285:in `load'
     # ./.bundle/ruby/2.5.0/gems/spring-commands-rspec-1.0.4/lib/spring/commands/rspec.rb:18:in `call'
     # ./.bundle/ruby/2.5.0/gems/spring-2.0.2/lib/spring/command_wrapper.rb:38:in `call'
     # ./.bundle/ruby/2.5.0/gems/spring-2.0.2/lib/spring/application.rb:201:in `block in serve'
     # ./.bundle/ruby/2.5.0/gems/spring-2.0.2/lib/spring/application.rb:171:in `fork'
     # ./.bundle/ruby/2.5.0/gems/spring-2.0.2/lib/spring/application.rb:171:in `serve'
     # ./.bundle/ruby/2.5.0/gems/spring-2.0.2/lib/spring/application.rb:141:in `block in run'
     # ./.bundle/ruby/2.5.0/gems/spring-2.0.2/lib/spring/application.rb:135:in `loop'
     # ./.bundle/ruby/2.5.0/gems/spring-2.0.2/lib/spring/application.rb:135:in `run'
     # ./.bundle/ruby/2.5.0/gems/spring-2.0.2/lib/spring/application/boot.rb:19:in `<top (required)>'
     # -e:1:in `<main>'

@kamillle Hey, I just pushed up a branch, fix-presence-validation-for-activemodel, that should fix this issue. Can you temporarily switch to it in your Gemfile and see if makes your tests pass?

@mcmire Sure. I'll check that!! Thank you!馃槏

@mcmire The fix is completely working in my case!!! Appriciate your working.馃槅馃檶馃檶

This fix is live in v4.1.1.

Was this page helpful?
0 / 5 - 0 ratings