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.

7 files changed, 20 insertions, 12 deletions

diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb
index 255f1f3569a..59f948959d6 100644
--- a/app/controllers/projects/snippets_controller.rb
+++ b/app/controllers/projects/snippets_controller.rb
@@ -7,7 +7,8 @@ class Projects::SnippetsController < Projects::ApplicationController
   include SnippetsActions
   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 :snippet, only: [:show, :edit, :destroy, :update, :raw, :toggle_award_emoji, :mark_as_spam]
diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb
index fa5bdbc7d49..b0998d7f3be 100644
--- a/app/controllers/projects/wikis_controller.rb
+++ b/app/controllers/projects/wikis_controller.rb
@@ -10,7 +10,8 @@ class Projects::WikisController < Projects::ApplicationController
   before_action :authorize_admin_wiki!, only: :destroy
   before_action :load_project_wiki
   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
     redirect_to(project_wiki_path(@project, @page))
   end
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index feefc7f8137..37ffd28bf9e 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -18,9 +18,11 @@ class ProjectsController < Projects::ApplicationController
   before_action :redirect_git_extension, only: [:show]
   before_action :project, 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 :tree, only: [:show], if: [:repo_exists?, :project_view_files?]
-  before_action :lfs_blob_ids, only: [:show], if: [:repo_exists?, :project_view_files?]
+  before_action :assign_ref_vars, if: -> { action_name == 'show' && repo_exists? }
+  before_action :tree,
+    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 :present_project, only: [:edit]
   before_action :authorize_download_code!, only: [:refs]
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index b2b151bbcf0..638934694e0 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -8,8 +8,7 @@ class RegistrationsController < Devise::RegistrationsController
   prepend_before_action :check_captcha, only: :create
   before_action :whitelist_query_limiting, only: [:destroy]
   before_action :ensure_terms_accepted,
-                if: -> { Gitlab::CurrentSettings.current_application_settings.enforce_terms? },
-                only: [:create]
+    if: -> { action_name == 'create' && Gitlab::CurrentSettings.current_application_settings.enforce_terms? }
 
   def new
     redirect_to(new_user_session_path)
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index a841859621e..7604b31467a 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -13,18 +13,17 @@ class SessionsController < Devise::SessionsController
 
   prepend_before_action :check_initial_setup, only: [:new]
   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 :store_redirect_uri, only: [:new]
   prepend_before_action :ldap_servers, 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 :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?
 
   CAPTCHA_HEADER = 'X-GitLab-Show-Login-Captcha'.freeze
diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb
index fad036b8df8..869655e9550 100644
--- a/app/controllers/snippets_controller.rb
+++ b/app/controllers/snippets_controller.rb
@@ -8,7 +8,8 @@ class SnippetsController < ApplicationController
   include RendersBlob
   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]
 
diff --git a/changelogs/unreleased/55564-remove-if-in-before-after-action.yml b/changelogs/unreleased/55564-remove-if-in-before-after-action.yml
new file mode 100644
index 00000000000..a787faa8a9c
--- /dev/null
+++ b/changelogs/unreleased/55564-remove-if-in-before-after-action.yml
@@ -0,0 +1,5 @@
+---
+title: Rewrite `if:` argument in before_action and alike when `only:` is also used
+merge_request: 24412
+author: George Thomas @thegeorgeous
+type: other