Code review comment for lp:~barry/launchpad/430065-person

Revision history for this message
Barry Warsaw (barry) wrote :

= Summary =

This branch fixes bug 430065 which converts the ~person/+editemails and
~person/+changepassword pages to UI 3.0. Along the way, I was able to close
an old bug 180349 which adds some useful links to the mailing list
subscription section on the +editemails page.

== Proposed fix ==

Update the templates and views, fix tests, and do a minimal redesign on the
+editemails page. That page is actually pretty horrendous, but time boxing
only allows for a moderate redesign and improvement. In particular, on the
+editemails page it would be nice to move the Add button to next to the "Add a
new address" text box, but given the way the form is laid out, this isn't
possible.

The ui has been approved by rockstar and edwin.

== Pre-implementation notes ==

None really. By now, conversions are pretty straightforward.

== Implementation details ==

Changes in this branch:

* Get rid of the person-changepassword.pt template altogether, substituting
  generic-edit.pt for it.

* Removed some pagetitles that are no longer necessary.

* Fixed a stylistic nit in launchpad.py Hierarchy class.

* Cleaned up the PersonEditEmailsView implementation. Also, change the
  mailing_list_widgets property implementation to return a dictionary instead
  of the widgets directly. This allowed me to fix bug 180349.

* Update a bunch of doctests.

== Tests ==

% bin/test -vv -t mailinglist -t stories/foaf -t stories/gpg-coc -t conduct

== Demo and Q/A ==

For a person with no preferred email address and no mailing list
subscriptions:

    http://launchpad.dev/~matsubara/+editemails

For a person with a preferred email address but no mailing list subscriptions:

    http://launchpad.dev/~no-priv/+editemails

For a person with several validated email addresses and a mailing list
subscription:

    http://launchpad.dev/~name16/+editemails

To see the change password page:

    http://launchpad.dev/~no-priv/+changepassword

= Launchpad lint =

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

Linting changed files:
  lib/lp/registry/browser/configure.zcml
  lib/lp/registry/stories/mailinglists/subscriptions.txt
  lib/canonical/launchpad/pagetitles.py
  lib/lp/registry/stories/foaf/xx-validate-email.txt
  lib/lp/registry/templates/person-editemails.pt
  lib/canonical/launchpad/browser/launchpad.py
  lib/lp/registry/stories/foaf/xx-setpreferredemail.txt
  lib/lp/registry/browser/person.py

== Pylint notices ==

lib/lp/registry/browser/person.py
    117: [F0401] Unable to import 'lazr.delegates' (No module named delegates)
    118: [F0401] Unable to import 'lazr.config' (No module named config)
    119: [F0401] Unable to import 'lazr.restful.interface' (No module named restful)

« Back to merge proposal