Commit 4048aafa authored by Igor Drozdov's avatar Igor Drozdov

Enforce using symbolic status codes in controllers

This commit removes Rails/HttpStatus cop out of todo
and corrects the existing offenses
parent 7ae80df4
...@@ -401,13 +401,6 @@ Rails/FilePath: ...@@ -401,13 +401,6 @@ Rails/FilePath:
Rails/HasManyOrHasOneDependent: Rails/HasManyOrHasOneDependent:
Enabled: false Enabled: false
# Offense count: 40
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: numeric, symbolic
Rails/HttpStatus:
Enabled: false
# Offense count: 2 # Offense count: 2
# Configuration parameters: Include. # Configuration parameters: Include.
# Include: app/controllers/**/*.rb # Include: app/controllers/**/*.rb
......
...@@ -44,7 +44,7 @@ class Admin::ApplicationsController < Admin::ApplicationController ...@@ -44,7 +44,7 @@ class Admin::ApplicationsController < Admin::ApplicationController
def destroy def destroy
@application.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 end
private private
......
...@@ -69,7 +69,7 @@ class Admin::GroupsController < Admin::ApplicationController ...@@ -69,7 +69,7 @@ class Admin::GroupsController < Admin::ApplicationController
Groups::DestroyService.new(@group, current_user).async_execute Groups::DestroyService.new(@group, current_user).async_execute
redirect_to admin_groups_path, redirect_to admin_groups_path,
status: 302, status: :found,
alert: _('Group %{group_name} was scheduled for deletion.') % { group_name: @group.name } alert: _('Group %{group_name} was scheduled for deletion.') % { group_name: @group.name }
end end
......
...@@ -38,9 +38,9 @@ class Admin::IdentitiesController < Admin::ApplicationController ...@@ -38,9 +38,9 @@ class Admin::IdentitiesController < Admin::ApplicationController
def destroy def destroy
if @identity.destroy if @identity.destroy
RepairLdapBlockedUserService.new(@user).execute 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 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
end end
......
...@@ -17,9 +17,9 @@ class Admin::KeysController < Admin::ApplicationController ...@@ -17,9 +17,9 @@ class Admin::KeysController < Admin::ApplicationController
respond_to do |format| respond_to do |format|
if key.destroy 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 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 end
end end
......
...@@ -43,7 +43,7 @@ class Admin::LabelsController < Admin::ApplicationController ...@@ -43,7 +43,7 @@ class Admin::LabelsController < Admin::ApplicationController
respond_to do |format| respond_to do |format|
format.html do 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 end
format.js format.js
end end
......
...@@ -41,7 +41,7 @@ class Admin::ProjectsController < Admin::ApplicationController ...@@ -41,7 +41,7 @@ class Admin::ProjectsController < Admin::ApplicationController
redirect_to admin_projects_path, status: :found redirect_to admin_projects_path, status: :found
rescue Projects::DestroyService::DestroyError => ex 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 end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
......
...@@ -13,7 +13,7 @@ class Admin::SpamLogsController < Admin::ApplicationController ...@@ -13,7 +13,7 @@ class Admin::SpamLogsController < Admin::ApplicationController
if params[:remove_user] if params[:remove_user]
spam_log.remove_user(deleted_by: current_user) spam_log.remove_user(deleted_by: current_user)
redirect_to admin_spam_logs_path, redirect_to admin_spam_logs_path,
status: 302, status: :found,
notice: _('User %{username} was successfully removed.') % { username: spam_log.user.username } notice: _('User %{username} was successfully removed.') % { username: spam_log.user.username }
else else
spam_log.destroy spam_log.destroy
......
...@@ -169,7 +169,7 @@ class Admin::UsersController < Admin::ApplicationController ...@@ -169,7 +169,7 @@ class Admin::UsersController < Admin::ApplicationController
user.delete_async(deleted_by: current_user, params: params.permit(:hard_delete)) user.delete_async(deleted_by: current_user, params: params.permit(:hard_delete))
respond_to do |format| 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 } format.json { head :ok }
end end
end end
......
...@@ -58,7 +58,7 @@ class ApplicationController < ActionController::Base ...@@ -58,7 +58,7 @@ class ApplicationController < ActionController::Base
rescue_from Encoding::CompatibilityError do |exception| rescue_from Encoding::CompatibilityError do |exception|
log_exception(exception) log_exception(exception)
render "errors/encoding", layout: "errors", status: 500 render "errors/encoding", layout: "errors", status: :internal_server_error
end end
rescue_from ActiveRecord::RecordNotFound do |exception| rescue_from ActiveRecord::RecordNotFound do |exception|
...@@ -197,19 +197,19 @@ class ApplicationController < ActionController::Base ...@@ -197,19 +197,19 @@ class ApplicationController < ActionController::Base
end end
def git_not_found! 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 end
def render_403 def render_403
respond_to do |format| respond_to do |format|
format.any { head :forbidden } 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
end end
def render_404 def render_404
respond_to do |format| 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 # 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.js { render json: '', status: :not_found, content_type: 'application/json' }
format.any { head :not_found } format.any { head :not_found }
......
...@@ -56,7 +56,7 @@ module LfsRequest ...@@ -56,7 +56,7 @@ module LfsRequest
documentation_url: help_url documentation_url: help_url
}, },
content_type: CONTENT_TYPE, content_type: CONTENT_TYPE,
status: 403 status: :forbidden
) )
end end
...@@ -67,7 +67,7 @@ module LfsRequest ...@@ -67,7 +67,7 @@ module LfsRequest
documentation_url: help_url documentation_url: help_url
}, },
content_type: CONTENT_TYPE, content_type: CONTENT_TYPE,
status: 404 status: :not_found
) )
end end
......
...@@ -22,7 +22,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController ...@@ -22,7 +22,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
respond_to do |format| respond_to do |format|
format.html do format.html do
redirect_to dashboard_todos_path, redirect_to dashboard_todos_path,
status: 302, status: :found,
notice: _('To-do item successfully marked as done.') notice: _('To-do item successfully marked as done.')
end end
format.js { head :ok } format.js { head :ok }
...@@ -34,7 +34,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController ...@@ -34,7 +34,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
updated_ids = TodoService.new.mark_todos_as_done(@todos, current_user) updated_ids = TodoService.new.mark_todos_as_done(@todos, current_user)
respond_to do |format| 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.js { head :ok }
format.json { render json: todos_counts.merge(updated_ids: updated_ids) } format.json { render json: todos_counts.merge(updated_ids: updated_ids) }
end end
......
...@@ -63,7 +63,7 @@ class Groups::LabelsController < Groups::ApplicationController ...@@ -63,7 +63,7 @@ class Groups::LabelsController < Groups::ApplicationController
respond_to do |format| respond_to do |format|
format.html do 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 end
format.js format.js
end end
......
...@@ -116,7 +116,7 @@ class GroupsController < Groups::ApplicationController ...@@ -116,7 +116,7 @@ class GroupsController < Groups::ApplicationController
def destroy def destroy
Groups::DestroyService.new(@group, current_user).async_execute 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 end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
......
...@@ -36,7 +36,7 @@ class HelpController < ApplicationController ...@@ -36,7 +36,7 @@ class HelpController < ApplicationController
render 'show.html.haml' render 'show.html.haml'
else else
# Force template to Haml # 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
end end
......
...@@ -57,7 +57,7 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController ...@@ -57,7 +57,7 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController
end end
rescue_from ActiveRecord::RecordNotFound do |exception| rescue_from ActiveRecord::RecordNotFound do |exception|
render "errors/not_found", layout: "errors", status: 404 render "errors/not_found", layout: "errors", status: :not_found
end end
def create_application_params def create_application_params
......
...@@ -13,7 +13,7 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio ...@@ -13,7 +13,7 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio
end end
redirect_to applications_profile_url, redirect_to applications_profile_url,
status: 302, status: :found,
notice: I18n.t(:notice, scope: [:doorkeeper, :flash, :authorized_applications, :destroy]) notice: I18n.t(:notice, scope: [:doorkeeper, :flash, :authorized_applications, :destroy])
end end
end end
...@@ -47,7 +47,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -47,7 +47,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
def omniauth_error def omniauth_error
@provider = params[:provider] @provider = params[:provider]
@error = params[:error] @error = params[:error]
render 'errors/omniauth_error', layout: "oauth_error", status: 422 render 'errors/omniauth_error', layout: "oauth_error", status: :unprocessable_entity
end end
def cas3 def cas3
......
...@@ -4,6 +4,6 @@ class Profiles::U2fRegistrationsController < Profiles::ApplicationController ...@@ -4,6 +4,6 @@ class Profiles::U2fRegistrationsController < Profiles::ApplicationController
def destroy def destroy
u2f_registration = current_user.u2f_registrations.find(params[:id]) u2f_registration = current_user.u2f_registrations.find(params[:id])
u2f_registration.destroy 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
end end
...@@ -76,7 +76,7 @@ class Projects::LabelsController < Projects::ApplicationController ...@@ -76,7 +76,7 @@ class Projects::LabelsController < Projects::ApplicationController
@labels = find_labels @labels = find_labels
redirect_to project_labels_path(@project), redirect_to project_labels_path(@project),
status: 302, status: :found,
notice: 'Label was removed' notice: 'Label was removed'
end end
......
...@@ -109,7 +109,7 @@ class Projects::LfsApiController < Projects::GitHttpClientController ...@@ -109,7 +109,7 @@ class Projects::LfsApiController < Projects::GitHttpClientController
message: lfs_read_only_message message: lfs_read_only_message
}, },
content_type: LfsRequest::CONTENT_TYPE, content_type: LfsRequest::CONTENT_TYPE,
status: 403 status: :forbidden
) )
end end
end end
......
...@@ -21,7 +21,7 @@ class Projects::PagesController < Projects::ApplicationController ...@@ -21,7 +21,7 @@ class Projects::PagesController < Projects::ApplicationController
respond_to do |format| respond_to do |format|
format.html do format.html do
redirect_to project_pages_path(@project), redirect_to project_pages_path(@project),
status: 302, status: :found,
notice: 'Pages were removed' notice: 'Pages were removed'
end end
end end
......
...@@ -42,7 +42,7 @@ class Projects::PagesDomainsController < Projects::ApplicationController ...@@ -42,7 +42,7 @@ class Projects::PagesDomainsController < Projects::ApplicationController
def update def update
if @domain.update(update_params) if @domain.update(update_params)
redirect_to project_pages_path(@project), redirect_to project_pages_path(@project),
status: 302, status: :found,
notice: 'Domain was updated' notice: 'Domain was updated'
else else
render 'edit' render 'edit'
...@@ -55,7 +55,7 @@ class Projects::PagesDomainsController < Projects::ApplicationController ...@@ -55,7 +55,7 @@ class Projects::PagesDomainsController < Projects::ApplicationController
respond_to do |format| respond_to do |format|
format.html do format.html do
redirect_to project_pages_path(@project), redirect_to project_pages_path(@project),
status: 302, status: :found,
notice: 'Domain was removed' notice: 'Domain was removed'
end end
format.js format.js
......
...@@ -84,7 +84,7 @@ class Projects::TagsController < Projects::ApplicationController ...@@ -84,7 +84,7 @@ class Projects::TagsController < Projects::ApplicationController
format.html do format.html do
redirect_to project_tags_path(@project), redirect_to project_tags_path(@project),
alert: @error, status: 303 alert: @error, status: :see_other
end end
format.js do format.js do
......
...@@ -110,7 +110,7 @@ class Projects::WikisController < Projects::ApplicationController ...@@ -110,7 +110,7 @@ class Projects::WikisController < Projects::ApplicationController
WikiPages::DestroyService.new(@project, current_user).execute(@page) WikiPages::DestroyService.new(@project, current_user).execute(@page)
redirect_to project_wiki_path(@project, :home), redirect_to project_wiki_path(@project, :home),
status: 302, status: :found,
notice: _("Page was successfully deleted") notice: _("Page was successfully deleted")
rescue Gitlab::Git::Wiki::OperationError => e rescue Gitlab::Git::Wiki::OperationError => e
@error = e @error = e
......
...@@ -154,7 +154,7 @@ class ProjectsController < Projects::ApplicationController ...@@ -154,7 +154,7 @@ class ProjectsController < Projects::ApplicationController
redirect_to dashboard_projects_path, status: :found redirect_to dashboard_projects_path, status: :found
rescue Projects::DestroyService::DestroyError => ex 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 end
def new_issuable_address def new_issuable_address
......
...@@ -45,9 +45,9 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -45,9 +45,9 @@ class RegistrationsController < Devise::RegistrationsController
if destroy_confirmation_valid? if destroy_confirmation_valid?
current_user.delete_async(deleted_by: current_user) current_user.delete_async(deleted_by: current_user)
session.try(:destroy) 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 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
end end
......
...@@ -25,7 +25,7 @@ module EE ...@@ -25,7 +25,7 @@ module EE
documentation_url: help_url documentation_url: help_url
}, },
content_type: ::LfsRequest::CONTENT_TYPE, content_type: ::LfsRequest::CONTENT_TYPE,
status: 406 status: :not_acceptable
) )
end end
......
...@@ -24,6 +24,6 @@ class OmniauthKerberosSpnegoController < ApplicationController ...@@ -24,6 +24,6 @@ class OmniauthKerberosSpnegoController < ApplicationController
# when the browser has given up. # when the browser has given up.
# #
headers['Www-Authenticate'] = spnego_challenge 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
end end
...@@ -25,7 +25,7 @@ module Projects ...@@ -25,7 +25,7 @@ module Projects
@package = project.packages.find(params[:id]) @package = project.packages.find(params[:id])
@package.destroy @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 end
end end
......
...@@ -637,7 +637,7 @@ describe ApplicationController do ...@@ -637,7 +637,7 @@ describe ApplicationController do
context 'given a 422 error page' do context 'given a 422 error page' do
controller do controller do
def index def index
render 'errors/omniauth_error', layout: 'errors', status: 422 render 'errors/omniauth_error', layout: 'errors', status: :unprocessable_entity
end end
end end
...@@ -651,7 +651,7 @@ describe ApplicationController do ...@@ -651,7 +651,7 @@ describe ApplicationController do
context 'given a 500 error page' do context 'given a 500 error page' do
controller do controller do
def index def index
render 'errors/omniauth_error', layout: 'errors', status: 500 render 'errors/omniauth_error', layout: 'errors', status: :internal_server_error
end end
end end
...@@ -665,7 +665,7 @@ describe ApplicationController do ...@@ -665,7 +665,7 @@ describe ApplicationController do
context 'given a 200 success page' do context 'given a 200 success page' do
controller do controller do
def index def index
render 'errors/omniauth_error', layout: 'errors', status: 200 render 'errors/omniauth_error', layout: 'errors', status: :ok
end end
end end
......
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