Skip to content

Clean up CAPTCHA view logic

Chad Woolley requested to merge clean-up-captcha-view-logic into master

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 the yield 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

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

Edited by Chad Woolley

Merge request reports