Commit 03da9d9c authored by Peter Leitzen's avatar Peter Leitzen

Merge branch 'issue_239356_fix_Rails/FindById' into 'master'

Resolve Rails/FindById rubocop offenses

See merge request gitlab-org/gitlab!81390
parents 2a2a322e dadd2772
......@@ -333,22 +333,6 @@ Rails/CreateTableWithTimestamps:
Rails/FilePath:
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
# Configuration parameters: Include.
# Include: app/models/**/*.rb
......
......@@ -268,7 +268,7 @@ class Projects::PipelinesController < Projects::ApplicationController
project
.all_pipelines
.includes(builds: :tags, user: :status)
.find_by!(id: params[:id])
.find(params[:id])
.present(current_user: current_user)
end
end
......
......@@ -11,7 +11,7 @@ module DeployTokenMethods
end
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
end
......
......@@ -29,7 +29,7 @@ module EE
# rubocop:disable Gitlab/ModuleWithInstanceVariables
# rubocop: disable CodeReuse/ActiveRecord
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)
......
......@@ -46,7 +46,7 @@ module API
level = ::Gitlab::Audit::Levels::Instance.new
# rubocop: disable CodeReuse/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
present audit_event, with: EE::API::Entities::AuditEvent
......
......@@ -8,7 +8,7 @@ module API
helpers do
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
......
......@@ -133,11 +133,11 @@ module EE
end
get '/:audit_event_id' do
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
audit_event = AuditLogFinder.new(level: level, params: audit_log_finder_params)
.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
end
......
......@@ -55,11 +55,11 @@ module EE
end
get '/:audit_event_id', feature_category: :audit_events do
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
audit_event = AuditLogFinder.new(level: level, params: audit_log_finder_params)
.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
end
......
......@@ -350,7 +350,7 @@ RSpec.describe AuditLogFinder do
describe '#find_by!' do
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) }
......
......@@ -200,7 +200,7 @@ module API
get ":id/user_agent_detail" do
authenticated_as_admin!
snippet = Snippet.find_by_id!(params[:id])
snippet = Snippet.find(params[:id])
break not_found!('UserAgentDetail') unless snippet.user_agent_detail
......
......@@ -30,6 +30,7 @@ RSpec.describe FinderMethods do
authorized_project.add_developer(user)
end
# rubocop:disable Rails/FindById
describe '#find_by!' do
it 'returns the project if the user has access' do
expect(finder.find_by!(id: authorized_project.id)).to eq(authorized_project)
......@@ -53,6 +54,7 @@ RSpec.describe FinderMethods do
finder.find_by!(id: authorized_project.id)
end
end
# rubocop:enable Rails/FindById
describe '#find' do
it 'returns the project if the user has access' do
......
......@@ -93,11 +93,11 @@ RSpec.describe FinderWithCrossProjectAccess do
it 'checks the accessibility of the subject directly' do
expect_access_check_on_result
finder.find_by!(id: result.id)
finder.find(result.id)
end
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))
.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