Hi Barry, The changes look good. Thanks for taking the time to make a lot of drive-by fixes. I've got a few questions below. --Brad > === modified file 'lib/lp/registry/browser/person.py' > --- lib/lp/registry/browser/person.py 2009-09-16 17:07:14 +0000 > +++ lib/lp/registry/browser/person.py 2009-09-16 21:19:47 +0000 > @@ -4086,6 +4086,8 @@ > custom_widget('mailing_list_auto_subscribe_policy', > LaunchpadRadioWidgetWithDescription) > > + label = 'Change your e-mail settings' > + > def initialize(self): > if self.context.is_team: > # +editemails is not available on teams. > @@ -4191,13 +4193,11 @@ > which the user is subscribed to this mailing list. > """ > subscription = mailing_list.getSubscription(self.context) > - if subscription is not None: > - if subscription.email_address is None: > - return "Preferred address" > - else: > - return subscription.email_address > - else: > + if subscription is None: > return "Don't subscribe" > + if subscription.email_address is None: Hmm, I'd have used an elif. > + return 'Preferred address' > + return subscription.email_address > > def _mailing_list_fields(self): > """Creates a field for each mailing list the user can subscribe to. > @@ -4233,8 +4235,22 @@ > @property > def mailing_list_widgets(self): > """Return all the mailing list subscription widgets.""" > - return [widget for widget in self.widgets > - if 'field.subscription.' in widget.name] > + mailing_list_set = getUtility(IMailingListSet) > + widgets = [] > + for widget in self.widgets: > + if 'field.subscription.' in widget.name: Why not use 'startswith'? Can it really be anywhere in the name? > + team_name = widget.label > + mailing_list = mailing_list_set.get(team_name) > + assert mailing_list is not None, 'Missing mailing list' > + widget_dict = dict( > + team=mailing_list.team, > + widget=widget, > + ) > + widgets.append(widget_dict) > + # We'll put the label in the first column, so don't include it > + # in the second column. > + widget.display_label = False > + return widgets > > def _validate_selected_address(self, data, field='VALIDATED_SELECTED'): > """A generic validator for this view's actions. > === modified file 'lib/lp/registry/stories/foaf/xx-setpreferredemail.txt' > --- lib/lp/registry/stories/foaf/xx-setpreferredemail.txt 2008-09-08 13:35:16 +0000 > +++ lib/lp/registry/stories/foaf/xx-setpreferredemail.txt 2009-09-16 21:00:11 +0000 > @@ -1,11 +1,14 @@ > -= A person's contact email address = > +================================ > +A person's contact email address > +================================ Thanks for converting this test. > === modified file 'lib/lp/registry/stories/mailinglists/subscriptions.txt' > --- lib/lp/registry/stories/mailinglists/subscriptions.txt 2009-09-14 01:28:41 +0000 > +++ lib/lp/registry/stories/mailinglists/subscriptions.txt 2009-09-16 21:00:11 +0000 > === removed file 'lib/lp/registry/templates/person-changepassword.pt' Excellent! > === modified file 'lib/lp/registry/templates/person-editemails.pt' > --- lib/lp/registry/templates/person-editemails.pt 2009-07-17 17:59:07 +0000 > +++ lib/lp/registry/templates/person-editemails.pt 2009-09-16 21:00:11 +0000 > @@ -3,35 +3,25 @@ > 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" > > > >

> > -

Change your e-mail settings

Your e-mail addresses

> - > -

Your preferred contact address for all Launchpad e-mail is:

> -

> -

> - > - > - > -
> - Currently you don't have a contact address in > - Launchpad. > -
> - > +
> + Your preferred contact address for all Launchpad e-mail is: > + > +
> +
+ id="no-contact-address"> > + Currently you don't have a contact address in Launchpad. > +
> > > > - > > > > @@ -87,24 +77,21 @@ > lists. You can subscribe to a team's mailing list using any of > your verified email addresses.
> > -
> +
> > - > - > + Missing semi-colon > + > > > - > - > - > - > - > - > - > + > + > + > + > >
For this mailing list... Subscribe with this address For mailing list Subscribe with
> - > -
Team Widget
> +
> + > +
> > >

For this mailing list...	Subscribe with this address	For mailing list	Subscribe with
	> - > -
Team	Widget