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

[Brad Crittenden]

[Brad Crittenden (bac)]

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

[Curtis Hovey (sinzui)]

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?

review: Approve (code)

[Brad Crittenden (bac)]

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.

=== modified file 'lib/canonical/launchpad/pagetitles.py'
--- lib/canonical/launchpad/pagetitles.py	2009-09-17 16:32:56 +0000
+++ lib/canonical/launchpad/pagetitles.py	2009-09-17 18:27:21 +0000
@@ -657,8 +657,6 @@
 person_answer_contact_for = ContextDisplayName(
     'Projects for which %s is an answer contact')
 
-person_edit = ContextDisplayName(smartquote("%s's details"))
-
 # person_foaf is an rdf file
 
 person_hwdb_submissions = ContextDisplayName(

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2009-09-17 16:12:20 +0000
+++ lib/lp/registry/browser/person.py	2009-09-17 18:48:15 +0000
@@ -3796,6 +3796,8 @@
 
     implements(IPersonEditMenu)
 
+    label = 'Change your personal details'
+
     # Will contain an hidden input when the user is renaming his
     # account with full knowledge of the consequences.
     i_know_this_is_an_openid_security_issue_input = None

=== renamed file 'lib/lp/registry/stories/foaf/xx-addemail.txt' => 'lib/lp/registry/stories/foaf/xx-add-email.txt'
--- 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.")
 
     >>> browser = setupBrowser(auth='Basic [e-mail]:test')
     >>> browser.open('http://launchpad.dev/~name12')
-    >>> browser.getLink('Change details').click()
-    >>> browser.getLink('E-mail Settings').click()
+    >>> browser.getLink(url='+editemails').click()
     >>> browser.url
     'http://launchpad.dev/~name12/+editemails'
 
@@ -72,11 +70,10 @@
 
 Now that the address is confirmed he sees it in the list of his confirmed
 addresses.
-    
+
     >>> from canonical.launchpad.testing.pages import strip_label
 
-    >>> browser.getLink('Change details').click()
-    >>> browser.getLink('E-mail Settings').click()
+    >>> browser.getLink(url='+editemails').click()
     >>> confirmed = browser.getControl(name="field.VALIDATED_SELECTED")
     >>> [strip_label(option) for option in confirmed.displayOptions]
     ['[e-mail]', '[e-mail]', '[e-mail]']

=== modified file 'lib/lp/registry/stories/foaf/xx-add-sshkey.txt'
--- lib/lp/registry/stories/foaf/xx-add-sshkey.txt	2009-09-16 17:56:17 +0000
+++ lib/lp/registry/stories/foaf/xx-add-sshkey.txt	2009-09-17 16:18:19 +0000
@@ -102,17 +102,6 @@
     ...
     Unauthorized: ...
 
-Nor is the option to edit the user's SSH keys displayed in the navigation
-menu:
-
-    >>> admin_browser.open('http://launchpad.dev/~salgado')
-    >>> admin_browser.getLink('Change details').click()
-    >>> print_navigation_links(admin_browser.contents)
-    Personal
-    E-mail Settings: .../~salgado/+editemails
-    OpenPGP Keys: .../~salgado/+editpgpkeys
-    Passwords: .../~salgado/+changepassword
-
 Salgado chooses to remove one of his ssh keys from Launchpad. The link
 to edit his keys is on the page.
 

=== renamed file 'lib/lp/registry/stories/foaf/xx-setpreferredemail.txt' => 'lib/lp/registry/stories/foaf/xx-set-preferredemail.txt'
--- lib/lp/registry/stories/foaf/xx-setpreferredemail.txt	2009-09-16 21:00:11 +0000
+++ lib/lp/registry/stories/foaf/xx-set-preferredemail.txt	2009-09-17 16:18:19 +0000
@@ -15,8 +15,7 @@
 
     >>> browser = setupBrowser(auth='Basic [e-mail]:test')
     >>> browser.open('http://launchpad.dev/~name12')
-    >>> browser.getLink('Change details').click()
-    >>> browser.getLink('E-mail Settings').click()
+    >>> browser.getLink(url='+editemails').click()
     >>> print browser.url
     http://launchpad.dev/~name12/+editemails
     >>> print browser.title

=== modified file 'lib/lp/registry/stories/foaf/xx-validate-email.txt'
--- lib/lp/registry/stories/foaf/xx-validate-email.txt	2009-09-16 21:00:11 +0000
+++ lib/lp/registry/stories/foaf/xx-validate-email.txt	2009-09-17 16:18:19 +0000
@@ -52,8 +52,7 @@
 
 Check that the email address now shows up as validated.
 
-    >>> browser.getLink('Change details').click()
-    >>> browser.getLink('E-mail Settings').click()
+    >>> browser.getLink(url='+editemails').click()
     >>> browser.getControl(name="field.VALIDATED_SELECTED").getControl(
     ...     value='[e-mail]')
     <ItemControl...optionValue='[e-mail]'...>

=== modified file 'lib/lp/registry/stories/gpg-coc/01-claimgpg.txt'
--- lib/lp/registry/stories/gpg-coc/01-claimgpg.txt	2009-09-10 20:12:12 +0000
+++ lib/lp/registry/stories/gpg-coc/01-claimgpg.txt	2009-09-17 16:18:19 +0000
@@ -20,8 +20,8 @@
 Start out with a clean page containing no imported keys:
 
     >>> browser = setupBrowser(auth='Basic [e-mail]:test')
-    >>> browser.open("http://launchpad.dev/~name12/+edit")
-    >>> browser.getLink('OpenPGP Keys').click()
+    >>> browser.open("http://launchpad.dev/~name12")
+    >>> browser.getLink(url='+editpgpkeys').click()
     >>> print browser.title
     +editpgpkeys : Sample Person
 

=== modified file 'lib/lp/registry/stories/mailinglists/subscriptions.txt'
--- lib/lp/registry/stories/mailinglists/subscriptions.txt	2009-09-16 21:00:11 +0000
+++ lib/lp/registry/stories/mailinglists/subscriptions.txt	2009-09-17 18:12:42 +0000
@@ -83,8 +83,7 @@
 it's currently the team contact method.
 
     >>> browser.open('http://launchpad.dev/~carlos')
-    >>> browser.getLink("Change details").click()
-    >>> browser.getLink("E-mail Settings").click()
+    >>> browser.getLink(url="+editemails").click()
 
     >>> from canonical.launchpad.helpers import backslashreplace
     >>> print backslashreplace(browser.title)
@@ -211,8 +210,7 @@
 Admins team, and he should know if the list is available.
 
     >>> browser.open('http://launchpad.dev/~carlos')
-    >>> browser.getLink("Change details").click()
-    >>> browser.getLink("E-mail Settings").click()
+    >>> browser.getLink(url="+editemails").click()
     >>> print backslashreplace(browser.title)
     +editemails : Carlos Perell\xf3 Mar\xedn
 
@@ -245,8 +243,7 @@
 screen.
 
     >>> browser.open('http://launchpad.dev/~jdub')
-    >>> browser.getLink("Change details").click()
-    >>> browser.getLink("E-mail Settings").click()
+    >>> browser.getLink(url="+editemails").click()
     >>> print browser.title
     +editemails : Jeff Waugh
 
@@ -270,8 +267,7 @@
 His mailing list subscription is now available to be managed.
 
     >>> browser.open('http://launchpad.dev/~jdub')
-    >>> browser.getLink("Change details").click()
-    >>> browser.getLink("E-mail Settings").click()
+    >>> browser.getLink(url="+editemails").click()
     >>> print browser.title
     +editemails : Jeff Waugh
 
@@ -346,8 +342,7 @@
 
     # Subscribe to the list using the normal technique.
     >>> browser.open('http://launchpad.dev/~carlos')
-    >>> browser.getLink("Change details").click()
-    >>> browser.getLink("E-mail Settings").click()
+    >>> browser.getLink(url="+editemails").click()
     >>> rosetta_admins = browser.getControl(
     ...     name='field.subscription.rosetta-admins')
     >>> rosetta_admins.value = ['Preferred address']
@@ -472,8 +467,7 @@
 
     >>> browser = setupBrowser(auth='Basic [e-mail]:test')
     >>> browser.open('http://launchpad.dev/~carlos')
-    >>> browser.getLink("Change details").click()
-    >>> browser.getLink("E-mail Settings").click()
+    >>> browser.getLink(url="+editemails").click()
     >>> print backslashreplace(browser.title)
     +editemails : Carlos Perell\xf3 Mar\xedn
 
@@ -551,8 +545,7 @@
     ...     auth='Basic [e-mail]:jblack')
 
     >>> browser.open('http://launchpad.dev/~jblack')
