Commit dadd2772 authored by Siddharth Asthana's avatar Siddharth Asthana Committed by Peter Leitzen

Resolve Rails/FindById rubocop offenses

Changelog: other
EE: true
parent 4c6b42a8
...@@ -333,22 +333,6 @@ Rails/CreateTableWithTimestamps: ...@@ -333,22 +333,6 @@ Rails/CreateTableWithTimestamps:
Rails/FilePath: Rails/FilePath:
Enabled: false Enabled: false
# Offense count: 15
# Cop supports --auto-correct.
Rails/FindById:
Exclude:
- 'app/controllers/projects/pipelines_controller.rb'
- 'app/services/concerns/deploy_token_methods.rb'
- 'ee/app/controllers/ee/groups/group_members_controller.rb'
- 'ee/lib/api/audit_events.rb'
- 'ee/lib/api/merge_request_approval_rules.rb'
- 'ee/lib/ee/api/groups.rb'
- 'ee/lib/ee/api/projects.rb'
- 'ee/spec/finders/audit_log_finder_spec.rb'
- 'lib/api/snippets.rb'
- 'spec/finders/concerns/finder_methods_spec.rb'
- 'spec/finders/concerns/finder_with_cross_project_access_spec.rb'
# Offense count: 354 # Offense count: 354
# Configuration parameters: Include. # Configuration parameters: Include.
# Include: app/models/**/*.rb # Include: app/models/**/*.rb
......
...@@ -268,7 +268,7 @@ class Projects::PipelinesController < Projects::ApplicationController ...@@ -268,7 +268,7 @@ class Projects::PipelinesController < Projects::ApplicationController
project project
.all_pipelines .all_pipelines
.includes(builds: :tags, user: :status) .includes(builds: :tags, user: :status)
.find_by!(id: params[:id]) .find(params[:id])
.present(current_user: current_user) .present(current_user: current_user)
end end
end end
......
...@@ -11,7 +11,7 @@ module DeployTokenMethods ...@@ -11,7 +11,7 @@ module DeployTokenMethods
end end
def destroy_deploy_token(entity, params) def destroy_deploy_token(entity, params)
deploy_token = entity.deploy_tokens.find_by_id!(params[:token_id]) deploy_token = entity.deploy_tokens.find(params[:token_id])
deploy_token.destroy deploy_token.destroy
end end
......
...@@ -29,7 +29,7 @@ module EE ...@@ -29,7 +29,7 @@ module EE
# rubocop:disable Gitlab/ModuleWithInstanceVariables # rubocop:disable Gitlab/ModuleWithInstanceVariables
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def override def override
member = membershipable_members.find_by!(id: params[:id]) member = membershipable_members.find(params[:id])
result = ::Members::UpdateService.new(current_user, override_params).execute(member, permission: :override) result = ::Members::UpdateService.new(current_user, override_params).execute(member, permission: :override)
......
...@@ -46,7 +46,7 @@ module API ...@@ -46,7 +46,7 @@ module API
level = ::Gitlab::Audit::Levels::Instance.new level = ::Gitlab::Audit::Levels::Instance.new
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
# This is not `find_by!` from ActiveRecord # This is not `find_by!` from ActiveRecord
audit_event = AuditLogFinder.new(level: level).find_by!(id: params[:id]) audit_event = AuditLogFinder.new(level: level).find(params[:id])
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
present audit_event, with: EE::API::Entities::AuditEvent present audit_event, with: EE::API::Entities::AuditEvent
......
...@@ -8,7 +8,7 @@ module API ...@@ -8,7 +8,7 @@ module API
helpers do helpers do
def find_merge_request_approval_rule(merge_request, id) def find_merge_request_approval_rule(merge_request, id)
merge_request.approval_rules.find_by_id!(id) merge_request.approval_rules.find(id)
end end
end end
......
...@@ -133,11 +133,11 @@ module EE ...@@ -133,11 +133,11 @@ module EE
end end
get '/:audit_event_id' do get '/:audit_event_id' do
level = ::Gitlab::Audit::Levels::Group.new(group: user_group) level = ::Gitlab::Audit::Levels::Group.new(group: user_group)
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord, Rails/FindById
# This is not `find_by!` from ActiveRecord # This is not `find_by!` from ActiveRecord
audit_event = AuditLogFinder.new(level: level, params: audit_log_finder_params) audit_event = AuditLogFinder.new(level: level, params: audit_log_finder_params)
.find_by!(id: params[:audit_event_id]) .find_by!(id: params[:audit_event_id])
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord, Rails/FindById
present audit_event, with: EE::API::Entities::AuditEvent present audit_event, with: EE::API::Entities::AuditEvent
end end
......
...@@ -55,11 +55,11 @@ module EE ...@@ -55,11 +55,11 @@ module EE
end end
get '/:audit_event_id', feature_category: :audit_events do get '/:audit_event_id', feature_category: :audit_events do
level = ::Gitlab::Audit::Levels::Project.new(project: user_project) level = ::Gitlab::Audit::Levels::Project.new(project: user_project)
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord, Rails/FindById
# This is not `find_by!` from ActiveRecord # This is not `find_by!` from ActiveRecord
audit_event = AuditLogFinder.new(level: level, params: audit_log_finder_params) audit_event = AuditLogFinder.new(level: level, params: audit_log_finder_params)
.find_by!(id: params[:audit_event_id]) .find_by!(id: params[:audit_event_id])
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord, Rails/FindById
present audit_event, with: EE::API::Entities::AuditEvent present audit_event, with: EE::API::Entities::AuditEvent
end end
......
...@@ -350,7 +350,7 @@ RSpec.describe AuditLogFinder do ...@@ -350,7 +350,7 @@ RSpec.describe AuditLogFinder do
describe '#find_by!' do describe '#find_by!' do
let(:id) { user_audit_event.id } let(:id) { user_audit_event.id }
subject { finder.find_by!(id: id) } subject { finder.find_by!(id: id) } # rubocop:disable Rails/FindById
it { is_expected.to eq(user_audit_event) } it { is_expected.to eq(user_audit_event) }
......
...@@ -200,7 +200,7 @@ module API ...@@ -200,7 +200,7 @@ module API
get ":id/user_agent_detail" do get ":id/user_agent_detail" do
authenticated_as_admin! authenticated_as_admin!
snippet = Snippet.find_by_id!(params[:id]) snippet = Snippet.find(params[:id])
break not_found!('UserAgentDetail') unless snippet.user_agent_detail break not_found!('UserAgentDetail') unless snippet.user_agent_detail
......
...@@ -30,6 +30,7 @@ RSpec.describe FinderMethods do ...@@ -30,6 +30,7 @@ RSpec.describe FinderMethods do
authorized_project.add_developer(user) authorized_project.add_developer(user)
end end
# rubocop:disable Rails/FindById
describe '#find_by!' do describe '#find_by!' do
it 'returns the project if the user has access' do it 'returns the project if the user has access' do
expect(finder.find_by!(id: authorized_project.id)).to eq(authorized_project) expect(finder.find_by!(id: authorized_project.id)).to eq(authorized_project)
...@@ -53,6 +54,7 @@ RSpec.describe FinderMethods do ...@@ -53,6 +54,7 @@ RSpec.describe FinderMethods do
finder.find_by!(id: authorized_project.id) finder.find_by!(id: authorized_project.id)
end end
end end
# rubocop:enable Rails/FindById
describe '#find' do describe '#find' do
it 'returns the project if the user has access' do it 'returns the project if the user has access' do
......
...@@ -93,11 +93,11 @@ RSpec.describe FinderWithCrossProjectAccess do ...@@ -93,11 +93,11 @@ RSpec.describe FinderWithCrossProjectAccess do
it 'checks the accessibility of the subject directly' do it 'checks the accessibility of the subject directly' do
expect_access_check_on_result expect_access_check_on_result
finder.find_by!(id: result.id) finder.find(result.id)
end end
it 're-enables the check after the find failed' do it 're-enables the check after the find failed' do
finder.find_by!(id: non_existing_record_id) rescue ActiveRecord::RecordNotFound finder.find(non_existing_record_id) rescue ActiveRecord::RecordNotFound
expect(finder.instance_variable_get(:@should_skip_cross_project_check)) expect(finder.instance_variable_get(:@should_skip_cross_project_check))
.to eq(false) .to eq(false)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment