Launchpad itself

Merge lp:~edwin-grubbs/launchpad/bug-615654-queue-addmember-emails into lp:launchpad/db-devel

  1. bug-615654-queue-addmember-emails
  2. Merge into db-devel
Proposed by Edwin Grubbs on 2010-09-17
Status: Merged
Approved by: Edwin Grubbs on 2010-09-20
Approved revision: no longer in the source branch.
Merged at revision: 9873
Proposed branch: lp:~edwin-grubbs/launchpad/bug-615654-queue-addmember-emails
Merge into: lp:launchpad/db-devel
Prerequisite: lp:~edwin-grubbs/launchpad/bug-615654-registry-jobqueue-schema
Diff against target: 875 lines (+551/-99)
8 files modified
lib/lp/registry/configure.zcml (+17/-0)
lib/lp/registry/doc/teammembership-email-notification.txt (+25/-11)
lib/lp/registry/enum.py (+12/-0)
lib/lp/registry/interfaces/persontransferjob.py (+70/-0)
lib/lp/registry/model/persontransferjob.py (+334/-0)
lib/lp/registry/model/teammembership.py (+7/-88)
lib/lp/registry/tests/test_persontransferjob.py (+62/-0)
lib/lp/testing/mail_helpers.py (+24/-0)
To merge this branch: bzr merge lp:~edwin-grubbs/launchpad/bug-615654-queue-addmember-emails
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code 2010-09-17 Approve on 2010-09-17
Review via email: mp+35862@code.launchpad.net

Description of the Change

Summary
-------

This branch adds the MembershipNotificationJob class to send emails
after the team membership status has changed, e.g. proposed, approved,
admin, expired.

A follow-up branch will be needed in order to add a cronjob to actually
process the job queue. This branch can't land until then.

Implementation details
----------------------

lib/lp/registry/model/teammembership.py

    lib/lp/registry/configure.zcml
    lib/lp/registry/enum.py
    lib/lp/registry/interfaces/persontransferjob.py
    lib/lp/registry/model/persontransferjob.py

Tests:
    lib/lp/registry/tests/test_persontransferjob.py
    lib/lp/testing/mail_helpers.py
    lib/lp/registry/doc/teammembership-email-notification.txt

Tests
-----

./bin/test -vv -t 'teammembership-email-notification.txt|test_persontransferjob'

To post a comment you must log in.
Brad Crittenden (bac) wrote :
Download full text (12.6 KiB)

Hi Edwin,

Very interesting branch. I've got some suggestions but overall it is great.

--Brad

> === added file 'lib/lp/registry/model/persontransferjob.py'
> --- lib/lp/registry/model/persontransferjob.py 1970-01-01 00:00:00 +0000
> +++ lib/lp/registry/model/persontransferjob.py 2010-09-17 18:59:05 +0000
> @@ -0,0 +1,322 @@
> +# Copyright 2010 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Job classes related to PersonTransferJob are in here."""

s/ are in here//

> +__metaclass__ = type
> +__all__ = [
> + 'MembershipNotificationJob',
> + 'PersonTransferJob',
> + ]
> +
> +from lazr.delegates import delegates
> +import simplejson
> +from sqlobject import SQLObjectNotFound
> +from storm.base import Storm
> +from storm.expr import And
> +from storm.locals import (
> + Int,
> + Reference,
> + Unicode,
> + )
> +from zope.component import getUtility
> +from zope.interface import (
> + classProvides,
> + implements,
> + )
> +
> +

extra blank line

> +from canonical.config import config
> +from canonical.database.enumcol import EnumCol
> +from canonical.launchpad.helpers import (
> + get_contact_email_addresses,
> + get_email_template,
> + )
> +from canonical.launchpad.interfaces.lpstorm import (
> + IMasterStore,
> + IStore,
> + )
> +from canonical.launchpad.mail import (
> + format_address,
> + simple_sendmail,
> + )
> +from canonical.launchpad.mailnotification import MailWrapper
> +from canonical.launchpad.webapp import canonical_url
> +
> +

Hmmm, two of them. is there supposed to be two? did you do that
manually or did henning's tool do it? if the latter, then keep it.

> +from lp.registry.enum import PersonTransferJobType
> +from lp.registry.interfaces.person import (
> + IPerson,
> + IPersonSet,
> + ITeam,
> + )
> +from lp.registry.interfaces.persontransferjob import (
> + IPersonTransferJob,
> + IPersonTransferJobSource,
> + IMembershipNotificationJob,
> + IMembershipNotificationJobSource,
> + )
> +from lp.registry.interfaces.teammembership import TeamMembershipStatus
> +from lp.registry.model.person import Person
> +from lp.services.job.model.job import Job
> +from lp.services.job.runner import BaseRunnableJob
> +
> +
> +class PersonTransferJob(Storm):
[...]
> + def __init__(self, minor_person, major_person, job_type, metadata):
> + """Constructor.
> +
> + :param minor_person: The person or team being added to or removed
> + from the major_person.
> + :param major_person: The person or team that is receiving or losing
> + the minor person.
> + :param job_type: The specific membership action being performed.
> + :param metadata: The type-specific variables, as a JSON-compatible
> + dict.

Indent to match.

> + """
> + super(PersonTransferJob, self).__init__()
> + self.job = Job()
> + self.job_type = job_type
> + self.major_person = major_person
> + self.minor_person = minor_person
> +
> + json_data = simplejson.dump...

