Clean up CAPTCHA view logic
What does this MR do?
This is part of the effort to clean up CAPTCHA logic and make it simpler to understand, with less technical debt.
Specifically, it is Part 2 of "Plan C" for addressing the technical debt around issue creation.
Part 1 was to rewrite and simplify the CAPTCHA controller and module logic.
For more context on how we got to this point, see the discussion on this thread: !66427 (comment 635143092)
Changes:
- Fixes bug related to
merge_request_to_resolve_discussions_of
which was introduced in gitlab-foss!15408 (diffs) when theyield
was removed from the view. - Combines existing separate layouts into a single view (this was the source of the above bug). The layout hardcoded the name of the separate view, and was never reused, so it served no useful purpose as a layout anyway.
- Removes unused
has_submit
flag - Renames views to be consistent "
captcha_check
" terminology used in controller and helper methods - Reorganizes and adds comments to views
Screenshots or Screencasts (strongly suggested)
How to setup and validate locally (strongly suggested)
See the Testing Notes section of the CAPTCHA epic for details on how to setup and test spam checking and CAPTCHA in a local dev environment.
Does this MR meet the acceptance criteria?
Conformity
-
I have included changelog trailers, or none are needed. (Does this MR need a changelog?) -
I have added/updated documentation, or it's not needed. (Is documentation required?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) -
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) -
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides. -
This change is backwards compatible across updates, or this does not apply.
Availability and Testing
See the Testing Notes section of the CAPTCHA epic for details on how to setup and test spam checking and CAPTCHA in a local dev environment.
Exploratory Testing Performed
CAPTCHA behavior was explored in the following, with both cancel and solve of the CAPTCHA dialog:
-
issue creation (The only place which uses HTML form support and these views, all other non-login CAPTCHA modals use JSON Apollo/Axios support) -
Hit submit button without solving CAPTCHA (form reloads) -
Solve CAPTCHA and submit (saves successfully)
-
Checklist
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) -
I have tested this MR in all supported browsers, or it's not needed. -
I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.