Skip to content

Resolve StrongParams rubocop TODOs for some admin controllers

Many admin controllers use params directly without using Rails' StrongParameters helper. This causes Rubocop exceptions, which were ignored when the rubocop was introduced in !155661 (merged)

Let's try to remove a bunch of admin controllers from .rubocop_todo/rails/strong_params.yml

  • app/controllers/admin/background_migrations_controller.rb
  • app/controllers/admin/batched_jobs_controller.rb
  • app/controllers/admin/broadcast_messages_controller.rb
  • app/controllers/admin/deploy_keys_controller.rb
  • app/controllers/admin/groups_controller.rb
  • app/controllers/admin/hook_logs_controller.rb
  • app/controllers/admin/hooks_controller.rb
  • app/controllers/admin/identities_controller.rb
  • app/controllers/admin/impersonation_tokens_controller.rb
  • app/controllers/admin/labels_controller.rb
  • app/controllers/admin/runner_projects_controller.rb
  • app/controllers/admin/sessions_controller.rb
  • app/controllers/admin/slacks_controller.rb
  • app/controllers/admin/spam_logs_controller.rb
  • app/controllers/admin/topics/avatars_controller.rb
  • app/controllers/admin/topics_controller.rb

The following were excluded from this effort for being, at a quick glance, too complicated or high risk for a "quick win" / easy MR review:

  • 'app/controllers/admin/abuse_reports_controller.rb' - being addressed in Resolves AbuseController StrongParam rubocop ex... (!161856 - merged)
  • app/controllers/admin/application_settings_controller.rb - lots of params being both overridden and potentially passed to ApplicationSettings::UpdateService
  • app/controllers/admin/applications_controller.rb - highish risk and a weird override of params[:owner] that I don't quickly understand
  • app/controllers/admin/projects_controller.rb - there are a number of parameters that go to ProjectsFinder; could be its own MR
  • app/controllers/admin/runners_controller.rb there is some relatively complex use of params in TagsFinder and GitRefsFinder
  • app/controllers/admin/users_controller.rb The logic in update is pretty straightforward, but probably still enough to warrant its own simple MR
  • app/controllers/admin/keys_controller.rb modifying this file led to undercoverage failure, so it needs an MR that includes new specs
Edited by Nick Malcolm