CODEWITHSUNDEEP
ruby-on-railsactiverecordassociations
John Donner
John Donner

Reputation: 542

Is it better to update an associated record through the parent or by itself?

Imagine we have an Article and a Comment model. We setup our routes like:

# routes.rb
resources :articles do
  resources :comments
end

Now, we can destroy a comment via the CommentController, but there are a number of approaches that I've seen been implemented.

# method 1
def destroy
  Comment.where(article_id: params[:article_id]).find(params[:id]).destroy
end

# method 2
def destroy
  Comment.find(params[:id]).destroy
end

# method 3
def destroy
  article = Article.find(params[:article_id])
  comment = article.comments.find(params[:id])
  comment.destroy
end

Which is better and why?

I've seen in old Railscasts episodes and blogs that we should do the former for "security" reasons or because it is better to ensure that comments can only be found within their respective article, but why is that better? I haven't been able to find anything that goes too deep into the answer.

Upvotes: 1

Views: 46

Answers (1)

Adam Lassek
Adam Lassek

Reputation: 35515

When you are working with nested data in this way, it's better to scope your model lookup under the parent to avoid people iterating through ids in a simplistic way. You generally want to discourage that, and it will protect you against more serious security problems if it's a habit.

For instance, say you have some sort of visibility permission on Article. With method 2, it is possible to use an article_id that you are allowed to see to access Comments that you aren't.

Methods 1 & 3 are ostensibly doing the same thing, but I would prefer 1 because it uses fewer trips to the DB.

Upvotes: 1

Related Questions