rubocop / rails-style-guide

A community-driven Ruby on Rails style guide
http://rails.rubystyle.guide
6.48k stars 1.06k forks source link

Add examples for enforcing FKs #294

Closed pirj closed 3 years ago

pirj commented 3 years ago

fixes #287

Rendered: https://github.com/rubocop/rails-style-guide/blob/add-example-for-fks/README.adoc#foreign-key-constraints

Rails docs: https://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/SchemaStatements.html#method-i-add_reference

andyw8 commented 3 years ago

To help beginners, I wonder if this should include another integer field which shouldn't have a foreign key (e.g. position). That would help illustrate that not every fields needs to be declared as a foreign key.

pirj commented 3 years ago

@andyw8 Something like this?

# bad - does not add foreign keys
create_table :comment do |t|
  t.string :email # does not need a foreign key constraint
  t.references :article
  t.belongs_to :user
  t.integer :category_id
end
# good
create_table :comment do |t|
  t.string :email # does not need a foreign key constraint
  t.references :article, foreign_key: true
  t.belongs_to :user, foreign_key: true
  t.references :category, foreign_key: { to_table: :comment_categories }
end

I would be troubled to understand, how is email related to FKs guideline. On the other hand, some non-integer status column may deserve to be a FK if it's a natural PK in statuses table. And it needs on_delete: :nullify/on_update: :cascade. I just don't want to dive that deep in this guideline.

andyw8 commented 3 years ago

Ok, I'm fine to leave as-is.