Merge branch 'clean-up-captcha-view-logic' into 'master'

Clean up CAPTCHA view logic

See merge request gitlab-org/gitlab!67195

app/controllers/concerns/spammable_actions/captcha_check/html_format_actions_support.rb

@@ -18,7 +18,7 @@ module SpammableActions::CaptchaCheck::HtmlFormatActionsSupport
   private
 
   def with_captcha_check_html_format(&block)
-    captcha_render_lambda = -> { render :verify }
+    captcha_render_lambda = -> { render :captcha_check }
     with_captcha_check_common(captcha_render_lambda: captcha_render_lambda, &block)
   end
 

app/views/layouts/_recaptcha_verification.html.haml

-- humanized_resource_name = spammable.class.model_name.human.downcase
-
-%h3.page-title
-  = _('Anti-spam verification')
-%hr
-
-%p
-  = _("We detected potential spam in the %{humanized_resource_name}. Please solve the reCAPTCHA to proceed.") % { humanized_resource_name: humanized_resource_name }
-
-= render 'shared/recaptcha_form', spammable: spammable

app/views/projects/issues/verify.html.haml → app/views/projects/issues/captcha_check.html.haml

-= render layout: 'layouts/recaptcha_verification', locals: { spammable: @issue } do
+= render layout: 'shared/captcha_check', locals: { spammable: @issue } do
+  -# These fields are for values which are passed via URL parameters, and not included in the
+  -# issue's params, so they must be yielded to the block to be rendered.
+  -# If these are removed and no longer passed via URL parameters, the support
+  -# for yielding in the layout can also be removed.
   = hidden_field_tag(:merge_request_to_resolve_discussions_of, params[:merge_request_to_resolve_discussions_of])
   = hidden_field_tag(:discussion_to_resolve, params[:discussion_to_resolve])

app/views/shared/_recaptcha_form.html.haml → app/views/shared/_captcha_check.html.haml

@@ -2,22 +2,36 @@
 - humanized_resource_name = spammable.class.model_name.human.downcase
 - script = local_assigns.fetch(:script, true)
 - method = params[:action] == 'create' ? :post : :put
-- has_submit = local_assigns.fetch(:has_submit, true)
+
+%h3.page-title
+  = _('Anti-spam verification')
+%hr
+
+%p
+  = _("We detected potential spam in the %{humanized_resource_name}. Please solve the reCAPTCHA to proceed.") % { humanized_resource_name: humanized_resource_name }
 
 = form_for resource_name, method: method, html: { class: 'recaptcha-form js-recaptcha-form' } do |f|
   .recaptcha
+    -# Create a hidden field for each param of the resource
     - params[resource_name].each do |field, value|
       = hidden_field(resource_name, field, value: value)
-    = hidden_field_tag(:spam_log_id, spammable.spam_log.id)
-    -# The reCAPTCHA response value will be returned in the 'g-recaptcha-response' field
+
+    -# The reCAPTCHA response value will be returned in the 'g-recaptcha-response' field in non-test environments
     = recaptcha_tags script: script, callback: 'recaptchaDialogCallback', nonce: content_security_policy_nonce unless Rails.env.test?
+
     -# Fake the 'g-recaptcha-response' field in the test environment, so that the feature spec
     -# can get to the (mocked) SpamVerdictService check.
     = hidden_field_tag('g-recaptcha-response', 'abc123') if Rails.env.test?
 
-    -# Yields a block with given extra params.
+    -# Create a hidden field to pass back the ID of the spam_log record which was previously created
+    = hidden_field_tag(:spam_log_id, spammable.spam_log.id)
+
+    -# Yields a block with given extra params which are not included in `params[resource_name]`.
+    -# Currently, this is only used for these params which are passed via URL parameters,
+    -# and can be removed once they are no longer needed to be passed:
+    -# - merge_request_to_resolve_discussions_of
+    -# - discussion_to_resolve
     = yield
 
-  - if has_submit
   .row-content-block.footer-block
     = f.submit _("Create %{humanized_resource_name}") % { humanized_resource_name: humanized_resource_name }, class: 'gl-button btn btn-confirm'

spec/controllers/concerns/spammable_actions/captcha_check/html_format_actions_support_spec.rb

@@ -54,8 +54,8 @@ RSpec.describe SpammableActions::CaptchaCheck::HtmlFormatActionsSupport do
     context 'when spammable.render_recaptcha? is true' do
       let(:render_recaptcha) { true }
 
-      it 'renders :verify' do
-        expect(controller).to receive(:render).with(:verify)
+      it 'renders :captcha_check' do
+        expect(controller).to receive(:render).with(:captcha_check)
 
         subject
       end