Commit a9707e8c authored by George Thomas's avatar George Thomas Committed by George Thomas

Rewrite `if:` argument in before_action and alike when `only:` is also used

Closes #55564
This is first discovered in #54739 (comment 122609857) that if both if: and only:
are used in a before_action or after_action or alike, if: is completely ignored.
parent 037096ef
...@@ -7,7 +7,8 @@ class Projects::SnippetsController < Projects::ApplicationController ...@@ -7,7 +7,8 @@ class Projects::SnippetsController < Projects::ApplicationController
include SnippetsActions include SnippetsActions
include RendersBlob include RendersBlob
skip_before_action :verify_authenticity_token, only: [:show], if: :js_request? skip_before_action :verify_authenticity_token,
if: -> { action_name == 'show' && js_request? }
before_action :check_snippets_available! before_action :check_snippets_available!
before_action :snippet, only: [:show, :edit, :destroy, :update, :raw, :toggle_award_emoji, :mark_as_spam] before_action :snippet, only: [:show, :edit, :destroy, :update, :raw, :toggle_award_emoji, :mark_as_spam]
......
...@@ -10,7 +10,8 @@ class Projects::WikisController < Projects::ApplicationController ...@@ -10,7 +10,8 @@ class Projects::WikisController < Projects::ApplicationController
before_action :authorize_admin_wiki!, only: :destroy before_action :authorize_admin_wiki!, only: :destroy
before_action :load_project_wiki before_action :load_project_wiki
before_action :load_page, only: [:show, :edit, :update, :history, :destroy] before_action :load_page, only: [:show, :edit, :update, :history, :destroy]
before_action :valid_encoding?, only: [:show, :edit, :update], if: :load_page before_action :valid_encoding?,
if: -> { %w[show edit update].include?(action_name) && load_page }
before_action only: [:edit, :update], unless: :valid_encoding? do before_action only: [:edit, :update], unless: :valid_encoding? do
redirect_to(project_wiki_path(@project, @page)) redirect_to(project_wiki_path(@project, @page))
end end
......
...@@ -18,9 +18,11 @@ class ProjectsController < Projects::ApplicationController ...@@ -18,9 +18,11 @@ class ProjectsController < Projects::ApplicationController
before_action :redirect_git_extension, only: [:show] before_action :redirect_git_extension, only: [:show]
before_action :project, except: [:index, :new, :create, :resolve] before_action :project, except: [:index, :new, :create, :resolve]
before_action :repository, except: [:index, :new, :create, :resolve] before_action :repository, except: [:index, :new, :create, :resolve]
before_action :assign_ref_vars, only: [:show], if: :repo_exists? before_action :assign_ref_vars, if: -> { action_name == 'show' && repo_exists? }
before_action :tree, only: [:show], if: [:repo_exists?, :project_view_files?] before_action :tree,
before_action :lfs_blob_ids, only: [:show], if: [:repo_exists?, :project_view_files?] if: -> { action_name == 'show' && repo_exists? && project_view_files? }
before_action :lfs_blob_ids,
if: -> { action_name == 'show' && repo_exists? && project_view_files? }
before_action :project_export_enabled, only: [:export, :download_export, :remove_export, :generate_new_export] before_action :project_export_enabled, only: [:export, :download_export, :remove_export, :generate_new_export]
before_action :present_project, only: [:edit] before_action :present_project, only: [:edit]
before_action :authorize_download_code!, only: [:refs] before_action :authorize_download_code!, only: [:refs]
......
...@@ -8,8 +8,7 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -8,8 +8,7 @@ class RegistrationsController < Devise::RegistrationsController
prepend_before_action :check_captcha, only: :create prepend_before_action :check_captcha, only: :create
before_action :whitelist_query_limiting, only: [:destroy] before_action :whitelist_query_limiting, only: [:destroy]
before_action :ensure_terms_accepted, before_action :ensure_terms_accepted,
if: -> { Gitlab::CurrentSettings.current_application_settings.enforce_terms? }, if: -> { action_name == 'create' && Gitlab::CurrentSettings.current_application_settings.enforce_terms? }
only: [:create]
def new def new
redirect_to(new_user_session_path) redirect_to(new_user_session_path)
......
...@@ -13,18 +13,17 @@ class SessionsController < Devise::SessionsController ...@@ -13,18 +13,17 @@ class SessionsController < Devise::SessionsController
prepend_before_action :check_initial_setup, only: [:new] prepend_before_action :check_initial_setup, only: [:new]
prepend_before_action :authenticate_with_two_factor, prepend_before_action :authenticate_with_two_factor,
if: :two_factor_enabled?, only: [:create] if: -> { action_name == 'create' && two_factor_enabled? }
prepend_before_action :check_captcha, only: [:create] prepend_before_action :check_captcha, only: [:create]
prepend_before_action :store_redirect_uri, only: [:new] prepend_before_action :store_redirect_uri, only: [:new]
prepend_before_action :ldap_servers, only: [:new, :create] prepend_before_action :ldap_servers, only: [:new, :create]
prepend_before_action :require_no_authentication_without_flash, only: [:new, :create] prepend_before_action :require_no_authentication_without_flash, only: [:new, :create]
prepend_before_action :ensure_password_authentication_enabled!, if: :password_based_login?, only: [:create] prepend_before_action :ensure_password_authentication_enabled!, if: -> { action_name == 'create' && password_based_login? }
before_action :auto_sign_in_with_provider, only: [:new] before_action :auto_sign_in_with_provider, only: [:new]
before_action :load_recaptcha before_action :load_recaptcha
after_action :log_failed_login, only: [:new], if: :failed_login? after_action :log_failed_login, if: -> { action_name == 'new' && failed_login? }
helper_method :captcha_enabled? helper_method :captcha_enabled?
CAPTCHA_HEADER = 'X-GitLab-Show-Login-Captcha'.freeze CAPTCHA_HEADER = 'X-GitLab-Show-Login-Captcha'.freeze
......
...@@ -8,7 +8,8 @@ class SnippetsController < ApplicationController ...@@ -8,7 +8,8 @@ class SnippetsController < ApplicationController
include RendersBlob include RendersBlob
include PreviewMarkdown include PreviewMarkdown
skip_before_action :verify_authenticity_token, only: [:show], if: :js_request? skip_before_action :verify_authenticity_token,
if: -> { action_name == 'show' && js_request? }
before_action :snippet, only: [:show, :edit, :destroy, :update, :raw] before_action :snippet, only: [:show, :edit, :destroy, :update, :raw]
......
---
title: Rewrite `if:` argument in before_action and alike when `only:` is also used
merge_request: 24412
author: George Thomas @thegeorgeous
type: other
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