Merge branch 'id-enforce-symbolic-status-codes-in-app' into 'master'

Enforce using symbolic status codes in controllers

See merge request gitlab-org/gitlab!19929

.rubocop_todo.yml

@@ -401,13 +401,6 @@ Rails/FilePath:
 Rails/HasManyOrHasOneDependent:
   Enabled: false
 
-# Offense count: 40
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: numeric, symbolic
-Rails/HttpStatus:
-  Enabled: false
-
 # Offense count: 2
 # Configuration parameters: Include.
 # Include: app/controllers/**/*.rb

app/controllers/admin/applications_controller.rb

@@ -44,7 +44,7 @@ class Admin::ApplicationsController < Admin::ApplicationController
 
   def destroy
     @application.destroy
-    redirect_to admin_applications_url, status: 302, notice: _('Application was successfully destroyed.')
+    redirect_to admin_applications_url, status: :found, notice: _('Application was successfully destroyed.')
   end
 
   private

app/controllers/admin/groups_controller.rb

@@ -69,7 +69,7 @@ class Admin::GroupsController < Admin::ApplicationController
     Groups::DestroyService.new(@group, current_user).async_execute
 
     redirect_to admin_groups_path,
-                status: 302,
+                status: :found,
                 alert: _('Group %{group_name} was scheduled for deletion.') % { group_name: @group.name }
   end
 

app/controllers/admin/identities_controller.rb

@@ -38,9 +38,9 @@ class Admin::IdentitiesController < Admin::ApplicationController
   def destroy
     if @identity.destroy
       RepairLdapBlockedUserService.new(@user).execute
-      redirect_to admin_user_identities_path(@user), status: 302, notice: _('User identity was successfully removed.')
+      redirect_to admin_user_identities_path(@user), status: :found, notice: _('User identity was successfully removed.')
     else
-      redirect_to admin_user_identities_path(@user), status: 302, alert: _('Failed to remove user identity.')
+      redirect_to admin_user_identities_path(@user), status: :found, alert: _('Failed to remove user identity.')
     end
   end
 

app/controllers/admin/keys_controller.rb

@@ -17,9 +17,9 @@ class Admin::KeysController < Admin::ApplicationController
 
     respond_to do |format|
       if key.destroy
-        format.html { redirect_to keys_admin_user_path(user), status: 302, notice: _('User key was successfully removed.') }
+        format.html { redirect_to keys_admin_user_path(user), status: :found, notice: _('User key was successfully removed.') }
       else
-        format.html { redirect_to keys_admin_user_path(user), status: 302, alert: _('Failed to remove user key.') }
+        format.html { redirect_to keys_admin_user_path(user), status: :found, alert: _('Failed to remove user key.') }
       end
     end
   end

app/controllers/admin/labels_controller.rb

@@ -43,7 +43,7 @@ class Admin::LabelsController < Admin::ApplicationController
 
     respond_to do |format|
       format.html do
-        redirect_to admin_labels_path, status: 302, notice: _('Label was removed')
+        redirect_to admin_labels_path, status: :found, notice: _('Label was removed')
       end
       format.js
     end

app/controllers/admin/projects_controller.rb

@@ -41,7 +41,7 @@ class Admin::ProjectsController < Admin::ApplicationController
 
     redirect_to admin_projects_path, status: :found
   rescue Projects::DestroyService::DestroyError => ex
-    redirect_to admin_projects_path, status: 302, alert: ex.message
+    redirect_to admin_projects_path, status: :found, alert: ex.message
   end
 
   # rubocop: disable CodeReuse/ActiveRecord

app/controllers/admin/spam_logs_controller.rb

@@ -13,7 +13,7 @@ class Admin::SpamLogsController < Admin::ApplicationController
     if params[:remove_user]
       spam_log.remove_user(deleted_by: current_user)
       redirect_to admin_spam_logs_path,
-                  status: 302,
+                  status: :found,
                   notice: _('User %{username} was successfully removed.') % { username: spam_log.user.username }
     else
       spam_log.destroy

app/controllers/admin/users_controller.rb

@@ -169,7 +169,7 @@ class Admin::UsersController < Admin::ApplicationController
     user.delete_async(deleted_by: current_user, params: params.permit(:hard_delete))
 
     respond_to do |format|
-      format.html { redirect_to admin_users_path, status: 302, notice: _("The user is being deleted.") }
+      format.html { redirect_to admin_users_path, status: :found, notice: _("The user is being deleted.") }
       format.json { head :ok }
     end
   end

app/controllers/application_controller.rb

@@ -58,7 +58,7 @@ class ApplicationController < ActionController::Base
 
   rescue_from Encoding::CompatibilityError do |exception|
     log_exception(exception)
-    render "errors/encoding", layout: "errors", status: 500
+    render "errors/encoding", layout: "errors", status: :internal_server_error
   end
 
   rescue_from ActiveRecord::RecordNotFound do |exception|
@@ -197,19 +197,19 @@ class ApplicationController < ActionController::Base
   end
 
   def git_not_found!
