Shoulda-matchers: validate_presence_of with belongs_to in Rails 5

[flyfy1]

Rails 5 has marked belongs_to to be optional: false by default. Therefore, if we have:

# app/model/student_book.rb
class StudentBook < ApplicationRecord
  belongs_to :student
end

then the test

require 'rails_helper'

RSpec.describe StudentBook do
  subject { StudentBook.new }

  it { is_expected.not_to be_valid }
  it { is_expected.to validate_presence_of(:student) }
end

should succeed for both cases. However, it would fail for the second case.

I've created a rails project to show the issue here: https://github.com/flyfy1/rspec-test-belongs-to

[mcmire]

@flyfy1 This should have been fixed recently. Are you on master?

[eugeneius]

I'm seeing the same after upgrading to Rails 5, and can confirm that it's not fixed on the latest master.

     Failure/Error: it { is_expected.to validate_presence_of(:student) }

       Feature did not properly validate that :student cannot be empty/falsy.
         After setting :student to ‹nil›, the matcher expected the Feature to be
         invalid and to produce the validation error "can't be blank" on
         :student. The record was indeed invalid, but it produced these
         validation errors instead:

         * student: ["must exist"]

This broke as a result of https://github.com/rails/rails/pull/18700: notice that the presence validation added by a belongs_to association now uses a different message than the default one from Active Model.

[mcmire]

Right, I gotcha.

You shouldn't need to manually test belongs_to, you should be able to use the belong_to matcher, which handles testing for the presence validation automatically (using the "must exist" validation message and not the other one). I know there were issues with this in the past, but it should work now. If it's still not working, then raise an issue (although I believe one may already exist, so check there).

[flyfy1]

could u give a sample code? thx 😁

On Fri, Apr 6, 2018, 1:20 AM Elliot Winkler [e-mail]
wrote:

Right, I gotcha.

You shouldn't need to manually test belongs_to, you should be able to use
the belong_to matcher, which handles testing for the presence validation
automatically (using the "must exist" validation message and not the other
one). If that's not working, then raise an issue (although I believe one
may already exist).

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/thoughtbot/shoulda-matchers/issues/1095#issuecomment-379011374,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABe1YItHV2b7CWN5-rM2fdBlzxeCbg2vks5tllI_gaJpZM4TDWCQ
.

[mcmire]

@flyfy1

If you have

class StudentBook < ApplicationRecord
  belongs_to :student
end

as a model, you should be able to simply write:

RSpec.describe StudentBook do
  it { is_expected_to.belong_to(:student) }
end

The belong_to matcher will test the presence validation for you automatically.

[flyfy1]

what if in the code, i have belongs_to optional:true ?

On Fri, Apr 6, 2018, 7:13 AM Elliot Winkler [e-mail]
wrote:

@flyfy1 https://github.com/flyfy1

If you have

class StudentBook < ApplicationRecord
belongs_to :studentend

as a model, you should be able to simply write:

RSpec.describe StudentBook do
it { is_expected_to.belong_to(:student) }end

The belong_to matcher will test the presence validation for you
automatically.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/thoughtbot/shoulda-matchers/issues/1095#issuecomment-379103738,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABe1YGaV3rvNQTvXtFd7qmLWnbddlD7Wks5tlqUkgaJpZM4TDWCQ
.

[mcmire]

@flyfy1 Then you can say:

RSpec.describe StudentBook do
  it { is_expected_to.belong_to(:student).optional }
end

[flyfy1]

OK IC.. thx 👍

[yagudaev]

Thanks for the tip @mcmire works great.

However, I do still think the validate_presence_of matcher should also be able to look for the 'must exist' message and pass. It is rather confusing behaviour at the moment.

[formigarafa]

I agree this is confusing but I don't think changing how validate_presence_of will improve this. It might create more confusion by mixing two different but similar concepts.

