Code review comment for lp:~bac/launchpad/bug-452491-captcha2-boogaloo

Revision history for this message
Brad Crittenden (bac) wrote :

= Summary =

Bug 452491 requests a simple captcha be placed on the forgotten password page.

== Proposed fix ==

The captcha already existed on the registration page. It was quick work to refactor
the bits into a mixin class and let the forgotten password page use it. Sadly both
of those pages are done without using LaunchpadFormView, made instead with grout and
twine, so it took a little massaging to get things to work.

The test helper set_captcha_answer had to be made a little smarter to account for
form prefixes.

The reset password test needed some clean up too. There is a fair amount of drive by
stuff in here but it's easy to sort out, I hope.

I also changed the spacing on the error messages for this page and the registration
page to fix some overlap problems. Screenshots are at:
http://people.canonical.com/~bac/captcha/

== Pre-implementation notes ==

None

== Implementation details ==

As above.

== Tests ==

bin/test -vvm lp.registry -t foaf -t xx-new-account-redirection-url.txt

== Demo and Q/A ==

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/templates/launchpad-login.pt
  lib/canonical/launchpad/templates/launchpad-forgottenpassword.pt
  lib/lp/registry/stories/foaf/xx-reg-with-existing-email.txt
  lib/lp/registry/stories/foaf/xx-createaccount.txt
  lib/canonical/launchpad/browser/tests/registration.py
  lib/canonical/launchpad/webapp/login.py
  lib/lp/testing/registration.py
  lib/canonical/launchpad/pagetests/standalone/xx-new-account-redirection-url.txt
  lib/lp/registry/stories/foaf/xx-resetpassword.txt

« Back to merge proposal