-    render "errors/git_not_found.html", layout: "errors", status: 404
+    render "errors/git_not_found.html", layout: "errors", status: :not_found
   end
 
   def render_403
     respond_to do |format|
       format.any { head :forbidden }
-      format.html { render "errors/access_denied", layout: "errors", status: 403 }
+      format.html { render "errors/access_denied", layout: "errors", status: :forbidden }
     end
   end
 
   def render_404
     respond_to do |format|
-      format.html { render "errors/not_found", layout: "errors", status: 404 }
+      format.html { render "errors/not_found", layout: "errors", status: :not_found }
       # Prevent the Rails CSRF protector from thinking a missing .js file is a JavaScript file
       format.js { render json: '', status: :not_found, content_type: 'application/json' }
       format.any { head :not_found }

app/controllers/concerns/lfs_request.rb

@@ -56,7 +56,7 @@ module LfsRequest
         documentation_url: help_url
       },
       content_type: CONTENT_TYPE,
-      status: 403
+      status: :forbidden
     )
   end
 
@@ -67,7 +67,7 @@ module LfsRequest
         documentation_url: help_url
       },
       content_type: CONTENT_TYPE,
-      status: 404
+      status: :not_found
     )
   end
 

app/controllers/dashboard/todos_controller.rb

@@ -22,7 +22,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
     respond_to do |format|
       format.html do
         redirect_to dashboard_todos_path,
-                    status: 302,
+                    status: :found,
                     notice: _('To-do item successfully marked as done.')
       end
       format.js { head :ok }
@@ -34,7 +34,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
     updated_ids = TodoService.new.mark_todos_as_done(@todos, current_user)
 
     respond_to do |format|
-      format.html { redirect_to dashboard_todos_path, status: 302, notice: _('Everything on your to-do list is marked as done.') }
+      format.html { redirect_to dashboard_todos_path, status: :found, notice: _('Everything on your to-do list is marked as done.') }
       format.js { head :ok }
       format.json { render json: todos_counts.merge(updated_ids: updated_ids) }
     end

app/controllers/groups/labels_controller.rb

@@ -63,7 +63,7 @@ class Groups::LabelsController < Groups::ApplicationController
 
     respond_to do |format|
       format.html do
-        redirect_to group_labels_path(@group), status: 302, notice: "#{@label.name} deleted permanently"
+        redirect_to group_labels_path(@group), status: :found, notice: "#{@label.name} deleted permanently"
       end
       format.js
     end

app/controllers/groups_controller.rb

@@ -116,7 +116,7 @@ class GroupsController < Groups::ApplicationController
   def destroy
     Groups::DestroyService.new(@group, current_user).async_execute
 
-    redirect_to root_path, status: 302, alert: "Group '#{@group.name}' was scheduled for deletion."
+    redirect_to root_path, status: :found, alert: "Group '#{@group.name}' was scheduled for deletion."
   end
 
   # rubocop: disable CodeReuse/ActiveRecord

app/controllers/help_controller.rb

@@ -36,7 +36,7 @@ class HelpController < ApplicationController
           render 'show.html.haml'
         else
           # Force template to Haml
-          render 'errors/not_found.html.haml', layout: 'errors', status: 404
+          render 'errors/not_found.html.haml', layout: 'errors', status: :not_found
         end
       end
 

app/controllers/oauth/applications_controller.rb

@@ -57,7 +57,7 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController
   end
 
   rescue_from ActiveRecord::RecordNotFound do |exception|
-    render "errors/not_found", layout: "errors", status: 404
+    render "errors/not_found", layout: "errors", status: :not_found
   end
 
   def create_application_params

app/controllers/oauth/authorized_applications_controller.rb

@@ -13,7 +13,7 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio
     end
 
     redirect_to applications_profile_url,
-                status: 302,
+                status: :found,
                 notice: I18n.t(:notice, scope: [:doorkeeper, :flash, :authorized_applications, :destroy])
   end
 end

app/controllers/omniauth_callbacks_controller.rb

@@ -47,7 +47,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
   def omniauth_error
     @provider = params[:provider]
     @error = params[:error]
-    render 'errors/omniauth_error', layout: "oauth_error", status: 422
+    render 'errors/omniauth_error', layout: "oauth_error", status: :unprocessable_entity
   end
 
   def cas3

app/controllers/profiles/u2f_registrations_controller.rb

@@ -4,6 +4,6 @@ class Profiles::U2fRegistrationsController < Profiles::ApplicationController
   def destroy
     u2f_registration = current_user.u2f_registrations.find(params[:id])
     u2f_registration.destroy
-    redirect_to profile_two_factor_auth_path, status: 302, notice: _("Successfully deleted U2F device.")
+    redirect_to profile_two_factor_auth_path, status: :found, notice: _("Successfully deleted U2F device.")
   end
 end

app/controllers/projects/labels_controller.rb

@@ -76,7 +76,7 @@ class Projects::LabelsController < Projects::ApplicationController
     @labels = find_labels
 
     redirect_to project_labels_path(@project),
-                status: 302,
+                status: :found,
                 notice: 'Label was removed'
   end
 

app/controllers/projects/lfs_api_controller.rb

