Option #3 would be my goto, but I have seen codebases using option #1, and the behaviour of that definition differs slightly from option #2 and #3. It differs when we want to create both an unpersisted post and an unpersisted comment at the same time.
Question: Are there any valid use cases to use validates :post_id, presence: true at all?
Here are some tests to illustrate how the definitions differ:
# frozen_string_literal: true
require "bundler/inline"
gemfile(true) do
source "https://rubygems.org"
git_source(:github) { |repo| "https://github.com/#{repo}.git" }
gem "rails"
# If you want to test against edge Rails replace the previous line with this:
# gem "rails", github: "rails/rails", branch: "main"
gem "sqlite3"
gem "debug"
end
require "active_record"
require "minitest/autorun"
require "logger"
# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
# ActiveRecord::Base.logger = Logger.new(STDOUT)
ActiveRecord::Schema.define do
create_table :posts, force: true do |t|
end
create_table :comments, force: true do |t|
t.integer :post_id
end
end
class Post < ActiveRecord::Base
has_many :comments
end
class Comment < ActiveRecord::Base
end
class Comment1 < Comment
belongs_to :post
validates :post_id, presence: true
end
class Comment2 < Comment
belongs_to :post
validates :post, presence: true
end
class Comment3 < Comment
belongs_to :post, optional: false
end
class TestCommentCreation < Minitest::Test
def test_create_comment_with_persisted_post
post = Post.create!
assert Comment1.create!(post: post)
assert Comment2.create!(post: post)
assert Comment3.create!(post: post)
end
def test_create_comment_with_unpersisted_post_that_gets_saved
# Comment1 - post_id must exist
post = Post.new
comment1 = Comment1.new(post: post)
post.save!
error = assert_raises(ActiveRecord::RecordInvalid) do
comment1.save!
end
assert_equal "Validation failed: Post can't be blank", error.message
# Comment2 - post must exist
post = Post.new
comment2 = Comment2.new(post: post)
post.save!
comment2.save!
assert_equal Post.last, comment2.reload.post
# Comment3 - post optional false
post = Post.new
comment3 = Comment3.new(post: post)
post.save!
comment3.save!
assert_equal Post.last, comment3.reload.post
end
def test_create_comment_with_unpersisted_post
# Comment1 - post_id must exist
error = assert_raises(ActiveRecord::RecordInvalid) do
Comment1.create!(post: Post.new)
end
assert_equal "Validation failed: Post can't be blank", error.message
# Comment2 - post must exist
comment2 = Comment2.create!(post: Post.new)
assert_equal Post.last, comment2.reload.post
# Comment3 - post optional false
comment3 = Comment3.create!(post: Post.new)
assert_equal Post.last, comment3.reload.post
end
end
Option #3 would be my preference. I would say if they are using validates presence on a belongs_to it’s maybe either old code or the person didn’t understand the API of belongs_to well enough.
This is handy if you want to allow a null value but when it’s not null you want to make sure the association is valid. This will prevent you from putting an invalid reference such as affiliate_id: 424242 when that ID doesn’t exist.
Looking at the combo, would you expect the same behaviour whether you assign a destroyed post or a destroyed post id?
class Post < ActiveRecord::Base
has_many :comments
end
class Comment < ActiveRecord::Base
belongs_to :post, optional: true
validates :post, presence: true, if: :post_id
end
class TestCommentCreation < Minitest::Test
def test_invalid_post_id
post = Post.create!
post.destroy!
comment1 = Comment.new(post: post)
comment2 = Comment.new(post_id: post.id)
# Comment 1 saved successfully - unexpected?
assert comment1.save
comment1.reload
assert_equal post.id, comment1.post_id # post_id set with destroyed post.id
assert_nil comment1.post
# Comment 2 not saved as expected
refute comment2.save
assert_equal ["Post can't be blank"], comment2.errors.full_messages
end
end
Adding a foreign key to the db on post_id will prevent the insert but it will raise because the validation does not catch ActiveRecord::InvalidForeignKey error when calling comment1.save
Because I think by default Rails will set validates :post, presence: true in the above case. Ultimately this means your unexpected test case for the optional case from my example combo would react the same as the default case.
I see thanks Nick, I haven’t tested it but if you’re sticking to the default behaviour and offer an extra check for an optional association when the association is not loaded but the id provided. That makes sense.
From what I understand assigning post_id or the association post does not behave the same way.
Assigning both like Comment.new(post_id: id, post: post) depends on the order of assignment and behave differently when nil is the argument too.
Unless we’re trying to convey the idea that a persisted record is required by validating the foreign key I would think that the Rails recommendation is to not validate foreign keys but associations only.
I’m trying to figure whether this assumption is true or not and I can’t find anything on the topic.
Rails validations are only useful for when users need to fix things. The best way to figure out if a validation is needed is by asking “can a user do something to fix this issue?”. If the answer is no, just error out, because you aren’t trying to tell a user something, you’re trying to tell an engineer something. For that we use error notifications, not validations.
I will say more: even if you have a dropdown select element on a page that only lets you choose a specific item, don’t validate that the item is in the list. Just straight up explode if the item is wrong. (If you use, say, enums, that should be enough. If it’s not in the enum, it will explode.) Because the only way a user could possibly do that is if they submitted a crafted request using some kind of console, in which case you don’t owe them much.
If you’re actually implementing a public API, then your “users” are now API client engineers, so validations are now appropriate in more cases. In that case, the “select dropdown” might deserve a proper validation because it’s easy to supply and invalid value via API, so you should probably provide an informative error.
If you have a front-end team, and they are the ones making the mistake, you can instruct them to view the logs and see what they submitted incorrectly, instead of trying to craft informative responses just for them. Unless it’s a big company with isolated services maintained by isolated departments, then treat it as a public API.
In your case a database constraint is probably all you need (and the optional: false). However, if you have a form, where a user must choose the one item that will get associated to this record, and the UI allows them to submit this form without choosing an item, then a presence validation is a good way to tell them about their mistake. However, you would normally have the kind of UI that makes this situation impossible without a hand-written request, so you probably don’t have to do that.
Aha, thanks @fatkodima.
That is indeed the main difference.
I understand now why someone would validate the foreign key and not the association.
Note: I saw recently that defining a belongs_to association is probably the best anyway as it tries to mitigate extra SQL queries when the association hasn’t changed. I guess this allows to capture orphan record with application code and nice validation messages instead of DB foreign key errors.
I think the belongs_to definition now tries to make the validation a bit smarter in rails 7
