launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06030
[Merge] lp:~wgrant/launchpad/mailinglists-use-people into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/mailinglists-use-people into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/mailinglists-use-people/+merge/87462
This branch makes MailingList(Set)?.get(Subscribed|Sender)Addresses() a bit more sensible.
MailingList's methods are only used by tests, so I've adjusted their return value to be more convenient and reimplemented them in terms of the Set's. This allows a lot of boilerplate to be removed from the tests.
MailingListSet.getSubscribedAddresses() has had a substantial makeover. Its queries now use inner rather than unnecessary outer joins, and the table list is not duplicated. It also now uses EmailAddress.person instead of the deprecated and soon to be removed EmailAddress.account.
--
https://code.launchpad.net/~wgrant/launchpad/mailinglists-use-people/+merge/87462
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/mailinglists-use-people into lp:launchpad.
=== modified file 'lib/lp/registry/browser/tests/team-join-views.txt'
--- lib/lp/registry/browser/tests/team-join-views.txt 2011-12-24 17:49:30 +0000
+++ lib/lp/registry/browser/tests/team-join-views.txt 2012-01-04 11:29:16 +0000
@@ -56,8 +56,7 @@
Your request to join moderated-team-with-list is awaiting approval.
Your mailing list subscription is awaiting approval.
- >>> sorted(email.email for email in
- ... moderated_list.getSubscribedAddresses())
+ >>> sorted(moderated_list.getSubscribedAddresses())
[]
# The subscription object is in the database, waiting for the
@@ -76,6 +75,5 @@
You have successfully joined open-team-with-list.
You have been subscribed to this team’s mailing list.
- >>> sorted(email.email for email in
- ... open_list.getSubscribedAddresses())
+ >>> sorted(open_list.getSubscribedAddresses())
[u'test@xxxxxxxxxxxxx']
=== modified file 'lib/lp/registry/doc/mailinglist-subscriptions.txt'
--- lib/lp/registry/doc/mailinglist-subscriptions.txt 2011-12-24 17:49:30 +0000
+++ lib/lp/registry/doc/mailinglist-subscriptions.txt 2012-01-04 11:29:16 +0000
@@ -48,7 +48,7 @@
mailing list, are available both through the mailing list object and through
the mailing list set.
- >>> sorted(email.email for email in list_one.getSenderAddresses())
+ >>> sorted(list_one.getSenderAddresses())
[u'anne.person@xxxxxxxxxxx', u'aperson@xxxxxxxxxxx',
u'bart.person@xxxxxxxxxxx', u'bperson@xxxxxxxxxxx',
u'no-priv@xxxxxxxxxxxxx']
@@ -76,7 +76,7 @@
>>> list_one.subscribe(anne)
>>> transaction.commit()
- >>> sorted(email.email for email in list_one.getSubscribedAddresses())
+ >>> sorted(list_one.getSubscribedAddresses())
[u'anne.person@xxxxxxxxxxx']
>>> sorted(person.displayname for person in list_one.getSubscribers())
[u'Anne Person']
@@ -94,7 +94,7 @@
>>> list_one.subscribe(bart, alternative_email)
>>> transaction.commit()
- >>> sorted(email.email for email in list_one.getSubscribedAddresses())
+ >>> sorted(list_one.getSubscribedAddresses())
[u'anne.person@xxxxxxxxxxx', u'bperson@xxxxxxxxxxx']
>>> sorted(person.displayname for person in list_one.getSubscribers())
[u'Anne Person', u'Bart Person']
@@ -114,7 +114,7 @@
messages. However, the user may post a message from any of the addresses
they've confirmed with Launchpad.
- >>> sorted(email.email for email in list_one.getSenderAddresses())
+ >>> sorted(list_one.getSenderAddresses())
[u'anne.person@xxxxxxxxxxx', u'aperson@xxxxxxxxxxx',
u'bart.person@xxxxxxxxxxx', u'bperson@xxxxxxxxxxx',
u'no-priv@xxxxxxxxxxxxx']
@@ -129,7 +129,7 @@
>>> alternative.email
u'anne.x.person@xxxxxxxxxxx'
- >>> sorted(email.email for email in list_one.getSenderAddresses())
+ >>> sorted(list_one.getSenderAddresses())
[u'anne.person@xxxxxxxxxxx', u'aperson@xxxxxxxxxxx',
u'bart.person@xxxxxxxxxxx', u'bperson@xxxxxxxxxxx',
u'no-priv@xxxxxxxxxxxxx']
@@ -140,7 +140,7 @@
>>> alternative.status = EmailAddressStatus.VALIDATED
>>> transaction.commit()
- >>> sorted(email.email for email in list_one.getSenderAddresses())
+ >>> sorted(list_one.getSenderAddresses())
[u'anne.person@xxxxxxxxxxx', u'anne.x.person@xxxxxxxxxxx',
u'aperson@xxxxxxxxxxx',
u'bart.person@xxxxxxxxxxx', u'bperson@xxxxxxxxxxx',
@@ -215,7 +215,7 @@
>>> list_one.subscribe(cris)
>>> transaction.commit()
- >>> sorted(email.email for email in list_one.getSubscribedAddresses())
+ >>> sorted(list_one.getSubscribedAddresses())
[u'anne.person@xxxxxxxxxxx', u'bperson@xxxxxxxxxxx',
u'cris.person@xxxxxxxxxxx']
>>> [person.displayname for person in list_one.getSubscribers()]
@@ -229,7 +229,7 @@
Cris may post to the mailing list using any of her registered and validated
email addresses.
- >>> sorted(email.email for email in list_one.getSenderAddresses())
+ >>> sorted(list_one.getSenderAddresses())
[u'anne.person@xxxxxxxxxxx', u'aperson@xxxxxxxxxxx',
u'bart.person@xxxxxxxxxxx', u'bperson@xxxxxxxxxxx',
u'cperson@xxxxxxxxxxx', u'cris.person@xxxxxxxxxxx',
@@ -250,7 +250,7 @@
>>> alternative.email
u'cris.x.person@xxxxxxxxxxx'
- >>> sorted(email.email for email in list_one.getSenderAddresses())
+ >>> sorted(list_one.getSenderAddresses())
[u'anne.person@xxxxxxxxxxx', u'aperson@xxxxxxxxxxx',
u'bart.person@xxxxxxxxxxx', u'bperson@xxxxxxxxxxx',
u'cperson@xxxxxxxxxxx', u'cris.person@xxxxxxxxxxx',
@@ -261,7 +261,7 @@
>>> alternative.status = EmailAddressStatus.VALIDATED
>>> transaction.commit()
- >>> sorted(email.email for email in list_one.getSenderAddresses())
+ >>> sorted(list_one.getSenderAddresses())
[u'anne.person@xxxxxxxxxxx', u'aperson@xxxxxxxxxxx',
u'bart.person@xxxxxxxxxxx', u'bperson@xxxxxxxxxxx',
u'cperson@xxxxxxxxxxx', u'cris.person@xxxxxxxxxxx',
@@ -297,10 +297,9 @@
>>> sub_team_list.subscribe(lars)
>>> transaction.commit()
- >>> sorted(email.email
- ... for email in super_team_list.getSubscribedAddresses())
+ >>> sorted(super_team_list.getSubscribedAddresses())
[u'lars.person@xxxxxxxxxxx']
- >>> sorted(email.email for email in sub_team_list.getSubscribedAddresses())
+ >>> sorted(sub_team_list.getSubscribedAddresses())
[u'lars.person@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSenderAddresses(team_names))
@@ -337,10 +336,10 @@
>>> list_three.subscribe(dirk)
>>> transaction.commit()
- >>> sorted(email.email for email in list_one.getSubscribedAddresses())
+ >>> sorted(list_one.getSubscribedAddresses())
[u'anne.person@xxxxxxxxxxx', u'bperson@xxxxxxxxxxx',
u'cris.person@xxxxxxxxxxx', u'dirk.person@xxxxxxxxxxx']
- >>> sorted(email.email for email in list_three.getSubscribedAddresses())
+ >>> sorted(list_three.getSubscribedAddresses())
[u'dirk.person@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSenderAddresses(team_names))
@@ -379,7 +378,7 @@
... TeamMembershipStatus.DEACTIVATED, salgado)
>>> transaction.commit()
- >>> sorted(email.email for email in list_one.getSubscribedAddresses())
+ >>> sorted(list_one.getSubscribedAddresses())
[u'anne.person@xxxxxxxxxxx', u'bperson@xxxxxxxxxxx',
u'cris.person@xxxxxxxxxxx']
@@ -426,7 +425,7 @@
...
CannotSubscribe: Dirk Person is already subscribed to list Team One
- >>> sorted(email.email for email in list_one.getSubscribedAddresses())
+ >>> sorted(list_one.getSubscribedAddresses())
[u'anne.person@xxxxxxxxxxx', u'bperson@xxxxxxxxxxx',
u'cris.person@xxxxxxxxxxx']
@@ -471,7 +470,7 @@
>>> list_four.subscribe(elle)
>>> transaction.commit()
- >>> sorted(email.email for email in list_four.getSubscribedAddresses())
+ >>> sorted(list_four.getSubscribedAddresses())
[u'elle.person@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSenderAddresses(team_names))
@@ -506,7 +505,7 @@
>>> elle.setPreferredEmail(elle_alternative)
>>> transaction.commit()
- >>> sorted(email.email for email in list_four.getSubscribedAddresses())
+ >>> sorted(list_four.getSubscribedAddresses())
[u'eperson@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSenderAddresses(team_names))
@@ -554,14 +553,14 @@
Any list member can unsubscribe from the mailing list.
- >>> sorted(email.email for email in list_one.getSubscribedAddresses())
+ >>> sorted(list_one.getSubscribedAddresses())
[u'anne.person@xxxxxxxxxxx', u'bperson@xxxxxxxxxxx',
u'cris.person@xxxxxxxxxxx']
>>> list_one.unsubscribe(anne)
>>> transaction.commit()
- >>> sorted(email.email for email in list_one.getSubscribedAddresses())
+ >>> sorted(list_one.getSubscribedAddresses())
[u'bperson@xxxxxxxxxxx', u'cris.person@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSenderAddresses(team_names))
@@ -607,7 +606,7 @@
>>> bart.leave(team_one)
>>> transaction.commit()
- >>> sorted(email.email for email in list_one.getSubscribedAddresses())
+ >>> sorted(list_one.getSubscribedAddresses())
[u'cris.person@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSenderAddresses(team_names))
@@ -647,7 +646,7 @@
>>> bart.join(team_two)
>>> transaction.commit()
- >>> sorted(email.email for email in list_one.getSubscribedAddresses())
+ >>> sorted(list_one.getSubscribedAddresses())
[u'bperson@xxxxxxxxxxx', u'cris.person@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSenderAddresses(team_names))
@@ -686,7 +685,7 @@
>>> cris.leave(team_two)
>>> transaction.commit()
- >>> sorted(email.email for email in list_one.getSubscribedAddresses())
+ >>> sorted(list_one.getSubscribedAddresses())
[u'bperson@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -720,7 +719,7 @@
>>> iona.join(team_five)
>>> iona.join(team_two)
- >>> sorted(email.email for email in list_one.getSubscribedAddresses())
+ >>> sorted(list_one.getSubscribedAddresses())
[u'bperson@xxxxxxxxxxx']
>>> list_one.subscribe(gwen)
@@ -728,7 +727,7 @@
>>> list_one.subscribe(iona)
>>> transaction.commit()
- >>> sorted(email.email for email in list_one.getSubscribedAddresses())
+ >>> sorted(list_one.getSubscribedAddresses())
[u'bperson@xxxxxxxxxxx', u'gwen.person@xxxxxxxxxxx',
u'hank.person@xxxxxxxxxxx', u'iona.person@xxxxxxxxxxx']
@@ -753,7 +752,7 @@
... TeamMembershipStatus.DEACTIVATED, salgado)
>>> transaction.commit()
- >>> sorted(email.email for email in list_one.getSubscribedAddresses())
+ >>> sorted(list_one.getSubscribedAddresses())
[u'bperson@xxxxxxxxxxx', u'iona.person@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -795,7 +794,7 @@
When a team's mailing list is deactivated, all subscriptions to that mailing
list are dropped.
- >>> sorted(email.email for email in list_one.getSubscribedAddresses())
+ >>> sorted(list_one.getSubscribedAddresses())
[u'bperson@xxxxxxxxxxx', u'iona.person@xxxxxxxxxxx']
# Deactivation isn't complete until the list's status is transitioned to
@@ -805,7 +804,7 @@
>>> list_one.transitionToStatus(MailingListStatus.INACTIVE)
>>> transaction.commit()
- >>> sorted(email.email for email in list_one.getSubscribedAddresses())
+ >>> sorted(list_one.getSubscribedAddresses())
[]
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -843,7 +842,7 @@
>>> list_six.subscribe(jack)
>>> transaction.commit()
- >>> sorted(email.email for email in list_six.getSubscribedAddresses())
+ >>> sorted(list_six.getSubscribedAddresses())
[u'jack.person@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -861,7 +860,7 @@
>>> jack.leave(team_six)
>>> transaction.commit()
- >>> sorted(email.email for email in list_six.getSubscribedAddresses())
+ >>> sorted(list_six.getSubscribedAddresses())
[]
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -877,7 +876,7 @@
>>> jack.join(team_six)
>>> transaction.commit()
- >>> sorted(email.email for email in list_six.getSubscribedAddresses())
+ >>> sorted(list_six.getSubscribedAddresses())
[u'jack.person@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -903,7 +902,7 @@
>>> list_six.subscribe(kara, kara_alternative)
>>> transaction.commit()
- >>> sorted(email.email for email in list_six.getSubscribedAddresses())
+ >>> sorted(list_six.getSubscribedAddresses())
[u'jack.person@xxxxxxxxxxx', u'kperson@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -921,7 +920,7 @@
>>> kara.leave(team_six)
>>> transaction.commit()
- >>> sorted(email.email for email in list_six.getSubscribedAddresses())
+ >>> sorted(list_six.getSubscribedAddresses())
[u'jack.person@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -939,7 +938,7 @@
>>> kara.join(team_six)
>>> transaction.commit()
- >>> sorted(email.email for email in list_six.getSubscribedAddresses())
+ >>> sorted(list_six.getSubscribedAddresses())
[u'jack.person@xxxxxxxxxxx', u'kperson@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -968,7 +967,7 @@
>>> list_six.changeAddress(kara, address)
>>> transaction.commit()
- >> sorted(email.email for email in list_six.getSubscribedAddresses())
+ >> sorted(list_six.getSubscribedAddresses())
[u'kara.person@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -987,7 +986,7 @@
>>> list_six.changeAddress(kara, address)
>>> transaction.commit()
- >> sorted(email.email for email in list_six.getSubscribedAddresses())
+ >> sorted(list_six.getSubscribedAddresses())
[u'kperson@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -1008,7 +1007,7 @@
>>> list_six.changeAddress(kara, None)
>>> transaction.commit()
- >>> sorted(email.email for email in list_six.getSubscribedAddresses())
+ >>> sorted(list_six.getSubscribedAddresses())
[u'kara.person@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -1028,7 +1027,7 @@
>>> kara.setPreferredEmail(address)
>>> transaction.commit()
- >>> sorted(email.email for email in list_six.getSubscribedAddresses())
+ >>> sorted(list_six.getSubscribedAddresses())
[u'kperson@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -1076,7 +1075,7 @@
>>> transaction.commit()
>>> list_seven.subscribe(sam)
- >>> sorted(email.email for email in list_seven.getSubscribedAddresses())
+ >>> sorted(list_seven.getSubscribedAddresses())
[]
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -1093,7 +1092,7 @@
Samuel may not post to the mailing list using any of his addresses.
- >>> sorted(email.email for email in list_seven.getSenderAddresses())
+ >>> sorted(list_seven.getSenderAddresses())
[u'no-priv@xxxxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSenderAddresses(team_names))
@@ -1125,9 +1124,9 @@
>>> sam.join(team_seven)
>>> transaction.commit()
- >>> sorted(email.email for email in list_seven.getSubscribedAddresses())
+ >>> sorted(list_seven.getSubscribedAddresses())
[u'samuel.person@xxxxxxxxxxx']
- >>> sorted(email.email for email in list_seven.getSenderAddresses())
+ >>> sorted(list_seven.getSenderAddresses())
[u'no-priv@xxxxxxxxxxxxx',
u'samuel.person@xxxxxxxxxxx', u'sperson@xxxxxxxxxxx']
@@ -1172,9 +1171,9 @@
>>> removeSecurityProxy(list_seven).status = MailingListStatus.INACTIVE
>>> transaction.commit()
- >>> sorted(email.email for email in list_seven.getSubscribedAddresses())
+ >>> sorted(list_seven.getSubscribedAddresses())
[]
- >>> sorted(email.email for email in list_seven.getSenderAddresses())
+ >>> sorted(list_seven.getSenderAddresses())
[]
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -1218,8 +1217,7 @@
>>> list_six.changeAddress(kara, kara_alternative)
>>> transaction.commit()
- >>> print sorted(email.email
- ... for email in list_six.getSubscribedAddresses())
+ >>> print sorted(list_six.getSubscribedAddresses())
[u'kperson@xxxxxxxxxxx']
# kara_alternative has been set as kara's preferred email during this
@@ -1231,8 +1229,7 @@
False
>>> kara_alternative.destroySelf()
>>> transaction.commit()
- >>> print sorted(email.email
- ... for email in list_six.getSubscribedAddresses())
+ >>> print sorted(list_six.getSubscribedAddresses())
[]
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -1286,11 +1283,10 @@
>>> list_nine.subscribe(vera)
>>> transaction.commit()
- >>> print sorted(
- ... email.email for email in list_nine.getSubscribedAddresses())
+ >>> print sorted(list_nine.getSubscribedAddresses())
[u'umma.person@xxxxxxxxxxx', u'vera.person@xxxxxxxxxxx']
- >>> print sorted(email.email for email in list_nine.getSenderAddresses())
+ >>> print sorted(list_nine.getSenderAddresses())
[u'no-priv@xxxxxxxxxxxxx',
u'umma.person@xxxxxxxxxxx', u'uperson@xxxxxxxxxxx',
u'vera.person@xxxxxxxxxxx', u'vperson@xxxxxxxxxxx']
@@ -1344,8 +1340,7 @@
Umma is no longer subscribed to the mailing list...
- >>> print sorted(
- ... email.email for email in list_nine.getSubscribedAddresses())
+ >>> print sorted(list_nine.getSubscribedAddresses())
[u'vera.person@xxxxxxxxxxx']
>>> print_addresses(mailinglist_set.getSubscribedAddresses(team_names))
@@ -1364,7 +1359,7 @@
...and she can no longer post directly to the mailing list.
- >>> print sorted(email.email for email in list_nine.getSenderAddresses())
+ >>> print sorted(list_nine.getSenderAddresses())
[u'no-priv@xxxxxxxxxxxxx',
u'vera.person@xxxxxxxxxxx', u'vperson@xxxxxxxxxxx']
=== modified file 'lib/lp/registry/doc/message-holds.txt'
--- lib/lp/registry/doc/message-holds.txt 2011-12-24 17:49:30 +0000
+++ lib/lp/registry/doc/message-holds.txt 2012-01-04 11:29:16 +0000
@@ -380,7 +380,7 @@
>>> login('no-priv@xxxxxxxxxxxxx')
>>> team_three, list_three = mailinglists_helper.new_team(
... 'test-three', True)
- >>> sorted(email.email for email in list_three.getSenderAddresses())
+ >>> sorted(list_three.getSenderAddresses())
[u'no-priv@xxxxxxxxxxxxx']
Salgado posts a message to the list, but because he is not a team member, it
@@ -405,7 +405,7 @@
>>> held_message.acknowledge()
>>> transaction.commit()
- >>> sorted(email.email for email in list_three.getSenderAddresses())
+ >>> sorted(list_three.getSenderAddresses())
[u'no-priv@xxxxxxxxxxxxx']
>>> from lp.registry.interfaces.mailinglist import IMailingListSet
@@ -434,7 +434,7 @@
Now, Salgado is on the list of approved posters.
- >>> sorted(email.email for email in list_three.getSenderAddresses())
+ >>> sorted(list_three.getSenderAddresses())
[u'guilherme.salgado@xxxxxxxxxxxxx', u'no-priv@xxxxxxxxxxxxx']
>>> bulk_addresses = mailinglist_set.getSenderAddresses(['test-three'])
@@ -444,7 +444,7 @@
But he will still not get messages delivered to his address, since he's not
subscribed to the team mailing list.
- >>> sorted(email.email for email in list_three.getSubscribedAddresses())
+ >>> sorted(list_three.getSubscribedAddresses())
[]
Of course, Salgado still cannot post to a mailing list that he has not been
@@ -452,7 +452,7 @@
>>> team_four, list_four = mailinglists_helper.new_team(
... 'test-four', True)
- >>> sorted(email.email for email in list_four.getSenderAddresses())
+ >>> sorted(list_four.getSenderAddresses())
[u'no-priv@xxxxxxxxxxxxx']
=== modified file 'lib/lp/registry/interfaces/mailinglist.py'
--- lib/lp/registry/interfaces/mailinglist.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/interfaces/mailinglist.py 2012-01-04 11:29:16 +0000
@@ -405,10 +405,8 @@
def getSubscribedAddresses():
"""Return the set of subscribed email addresses for members.
- :return: an iterator over the subscribed IEmailAddresses for all
- subscribed members of the mailing list, in no particular order.
- This represents all the addresses which will receive messages
- posted to the mailing list.
+ :return: a list of email addresses (as strings) for all
+ subscribed members of the mailing list.
"""
def getSubscribers():
@@ -421,9 +419,9 @@
def getSenderAddresses():
"""Return the set of all email addresses for members.
- :return: an iterator over the all the registered and validated
- IEmailAddresses for all members of the mailing list's team, in
- no particular order. These represent all the addresses which are
+ :return: a list of the registered and validated email addresses
+ (as strings) for all members of the mailing list's team, in no
+ particular order. These represent all the addresses which are
allowed to post to the mailing list.
"""
=== modified file 'lib/lp/registry/model/mailinglist.py'
--- lib/lp/registry/model/mailinglist.py 2011-12-30 08:03:42 +0000
+++ lib/lp/registry/model/mailinglist.py 2012-01-04 11:29:16 +0000
@@ -38,7 +38,6 @@
from storm.expr import (
And,
Join,
- LeftJoin,
)
from storm.info import ClassAlias
from storm.store import Store
@@ -442,113 +441,17 @@
def getSubscribedAddresses(self):
"""See `IMailingList`."""
- store = Store.of(self)
- # In order to handle the case where the preferred email address is
- # used (i.e. where MailingListSubscription.email_address is NULL), we
- # need to UNION, those using a specific address and those using the
- # preferred address.
- tables = (
- EmailAddress,
- LeftJoin(Account, Account.id == EmailAddress.accountID),
- LeftJoin(MailingListSubscription,
- MailingListSubscription.personID
- == EmailAddress.personID),
- # pylint: disable-msg=C0301
- LeftJoin(
- MailingList,
- MailingList.id == MailingListSubscription.mailing_listID),
- LeftJoin(TeamParticipation,
- TeamParticipation.personID
- == MailingListSubscription.personID),
- )
- preferred = store.using(*tables).find(
- EmailAddress,
- And(MailingListSubscription.mailing_list == self,
- TeamParticipation.team == self.team,
- MailingList.status != MailingListStatus.INACTIVE,
- MailingListSubscription.email_addressID == None,
- EmailAddress.status == EmailAddressStatus.PREFERRED,
- Account.status == AccountStatus.ACTIVE))
- tables = (
- EmailAddress,
- LeftJoin(Account, Account.id == EmailAddress.accountID),
- LeftJoin(MailingListSubscription,
- MailingListSubscription.email_addressID
- == EmailAddress.id),
- # pylint: disable-msg=C0301
- LeftJoin(
- MailingList,
- MailingList.id == MailingListSubscription.mailing_listID),
- LeftJoin(TeamParticipation,
- TeamParticipation.personID
- == MailingListSubscription.personID),
- )
- explicit = store.using(*tables).find(
- EmailAddress,
- And(MailingListSubscription.mailing_list == self,
- TeamParticipation.team == self.team,
- MailingList.status != MailingListStatus.INACTIVE,
- Account.status == AccountStatus.ACTIVE))
- # Union the two queries together to give us the complete list of email
- # addresses allowed to post. Note that while we're retrieving both
- # the EmailAddress and Person records, this method is defined as only
- # returning EmailAddresses. The reason why we include the Person in
- # the query is because the consumer of this method will access
- # email_address.person.displayname, so the prejoin to Person is
- # critical to acceptable performance. Indeed, without the prejoin, we
- # were getting tons of timeout OOPSes. See bug 259440.
- for email_address in preferred.union(explicit):
- yield email_address
+ return [
+ address for (name, address) in
+ getUtility(IMailingListSet).getSubscribedAddresses(
+ [self.team.name]).get(self.team.name, [])]
def getSenderAddresses(self):
"""See `IMailingList`."""
- store = Store.of(self)
- # First, we need to find all the members of the team this mailing list
- # is associated with. Find all of their validated and preferred email
- # addresses of those team members. Every one of those email addresses
- # are allowed to post to the mailing list.
- tables = (
- Person,
- Join(Account, Account.id == Person.accountID),
- Join(EmailAddress, EmailAddress.personID == Person.id),
- Join(TeamParticipation, TeamParticipation.personID == Person.id),
- Join(MailingList, MailingList.teamID == TeamParticipation.teamID),
- )
- team_members = store.using(*tables).find(
- EmailAddress,
- And(TeamParticipation.team == self.team,
- MailingList.status != MailingListStatus.INACTIVE,
- Person.teamowner == None,
- EmailAddress.status.is_in(EMAIL_ADDRESS_STATUSES),
- Account.status == AccountStatus.ACTIVE,
- ))
- # Second, find all of the email addresses for all of the people who
- # have been explicitly approved for posting to this mailing list.
- # This occurs as part of first post moderation, but since they've
- # already been approved for this list, we don't need to wait for three
- # global approvals.
- tables = (
- Person,
- Join(Account, Account.id == Person.accountID),
- Join(EmailAddress, EmailAddress.personID == Person.id),
- Join(MessageApproval, MessageApproval.posted_byID == Person.id),
- )
- approved_posters = store.using(*tables).find(
- EmailAddress,
- And(MessageApproval.mailing_list == self,
- MessageApproval.status.is_in(MESSAGE_APPROVAL_STATUSES),
- EmailAddress.status.is_in(EMAIL_ADDRESS_STATUSES),
- Account.status == AccountStatus.ACTIVE,
- ))
- # Union the two queries together to give us the complete list of email
- # addresses allowed to post. Note that while we're retrieving both
- # the EmailAddress and Person records, this method is defined as only
- # returning EmailAddresses. The reason why we include the Person in
- # the query is because the consumer of this method will access
- # email_address.person.displayname, so the prejoin to Person is
- # critical to acceptable performance. Indeed, without the prejoin, we
- # were getting tons of timeout OOPSes. See bug 259440.
- return team_members.union(approved_posters)
+ return [
+ address for (name, address) in
+ getUtility(IMailingListSet).getSenderAddresses(
+ [self.team.name]).get(self.team.name, [])]
def holdMessage(self, message):
"""See `IMailingList`."""
@@ -686,21 +589,16 @@
Team = ClassAlias(Person)
tables = (
EmailAddress,
- LeftJoin(Account, Account.id == EmailAddress.accountID),
- LeftJoin(MailingListSubscription,
- MailingListSubscription.personID
- == EmailAddress.personID),
- # pylint: disable-msg=C0301
- LeftJoin(
+ Join(Person, Person.id == EmailAddress.personID),
+ Join(Account, Account.id == Person.accountID),
+ Join(TeamParticipation, TeamParticipation.personID == Person.id),
+ Join(
+ MailingListSubscription,
+ MailingListSubscription.personID == Person.id),
+ Join(
MailingList,
MailingList.id == MailingListSubscription.mailing_listID),
- LeftJoin(TeamParticipation,
- TeamParticipation.personID
- == MailingListSubscription.personID),
- LeftJoin(Person,
- Person.id == TeamParticipation.personID),
- LeftJoin(Team,
- Team.id == MailingList.teamID),
+ Join(Team, Team.id == MailingList.teamID),
)
team_ids, list_ids = self._getTeamIdsAndMailingListIds(team_names)
# Find all the people who are subscribed with their preferred address.
@@ -720,29 +618,12 @@
'Unexpected team name in results: %s' % team_name)
value = (display_name, email)
by_team[team_name].add(value)
- tables = (
- EmailAddress,
- LeftJoin(Account, Account.id == EmailAddress.accountID),
- LeftJoin(MailingListSubscription,
- MailingListSubscription.email_addressID
- == EmailAddress.id),
- # pylint: disable-msg=C0301
- LeftJoin(
- MailingList,
- MailingList.id == MailingListSubscription.mailing_listID),
- LeftJoin(TeamParticipation,
- TeamParticipation.personID
- == MailingListSubscription.personID),
- LeftJoin(Person,
- Person.id == TeamParticipation.personID),
- LeftJoin(Team,
- Team.id == MailingList.teamID),
- )
explicit = store.using(*tables).find(
(EmailAddress.email, Person.displayname, Team.name),
And(MailingListSubscription.mailing_listID.is_in(list_ids),
TeamParticipation.teamID.is_in(team_ids),
MailingList.status != MailingListStatus.INACTIVE,
+ EmailAddress.id == MailingListSubscription.email_addressID,
Account.status == AccountStatus.ACTIVE))
for email, display_name, team_name in explicit:
assert team_name in team_names, (
=== modified file 'lib/lp/registry/tests/test_mlists.py'
--- lib/lp/registry/tests/test_mlists.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/tests/test_mlists.py 2012-01-04 11:29:16 +0000
@@ -90,9 +90,7 @@
def assertAddresses(self, *addresses):
"""Assert that `addresses` are subscribed to the mailing list."""
- subscribers = set(
- email.email
- for email in self.mailing_list.getSubscribedAddresses())
+ subscribers = set(self.mailing_list.getSubscribedAddresses())
expected = set(addresses)
self.assertEqual(subscribers, expected)