review: Approve (code)
Edwin Grubbs (edwin-grubbs) wrote :
Download full text (13.7 KiB)

On Fri, Sep 17, 2010 at 3:12 PM, Brad Crittenden <email address hidden> wrote:
> Review: Approve code
> Hi Edwin,
>
> Very interesting branch.  I've got some suggestions but overall it is great.
>
> --Brad

Thanks for the review.

>> === added file 'lib/lp/registry/model/persontransferjob.py'
>> --- lib/lp/registry/model/persontransferjob.py        1970-01-01 00:00:00 +0000
>> +++ lib/lp/registry/model/persontransferjob.py        2010-09-17 18:59:05 +0000
>> @@ -0,0 +1,322 @@
>> +# Copyright 2010 Canonical Ltd.  This software is licensed under the
>> +# GNU Affero General Public License version 3 (see the file LICENSE).
>> +
>> +"""Job classes related to PersonTransferJob are in here."""
>
> s/ are in here//

Fixed. That's just odd. I'm blaming it on my rampant plagiarism in this branch.

>> +__metaclass__ = type
>> +__all__ = [
>> +    'MembershipNotificationJob',
>> +    'PersonTransferJob',
>> +    ]
>> +
>> +from lazr.delegates import delegates
>> +import simplejson
>> +from sqlobject import SQLObjectNotFound
>> +from storm.base import Storm
>> +from storm.expr import And
>> +from storm.locals import (
>> +    Int,
>> +    Reference,
>> +    Unicode,
>> +    )
>> +from zope.component import getUtility
>> +from zope.interface import (
>> +    classProvides,
>> +    implements,
>> +    )
>> +
>> +
>
> extra blank line

Fixed with henning's tool.

>> +from canonical.config import config
>> +from canonical.database.enumcol import EnumCol
>> +from canonical.launchpad.helpers import (
>> +    get_contact_email_addresses,
>> +    get_email_template,
>> +    )
>> +from canonical.launchpad.interfaces.lpstorm import (
>> +    IMasterStore,
>> +    IStore,
>> +    )
>> +from canonical.launchpad.mail import (
>> +    format_address,
>> +    simple_sendmail,
>> +    )
>> +from canonical.launchpad.mailnotification import MailWrapper
>> +from canonical.launchpad.webapp import canonical_url
>> +
>> +
>
> Hmmm, two of them.  is there supposed to be two?  did you do that
> manually or did henning's tool do it?  if the latter, then keep it.

Fixed. I really need to update my vim plugin.