-    >>> browser.getLink("Change details").click()
-    >>> browser.getLink("E-mail Settings").click()
+    >>> browser.getLink(url="+editemails").click()
     >>> print_radio_button_field(browser.contents,
     ...     'mailing_list_auto_subscribe_policy')
     ( ) Never subscribe to mailing lists
@@ -569,8 +562,7 @@
 
     # Change James' setting
     >>> browser.open('http://launchpad.dev/~jblack')
-    >>> browser.getLink("Change details").click()
-    >>> browser.getLink("E-mail Settings").click()
+    >>> browser.getLink(url="+editemails").click()
     >>> set_autosubscribe_policy_and_submit('ALWAYS')
     ( ) Never subscribe to mailing lists
     ( ) Ask me when I join a team
@@ -586,8 +578,7 @@
 
     # Change James' setting
     >>> browser.open('http://launchpad.dev/~jblack')
-    >>> browser.getLink("Change details").click()
-    >>> browser.getLink("E-mail Settings").click()
+    >>> browser.getLink(url="+editemails").click()
     >>> set_autosubscribe_policy_and_submit('NEVER')
     (*) Never subscribe to mailing lists
     ( ) Ask me when I join a team
@@ -600,8 +591,7 @@
 
     # Restore James' setting.
     >>> browser.open('http://launchpad.dev/~jblack')
-    >>> browser.getLink("Change details").click()
-    >>> browser.getLink("E-mail Settings").click()
+    >>> browser.getLink(url="+editemails").click()
     >>> set_autosubscribe_policy_and_submit('ON_REGISTRATION')
     ( ) Never subscribe to mailing lists
     (*) Ask me when I join a team

=== modified file 'lib/lp/registry/templates/person-edit.pt'
--- lib/lp/registry/templates/person-edit.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/registry/templates/person-edit.pt	2009-09-17 18:48:15 +0000
@@ -3,16 +3,12 @@
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  xml:lang="en"
-  lang="en"
-  dir="ltr"
-  metal:use-macro="view/macro:page/onecolumn"
+  metal:use-macro="view/macro:page/main_only"
   i18n:domain="launchpad"
 >
   <body>
     <div metal:fill-slot="main"
       tal:define="overview_menu context/menu:overview">
-      <h1>Change your personal details</h1>
 
       <div metal:use-macro="context/@@launchpad_form/form">