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

Rails 5: belongs_to :class, optional: true #870

Closed
stephannv opened this issue Dec 27, 2015 · 24 comments
Closed

Rails 5: belongs_to :class, optional: true #870

stephannv opened this issue Dec 27, 2015 · 24 comments

Comments

@stephannv
Copy link
Contributor

belongs_to will now trigger a validation error by default if the association is not present.
We can turn this off on a per-association basis with optional: true.
rails/rails#18937

We will need a qualifier for this.

@mcmire
Copy link
Collaborator

mcmire commented Dec 27, 2015

Okay -- do you plan on using optional: true? We tend to add qualifiers on an as-needed basis.

@stephannv
Copy link
Contributor Author

Yes. I'm developing a app where a Amiibo may or may not to belongs to Series. ex.: Sonic amiibo belongs to Smash Bros. Series, but Wolf Link amiibo belongs to none.

@mcmire
Copy link
Collaborator

mcmire commented Dec 27, 2015

Cool. I'm not sure when I'll be able to get to this, as I am working on resolving some regressions with the 3.0 release right now, but I would happily take a PR, otherwise I'll get to it at some point down the line. I'll leave this open in the meantime since it's related to Rails 5.

@thomasbiddle
Copy link

I have a use-case for this as well.

@leoebfolsom
Copy link

leoebfolsom commented Nov 29, 2016

I also have a use case for this. A food database where most of the foods are not created by users, but users can create foods and 'own' the foods that they create. Workaround: the admin user profile will own foods by default. Please correct me if there's a better workaround or existing solution.
Edit: It seems food does not have to belong_to anything, per this SO post. http://stackoverflow.com/questions/16699877/rails-optional-belongs-to Perhaps I have misunderstood the issue.

@stephannv
Copy link
Contributor Author

stephannv commented Nov 29, 2016

You can use belongs_to :user, optional: true for specific case or just put on application.rb config.active_record.belongs_to_required_by_default = false to config all application.
But shoulda-matcher doesn't have a matcher for this

@leoebfolsom
Copy link

@stephanngamedev I did neither of those things and it seems like I can create foods with user_id: nil no problem--am I missing something? My migration:

class AddUserToFoods < ActiveRecord::Migration
  def change
    add_reference :foods, :user, index: true, foreign_key: true
  end
end

@stephannv
Copy link
Contributor Author

Are you using Rails 5?

@leoebfolsom
Copy link

leoebfolsom commented Nov 29, 2016

I am using Rails 4. Sorry, I see now this thread is labeled Rails 5. Would your recommendations be necessary in Rails 5?

@stephannv
Copy link
Contributor Author

Yes. My recomendations is to make rails 5 belongs_to behavior like rails 4 belongs_to

@mcmire
Copy link
Collaborator

mcmire commented Nov 29, 2016

@leoebrown Rails 5 changed the behavior of belongs_to so that it implicitly adds a presence validation for the association; optional: true disables that behavior. But in Rails 4 you don't really have to worry about this. If I remember correctly, if you leave your *_id column blank then your association will return nil (this works for both Rails 4 and 5). So you can use the belongs_to matcher in shoulda-matchers just fine without having to specify any additional qualifiers.

@vizcay
Copy link

vizcay commented Mar 22, 2017

Hi! there is some progress on this?

I used to have validate association, presence: true back in Rails 4 to check that the referenced model exists and it was verified by a validate_presence_of. Now, in Rails 5, this is the default behaviour but if I remove the validation in the model to dry it up, the spec now fails because the errors messages diffier ("must exist" vs "can't be blank").

According to the new Rails behaviour, do you think that shoulda-matchers should test this by assigning nil and checking if and error was added? And if a qualifier was passed, the inverse check..

@xDAGRONx
Copy link

xDAGRONx commented Apr 4, 2017

@vizcay for what it's worth, you can get the spec to pass by adding with_message(:required) like so:

is_expected.to validate_presence_of(association).with_message(:required)

@ybakos
Copy link

ybakos commented Jul 6, 2017

Given a Rails 5 model with the association,

belongs_to :featured_plant, class_name: 'Plant', optional: true

I should be able to test the association with:

it { is_expected.to belong_to(:featured_plant).class_name('Plant').optional(true) }

to verify that the presence of an associated model is not required.

@chamnap
Copy link

chamnap commented Jul 10, 2017

@ybakos, I tested it doesn't work on v3.1.1.

@lekansogunle
Copy link

lekansogunle commented Aug 9, 2017

We are only testing for validation of the presence of the association ID, what if the given ID is invalid. And app behaviour is not to validate the existence of such association?

@robbl-as
Copy link

robbl-as commented Jan 8, 2018

@mcmire @stephanngamedev Isn't that issue resolved by now?

@Hyperadministrator
Copy link

Hyperadministrator commented Jan 11, 2018

Doesn't seem to work in version 3.1.2 for I am getting:
undefined method optional' for #<Shoulda::Matchers::ActiveRecord::AssociationMatcher
when I'm doing it { is_expected.to belong_to(:user).optional(true) }

@mcmire
Copy link
Collaborator

mcmire commented Jan 24, 2018

Not sure why this issue wasn't closed automatically, but yes, this is resolved by #1058. It'll be available in the next version, or you're feeling adventurous, you can point to the master branch.

@mcmire mcmire closed this as completed Jan 24, 2018
@giriss
Copy link

giriss commented Jul 6, 2018

@mcmire When will this version be released?
The new code was merged 19th Oct 2017 but is still not present in the latest version!

@lucascaton
Copy link

Any news on this? Thanks :)

@namtx
Copy link

namtx commented Oct 17, 2018

Failure/Error: it{is_expected.to belong_to(:position).optional true}
     
     NoMethodError:
       undefined method `optional' for #<Shoulda::Matchers::ActiveRecord::AssociationMatcher:0x00000000024649c8>
       Did you mean?  options

@mcmire
Copy link
Collaborator

mcmire commented Oct 17, 2018

@namtx @akhilesh05 @lucascaton We have a pre-release version out now! Try v4.0.0.rc1 to get optional.

@vinibrsl
Copy link

In v4.0.0+, the usage is:

it { should belong_to(:sender).optional }

without the bool argument.

jclopezdev added a commit to jclopezdev/siai that referenced this issue Oct 31, 2019
We previously updated the shoulda-matchers gem, and now it is
necessary to specify in the spec if the relationship is optional

More info:
thoughtbot/shoulda-matchers#870 (comment)

Also the course relationship was validated as required,
and at the same time as optional, so I left it as required
repent added a commit to repent/acromatic that referenced this issue Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests