launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06333
[Merge] lp:~sinzui/launchpad/contact-team into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/contact-team into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #246022 in Launchpad itself: "No way for team admins to contact all members of the team when a list is configured"
https://bugs.launchpad.net/launchpad/+bug/246022
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/contact-team/+merge/92589
Allow team members to contact all the team members, non-members contact the
team admins.
Launchpad bug: https://bugs.launchpad.net/bugs/246022
Pre-implementation: lifeless
A team administrator cannot contact all members at once if, for example,
the contact address for the team is a mailing list, and not all members
are subscribed to it.
This issues has mutated over the years. When bug mail was out of control,
we decided to honour a team's choice to set a contact address to send
all email to a black hole. Bug mail is no longer an issue. When a member
choose the contact-this-team form, Launchpad will send email to all members
so that important messages can be sent. contact-this-team is still not
substitute for mailing lists because we limit its use to 3 time a day.
While discussing the rules recently, we discovered that we did not update
contact-this-team to contact team admins instead of the owner. We allow
the owner to leave the team, delegating all responsibility to the admins.
--------------------------------------------------------------------
RULES
* Delete the rules for when the team has a contact address, allowing
the team member rules to be the only rule in play.
* Change the contact owner rule to contact admins.
* ADDENDUM: I cannot 'hit' the cancel link. I do not think the cancel
is a verb and suffices.
QA
* Visit https://qastaging.launchpad.net/~registry
* Set the contact address to the mailing list.
* Choose contact this team's members.
* Verify the page says
You are contacting nn members of the Registry Administrators (registry)
team directly.
* Verify the page says
If you do not want Registry Administrators to know your email address,
_cancel_ now.
* Visit https://qastaging.launchpad.net/~bzr (or a
team you are not a member of)
* Choose contact this team's admins.
* Verify the page says
You are contacting the Bazaar Developers (bzr) team admins.
LINT
lib/lp/registry/browser/person.py
lib/lp/registry/browser/tests/person-views.txt
lib/lp/registry/browser/tests/team-views.txt
lib/lp/registry/browser/tests/user-to-user-views.txt
lib/lp/registry/stories/team/xx-team-home.txt
lib/lp/registry/templates/contact-user.pt
TEST
./bin/test -vcc -t user-to-user-views -t person-views -t team-views \
-t xx-team-home lp.registry
IMPLEMENTATION
Removed the TO_TEAM state of ContactViaWebNotificationRecipientSet, deleted
all callsites that accessed it, deleted tests that check that the team
email address was contacted.
Renamed TO_OWNER to TO_ADMINS and revised the descriptive test. Updated
tests to verify team admins are contacted. Replaced
_getPrimaryRecipient() with a block to get the team admins (make the
tests pass). While there was no method that returned person and email
address, my implementation uses the get_recipients helper that caches
the user/team email addresses.
--
https://code.launchpad.net/~sinzui/launchpad/contact-team/+merge/92589
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/contact-team into lp:launchpad.
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2012-02-03 22:03:43 +0000
+++ lib/lp/registry/browser/person.py 2012-02-10 21:48:22 +0000
@@ -217,6 +217,7 @@
Milestone,
milestone_sort_key,
)
+from lp.registry.model.person import get_recipients
from lp.services.config import config
from lp.services.database.sqlbase import flush_database_updates
from lp.services.feeds.browser import FeedsMixin
@@ -2581,8 +2582,7 @@
:return: the recipients of the message.
:rtype: `ContactViaWebNotificationRecipientSet` constant:
TO_USER
- TO_TEAM (Send to team's preferredemail)
- TO_OWNER
+ TO_ADMINS
TO_MEMBERS
"""
return ContactViaWebNotificationRecipientSet(
@@ -2597,13 +2597,10 @@
return 'Send an email to yourself through Launchpad'
else:
return 'Send an email to this user through Launchpad'
- elif self.group_to_contact == ContactViaWeb.TO_TEAM:
- return ("Send an email to your team's contact email address "
- "through Launchpad")
elif self.group_to_contact == ContactViaWeb.TO_MEMBERS:
return "Send an email to your team's members through Launchpad"
- elif self.group_to_contact == ContactViaWeb.TO_OWNER:
- return "Send an email to this team's owner through Launchpad"
+ elif self.group_to_contact == ContactViaWeb.TO_ADMINS:
+ return "Send an email to this team's admins through Launchpad"
else:
raise AssertionError('Unknown group to contact.')
@@ -2615,12 +2612,10 @@
# Note that we explicitly do not change the text to "Contact
# yourself" when viewing your own page.
return 'Contact this user'
- elif self.group_to_contact == ContactViaWeb.TO_TEAM:
- return "Contact this team's email address"
elif self.group_to_contact == ContactViaWeb.TO_MEMBERS:
return "Contact this team's members"
- elif self.group_to_contact == ContactViaWeb.TO_OWNER:
- return "Contact this team's owner"
+ elif self.group_to_contact == ContactViaWeb.TO_ADMINS:
+ return "Contact this team's admins"
else:
raise AssertionError('Unknown group to contact.')
@@ -4823,9 +4818,8 @@
# Primary reason enumerations.
TO_USER = object()
- TO_TEAM = object()
TO_MEMBERS = object()
- TO_OWNER = object()
+ TO_ADMINS = object()
def __init__(self, user, person_or_team):
"""Initialize the state based on the context and the user.
@@ -4861,36 +4855,16 @@
"""
if person_or_team.is_team:
if self.user.inTeam(person_or_team):
- if removeSecurityProxy(person_or_team).preferredemail is None:
- # Send to each team member.
- return self.TO_MEMBERS
- else:
- # Send to the team's contact address.
- return self.TO_TEAM
+ return self.TO_MEMBERS
else:
# A non-member can only send emails to a single person to
# hinder spam and to prevent leaking membership
# information for private teams when the members reply.
- return self.TO_OWNER
+ return self.TO_ADMINS
else:
# Send to the user
return self.TO_USER
- def _getPrimaryRecipient(self, person_or_team):
- """Return the primary recipient.
-
- The primary recipient is the ``person_or_team`` in all cases
- except for when the email is restricted to a team owner.
-
- :param person_or_team: The party that is the context of the email.
- :type person_or_team: `IPerson`.
- """
- if self._primary_reason is self.TO_OWNER:
- person_or_team = person_or_team.teamowner
- while person_or_team.is_team:
- person_or_team = person_or_team.teamowner
- return person_or_team
-
def _getReasonAndHeader(self, person_or_team):
"""Return the reason and header why the email was received.
@@ -4902,20 +4876,13 @@
'using the "Contact this user" link on your profile page\n'
'(%s)' % canonical_url(person_or_team))
header = 'ContactViaWeb user'
- elif self._primary_reason is self.TO_OWNER:
+ elif self._primary_reason is self.TO_ADMINS:
reason = (
- 'using the "Contact this team\'s owner" link on the '
+ 'using the "Contact this team\'s admins" link on the '
'%s team page\n(%s)' % (
person_or_team.displayname,
canonical_url(person_or_team)))
header = 'ContactViaWeb owner (%s team)' % person_or_team.name
- elif self._primary_reason is self.TO_TEAM:
- reason = (
- 'using the "Contact this team" link on the '
- '%s team page\n(%s)' % (
- person_or_team.displayname,
- canonical_url(person_or_team)))
- header = 'ContactViaWeb member (%s team)' % person_or_team.name
else:
# self._primary_reason is self.TO_MEMBERS.
reason = (
@@ -4937,15 +4904,9 @@
return (
'You are contacting %s (%s).' %
(person_or_team.displayname, person_or_team.name))
- elif self._primary_reason is self.TO_OWNER:
- return (
- 'You are contacting the %s (%s) team owner, %s (%s).' %
- (person_or_team.displayname, person_or_team.name,
- self._primary_recipient.displayname,
- self._primary_recipient.name))
- elif self._primary_reason is self.TO_TEAM:
- return (
- 'You are contacting the %s (%s) team.' %
+ elif self._primary_reason is self.TO_ADMINS:
+ return (
+ 'You are contacting the %s (%s) team admins.' %
(person_or_team.displayname, person_or_team.name))
else:
# This is a team without a contact address (self.TO_MEMBERS).
@@ -4968,6 +4929,16 @@
for recipient in team.getMembersWithPreferredEmails():
email = removeSecurityProxy(recipient).preferredemail.email
all_recipients[email] = recipient
+ elif self._primary_reason is self.TO_ADMINS:
+ team = self._primary_recipient
+ for admin in team.adminmembers:
+ # This method is similar to getTeamAdminsEmailAddresses, but
+ # this case needs to know the user. Since both methods
+ # ultimately iterate over get_recipients, this case is not
+ # in a different performance class.
+ for recipient in get_recipients(admin):
+ email = removeSecurityProxy(recipient).preferredemail.email
+ all_recipients[email] = recipient
elif self._primary_recipient.is_valid_person_or_team:
email = removeSecurityProxy(
self._primary_recipient).preferredemail.email
@@ -5040,7 +5011,7 @@
"""
self._reset_state()
self._primary_reason = self._getPrimaryReason(person)
- self._primary_recipient = self._getPrimaryRecipient(person)
+ self._primary_recipient = person
if reason is None:
reason, header = self._getReasonAndHeader(person)
self._reason = reason
=== modified file 'lib/lp/registry/browser/tests/person-views.txt'
--- lib/lp/registry/browser/tests/person-views.txt 2012-01-15 13:32:27 +0000
+++ lib/lp/registry/browser/tests/person-views.txt 2012-02-10 21:48:22 +0000
@@ -525,14 +525,6 @@
Non-member contacting a Team
----------------------------
-Users can contact teams, but the behaviour depends upon whether the user
-is a member of the team. No Privileges Person is not a member of the
-Landscape Developers team.
-
- >>> view = create_initialized_view(landscape_developers, '+index')
- >>> print view.contact_link_title
- Send an email to this team's owner through Launchpad
-
The EmailToPersonView can be used by non-members to contact the team
owner.
@@ -545,7 +537,7 @@
>>> print view.recipients.description
You are contacting the Landscape Developers (landscape-developers) team
- owner, Sample Person (name12).
+ admins.
>>> [recipient.name for recipient in view.recipients]
[u'name12']
@@ -608,19 +600,6 @@
>>> [recipient.name for recipient in view.recipients]
[u'name12']
-EmailToPersonView will use the contact address when the team has one.
-
- >>> email_address = factory.makeEmail(
- ... 'landscapers@xxxxxxxxxxxxx', landscape_developers)
- >>> landscape_developers.setContactAddress(email_address)
-
- >>> view = create_initialized_view(landscape_developers, '+contactuser')
- >>> print view.recipients.description
- You are contacting the Landscape Developers (landscape-developers) team.
-
- >>> [recipient.name for recipient in view.recipients]
- [u'landscape-developers']
-
Contact this user/team valid addresses and quotas
-------------------------------------------------
=== modified file 'lib/lp/registry/browser/tests/team-views.txt'
--- lib/lp/registry/browser/tests/team-views.txt 2012-01-09 12:39:11 +0000
+++ lib/lp/registry/browser/tests/team-views.txt 2012-02-10 21:48:22 +0000
@@ -213,9 +213,9 @@
>>> view = create_initialized_view(guadamen, '+index')
>>> print view.contact_link_title
- Send an email to this team's owner through Launchpad
+ Send an email to this team's admins through Launchpad
>>> print view.specific_contact_text
- Contact this team's owner
+ Contact this team's admins
Mugshots
=== modified file 'lib/lp/registry/browser/tests/user-to-user-views.txt'
--- lib/lp/registry/browser/tests/user-to-user-views.txt 2011-12-29 05:29:36 +0000
+++ lib/lp/registry/browser/tests/user-to-user-views.txt 2012-02-10 21:48:22 +0000
@@ -305,13 +305,14 @@
--
This message was sent from Launchpad by
Bart (http://launchpad.dev/~bart)
- using the "Contact this team's owner" link on the GuadaMen team page
+ using the "Contact this team's admins" link on the GuadaMen team page
(http://launchpad.dev/~guadamen).
For more information see
https://help.launchpad.net/YourAccount/ContactingPeople
- # of Messages: 1
+ # of Messages: 2
Recipients:
Foo Bar <foo.bar@xxxxxxxxxxxxx>
+ Ubuntu Team <support@xxxxxxxxxx>
Member to team
@@ -363,110 +364,7 @@
Steve Alexander <steve.alexander@xxxxxxxxxxxxxxx>
Ubuntu Team <support@xxxxxxxxxx>
-The Guadamen team creates a mailing list but does not set it to be the
-contact address. The mailing list will not be used.
-
- >>> team, mailing_list = factory.makeTeamAndMailingList(
- ... guadamen.name, guadamen.teamowner.name)
-
- # Ignore the 'new mailing list message'
-
- >>> transaction.commit()
- >>> del stub.test_emails[:]
-
-Foo Bar now contacts them again, which he can do because his quota is
-still not met. This message includes a "%s" combination; it is not a
-interpolation instruction.
-
- >>> view = create_view(
- ... foo_bar, guadamen, {
- ... 'field.field.from_': 'foo.bar@xxxxxxxxxxxxx',
- ... 'field.subject': 'My last question for Guadamen',
- ... 'field.message': 'Can one of you help me with "%s" usage!',
- ... 'field.actions.send': 'Send',
- ... })
- >>> print_notifications(view)
- Message sent to GuadaMen
-
-Foo Bar's message gets sent to each individual member of he team.
-
- >>> transaction.commit()
- >>> print_messages()
- Senders: set(['Foo Bar <foo.bar@xxxxxxxxxxxxx>'])
- Subjects: set(['My last question for Guadamen'])
- Bodies:
- Can one of you help me with "%s" usage!
- --
- This message was sent from Launchpad by
- Foo Bar (http://launchpad.dev/~name16)
- to each member of the GuadaMen team
- using the "Contact this team" link on the GuadaMen
- team page (http://launchpad.dev/~guadamen).
- For more information see
- https://help.launchpad.net/YourAccount/ContactingPeople
- # of Messages: 10
- Recipients:
- Alexander Limi <limi@xxxxxxxxx>
- Celso Providelo <celso.providelo@xxxxxxxxxxxxx>
- Colin Watson <colin.watson@xxxxxxxxxxxxxxx>
- Daniel Silverstone <daniel.silverstone@xxxxxxxxxxxxx>
- Edgar Bursic <edgar@xxxxxxxxxxxxxxxx>
- Foo Bar <foo.bar@xxxxxxxxxxxxx>
- Jeff Waugh <jeff.waugh@xxxxxxxxxxxxxxx>
- Mark Shuttleworth <mark@xxxxxxxxxxx>
- Steve Alexander <steve.alexander@xxxxxxxxxxxxxxx>
- Ubuntu Team <support@xxxxxxxxxx>
-
-The Guadamen team now registers an external contact address (they could
-have also used their Launchpad mailing list address).
-
- >>> from lp.services.identity.interfaces.emailaddress import (
- ... IEmailAddressSet)
- >>> email_set = getUtility(IEmailAddressSet)
- >>> address = email_set.new('guadamen@xxxxxxxxxxx', guadamen)
- >>> guadamen.setContactAddress(address)
-
-Foo Bar contacts the Guadamen team again, which is allowed because his
-quota was not met by his first message. This time only one message is
-sent, and that to the new contact address.
-
- >>> view = create_view(
- ... foo_bar, guadamen, {
- ... 'field.field.from_': 'foo.bar@xxxxxxxxxxxxx',
- ... 'field.subject': 'Hello again Guadamen',
- ... 'field.message': 'Can one of you help me?',
- ... 'field.actions.send': 'Send',
- ... })
- >>> print_notifications(view)
- Message sent to GuadaMen
-
- >>> transaction.commit()
- >>> len(stub.test_emails)
- 1
-
- >>> from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- >>> print from_addr, to_addrs
- bounces@xxxxxxxxxxxxx [u'GuadaMen <guadamen@xxxxxxxxxxx>']
-
- >>> print raw_msg
- Content-Type: text/plain; charset="us-ascii"
- ...
- From: Foo Bar <foo.bar@xxxxxxxxxxxxx>
- To: GuadaMen <guadamen@xxxxxxxxxxx>
- Subject: Hello again Guadamen
- ...
- X-Launchpad-Message-Rationale: ContactViaWeb member (guadamen team)
- ...
- <BLANKLINE>
- Can one of you help me?
- --
- This message was sent from Launchpad by
- Foo Bar (http://launchpad.dev/~name16)
- using the "Contact this team" link on the GuadaMen team page
- (http://launchpad.dev/~guadamen).
- For more information see
- https://help.launchpad.net/YourAccount/ContactingPeople
-
+ >>> transaction.commit()
Message quota
-------------
@@ -477,6 +375,22 @@
emails again. The is_possible property will be False if is_allowed is
False.
+ >>> view = create_view(
+ ... foo_bar, guadamen, {
+ ... 'field.field.from_': 'foo.bar@xxxxxxxxxxxxx',
+ ... 'field.subject': 'Hello Guadamen',
+ ... 'field.message': 'Can one of you help me?',
+ ... 'field.actions.send': 'Send',
+ ... })
+ >>> view.contact_is_allowed
+ True
+
+ >>> view.contact_is_possible
+ True
+
+ >>> view.initialize()
+ >>> transaction.commit()
+
Foo Bar has now reached his quota and can send no more contact messages
today.
@@ -500,16 +414,6 @@
Your message was not sent because you have exceeded your daily quota of
3 messages to contact users. Try again in ...
-Bart has sent 1 message, he may send more messages.
-
- >>> login_person(bart)
- >>> view = create_view(bart, guadamen)
- >>> view.contact_is_allowed
- True
-
- >>> view.contact_is_possible
- True
-
Identifying information
-----------------------
=== modified file 'lib/lp/registry/stories/team/xx-team-home.txt'
--- lib/lp/registry/stories/team/xx-team-home.txt 2011-01-04 16:08:57 +0000
+++ lib/lp/registry/stories/team/xx-team-home.txt 2012-02-10 21:48:22 +0000
@@ -1,4 +1,5 @@
-= A team's home page =
+A team's home page
+==================
The home page of a public team is visible to everyone.
@@ -133,18 +134,6 @@
... find_tag_by_id(browser.contents, 'your-involvement'))
You are a member of this team...
-He can also see the contact email address and a link to contact the team.
-
- >>> browser.open('http://launchpad.dev/~ubuntu-team')
- >>> print extract_text(
- ... find_tag_by_id(browser.contents, 'contact-email'))
- Email:
- support@xxxxxxxxxx
- Set contact address
- >>> print extract_text(
- ... find_tag_by_id(browser.contents, 'contact-user'))
- Contact this team's email address
-
Member can contact their team even if the team does not have a contact
address:
@@ -190,7 +179,8 @@
...
-== Team admins ==
+Team admins
+-----------
Team owners and admins can see a link to approve and decline applicants.
@@ -207,7 +197,8 @@
<Link text='Approve or decline members' url='.../+editproposedmembers'>
-== Non members ==
+Non members
+-----------
No Privileges Person is not a member of the Ubuntu team.
@@ -220,13 +211,14 @@
He can see the contact address, and the link explains the email
will actually go to the team's administrators.
- >>> print extract_text(find_tag_by_id(user_browser.contents, 'contact-email'))
+ >>> print extract_text(find_tag_by_id(
+ ... user_browser.contents, 'contact-email'))
Email:
support@xxxxxxxxxx
>>> content = find_tag_by_id(user_browser.contents, 'contact-user')
>>> print extract_text(content)
- Contact this team's owner
+ Contact this team's admins
>>> content.a
<a href="+contactuser"...
- title="Send an email to this team's owner through Launchpad">...
+ title="Send an email to this team's admins through Launchpad">...
=== modified file 'lib/lp/registry/templates/contact-user.pt'
--- lib/lp/registry/templates/contact-user.pt 2011-10-13 05:42:45 +0000
+++ lib/lp/registry/templates/contact-user.pt 2012-02-10 21:48:22 +0000
@@ -17,7 +17,7 @@
specified below. If you do not want
<tal:person
tal:content="context/displayname">Anne Person</tal:person>
- to know your email address, hit
+ to know your email address,
<a href="" tal:attributes="href view/cancel_url">Cancel</a> now.
</p>
</div>
