Merge branch 'enable-rubocop-application-record' into 'master'

Document ApplicationRecord / pluck_primary_key

Closes #59690

See merge request gitlab-org/gitlab-ce!26764
This commit is contained in:
Lin Jen-Shin 2019-04-01 15:55:46 +00:00
commit 700e8d1917
4 changed files with 31 additions and 2 deletions

View file

@ -146,6 +146,20 @@ Naming/FileName:
- XSS - XSS
- GRPC - GRPC
Rails/ApplicationRecord:
Enabled: true
Exclude:
# Models in database migrations should not subclass from ApplicationRecord
# as they need to be as decoupled from application code as possible
- db/**/*.rb
- lib/gitlab/background_migration/**/*.rb
- lib/gitlab/database/**/*.rb
- spec/**/*.rb
- ee/db/**/*.rb
- ee/lib/gitlab/background_migration/**/*.rb
- ee/lib/ee/gitlab/background_migration/**/*.rb
- ee/spec/**/*.rb
# GitLab ################################################################### # GitLab ###################################################################
Gitlab/ModuleWithInstanceVariables: Gitlab/ModuleWithInstanceVariables:

View file

@ -155,6 +155,21 @@ The _only_ time you should use `pluck` is when you actually need to operate on
the values in Ruby itself (e.g. write them to a file). In almost all other cases the values in Ruby itself (e.g. write them to a file). In almost all other cases
you should ask yourself "Can I not just use a sub-query?". you should ask yourself "Can I not just use a sub-query?".
In line with our `CodeReuse/ActiveRecord` cop, you should only use forms like
`pluck(:id)` or `pluck(:user_id)` within model code. In the former case, you can
use the `ApplicationRecord`-provided `.pluck_primary_key` helper method instead.
In the latter, you should add a small helper method to the relevant model.
## Inherit from ApplicationRecord
Most models in the GitLab codebase should inherit from `ApplicationRecord`,
rather than from `ActiveRecord::Base`. This allows helper methods to be easily
added.
An exception to this rule exists for models created in database migrations. As
these should be isolated from application code, they should continue to subclass
from `ActiveRecord::Base`.
## Use UNIONs ## Use UNIONs
UNIONs aren't very commonly used in most Rails applications but they're very UNIONs aren't very commonly used in most Rails applications but they're very

View file

@ -24,7 +24,7 @@ namespace :tokens do
end end
end end
class TmpUser < ActiveRecord::Base class TmpUser < ActiveRecord::Base # rubocop:disable Rails/ApplicationRecord
include TokenAuthenticatable include TokenAuthenticatable
self.table_name = 'users' self.table_name = 'users'

View file

@ -22,7 +22,7 @@ module RspecProfiling
ENV['RSPEC_PROFILING_POSTGRES_URL'] ENV['RSPEC_PROFILING_POSTGRES_URL']
end end
class Result < ActiveRecord::Base class Result < ActiveRecord::Base # rubocop:disable Rails/ApplicationRecord
acts_as_copy_target acts_as_copy_target
end end
end end