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/CreateTableWithTimestamps shouldn't warn when id: false is used #34

Closed
connorshea opened this issue Jan 3, 2019 · 3 comments · Fixed by #975
Closed

Rails/CreateTableWithTimestamps shouldn't warn when id: false is used #34

connorshea opened this issue Jan 3, 2019 · 3 comments · Fixed by #975

Comments

@connorshea
Copy link

connorshea commented Jan 3, 2019

In migrations you can create a join table like so:

create_table :games, :genres, id: false do |t|
  t.integer :game_id
  t.integer :genre_id
end

But Rails/CreateTableWithTimestamps will warn about this code because the table is being created without timestamps. Join tables shouldn't have timestamps, so this is a false positive.

https://guides.rubyonrails.org/association_basics.html#creating-join-tables-for-has-and-belongs-to-many-associations

@andyw8
Copy link
Contributor

andyw8 commented Jan 3, 2019

I think it's still useful to have timestamps on a join table, even if just for troubleshooting.

@alexey
Copy link

alexey commented Sep 10, 2020

I disagree with @andyw8, because it's seems useless for habtm tables.
It useful for more complex join tables.
Simple join/habtm tables easily can increase time of response and overflood your db with unusable two magic time fields (think about that join record will be touch updated with updated_at when associated records will also update)

My opinion is to have only id's fields and when you really need timestamps - add them(during my 15y Rails career never had to use timestamps in simple habtm's)

Therefore I totally agree with @connorshea that this is false positive

koic added a commit to koic/rubocop-rails that referenced this issue Apr 4, 2023
…lse` and not include `timestamps`

Fixes rubocop#34.

This PR allows `CreateTableWithTimestamps` when using `id: false` and not include `timestamps`.

There are design arguments for both allowing and disallowing migration file has `timestamps`
when `id: false` (join table) . For the following reasons, allow it when `id: false`:

For example, when running `bin/rails g migration create_articles`, the migration file containing
`timestamps` is created by default. Perhaps editing `id: false` and removing `timestamps` will be
intentional. On the other hand, if it is not intentionally deleted, it will remain.

So I think that it is possible to allow the behavior that `timestamps` is not written for `id: false`.
This emphasizes respecting user's editing intentions.
@koic koic closed this as completed in #975 Apr 5, 2023
koic added a commit that referenced this issue Apr 5, 2023
…for_id_false

[Fix #34] Allow `CreateTableWithTimestamps` when using `id: false` and not include `timestamps`
@connorshea
Copy link
Author

Thank you Koichi!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants