Merge lp:~bac/launchpad/bug-432026-person-edit into lp:launchpad

= Summary =

The person-edit.pt template needs to be converted, as stated in bug 432026.

== Proposed fix ==

Simple conversion of the template and then fixing failed tests.

== Pre-implementation notes ==

Brief chat with Curtis.

== Implementation details ==

In the old page there were navigation lozenges that linked to lower edit pages, e.g.
for editing emails, wikiname, etc. A lot of the pagetest tested going to the 'Change
details' page and then navigating to correct page. The links to the individual edit
pages are now available off the person index page. The tests were changed to go
directly to the lower edit page, which saves a page load, which will make Julian happy.

A few tests were renamed for consistency.

== Tests ==

Run all of the registry tests:
bin/test -vvm lp.registry

== Demo and Q/A ==

https://launchpad.dev/~mark and click on 'Change details'.

= 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/stories/mailinglists/subscriptions.txt
  lib/canonical/launchpad/pagetitles.py
  lib/lp/registry/stories/foaf/xx-validate-email.txt
  lib/lp/registry/stories/foaf/xx-add-sshkey.txt
  lib/lp/registry/stories/foaf/xx-set-preferredemail.txt
  lib/lp/registry/stories/foaf/xx-add-email.txt
  lib/lp/registry/stories/gpg-coc/01-claimgpg.txt
  lib/lp/registry/browser/person.py
  lib/lp/registry/templates/person-edit.pt

== 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)

--
Brad Crittenden
<email address hidden>

Thanks for doing this Brad.

This branch looks fine to land. I have a few suggestions, but don't let them stop you from landing the code today.

> === renamed file 'lib/lp/registry/stories/foaf/xx-addemail.txt' => 'lib/lp/registry/stories/foaf/xx-add-email.txt'

I like the rename.

> --- lib/lp/registry/stories/foaf/xx-addemail.txt 2009-05-12 01:39:29 +0000
> +++ lib/lp/registry/stories/foaf/xx-add-email.txt 2009-09-17 16:18:19 +0000
> @@ -7,15 +7,13 @@
>
> Sample Person will now add a couple email addresses to his account.
>
> - # Workaround while https://launchpad.net/launchpad/+bug/39016 is not
> - # fixed.
> >>> from lp.services.mail import stub
> - >>> stub.test_emails[:] = []
> + >>> assert not stub.test_emails, (
> + ... "stub.test_emails should be empty at the start of the test.")

You can remove the assert. The problem was fixed a long time ago and lots
of tests use the stub without clearing it.

> >>> browser = setupBrowser(auth='Basic <email address hidden>:test')
> >>> browser.open('http://launchpad.dev/~name12')
> - >>> browser.getLink('Change details').click()
> - >>> browser.getLink('E-mail Settings').click()
> + >>> browser.getLink(url='+editemails').click()

This is fine as it is, but I usually use the alt text of an image to get the
link.

    >>> browser.getLink('Change e-mail settings').click()

^ Did that not work?

On Sep 17, 2009, at 15:53 , Curtis Hovey wrote:

> Review: Approve code
> Thanks for doing this Brad.
>
> This branch looks fine to land. I have a few suggestions, but don't
> let them stop you from landing the code today.
>
>> === renamed file 'lib/lp/registry/stories/foaf/xx-addemail.txt' =>
>> 'lib/lp/registry/stories/foaf/xx-add-email.txt'
>
> I like the rename.
>
>> --- lib/lp/registry/stories/foaf/xx-addemail.txt 2009-05-12
>> 01:39:29 +0000
>> +++ lib/lp/registry/stories/foaf/xx-add-email.txt 2009-09-17
>> 16:18:19 +0000
>> @@ -7,15 +7,13 @@
>>
>> Sample Person will now add a couple email addresses to his account.
>>
>> - # Workaround while https://launchpad.net/launchpad/+bug/39016
>> is not
>> - # fixed.
>>>>> from lp.services.mail import stub
>> - >>> stub.test_emails[:] = []
>> + >>> assert not stub.test_emails, (
>> + ... "stub.test_emails should be empty at the start of the
>> test.")
>
> You can remove the assert. The problem was fixed a long time ago and
> lots
> of tests use the stub without clearing it.

Great. We're not so good about cleaning up old XXX work-arounds and
comments.

>
>>>>> browser = setupBrowser(auth='Basic <email address hidden>:test')
>>>>> browser.open('http://launchpad.dev/~name12')
>> - >>> browser.getLink('Change details').click()
>> - >>> browser.getLink('E-mail Settings').click()
>> + >>> browser.getLink(url='+editemails').click()
>
> This is fine as it is, but I usually use the alt text of an image to
> get the
> link.

I'll leave this test as is but keep that in mind.

>
>>>> browser.getLink('Change e-mail settings').click()
>
> ^ Did that not work?

Didn't try it. I'll have a go next time around.