launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #09845
[Merge] lp:~wallyworld/launchpad/remove-new-team-contact-email into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/remove-new-team-contact-email into lp:launchpad.
Requested reviews:
Curtis Hovey (sinzui)
Related bugs:
Bug #31590 in Launchpad itself: "Team's contact e-mail address options should be more obvious"
https://bugs.launchpad.net/launchpad/+bug/31590
Bug #1020296 in Launchpad itself: "Setting a team contact address is prominently displayed, but is not recommended"
https://bugs.launchpad.net/launchpad/+bug/1020296
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/remove-new-team-contact-email/+merge/114549
== Implementation ==
After much discussion, it was decided that this branch would:
- remove the contact address from the new team form
- add a warning to the contact address form saying that if a mailing list or external contact address were used, private information may be disclosed.
This branch also changes the Email edit link on the team home page to say "None, members emailed directly" when there is no contact address set.
Attempts to find a rational, universally applicable solution for limiting the use of mailing lists or external contact addresses for exclusive teams did not prove feasible. So a warning will suffice.
I confirmed that if an external contact address which is already registered in Lp is used, it is rejected.
== Demo ==
Screenshot of the contact address page with the new warning:
http://people.canonical.com/~ianb/team-contact-address.png
== Tests ==
Adjust various doc tests as necessary.
Adjust the TestTeamCreationView test.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/app/stories/form/xx-form-layout.txt
lib/lp/registry/browser/team.py
lib/lp/registry/browser/tests/test_person.py
lib/lp/registry/interfaces/person.py
lib/lp/registry/stories/team/xx-team-contactemail.txt
lib/lp/registry/stories/team/xx-team-home.txt
lib/lp/registry/stories/teammembership/xx-teammembership.txt
lib/lp/registry/templates/person-macros.pt
lib/lp/registry/templates/team-contactaddress.pt
There was just the usual "narrative uses a moin header" stuff from the doc tests.
--
https://code.launchpad.net/~wallyworld/launchpad/remove-new-team-contact-email/+merge/114549
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/app/stories/form/xx-form-layout.txt'
--- lib/lp/app/stories/form/xx-form-layout.txt 2012-01-15 11:06:57 +0000
+++ lib/lp/app/stories/form/xx-form-layout.txt 2012-07-12 01:18:23 +0000
@@ -40,10 +40,10 @@
<tr>
<td colspan="2">
<div>
- <label for="field.contactemail">Contact Email Address:</label>
+ <label for="field.defaultmembershipperiod">Subscription period:</label>
<span class="fieldRequired">(Optional)</span>
<div>
- <input ... id="field.contactemail" ... />
+ <input ... id="field.defaultmembershipperiod" ... />
</div>
<p class="formHelp">...</p>
</div>
=== modified file 'lib/lp/registry/browser/team.py'
--- lib/lp/registry/browser/team.py 2012-07-03 08:04:35 +0000
+++ lib/lp/registry/browser/team.py 2012-07-12 01:18:23 +0000
@@ -123,7 +123,6 @@
IPersonSet,
ITeam,
ITeamContactAddressForm,
- ITeamCreation,
ITeamReassignment,
OPEN_TEAM_POLICY,
PersonVisibility,
@@ -228,7 +227,7 @@
* The user has a current commercial subscription.
"""
field_names = [
- "name", "visibility", "displayname", "contactemail",
+ "name", "visibility", "displayname",
"teamdescription", "subscriptionpolicy",
"defaultmembershipperiod", "renewal_policy",
"defaultrenewalperiod", "teamowner",
@@ -302,14 +301,10 @@
custom_widget('teamdescription', TextAreaWidget, height=10, width=30)
def setUpFields(self):
- """See `LaunchpadViewForm`.
-
- When editing a team the contactemail field is not displayed.
- """
+ """See `LaunchpadViewForm`."""
# Make an instance copy of field_names so as to not modify the single
# class list.
self.field_names = list(self.field_names)
- self.field_names.remove('contactemail')
self.field_names.remove('teamowner')
super(TeamEditView, self).setUpFields()
self.setUpVisibilityField(render_context=True)
@@ -994,7 +989,7 @@
class TeamAddView(TeamFormMixin, HasRenewalPolicyMixin, LaunchpadFormView):
"""View for adding a new team."""
- schema = ITeamCreation
+ schema = ITeam
page_title = 'Register a new team in Launchpad'
label = page_title
@@ -1031,16 +1026,6 @@
if visibility:
team.transitionVisibility(visibility, self.user)
del data['visibility']
- email = data.get('contactemail')
- if email is not None:
- generateTokenAndValidationEmail(email, team)
- self.request.response.addNotification(
- "A confirmation message has been sent to '%s'. Follow the "
- "instructions in that message to confirm the new "
- "contact address for this team. "
- "(If the message doesn't arrive in a few minutes, your mail "
- "provider might use 'greylisting', which could delay the "
- "message for up to an hour or two.)" % email)
if self.request.is_ajax:
return ''
=== modified file 'lib/lp/registry/browser/tests/test_person.py'
--- lib/lp/registry/browser/tests/test_person.py 2012-07-07 19:30:24 +0000
+++ lib/lp/registry/browser/tests/test_person.py 2012-07-12 01:18:23 +0000
@@ -611,7 +611,6 @@
def test_team_creation_good_data(self):
form = {
'field.actions.create': 'Create Team',
- 'field.contactemail': 'contactemail@xxxxxxxxxxx',
'field.displayname': 'liberty-land',
'field.name': 'libertyland',
'field.renewal_policy': 'NONE',
@@ -626,32 +625,6 @@
self.assertTrue(team is not None)
self.assertEqual('libertyland', team.name)
- def test_validate_email_catches_taken_emails(self):
- email_address = self.factory.getUniqueEmailAddress()
- self.factory.makePerson(
- name='libertylandaccount',
- displayname='libertylandaccount',
- email=email_address,
- account_status=AccountStatus.NOACCOUNT)
- form = {
- 'field.actions.create': 'Create Team',
- 'field.contactemail': email_address,
- 'field.displayname': 'liberty-land',
- 'field.name': 'libertyland',
- 'field.renewal_policy': 'NONE',
- 'field.renewal_policy-empty-marker': 1,
- 'field.subscriptionpolicy': 'RESTRICTED',
- 'field.subscriptionpolicy-empty-marker': 1,
- }
- person_set = getUtility(IPersonSet)
- view = create_initialized_view(person_set, '+newteam', form=form)
- expected_msg = (
- '%s is already registered in Launchpad and is associated with '
- '<a href="http://launchpad.dev/~libertylandaccount";>'
- 'libertylandaccount</a>.' % email_address)
- error_msg = view.errors[0].errors[0]
- self.assertEqual(expected_msg, error_msg)
-
class TestPersonParticipationView(TestCaseWithFactory):
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py 2012-07-07 19:30:24 +0000
+++ lib/lp/registry/interfaces/person.py 2012-07-12 01:18:23 +0000
@@ -28,7 +28,6 @@
'ISoftwareCenterAgentApplication',
'ITeam',
'ITeamContactAddressForm',
- 'ITeamCreation',
'ITeamReassignment',
'ImmutableVisibilityError',
'NoSuchPerson',
@@ -112,7 +111,6 @@
from lp.app.validators import LaunchpadValidationError
from lp.app.validators.email import email_validator
from lp.app.validators.name import name_validator
-from lp.app.validators.validation import validate_new_team_email
from lp.blueprints.interfaces.specificationtarget import IHasSpecifications
from lp.bugs.interfaces.bugtarget import IHasBugs
from lp.code.interfaces.hasbranches import (
@@ -2485,24 +2483,6 @@
title=_('New'), vocabulary='ValidTeamOwner', required=True)
-class ITeamCreation(ITeam):
- """An interface to be used by the team creation form.
-
- We need this special interface so we can allow people to specify a contact
- email address for a team upon its creation.
- """
-
- contactemail = TextLine(
- title=_("Contact Email Address"), required=False, readonly=False,
- description=_(
- "This is the email address we'll send all notifications to this "
- "team. If no contact address is chosen, notifications directed "
- "to this team will be sent to all team members. After finishing "
- "the team creation, a new message will be sent to this address "
- "with instructions on how to finish its registration."),
- constraint=validate_new_team_email)
-
-
class TeamContactMethod(EnumeratedType):
"""The method used by Launchpad to contact a given team."""
=== modified file 'lib/lp/registry/stories/team/xx-team-contactemail.txt'
--- lib/lp/registry/stories/team/xx-team-contactemail.txt 2011-12-22 05:09:10 +0000
+++ lib/lp/registry/stories/team/xx-team-contactemail.txt 2012-07-12 01:18:23 +0000
@@ -21,6 +21,14 @@
>>> browser.title
'Landscape Developers contact address...
+A warning is rendered about the privacy implications of using a mailing list or
+external contact address.
+
+ >>> from BeautifulSoup import BeautifulSoup
+ >>> soup = BeautifulSoup(browser.contents)
+ >>> soup.find(id='email-warning')
+ <p ... E-mail sent to a mailing list or external contact address may ...
+
As we can see, the landscape-developers team has no contact address.
>>> from lp.testing.pages import strip_label
=== modified file 'lib/lp/registry/stories/team/xx-team-home.txt'
--- lib/lp/registry/stories/team/xx-team-home.txt 2012-07-07 14:00:30 +0000
+++ lib/lp/registry/stories/team/xx-team-home.txt 2012-07-12 01:18:23 +0000
@@ -142,8 +142,7 @@
>>> print extract_text(
... find_tag_by_id(sample_browser.contents, 'contact-email'))
Email:
- No contact email
- Set contact address
+ None, members emailed directly
>>> print extract_text(
... find_tag_by_id(sample_browser.contents, 'contact-user'))
Contact this team's members
=== modified file 'lib/lp/registry/stories/teammembership/xx-teammembership.txt'
--- lib/lp/registry/stories/teammembership/xx-teammembership.txt 2012-01-15 11:06:57 +0000
+++ lib/lp/registry/stories/teammembership/xx-teammembership.txt 2012-07-12 01:18:23 +0000
@@ -35,52 +35,6 @@
>>> [a.name for a in Person.byName('myemail').adminmembers]
[u'name12']
-When creating a new team you can optionally provide a contact email address
-for that team. This contact address must not be registered in Launchpad.
-
- >>> browser.open('http://launchpad.dev/people/+newteam')
-
- >>> browser.getControl(name='field.name').value = 'newtestteam'
- >>> browser.getControl('Display Name').value = 'newtestteam'
- >>> browser.getControl(
- ... 'Contact Email Address').value = 'test@xxxxxxxxxxxxx'
- >>> browser.getControl('Team Description').value = 'another test team'
- >>> browser.getControl('Moderated Team').selected = True
- >>> browser.getControl('Create').click()
-
- >>> for tag in find_tags_by_class(browser.contents, 'message'):
- ... print extract_text(tag)
- There is 1 error.
- test@xxxxxxxxxxxxx is already registered in Launchpad and is associated
- with Sample Person.
-
-We also check if the given email address is syntatically valid.
-
- >>> browser.getControl(
- ... 'Contact Email Address').value = 'testcanonical.com'
- >>> browser.getControl('Create').click()
-
- >>> for tag in find_tags_by_class(browser.contents, 'message'):
- ... print extract_text(tag)
- There is 1 error.
- testcanonical.com isn't a valid email address.
-
-Now we enter a valid email address that's not registered in Launchpad.
-
- >>> browser.getControl('Contact Email Address').value = 'team-foo@xxxxxxx'
- >>> browser.getControl('Create').click()
-
- >>> browser.url
- 'http://launchpad.dev/~newtestteam'
-
- >>> for tag in find_tags_by_class(
- ... browser.contents, 'informational message'):
- ... print tag.renderContents()
- A confirmation message has been sent to 'team-foo@xxxxxxx'. Follow the
- instructions in that message to confirm the new contact address for this
- team. (If the message doesn't arrive in a few minutes, your mail provider
- might use 'greylisting', which could delay the message for up to an hour
- or two.)
== As an administrator ==
=== modified file 'lib/lp/registry/templates/person-macros.pt'
--- lib/lp/registry/templates/person-macros.pt 2012-06-15 16:23:50 +0000
+++ lib/lp/registry/templates/person-macros.pt 2012-07-12 01:18:23 +0000
@@ -20,7 +20,7 @@
<tal:no_preferredemail
condition="view/email_address_visibility/are_none_available">
- No contact email
+ None, members emailed directly
</tal:no_preferredemail>
<tal:emails repeat="email view/visible_email_addresses">
@@ -31,7 +31,7 @@
</div>
</tal:emails>
<a tal:condition="not: view/visible_email_addresses"
- tal:replace="structure overview_menu/editemail/fmt:link"></a>
+ tal:replace="structure overview_menu/editemail/fmt:icon"></a>
</dd>
</dl>
=== modified file 'lib/lp/registry/templates/team-contactaddress.pt'
--- lib/lp/registry/templates/team-contactaddress.pt 2009-08-28 16:23:47 +0000
+++ lib/lp/registry/templates/team-contactaddress.pt 2012-07-12 01:18:23 +0000
@@ -41,11 +41,22 @@
</div>
</metal:widgets>
- <p metal:fill-slot="extra_bottom" id="set-up-a-mailing-list">
+ <div metal:fill-slot="extra_bottom">
+ <p id="set-up-a-mailing-list">
If you <a href="+mailinglist">set up a mailing list for this
team</a>, you can use the mailing list as the team contact
address.
</p>
+
+ <p id="email-warning" class="large-warning" style="padding:2px 2px 0 36px;">
+ E-mail sent to a mailing list or external contact address may
+ be publicly accessible.<br/>If this team is subscribed to private
+ bug or branches, private information may be disclosed.<br/>
+ The safest option to avoid leaking private information is to
+ send notifications to each member individually.
+ </p>
+ </div>
+
</div>
</div>
Follow ups