Probably, a note in the validate_presence_of error message nodding to the possibly expected code would help to learn and avoid 5 minutes searches to get to this thread.

[yagudaev]

@formigarafa you don't think those concepts are similar enough that it should pass when you have validates_presence_of?

'student' must exist is very similar to 'student' can't be blank no? It is just slightly different messages.

[formigarafa]

If those were the same on rails, yes. but they are not.
one is defined by:
belongs_to :student, required: true
the other is defined by:
validates :student, presence: true

I am assuming something else, besides the message might be different underneath.

Probably, if validate_presence_of error message suggest how to fix the failure we wouldn't be here talking about that.

[yagudaev]

@formigarafa that makes sense. Yeah having a nicer message from validate_presence_of would be good. Thanks for the clarification

I guess the matcher would just check to see if there is a belongs to relationship and mentioned that validate_presence is different than required: true in rails, therefore you should either use a belong_to matcher or add a presence validation to the model.

[mcmire]

@formigarafa @yagudaev Thanks for the suggestions guys. That seems like a nice UX improvement. I'll add it to the queue for the next release.

[felipero]

I believe there is another case to be dealt with.

Consider the model:

class FitnessCriterium < ApplicationRecord
  belongs_to :project, optional: true
  belongs_to :list, optional: true
  validate :fittable_present?

  def fittable_present?
    if project.blank? && list.blank?
      errors.add(:project, I18n.t('errors.messages.blank'))
      errors.add(:list, I18n.t('errors.messages.blank'))
    elsif project.present? && list.present?
      errors.add(:list, I18n.t('fitness_criteria.errors.list_along_with_project'))
    end
  end
end

and the spec:

context 'associations' do
  it { is_expected.to belong_to(:project).optional }
  it { is_expected.to belong_to(:list).optional }
end

I get the following faillure:

Expected FitnessCriterium to have a belongs_to association called project (and for the record not to fail validation if :project is unset; i.e., either the association should have been defined with `optional: true`, or there should not be a presence validation on :project)

[mcmire]

@felipero In your case, just because you've defined your belongs_to initially as optional: true doesn't mean the association is actually optional. The optional qualifier on the belong_to matcher basically ensures that you can set that association to nil and everything will be okay. Judging by the validations you've added, this is clearly not the case. belongs_to supports another qualifier, without_validating_presence, that lets you bypass the automatic check on presence that it does. I would use this instead of optional, and this should allow you to test the logic that you have in fittable_present? using validate_presence_of, allow_value, or whatever method you so choose.

[felipero]

@mcmire I see what you mean. The trick is maybe figure that just by using without_validating_presence it will ignore the optional. Not that intuitive. Thanks for the answer.

[mcmire]

@formigarafa @yagudaev I decided to add a little bit to the end of the failure message that the presence matcher generates. How does this sound?

Expected Child to validate that :parent cannot be empty/falsy, but this
could not be proved.
  After setting :parent to ‹nil›, the matcher expected the Child to be
  invalid and to produce the validation error "can't be blank" on
  :parent. The record was indeed invalid, but it produced these
  validation errors instead:

  * parent: ["must exist"]

  You're getting this error because you've instructed your `belongs_to`
  association to add a presence validation to the attribute. *This*
  presence validation doesn't use "can't be blank", the usual validation
  message, but "must exist" instead.

  With that said, did you know that the `belong_to` matcher can test
  this validation for you? Instead of using `validate_presence_of`, try
  the following instead:

      it { should belong_to(:parent) }

[formigarafa]

That looks very good.
That is exactly the kind of message I would have read and solved the problem immediately or taken me to the right documentation spot of shoulda-matcher or rails.
Great work,
Thank you.

[yagudaev]

@mcmire great work 👍

[chase439]

There's a typo the solutions above. It should be:

RSpec.describe StudentBook do
  it { is_expected.to belong_to(:student) }
  it { is_expected.to belong_to(:teacher).optional }
end

is_expected.to instead of is_expected_to