@@ -109,7 +109,7 @@ class Projects::LfsApiController < Projects::GitHttpClientController
           message: lfs_read_only_message
         },
         content_type: LfsRequest::CONTENT_TYPE,
-        status: 403
+        status: :forbidden
       )
     end
   end

app/controllers/projects/pages_controller.rb

@@ -21,7 +21,7 @@ class Projects::PagesController < Projects::ApplicationController
     respond_to do |format|
       format.html do
         redirect_to project_pages_path(@project),
-                    status: 302,
+                    status: :found,
                     notice: 'Pages were removed'
       end
     end

app/controllers/projects/pages_domains_controller.rb

@@ -43,7 +43,7 @@ class Projects::PagesDomainsController < Projects::ApplicationController
   def update
     if @domain.update(update_params)
       redirect_to project_pages_path(@project),
-        status: 302,
+        status: :found,
         notice: 'Domain was updated'
     else
       render 'edit'
@@ -56,7 +56,7 @@ class Projects::PagesDomainsController < Projects::ApplicationController
     respond_to do |format|
       format.html do
         redirect_to project_pages_path(@project),
-                    status: 302,
+                    status: :found,
                     notice: 'Domain was removed'
       end
       format.js

app/controllers/projects/tags_controller.rb

@@ -84,7 +84,7 @@ class Projects::TagsController < Projects::ApplicationController
 
         format.html do
           redirect_to project_tags_path(@project),
-            alert: @error, status: 303
+            alert: @error, status: :see_other
         end
 
         format.js do

app/controllers/projects/wikis_controller.rb

@@ -110,7 +110,7 @@ class Projects::WikisController < Projects::ApplicationController
     WikiPages::DestroyService.new(@project, current_user).execute(@page)
 
     redirect_to project_wiki_path(@project, :home),
-                status: 302,
+                status: :found,
                 notice: _("Page was successfully deleted")
   rescue Gitlab::Git::Wiki::OperationError => e
     @error = e

app/controllers/projects_controller.rb

@@ -154,7 +154,7 @@ class ProjectsController < Projects::ApplicationController
 
     redirect_to dashboard_projects_path, status: :found
   rescue Projects::DestroyService::DestroyError => ex
-    redirect_to edit_project_path(@project), status: 302, alert: ex.message
+    redirect_to edit_project_path(@project), status: :found, alert: ex.message
   end
 
   def new_issuable_address

app/controllers/registrations_controller.rb

@@ -45,9 +45,9 @@ class RegistrationsController < Devise::RegistrationsController
     if destroy_confirmation_valid?
       current_user.delete_async(deleted_by: current_user)
       session.try(:destroy)
-      redirect_to new_user_session_path, status: 303, notice: s_('Profiles|Account scheduled for removal.')
+      redirect_to new_user_session_path, status: :see_other, notice: s_('Profiles|Account scheduled for removal.')
     else
-      redirect_to profile_account_path, status: 303, alert: destroy_confirmation_failure_message
+      redirect_to profile_account_path, status: :see_other, alert: destroy_confirmation_failure_message
     end
   end
 

ee/app/controllers/concerns/ee/lfs_request.rb

@@ -25,7 +25,7 @@ module EE
           documentation_url: help_url
         },
         content_type: ::LfsRequest::CONTENT_TYPE,
-        status: 406
+        status: :not_acceptable
       )
     end
 

ee/app/controllers/omniauth_kerberos_spnego_controller.rb

@@ -24,6 +24,6 @@ class OmniauthKerberosSpnegoController < ApplicationController
     # when the browser has given up.
     #
     headers['Www-Authenticate'] = spnego_challenge
-    render 'errors/kerberos_denied.html.haml', layout: 'errors', status: 401
+    render 'errors/kerberos_denied.html.haml', layout: 'errors', status: :unauthorized
   end
 end

ee/app/controllers/projects/packages/packages_controller.rb

@@ -25,7 +25,7 @@ module Projects
         @package = project.packages.find(params[:id])
         @package.destroy
 
-        redirect_to project_packages_path(@project), status: 302, notice: _('Package was removed')
+        redirect_to project_packages_path(@project), status: :found, notice: _('Package was removed')
       end
     end
   end

spec/controllers/application_controller_spec.rb

@@ -637,7 +637,7 @@ describe ApplicationController do
     context 'given a 422 error page' do
       controller do
         def index
-          render 'errors/omniauth_error', layout: 'errors', status: 422
+          render 'errors/omniauth_error', layout: 'errors', status: :unprocessable_entity
         end
       end
 
@@ -651,7 +651,7 @@ describe ApplicationController do
     context 'given a 500 error page' do
       controller do
         def index
-          render 'errors/omniauth_error', layout: 'errors', status: 500
+          render 'errors/omniauth_error', layout: 'errors', status: :internal_server_error
         end
       end
 
@@ -665,7 +665,7 @@ describe ApplicationController do
     context 'given a 200 success page' do
       controller do
         def index
-          render 'errors/omniauth_error', layout: 'errors', status: 200
+          render 'errors/omniauth_error', layout: 'errors', status: :ok
         end
       end