>> +from lp.registry.enum import PersonTransferJobType
>> +from lp.registry.interfaces.person import (
>> +    IPerson,
>> +    IPersonSet,
>> +    ITeam,
>> +    )
>> +from lp.registry.interfaces.persontransferjob import (
>> +    IPersonTransferJob,
>> +    IPersonTransferJobSource,
>> +    IMembershipNotificationJob,
>> +    IMembershipNotificationJobSource,
>> +    )
>> +from lp.registry.interfaces.teammembership import TeamMembershipStatus
>> +from lp.registry.model.person import Person
>> +from lp.services.job.model.job import Job
>> +from lp.services.job.runner import BaseRunnableJob
>> +
>> +
>> +class PersonTransferJob(Storm):
> [...]
>> +    def __init__(self, minor_person, major_person, job_type, metadata):
>> +        """Constructor.
>> +
>> +        :param minor_person: The person or team being added to or removed
>> +                             from the major_person.
>> +        :param major_person: The person or team that is receiving or losing
>> +                             the minor person.
>> +        :param job_type: The specific membership action b...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/configure.zcml'
2--- lib/lp/registry/configure.zcml 2010-09-14 10:51:25 +0000
3+++ lib/lp/registry/configure.zcml 2010-09-17 22:26:48 +0000
4@@ -84,6 +84,23 @@
5 __call__"/>
6 </class>
7
8+ <!-- IPersonTransferJob -->
9+ <securedutility
10+ component="lp.registry.model.persontransferjob.MembershipNotificationJob"
11+ provides="lp.registry.interfaces.persontransferjob.IMembershipNotificationJobSource">
12+ <allow
13+ interface="lp.registry.interfaces.persontransferjob.IMembershipNotificationJobSource"/>
14+ </securedutility>
15+
16+ <class class="lp.registry.model.persontransferjob.PersonTransferJob">
17+ <allow interface="lp.registry.interfaces.persontransferjob.IPersonTransferJob"/>
18+ </class>
19+
20+ <class class="lp.registry.model.persontransferjob.MembershipNotificationJob">
21+ <allow
22+ interface="lp.registry.interfaces.persontransferjob.IMembershipNotificationJob"/>
23+ </class>
24+
25 <!-- Location -->
26
27 <class
28
29=== modified file 'lib/lp/registry/doc/teammembership-email-notification.txt'
30--- lib/lp/registry/doc/teammembership-email-notification.txt 2010-03-26 00:45:36 +0000
31+++ lib/lp/registry/doc/teammembership-email-notification.txt 2010-09-17 22:26:48 +0000
32@@ -24,7 +24,7 @@
33
34 >>> from lp.services.mail import stub
35 >>> from lp.testing.mail_helpers import (
36- ... pop_notifications, print_distinct_emails)
37+ ... pop_notifications, print_distinct_emails, run_mail_jobs)
38
39 >>> from zope.component import getUtility
40 >>> from canonical.launchpad.interfaces import (
41@@ -49,7 +49,7 @@
42 >>> membership.status.title
43 'Proposed'
44
45- >>> transaction.commit()
46+ >>> run_mail_jobs()
47 >>> len(stub.test_emails)
48 5
49 >>> print_distinct_emails(include_reply_to=True)
50@@ -87,6 +87,11 @@
51 # that team.
52 >>> login('mark@example.com')
53 >>> setStatus(membership, TeamMembershipStatus.DECLINED, reviewer=mark)
54+
55+addMember() has queued up a job to send out the emails. We'll run the
56+job now.
57+
58+ >>> run_mail_jobs()
59 >>> len(stub.test_emails)
60 6
61
62@@ -125,6 +130,7 @@
63
64 >>> setStatus(daf_membership, TeamMembershipStatus.APPROVED,
65 ... reviewer=mark, comment='This is a nice guy; I like him')
66+ >>> run_mail_jobs()
67 >>> stub.test_emails.sort(by_to_addrs)
68 >>> len(stub.test_emails)
69 6
70@@ -158,6 +164,7 @@
71
72 >>> setStatus(daf_membership, TeamMembershipStatus.DEACTIVATED,
73 ... reviewer=mark)
74+ >>> run_mail_jobs()
75 >>> stub.test_emails.sort(by_to_addrs)
76 >>> len(stub.test_emails)
77 6
78@@ -187,7 +194,7 @@
79
80 >>> admins = personset.getByName('admins')
81 >>> admins.join(ubuntu_team, requester=mark)
82- >>> transaction.commit()
83+ >>> run_mail_jobs()
84 >>> len(stub.test_emails)
85 5
86 >>> print_distinct_emails(include_reply_to=True)
87@@ -228,7 +235,10 @@
88 >>> cprov = personset.getByName('cprov')
89 >>> marilize = personset.getByName('marilize')
90 >>> ignored = ubuntu_team.addMember(marilize, reviewer=cprov)
91- >>> transaction.commit()
92+ >>> run_mail_jobs()
93+
94+Now, the emails have been sent.
95+
96 >>> len(stub.test_emails)
97 6
98 >>> print_distinct_emails()
99@@ -277,7 +287,7 @@
100 >>> mirror_admins.getTeamAdminsEmailAddresses()
101 ['mark@example.com']
102 >>> ignored = ubuntu_team.addMember(mirror_admins, reviewer=cprov)
103- >>> transaction.commit()
104+ >>> run_mail_jobs()
105 >>> len(stub.test_emails)
106 1
107 >>> print_distinct_emails()
108@@ -304,7 +314,7 @@
109 >>> comment = "Of course I want to be part of ubuntu!"
110 >>> mirror_admins.acceptInvitationToBeMemberOf(ubuntu_team, comment)
111 >>> flush_database_updates()
112- >>> transaction.commit()
113+ >>> run_mail_jobs()
114
115 >>> len(stub.test_emails)
116 6
117@@ -337,7 +347,7 @@
118 >>> comment = "Landscape has nothing to do with ubuntu, unfortunately."
119 >>> landscape.declineInvitationToBeMemberOf(ubuntu_team, comment)
120 >>> flush_database_updates()
121- >>> transaction.commit()
122+ >>> run_mail_jobs()
123
124 >>> len(stub.test_emails)
125 7
126@@ -363,7 +373,7 @@
127 >>> ignored = ubuntu_team.addMember(
128 ... launchpad, reviewer=cprov, force_team_add=True)
129 >>> flush_database_updates()
130- >>> transaction.commit()
131+ >>> run_mail_jobs()
132 >>> len(stub.test_emails)
133 5
134 >>> print_distinct_emails()
135@@ -610,7 +620,7 @@
136 >>> membershipset.handleMembershipsExpiringToday(
137 ... reviewer=getUtility(ILaunchpadCelebrities).janitor)
138 >>> flush_database_updates()
139- >>> transaction.commit()
140+ >>> run_mail_jobs()
141
142 >>> len(stub.test_emails)
143 8
144@@ -690,7 +700,7 @@
145
146 >>> login_person(karl)
147 >>> karl.renewTeamMembership(mirror_admins)
148- >>> transaction.commit()
149+ >>> run_mail_jobs()
150 >>> len(stub.test_emails)
151 1
152
153@@ -713,7 +723,7 @@
154 approved to admin, but he won't get a notification of that.
155
156 >>> team = personset.newTeam(mark, 'testteam', 'Test')
157- >>> transaction.commit()
158+ >>> run_mail_jobs()
159 >>> len(stub.test_emails)
160 0
161
162@@ -730,6 +740,7 @@
163 >>> login('mark@example.com')
164 >>> setStatus(
165 ... cprov_membership, TeamMembershipStatus.ADMIN, reviewer=mark)
166+ >>> run_mail_jobs()
167 >>> len(stub.test_emails)
168 6
169 >>> print_distinct_emails()
170@@ -760,6 +771,7 @@
171 >>> jdub_membership = membershipset.getByPersonAndTeam(jdub, ubuntu_team)
172 >>> setStatus(jdub_membership, TeamMembershipStatus.APPROVED,
173 ... reviewer=jdub)
174+ >>> run_mail_jobs()
175 >>> len(stub.test_emails)
176 5
177 >>> print_distinct_emails()
178@@ -784,6 +796,7 @@
179 ... mirror_admins, ubuntu_team)
180 >>> setStatus(mirror_admins_membership, TeamMembershipStatus.DEACTIVATED,
181 ... reviewer=mark, silent=False)
182+ >>> run_mail_jobs()
183 >>> len(stub.test_emails)
184 6
185
186@@ -811,6 +824,7 @@
187 Approved
188 >>> setStatus(dumper_hwdb_membership,
189 ... TeamMembershipStatus.DEACTIVATED, reviewer=mark, silent=True)
190+ >>> run_mail_jobs()
191 >>> len(stub.test_emails)
192 0
193 >>> print dumper_hwdb_membership.status.title
194
195=== modified file 'lib/lp/registry/enum.py'
196--- lib/lp/registry/enum.py 2010-09-10 10:08:58 +0000
197+++ lib/lp/registry/enum.py 2010-09-17 22:26:48 +0000
198@@ -5,6 +5,7 @@
199
200 __metaclass__ = type
201 __all__ = [
202+ 'PersonTransferJobType',
203 'BugNotificationLevel',
204 'DistroSeriesDifferenceStatus',
205 'DistroSeriesDifferenceType',
206@@ -82,6 +83,7 @@
207 This difference has been resolved and versions are now equal.
208 """)
209
210+
211 class DistroSeriesDifferenceType(DBEnumeratedType):
212 """Distribution series difference type."""
213
214@@ -104,3 +106,13 @@
215
216 This package is present in both series with different versions.
217 """)
218+
219+
220+class PersonTransferJobType(DBEnumeratedType):
221+ """Values that IPersonTransferJob.job_type can take."""
222+
223+ MEMBERSHIP_NOTIFICATION = DBItem(0, """
224+ Add-member notification
225+
226+ Notify affected users of new team membership.
227+ """)
228
229=== added file 'lib/lp/registry/interfaces/persontransferjob.py'
230--- lib/lp/registry/interfaces/persontransferjob.py 1970-01-01 00:00:00 +0000
231+++ lib/lp/registry/interfaces/persontransferjob.py 2010-09-17 22:26:48 +0000
232@@ -0,0 +1,70 @@
233+# Copyright 2010 Canonical Ltd. This software is licensed under the
234+# GNU Affero General Public License version 3 (see the file LICENSE).
235+
236+"""Interface for the Jobs system to change memberships or merge persons."""
237+
238+__metaclass__ = type
239+__all__ = [
240+ 'IMembershipNotificationJob',
241+ 'IMembershipNotificationJobSource',
242+ 'IPersonTransferJob',
243+ 'IPersonTransferJobSource',
244+ ]
245+
246+from zope.interface import Attribute
247+from zope.schema import (
248+ Int,
249+ Object,
250+ )
251+
252+from canonical.launchpad import _
253+from lp.services.fields import PublicPersonChoice
254+from lp.services.job.interfaces.job import (
255+ IJob,
256+ IJobSource,
257+ IRunnableJob,
258+ )
259+
260+
261+class IPersonTransferJob(IRunnableJob):
262+ """A Job related to team membership or a person merge."""
263+
264+ id = Int(
265+ title=_('DB ID'), required=True, readonly=True,
266+ description=_("The tracking number for this job."))
267+
268+ job = Object(
269+ title=_('The common Job attributes'),
270+ schema=IJob,
271+ required=True)
272+
273+ minor_person = PublicPersonChoice(
274+ title=_('The person being added to the major person/team'),
275+ vocabulary='ValidPersonOrTeam',
276+ required=True)
277+
278+ major_person = PublicPersonChoice(
279+ title=_('The person or team receiving the minor person'),
280+ vocabulary='ValidPersonOrTeam',
281+ required=True)
282+
283+ metadata = Attribute('A dict of data about the job.')
284+
285+
286+class IPersonTransferJobSource(IJobSource):
287+ """An interface for acquiring IPersonTransferJobs."""
288+
289+ def create(minor_person, major_person, metadata):
290+ """Create a new IPersonTransferJob."""
291+
292+
293+class IMembershipNotificationJob(IPersonTransferJob):
294+ """A Job to notify new members of a team of that change."""
295+
296+
297+class IMembershipNotificationJobSource(IJobSource):
298+ """An interface for acquiring IMembershipNotificationJobs."""
299+
300+ def create(member, team, reviewer, old_status, new_status,
301+ last_change_comment=None):
302+ """Create a new IMembershipNotificationJob."""
303
304=== added file 'lib/lp/registry/model/persontransferjob.py'
305--- lib/lp/registry/model/persontransferjob.py 1970-01-01 00:00:00 +0000
306+++ lib/lp/registry/model/persontransferjob.py 2010-09-17 22:26:48 +0000
307@@ -0,0 +1,334 @@
308+# Copyright 2010 Canonical Ltd. This software is licensed under the
309+# GNU Affero General Public License version 3 (see the file LICENSE).
310+
311+"""Job classes related to PersonTransferJob."""
312+
313+__metaclass__ = type
314+__all__ = [
315+ 'MembershipNotificationJob',
316+ 'PersonTransferJob',
317+ ]
318+
319+from lazr.delegates import delegates
320+import simplejson
321+from sqlobject import SQLObjectNotFound
322+from storm.base import Storm
323+from storm.expr import And
324+from storm.locals import (
325+ Int,
326+ Reference,
327+ Unicode,
328+ )
329+from zope.component import getUtility
330+from zope.interface import (
331+ classProvides,
332+ implements,
333+ )
334+
335+from canonical.config import config
336+from canonical.database.enumcol import EnumCol
337+from canonical.launchpad.helpers import (
338+ get_contact_email_addresses,
339+ get_email_template,
340+ )
341+from canonical.launchpad.interfaces.lpstorm import IMasterStore
342+from canonical.launchpad.mail import (
343+ format_address,
344+ simple_sendmail,
345+ )
346+from canonical.launchpad.mailnotification import MailWrapper
347+from canonical.launchpad.webapp import canonical_url
348+from lp.registry.enum import PersonTransferJobType
349+from lp.registry.interfaces.person import (
350+ IPerson,
351+ IPersonSet,
352+ ITeam,
353+ )
354+from lp.registry.interfaces.persontransferjob import (
355+ IMembershipNotificationJob,
356+ IMembershipNotificationJobSource,
357+ IPersonTransferJob,
358+ IPersonTransferJobSource,
359+ )
360+from lp.registry.interfaces.teammembership import TeamMembershipStatus
361+from lp.registry.model.person import Person
362+from lp.services.job.model.job import Job
363+from lp.services.job.runner import BaseRunnableJob
364+
365+
366+class PersonTransferJob(Storm):
367+ """Base class for team membership and person merge jobs."""
368+
369+ implements(IPersonTransferJob)
370+
371+ __storm_table__ = 'PersonTransferJob'
372+
373+ id = Int(primary=True)
374+
375+ job_id = Int(name='job')
376+ job = Reference(job_id, Job.id)
377+
378+ major_person_id = Int(name='major_person')
379+ major_person = Reference(major_person_id, Person.id)
380+
381+ minor_person_id = Int(name='minor_person')
382+ minor_person = Reference(minor_person_id, Person.id)
383+
384+ job_type = EnumCol(enum=PersonTransferJobType, notNull=True)
385+
386+ _json_data = Unicode('json_data')
387+
388+ @property
389+ def metadata(self):
390+ return simplejson.loads(self._json_data)
391+
392+ def __init__(self, minor_person, major_person, job_type, metadata):
393+ """Constructor.
394+
395+ :param minor_person: The person or team being added to or removed
396+ from the major_person.
397+ :param major_person: The person or team that is receiving or losing
398+ the minor person.
399+ :param job_type: The specific membership action being performed.
400+ :param metadata: The type-specific variables, as a JSON-compatible
401+ dict.
402+ """
403+ super(PersonTransferJob, self).__init__()
404+ self.job = Job()
405+ self.job_type = job_type
406+ self.major_person = major_person
407+ self.minor_person = minor_person
408+
409+ json_data = simplejson.dumps(metadata)
410+ # XXX AaronBentley 2009-01-29 bug=322819: This should be a bytestring,
411+ # but the DB representation is unicode.
412+ self._json_data = json_data.decode('utf-8')
413+
414+ @classmethod
415+ def get(cls, key):
416+ """Return the instance of this class whose key is supplied."""
417+ store = IMasterStore(PersonTransferJob)
418+ instance = store.get(cls, key)
419+ if instance is None:
420+ raise SQLObjectNotFound(
421+ 'No occurrence of %s has key %s' % (cls.__name__, key))
422+ return instance
423+
424+
425+class PersonTransferJobDerived(BaseRunnableJob):
426+ """Intermediate class for deriving from PersonTransferJob.
427+
428+ Storm classes can't simply be subclassed or you can end up with
429+ multiple objects referencing the same row in the db. This class uses
430+ lazr.delegates, which is a little bit simpler than storm's
431+ infoheritance solution to the problem. Subclasses need to override
432+ the run() method.
433+ """
434+
435+ delegates(IPersonTransferJob)
436+ classProvides(IPersonTransferJobSource)
437+
438+ def __init__(self, job):
439+ self.context = job
440+
441+ def __repr__(self):
442+ return (
443+ '<%(job_type)s branch job (%(id)s) for %(minor_person)s '
444+ 'as part of %(major_person)s. status=%(status)s>' % {
445+ 'job_type': self.context.job_type.name,
446+ 'id': self.context.id,
447+ 'minor_person': self.minor_person.name,
448+ 'major_person': self.major_person.name,
449+ 'status': self.job.status,
450+ })
451+
452+ @classmethod
453+ def create(cls, minor_person, major_person, metadata):
454+ """See `IPersonTransferJob`."""
455+ if not IPerson.providedBy(minor_person):
456+ raise TypeError("minor_person must be IPerson: %s"
457+ % repr(minor_person))
458+ if not IPerson.providedBy(major_person):
459+ raise TypeError("major_person must be IPerson: %s"
460+ % repr(major_person))
461+ job = PersonTransferJob(
462+ minor_person=minor_person,
463+ major_person=major_person,
464+ job_type=cls.class_job_type,
465+ metadata=metadata)
466+ return cls(job)
467+
468+ @classmethod
469+ def iterReady(cls):
470+ """Iterate through all ready PersonTransferJobs."""
471+ store = IMasterStore(PersonTransferJob)
472+ jobs = store.find(
473+ PersonTransferJob,
474+ And(PersonTransferJob.job_type == cls.class_job_type,
475+ PersonTransferJob.job_id.is_in(Job.ready_jobs)))
476+ return (cls(job) for job in jobs)
477+
478+ def getOopsVars(self):
479+ """See `IRunnableJob`."""
480+ vars = BaseRunnableJob.getOopsVars(self)
481+ vars.extend([
482+ ('major_person_name', self.context.major_person.name),
483+ ('minor_person_name', self.context.minor_person.name),
484+ ])
485+ return vars
486+
487+
488+class MembershipNotificationJob(PersonTransferJobDerived):
489+ """A Job that sends notifications about team membership changes."""
490+
491+ implements(IMembershipNotificationJob)
492+ classProvides(IMembershipNotificationJobSource)
493+
494+ class_job_type = PersonTransferJobType.MEMBERSHIP_NOTIFICATION
495+
496+ @classmethod
497+ def create(cls, member, team, reviewer, old_status, new_status,
498+ last_change_comment=None):
499+ if not ITeam.providedBy(team):
500+ raise TypeError('team must be ITeam: %s' % repr(team))
501+ if not IPerson.providedBy(reviewer):
502+ raise TypeError('reviewer must be IPerson: %s' % repr(reviewer))
503+ if old_status not in TeamMembershipStatus:
504+ raise TypeError("old_status must be TeamMembershipStatus: %s"
505+ % repr(old_status))
506+ if new_status not in TeamMembershipStatus:
507+ raise TypeError("new_status must be TeamMembershipStatus: %s"
508+ % repr(new_status))
509+ metadata = {
510+ 'reviewer': reviewer.id,
511+ 'old_status': old_status.name,
512+ 'new_status': new_status.name,
513+ 'last_change_comment': last_change_comment,
514+ }
515+ return super(MembershipNotificationJob, cls).create(
516+ minor_person=member, major_person=team, metadata=metadata)
517+
518+ @property
519+ def member(self):
520+ return self.minor_person
521+
522+ @property
523+ def team(self):
524+ return self.major_person
525+
526+ @property
527+ def reviewer(self):
528+ return getUtility(IPersonSet).get(self.metadata['reviewer'])
529+
530+ @property
531+ def old_status(self):
532+ return TeamMembershipStatus.items[self.metadata['old_status']]
533+
534+ @property
535+ def new_status(self):
536+ return TeamMembershipStatus.items[self.metadata['new_status']]
537+
538+ @property
539+ def last_change_comment(self):
540+ return self.metadata['last_change_comment']
541+
542+ def run(self):
543+ """See `IMembershipNotificationJob`."""
544+ from canonical.launchpad.scripts import log
545+ from_addr = format_address(
546+ self.team.displayname, config.canonical.noreply_from_address)
547+ admin_emails = self.team.getTeamAdminsEmailAddresses()
548+ # person might be a self.team, so we can't rely on its preferredemail.
549+ self.member_email = get_contact_email_addresses(self.member)
550+ # Make sure we don't send the same notification twice to anybody.
551+ for email in self.member_email:
552+ if email in admin_emails:
553+ admin_emails.remove(email)
554+
555+ if self.reviewer != self.member:
556+ self.reviewer_name = self.reviewer.unique_displayname
557+ else:
558+ # The user himself changed his self.membership.
559+ self.reviewer_name = 'the user himself'
560+
561+ if self.last_change_comment:
562+ comment = ("\n%s said:\n %s\n" % (
563+ self.reviewer.displayname, self.last_change_comment.strip()))
564+ else:
565+ comment = ""
566+
567+ replacements = {
568+ 'member_name': self.member.unique_displayname,
569+ 'recipient_name': self.member.displayname,
570+ 'team_name': self.team.unique_displayname,
571+ 'team_url': canonical_url(self.team),
572+ 'old_status': self.old_status.title,
573+ 'new_status': self.new_status.title,
574+ 'reviewer_name': self.reviewer_name,
575+ 'comment': comment}
576+
577+ template_name = 'membership-statuschange'
578+ subject = (
579+ 'Membership change: %(member)s in %(team)s'
580+ % {
581+ 'member': self.member.name,
582+ 'team': self.team.name,
583+ })
584+ if self.new_status == TeamMembershipStatus.EXPIRED:
585+ template_name = 'membership-expired'
586+ subject = '%s expired from team' % self.member.name
587+ elif (self.new_status == TeamMembershipStatus.APPROVED and
588+ self.old_status != TeamMembershipStatus.ADMIN):
589+ if self.old_status == TeamMembershipStatus.INVITED:
590+ subject = ('Invitation to %s accepted by %s'
591+ % (self.member.name, self.reviewer.name))
592+ template_name = 'membership-invitation-accepted'
593+ elif self.old_status == TeamMembershipStatus.PROPOSED:
594+ subject = '%s approved by %s' % (
595+ self.member.name, self.reviewer.name)
596+ else:
597+ subject = '%s added by %s' % (
598+ self.member.name, self.reviewer.name)
599+ elif self.new_status == TeamMembershipStatus.INVITATION_DECLINED:
600+ subject = ('Invitation to %s declined by %s'
601+ % (self.member.name, self.reviewer.name))
602+ template_name = 'membership-invitation-declined'
603+ elif self.new_status == TeamMembershipStatus.DEACTIVATED:
604+ subject = '%s deactivated by %s' % (
605+ self.member.name, self.reviewer.name)
606+ elif self.new_status == TeamMembershipStatus.ADMIN:
607+ subject = '%s made admin by %s' % (
608+ self.member.name, self.reviewer.name)
609+ elif self.new_status == TeamMembershipStatus.DECLINED:
610+ subject = '%s declined by %s' % (
611+ self.member.name, self.reviewer.name)
612+ else:
613+ # Use the default template and subject.
614+ pass
615+
616+ if len(admin_emails) != 0:
617+ admin_template = get_email_template(
618+ "%s-bulk.txt" % template_name)
619+ for address in admin_emails:
620+ recipient = getUtility(IPersonSet).getByEmail(address)
621+ replacements['recipient_name'] = recipient.displayname
622+ msg = MailWrapper().format(
623+ admin_template % replacements, force_wrap=True)
624+ simple_sendmail(from_addr, address, subject, msg)
625+
626+ # The self.member can be a self.self.team without any
627+ # self.members, and in this case we won't have a single email
628+ # address to send this notification to.
629+ if self.member_email and self.reviewer != self.member:
630+ if self.member.isTeam():
631+ template = '%s-bulk.txt' % template_name
632+ else:
633+ template = '%s-personal.txt' % template_name
634+ self.member_template = get_email_template(template)
635+ for address in self.member_email:
636+ recipient = getUtility(IPersonSet).getByEmail(address)
637+ replacements['recipient_name'] = recipient.displayname
638+ msg = MailWrapper().format(
639+ self.member_template % replacements, force_wrap=True)
640+ simple_sendmail(from_addr, address, subject, msg)
641+ log.debug('MembershipNotificationJob sent email')
642
643=== modified file 'lib/lp/registry/model/teammembership.py'
644--- lib/lp/registry/model/teammembership.py 2010-08-20 20:31:18 +0000
645+++ lib/lp/registry/model/teammembership.py 2010-09-17 22:26:48 +0000
646@@ -5,6 +5,7 @@
647
648 __metaclass__ = type
649 __all__ = [
650+ 'sendStatusChangeNotification',
651 'TeamMembership',
652 'TeamMembershipSet',
653 'TeamParticipation',
654@@ -51,6 +52,9 @@
655 TeamMembershipRenewalPolicy,
656 validate_public_person,
657 )
658+from lp.registry.interfaces.persontransferjob import (
659+ IMembershipNotificationJobSource,
660+ )
661 from lp.registry.interfaces.teammembership import (
662 CyclicalTeamMembershipError,
663 DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT,
664@@ -400,96 +404,11 @@
665 """Send a status change notification to all team admins and the
666 member whose membership's status changed.
667 """
668- team = self.team
669- member = self.person
670 reviewer = self.last_changed_by
671- from_addr = format_address(
672- team.displayname, config.canonical.noreply_from_address)
673 new_status = self.status
674- admins_emails = team.getTeamAdminsEmailAddresses()
675- # self.person might be a team, so we can't rely on its preferredemail.
676- member_email = get_contact_email_addresses(member)
677- # Make sure we don't send the same notification twice to anybody.
678- for email in member_email:
679- if email in admins_emails:
680- admins_emails.remove(email)
681-
682- if reviewer != member:
683- reviewer_name = reviewer.unique_displayname
684- else:
685- # The user himself changed his membership.
686- reviewer_name = 'the user himself'
687-
688- if self.last_change_comment:
689- comment = ("\n%s said:\n %s\n" % (
690- reviewer.displayname, self.last_change_comment.strip()))
691- else:
692- comment = ""
693-
694- replacements = {
695- 'member_name': member.unique_displayname,
696- 'recipient_name': member.displayname,
697- 'team_name': team.unique_displayname,
698- 'team_url': canonical_url(team),
699- 'old_status': old_status.title,
700- 'new_status': new_status.title,
701- 'reviewer_name': reviewer_name,
702- 'comment': comment}
703-
704- template_name = 'membership-statuschange'
705- subject = ('Membership change: %(member)s in %(team)s'
706- % {'member': member.name, 'team': team.name})
707- if new_status == TeamMembershipStatus.EXPIRED:
708- template_name = 'membership-expired'
709- subject = '%s expired from team' % member.name
710- elif (new_status == TeamMembershipStatus.APPROVED and
711- old_status != TeamMembershipStatus.ADMIN):
712- if old_status == TeamMembershipStatus.INVITED:
713- subject = ('Invitation to %s accepted by %s'
714- % (member.name, reviewer.name))
715- template_name = 'membership-invitation-accepted'
716- elif old_status == TeamMembershipStatus.PROPOSED:
717- subject = '%s approved by %s' % (member.name, reviewer.name)
718- else:
719- subject = '%s added by %s' % (member.name, reviewer.name)
720- elif new_status == TeamMembershipStatus.INVITATION_DECLINED:
721- subject = ('Invitation to %s declined by %s'
722- % (member.name, reviewer.name))
723- template_name = 'membership-invitation-declined'
724- elif new_status == TeamMembershipStatus.DEACTIVATED:
725- subject = '%s deactivated by %s' % (member.name, reviewer.name)
726- elif new_status == TeamMembershipStatus.ADMIN:
727- subject = '%s made admin by %s' % (member.name, reviewer.name)
728- elif new_status == TeamMembershipStatus.DECLINED:
729- subject = '%s declined by %s' % (member.name, reviewer.name)
730- else:
731- # Use the default template and subject.
732- pass
733-
734- if admins_emails:
735- admins_template = get_email_template(
736- "%s-bulk.txt" % template_name)
737- for address in admins_emails:
738- recipient = getUtility(IPersonSet).getByEmail(address)
739- replacements['recipient_name'] = recipient.displayname
740- msg = MailWrapper().format(
741- admins_template % replacements, force_wrap=True)
742- simple_sendmail(from_addr, address, subject, msg)
743-
744- # The member can be a team without any members, and in this case we
745- # won't have a single email address to send this notification to.
746- if member_email and reviewer != member:
747- if member.isTeam():
748- template = '%s-bulk.txt' % template_name
749- else:
750- template = '%s-personal.txt' % template_name
751- member_template = get_email_template(template)
752- for address in member_email:
753- recipient = getUtility(IPersonSet).getByEmail(address)
754- replacements['recipient_name'] = recipient.displayname
755- msg = MailWrapper().format(
756- member_template % replacements, force_wrap=True)
757- simple_sendmail(from_addr, address, subject, msg)
758+ getUtility(IMembershipNotificationJobSource).create(
759+ self.person, self.team, reviewer, old_status, new_status,
760+ self.last_change_comment)
761
762
763 class TeamMembershipSet:
764
765=== added file 'lib/lp/registry/tests/test_persontransferjob.py'
766--- lib/lp/registry/tests/test_persontransferjob.py 1970-01-01 00:00:00 +0000
767+++ lib/lp/registry/tests/test_persontransferjob.py 2010-09-17 22:26:48 +0000
768@@ -0,0 +1,62 @@
769+# Copyright 2010 Canonical Ltd. This software is licensed under the
770+# GNU Affero General Public License version 3 (see the file LICENSE).
771+
772+"""Tests for PersonTransferJobs."""
773+
774+__metaclass__ = type
775+
776+from canonical.testing import LaunchpadZopelessLayer
777+from lp.registry.enum import PersonTransferJobType
778+from lp.registry.model.persontransferjob import (
779+ PersonTransferJob,
780+ PersonTransferJobDerived,
781+ )
782+from lp.testing import TestCaseWithFactory
783+
784+
785+class PersonTransferJobTestCase(TestCaseWithFactory):
786+ """Test case for basic PersonTransferJob class."""
787+
788+ layer = LaunchpadZopelessLayer
789+
790+ def test_instantiate(self):
791+ # PersonTransferJob.__init__() instantiates a
792+ # PersonTransferJob instance.
793+ person = self.factory.makePerson()
794+ team = self.factory.makeTeam()
795+
796+ metadata = ('some', 'arbitrary', 'metadata')
797+ person_transfer_job = PersonTransferJob(
798+ person,
799+ team,
800+ PersonTransferJobType.MEMBERSHIP_NOTIFICATION,
801+ metadata)
802+
803+ self.assertEqual(person, person_transfer_job.minor_person)
804+ self.assertEqual(team, person_transfer_job.major_person)
805+ self.assertEqual(
806+ PersonTransferJobType.MEMBERSHIP_NOTIFICATION,
807+ person_transfer_job.job_type)
808+
809+ # When we actually access the PersonTransferJob's metadata it
810+ # gets unserialized from JSON, so the representation returned by
811+ # person_transfer_job.metadata will be different from what we
812+ # originally passed in.
813+ metadata_expected = [u'some', u'arbitrary', u'metadata']
814+ self.assertEqual(metadata_expected, person_transfer_job.metadata)
815+
816+
817+class PersonTransferJobDerivedTestCase(TestCaseWithFactory):
818+ """Test case for the PersonTransferJobDerived class."""
819+
820+ layer = LaunchpadZopelessLayer
821+
822+ def test_create_explodes(self):
823+ # PersonTransferJobDerived.create() will blow up because it
824+ # needs to be subclassed to work properly.
825+ person = self.factory.makePerson()
826+ team = self.factory.makeTeam()
827+ metadata = {'foo': 'bar'}
828+ self.assertRaises(
829+ AttributeError,
830+ PersonTransferJobDerived.create, person, team, metadata)
831
832=== modified file 'lib/lp/testing/mail_helpers.py'
833--- lib/lp/testing/mail_helpers.py 2010-08-20 20:31:18 +0000
834+++ lib/lp/testing/mail_helpers.py 2010-09-17 22:26:48 +0000
835@@ -10,7 +10,14 @@
836
837 import transaction
838
839+from zope.component import getUtility
840+
841+from lp.registry.interfaces.persontransferjob import (
842+ IMembershipNotificationJobSource,
843+ )
844+from lp.services.job.runner import JobRunner
845 from lp.services.mail import stub
846+from lp.testing.logger import MockLogger
847
848
849 def pop_notifications(sort_key=None, commit=True):
850@@ -91,8 +98,25 @@
851 print body
852 print "-"*40
853
854+
855 def print_distinct_emails(include_reply_to=False, include_rationale=True):
856 """A convenient shortcut for `print_emails`(group_similar=True)."""
857 return print_emails(group_similar=True,
858 include_reply_to=include_reply_to,
859 include_rationale=include_rationale)
860+
861+
862+def run_mail_jobs():
863+ """Process job queues that send out emails.
864+
865+ If a new job type is added that sends emails, this function can be
866+ extended to run those jobs, so that testing emails doesn't require a
867+ bunch of different function calls to process different queues.
868+ """
869+ # Commit the transaction to make sure that the JobRunner can find
870+ # the queued jobs.
871+ transaction.commit()
872+ job_source = getUtility(IMembershipNotificationJobSource)
873+ logger = MockLogger()
874+ runner = JobRunner.fromReady(job_source, logger)
875+ runner.runAll()

Subscribers

People subscribed via source and target branches

to status/vote changes: