Launchpad itself

Merge lp:~thumper/launchpad/new-reviewer-email-job into lp:launchpad

  1. new-reviewer-email-job
  2. Merge into devel
Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merge reported by: Tim Penhey
Merged at revision: not available
Proposed branch: lp:~thumper/launchpad/new-reviewer-email-job
Merge into: lp:launchpad
Prerequisite: lp:~thumper/launchpad/code-review-comment-email-job
Diff against target: 415 lines (+201/-34)
8 files modified
lib/lp/code/configure.zcml (+10/-0)
lib/lp/code/interfaces/branchmergeproposal.py (+19/-0)
lib/lp/code/mail/branch.py (+6/-7)
lib/lp/code/mail/branchmergeproposal.py (+3/-16)
lib/lp/code/mail/tests/test_branch.py (+7/-3)
lib/lp/code/mail/tests/test_branchmergeproposal.py (+79/-5)
lib/lp/code/model/branchmergeproposal.py (+2/-1)
lib/lp/code/model/branchmergeproposaljob.py (+75/-2)
To merge this branch: bzr merge lp:~thumper/launchpad/new-reviewer-email-job
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+21610@code.launchpad.net

Description of the change

This branch adds the job to send the notification email for review requests.

lib/lp/code/configure.zcml
 - simple registering of the job and the job source

lib/lp/code/interfaces/branchmergeproposal.py
 - define the interface for the job itself and the utility to create them

lib/lp/code/mail/branch.py
 - since the details are now stored in the job, we pass through a few
   more details into the BMPMailer.forReviewer static function.

lib/lp/code/mail/branchmergeproposal.py
 - vastly simplify the event listner function.
   NOTE: this method is renamed in a later pipe.

lib/lp/code/mail/tests/test_branch.py and
lib/lp/code/model/branchmergeproposal.py
 - since the static constructor function changed the parameters, the tests
   also had to be updated.

lib/lp/code/mail/tests/test_branchmergeproposal.py
 - some of the existing tests had to be augmented with job.run in order
   to send the email that they were checking before
 - other tests are checking the job creation.

lib/lp/code/model/branchmergeproposaljob.py
 - the implementation of the new job
 - just defers to the existing BMPMailer functionality

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :
Download full text (8.1 KiB)

Hi Tim,

There are some real issues that 'make lint' exposes. Good luck
finding them amongst all of the noise. :)

Otherwise this is a fine branch with the exception of a few items marked below.

Thanks.

--Brad

> === modified file 'lib/lp/code/interfaces/branchmergeproposal.py'
> --- lib/lp/code/interfaces/branchmergeproposal.py 2010-03-29 00:59:29 +0000
> +++ lib/lp/code/interfaces/branchmergeproposal.py 2010-03-29 00:59:31 +0000
> @@ -613,6 +615,23 @@
> """Create a job to email subscribers about the comment."""
>
>
> +class IReviewRequestedEmailJob(IRunnableJob):
> + """Interface for the job to sends review request emails."""
> +
> + reviewer = Attribute('The person or team asked to do the review.')
> + requester = Attribute('The person who as asked for the review.')

s/as/has/

> === modified file 'lib/lp/code/mail/tests/test_branchmergeproposal.py'
> --- lib/lp/code/mail/tests/test_branchmergeproposal.py 2010-03-05 03:35:10 +0000
> +++ lib/lp/code/mail/tests/test_branchmergeproposal.py 2010-03-29 00:59:31 +0000
> @@ -431,6 +437,76 @@
> login_person(self.owner)
> return person
>
> + def getReviewNotificationEmail(self):
> + """Return the review requested email job for the test's proposal."""
> + [job] = list(
> + IStore(BranchMergeProposalJob).find(
> + BranchMergeProposalJob,
> + BranchMergeProposalJob.branch_merge_proposal == self.bmp,
> + BranchMergeProposalJob.job_type ==
> + BranchMergeProposalJobType.REVIEW_REQUEST_EMAIL))
> + return ReviewRequestedEmailJob(job)
> +
> + def test_nominateReview_creates_job(self):
> + # When a reviewer is nominated, a job is created to send out the
> + # review request email.
> + reviewer = self.factory.makePerson()
> + pop_notifications()
> + vote_reference = self.bmp.nominateReviewer(reviewer, self.owner, None)
> + # No email is sent.
> + sent_mail = pop_notifications()
> + self.assertEqual([], sent_mail)
> + # A job is created.
> + review_request_job = self.getReviewNotificationEmail()
> + self.assertEqual(self.bmp, review_request_job.branch_merge_proposal)
> + self.assertEqual(reviewer, review_request_job.reviewer)
> + self.assertEqual(self.bmp.registrant, review_request_job.requester)
> +
> + def test_nominateReview_email_content(self):
> + # The email that is sent contains the description of the proposal, and
> + # a link to the proposal.
> + reviewer = self.factory.makePerson()
> + self.bmp.description = 'This branch is awesome.'
> + vote_reference = self.bmp.nominateReviewer(reviewer, self.owner, None)
> + review_request_job = self.getReviewNotificationEmail()
> + review_request_job.run()
> + [sent_mail] = pop_notifications()
> + self.assertEqual(
> + dedent("""\
> + You have been requested to review the proposed merge of %(source)s into %(target)s.
> +
> + This branch is awesome.
> +
> + --
> + %(bmp)s
> + You ...

Read more...

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2010-03-29 00:59:29 +0000
+++ lib/lp/code/configure.zcml 2010-03-29 00:59:31 +0000
@@ -933,6 +933,16 @@
933 </class>933 </class>
934934
935 <securedutility935 <securedutility
936 component="lp.code.model.branchmergeproposaljob.ReviewRequestedEmailJob"
937 provides="lp.code.interfaces.branchmergeproposal.IReviewRequestedEmailJobSource">
938 <allow interface="lp.code.interfaces.branchmergeproposal.IReviewRequestedEmailJobSource"/>
939 </securedutility>
940 <class class="lp.code.model.branchmergeproposaljob.ReviewRequestedEmailJob">
941 <allow interface="lp.services.job.interfaces.job.IRunnableJob" />
942 <allow interface="lp.code.interfaces.branchmergeproposal.IBranchMergeProposalJob" />
943 </class>
944
945 <securedutility
936 component="lp.code.model.branchjob.BranchUpgradeJob"946 component="lp.code.model.branchjob.BranchUpgradeJob"
937 provides="lp.code.interfaces.branchjob.IBranchUpgradeJobSource">947 provides="lp.code.interfaces.branchjob.IBranchUpgradeJobSource">
938 <allow interface="lp.code.interfaces.branchjob.IBranchUpgradeJobSource"/>948 <allow interface="lp.code.interfaces.branchjob.IBranchUpgradeJobSource"/>
939949
=== modified file 'lib/lp/code/interfaces/branchmergeproposal.py'
--- lib/lp/code/interfaces/branchmergeproposal.py 2010-03-29 00:59:29 +0000
+++ lib/lp/code/interfaces/branchmergeproposal.py 2010-03-29 00:59:31 +0000
@@ -18,6 +18,8 @@
18 'ICreateMergeProposalJobSource',18 'ICreateMergeProposalJobSource',
19 'IMergeProposalCreatedJob',19 'IMergeProposalCreatedJob',
20 'IMergeProposalCreatedJobSource',20 'IMergeProposalCreatedJobSource',
21 'IReviewRequestedEmailJob',
22 'IReviewRequestedEmailJobSource',
21 'IUpdatePreviewDiffJobSource',23 'IUpdatePreviewDiffJobSource',
22 'notify_modified',24 'notify_modified',
23 ]25 ]
@@ -613,6 +615,23 @@
613 """Create a job to email subscribers about the comment."""615 """Create a job to email subscribers about the comment."""
614616
615617
618class IReviewRequestedEmailJob(IRunnableJob):
619 """Interface for the job to sends review request emails."""
620
621 reviewer = Attribute('The person or team asked to do the review.')
622 requester = Attribute('The person who as asked for the review.')
623
624
625class IReviewRequestedEmailJobSource(IJobSource):
626 """Create or retrieve jobs that email review requests."""
627
628 def create(review_request):
629 """Create a job to email a review request.
630
631 :param review_request: A vote reference for the requested review.
632 """
633
634
616# XXX: JonathanLange 2010-01-06: This is only used in the scanner, perhaps it635# XXX: JonathanLange 2010-01-06: This is only used in the scanner, perhaps it
617# should be moved there.636# should be moved there.
618def notify_modified(proposal, func, *args, **kwargs):637def notify_modified(proposal, func, *args, **kwargs):
619638
=== modified file 'lib/lp/code/mail/branch.py'
--- lib/lp/code/mail/branch.py 2010-03-09 16:58:30 +0000
+++ lib/lp/code/mail/branch.py 2010-03-29 00:59:31 +0000
@@ -69,23 +69,22 @@
69 review_level=subscription.review_level)69 review_level=subscription.review_level)
7070
71 @classmethod71 @classmethod
72 def forReviewer(cls, vote_reference, recipient,72 def forReviewer(cls, branch_merge_proposal, pending_review, reviewer,
73 branch_identity_cache=None):73 branch_identity_cache=None):
74 """Construct RecipientReason for a reviewer.74 """Construct RecipientReason for a reviewer.
7575
76 The reviewer will be the sole recipient.76 The reviewer will be the sole recipient.
77 """77 """
78 merge_proposal = vote_reference.branch_merge_proposal78 branch = branch_merge_proposal.source_branch
79 branch = merge_proposal.source_branch79 if pending_review:
80 if vote_reference.comment is None:
81 reason_template = (80 reason_template = (
82 '%(entity_is)s requested to review %(merge_proposal)s.')81 '%(entity_is)s requested to review %(merge_proposal)s.')
83 else:82 else:
84 reason_template = (83 reason_template = (
85 '%(entity_is)s reviewing %(merge_proposal)s.')84 '%(entity_is)s reviewing %(merge_proposal)s.')
86 return cls(vote_reference.reviewer, recipient, branch,85 return cls(reviewer, reviewer, branch, 'Reviewer',
87 'Reviewer', reason_template, merge_proposal,86 reason_template, branch_merge_proposal,
88 branch_identity_cache=branch_identity_cache)87 branch_identity_cache=branch_identity_cache)
8988
90 @classmethod89 @classmethod
91 def forRegistrant(cls, merge_proposal, branch_identity_cache=None):90 def forRegistrant(cls, merge_proposal, branch_identity_cache=None):
9291
=== modified file 'lib/lp/code/mail/branchmergeproposal.py'
--- lib/lp/code/mail/branchmergeproposal.py 2010-02-17 08:57:04 +0000
+++ lib/lp/code/mail/branchmergeproposal.py 2010-03-29 00:59:31 +0000
@@ -15,8 +15,8 @@
15from lp.code.adapters.branch import BranchMergeProposalDelta15from lp.code.adapters.branch import BranchMergeProposalDelta
16from lp.code.enums import CodeReviewNotificationLevel16from lp.code.enums import CodeReviewNotificationLevel
17from lp.code.interfaces.branchmergeproposal import (17from lp.code.interfaces.branchmergeproposal import (
18 IMergeProposalCreatedJobSource)18 IMergeProposalCreatedJobSource, IReviewRequestedEmailJobSource)
19from lp.code.mail.branch import BranchMailer, RecipientReason19from lp.code.mail.branch import BranchMailer
20from lp.registry.interfaces.person import IPerson20from lp.registry.interfaces.person import IPerson
21from lp.services.mail.basemailer import BaseMailer21from lp.services.mail.basemailer import BaseMailer
2222
@@ -46,20 +46,7 @@
4646
47def send_review_requested_notifications(vote_reference, event):47def send_review_requested_notifications(vote_reference, event):
48 """Notify the reviewer that they have been requested to review."""48 """Notify the reviewer that they have been requested to review."""
49 # XXX: rockstar - 9 Oct 2008 - If the reviewer is a team, don't send49 getUtility(IReviewRequestedEmailJobSource).create(vote_reference)
50 # email. This is to stop the abuse of a user spamming all members of
51 # a team by requesting them to review a (possibly unrelated) branch.
52 # Ideally we'd come up with a better solution, but I can't think of
53 # one yet. In all other places we are emailing subscribers directly
54 # rather than people that haven't subscribed.
55 # See bug #281056. (affects IBranchMergeProposal)
56 if not vote_reference.reviewer.is_team:
57 reason = RecipientReason.forReviewer(
58 vote_reference, vote_reference.reviewer)
59 mailer = BMPMailer.forReviewRequest(
60 reason, vote_reference.branch_merge_proposal,
61 vote_reference.registrant)
62 mailer.sendAll()
6350
6451
65class BMPMailer(BranchMailer):52class BMPMailer(BranchMailer):
6653
=== modified file 'lib/lp/code/mail/tests/test_branch.py'
--- lib/lp/code/mail/tests/test_branch.py 2009-11-12 09:00:35 +0000
+++ lib/lp/code/mail/tests/test_branch.py 2010-03-29 00:59:31 +0000
@@ -59,8 +59,11 @@
5959
60 def test_forReviewer(self):60 def test_forReviewer(self):
61 """Test values when created from a branch subscription."""61 """Test values when created from a branch subscription."""
62 ignored, vote_reference, subscriber = self.makeReviewerAndSubscriber()62 merge_proposal, vote_reference, subscriber = (
63 reason = RecipientReason.forReviewer(vote_reference, subscriber)63 self.makeReviewerAndSubscriber())
64 pending_review = vote_reference.comment is None
65 reason = RecipientReason.forReviewer(
66 merge_proposal, pending_review, subscriber)
64 self.assertEqual(subscriber, reason.subscriber)67 self.assertEqual(subscriber, reason.subscriber)
65 self.assertEqual(subscriber, reason.recipient)68 self.assertEqual(subscriber, reason.recipient)
66 self.assertEqual(69 self.assertEqual(
@@ -68,7 +71,8 @@
6871
69 def test_getReasonReviewer(self):72 def test_getReasonReviewer(self):
70 bmp, vote_reference, subscriber = self.makeReviewerAndSubscriber()73 bmp, vote_reference, subscriber = self.makeReviewerAndSubscriber()
71 reason = RecipientReason.forReviewer(vote_reference, subscriber)74 pending_review = vote_reference.comment is None
75 reason = RecipientReason.forReviewer(bmp, pending_review, subscriber)
72 self.assertEqual(76 self.assertEqual(
73 'You are requested to review the proposed merge of %s into %s.'77 'You are requested to review the proposed merge of %s into %s.'
74 % (bmp.source_branch.bzr_identity,78 % (bmp.source_branch.bzr_identity,
7579
=== modified file 'lib/lp/code/mail/tests/test_branchmergeproposal.py'
--- lib/lp/code/mail/tests/test_branchmergeproposal.py 2010-03-05 03:35:10 +0000
+++ lib/lp/code/mail/tests/test_branchmergeproposal.py 2010-03-29 00:59:31 +0000
@@ -5,10 +5,12 @@
55
6from difflib import unified_diff6from difflib import unified_diff
7from unittest import TestLoader7from unittest import TestLoader
8from textwrap import dedent
8import transaction9import transaction
910
10from zope.security.proxy import removeSecurityProxy11from zope.security.proxy import removeSecurityProxy
1112
13from canonical.launchpad.interfaces import IStore
12from canonical.testing import (14from canonical.testing import (
13 DatabaseFunctionalLayer, LaunchpadFunctionalLayer)15 DatabaseFunctionalLayer, LaunchpadFunctionalLayer)
1416
@@ -21,6 +23,9 @@
21from lp.code.mail.branchmergeproposal import (23from lp.code.mail.branchmergeproposal import (
22 BMPMailer, send_merge_proposal_modified_notifications)24 BMPMailer, send_merge_proposal_modified_notifications)
23from lp.code.model.branch import update_trigger_modified_fields25from lp.code.model.branch import update_trigger_modified_fields
26from lp.code.model.branchmergeproposaljob import (
27 BranchMergeProposalJob, BranchMergeProposalJobType,
28 ReviewRequestedEmailJob)
24from lp.code.model.codereviewvote import CodeReviewVoteReference29from lp.code.model.codereviewvote import CodeReviewVoteReference
25from canonical.launchpad.webapp import canonical_url30from canonical.launchpad.webapp import canonical_url
26from lp.testing import login_person, TestCaseWithFactory31from lp.testing import login_person, TestCaseWithFactory
@@ -381,7 +386,8 @@
381 request = CodeReviewVoteReference(386 request = CodeReviewVoteReference(
382 branch_merge_proposal=merge_proposal, reviewer=candidate,387 branch_merge_proposal=merge_proposal, reviewer=candidate,
383 registrant=requester)388 registrant=requester)
384 return RecipientReason.forReviewer(request, candidate), requester389 reason = RecipientReason.forReviewer(merge_proposal, True, candidate)
390 return reason, requester
385391
386 def test_forReviewRequest(self):392 def test_forReviewRequest(self):
387 """Test creating a mailer for a review request."""393 """Test creating a mailer for a review request."""
@@ -431,6 +437,76 @@
431 login_person(self.owner)437 login_person(self.owner)
432 return person438 return person
433439
440 def getReviewNotificationEmail(self):
441 """Return the review requested email job for the test's proposal."""
442 [job] = list(
443 IStore(BranchMergeProposalJob).find(
444 BranchMergeProposalJob,
445 BranchMergeProposalJob.branch_merge_proposal == self.bmp,
446 BranchMergeProposalJob.job_type ==
447 BranchMergeProposalJobType.REVIEW_REQUEST_EMAIL))
448 return ReviewRequestedEmailJob(job)
449
450 def test_nominateReview_creates_job(self):
451 # When a reviewer is nominated, a job is created to send out the
452 # review request email.
453 reviewer = self.factory.makePerson()
454 pop_notifications()
455 vote_reference = self.bmp.nominateReviewer(reviewer, self.owner, None)
456 # No email is sent.
457 sent_mail = pop_notifications()
458 self.assertEqual([], sent_mail)
459 # A job is created.
460 review_request_job = self.getReviewNotificationEmail()
461 self.assertEqual(self.bmp, review_request_job.branch_merge_proposal)
462 self.assertEqual(reviewer, review_request_job.reviewer)
463 self.assertEqual(self.bmp.registrant, review_request_job.requester)
464
465 def test_nominateReview_email_content(self):
466 # The email that is sent contains the description of the proposal, and
467 # a link to the proposal.
468 reviewer = self.factory.makePerson()
469 self.bmp.description = 'This branch is awesome.'
470 vote_reference = self.bmp.nominateReviewer(reviewer, self.owner, None)
471 review_request_job = self.getReviewNotificationEmail()
472 review_request_job.run()
473 [sent_mail] = pop_notifications()
474 self.assertEqual(
475 dedent("""\
476 You have been requested to review the proposed merge of %(source)s into %(target)s.
477
478 This branch is awesome.
479
480 --
481 %(bmp)s
482 You are requested to review the proposed merge of %(source)s into %(target)s.
483 """ % {
484 'source': self.bmp.source_branch.bzr_identity,
485 'target': self.bmp.target_branch.bzr_identity,
486 'bmp': canonical_url(self.bmp)}),
487 sent_mail.get_payload(decode=True))
488
489 def test_nominateReview_emails_team_address(self):
490 # If a review request is made for a team, the members of the team are
491 # sent an email.
492 eric = self.factory.makePerson(
493 displayname='Eric the Viking', email='eric@vikings.example.com')
494 black_beard = self.factory.makePerson(
495 displayname='Black Beard', email='black@pirates.example.com')
496 review_team = self.factory.makeTeam(owner=eric)
497 login_person(eric)
498 review_team.addMember(black_beard, eric)
499 pop_notifications()
500 login_person(self.owner)
501 vote_reference = self.bmp.nominateReviewer(review_team, self.owner, None)
502 review_request_job = self.getReviewNotificationEmail()
503 review_request_job.run()
504 sent_mail = pop_notifications()
505 self.assertEqual(
506 ['Black Beard <black@pirates.example.com>',
507 'Eric the Viking <eric@vikings.example.com>'],
508 sorted(mail['to'] for mail in sent_mail))
509
434 def test_requestReviewWithPrivateEmail(self):510 def test_requestReviewWithPrivateEmail(self):
435 # We can request a review, even when one of the parties involved has a511 # We can request a review, even when one of the parties involved has a
436 # private email address.512 # private email address.
@@ -438,11 +514,9 @@
438 # Request a review and prepare the mailer.514 # Request a review and prepare the mailer.
439 vote_reference = self.bmp.nominateReviewer(515 vote_reference = self.bmp.nominateReviewer(
440 candidate, self.owner, None)516 candidate, self.owner, None)
441 reason = RecipientReason.forReviewer(vote_reference, candidate)
442 mailer = BMPMailer.forReviewRequest(reason, self.bmp, self.owner)
443 # Send the mail.517 # Send the mail.
444 pop_notifications()518 review_request_job = self.getReviewNotificationEmail()
445 mailer.sendAll()519 review_request_job.run()
446 mails = pop_notifications()520 mails = pop_notifications()
447 self.assertEqual(1, len(mails))521 self.assertEqual(1, len(mails))
448 candidate = removeSecurityProxy(candidate)522 candidate = removeSecurityProxy(candidate)
449523
=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py 2010-02-26 09:54:20 +0000
+++ lib/lp/code/model/branchmergeproposal.py 2010-03-29 00:59:31 +0000
@@ -281,8 +281,9 @@
281 for review in self.votes:281 for review in self.votes:
282 reviewer = review.reviewer282 reviewer = review.reviewer
283 if not reviewer.is_team:283 if not reviewer.is_team:
284 pending = review.comment is None
284 recipients[reviewer] = RecipientReason.forReviewer(285 recipients[reviewer] = RecipientReason.forReviewer(
285 review, reviewer,286 self, pending, reviewer,
286 branch_identity_cache=branch_identity_cache)287 branch_identity_cache=branch_identity_cache)
287 # If the registrant of the proposal is getting emails, update the288 # If the registrant of the proposal is getting emails, update the
288 # rationale to say that they registered it. Don't however send them289 # rationale to say that they registered it. Don't however send them
289290
=== modified file 'lib/lp/code/model/branchmergeproposaljob.py'
--- lib/lp/code/model/branchmergeproposaljob.py 2010-03-29 00:59:29 +0000
+++ lib/lp/code/model/branchmergeproposaljob.py 2010-03-29 00:59:31 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009, 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
44
@@ -20,6 +20,7 @@
20 'CodeReviewCommentEmailJob',20 'CodeReviewCommentEmailJob',
21 'CreateMergeProposalJob',21 'CreateMergeProposalJob',
22 'MergeProposalCreatedJob',22 'MergeProposalCreatedJob',
23 'ReviewRequestedEmailJob',
23 'UpdatePreviewDiffJob',24 'UpdatePreviewDiffJob',
24 ]25 ]
2526
@@ -51,13 +52,16 @@
51 IBranchMergeProposalJob, ICodeReviewCommentEmailJob,52 IBranchMergeProposalJob, ICodeReviewCommentEmailJob,
52 ICodeReviewCommentEmailJobSource, ICreateMergeProposalJob,53 ICodeReviewCommentEmailJobSource, ICreateMergeProposalJob,
53 ICreateMergeProposalJobSource, IMergeProposalCreatedJob,54 ICreateMergeProposalJobSource, IMergeProposalCreatedJob,
54 IMergeProposalCreatedJobSource, IUpdatePreviewDiffJobSource,55 IMergeProposalCreatedJobSource, IReviewRequestedEmailJob,
56 IReviewRequestedEmailJobSource, IUpdatePreviewDiffJobSource,
55 )57 )
58from lp.code.mail.branch import RecipientReason
56from lp.code.mail.branchmergeproposal import BMPMailer59from lp.code.mail.branchmergeproposal import BMPMailer
57from lp.code.mail.codereviewcomment import CodeReviewCommentMailer60from lp.code.mail.codereviewcomment import CodeReviewCommentMailer
58from lp.code.model.branchmergeproposal import BranchMergeProposal61from lp.code.model.branchmergeproposal import BranchMergeProposal
59from lp.code.model.diff import PreviewDiff62from lp.code.model.diff import PreviewDiff
60from lp.codehosting.vfs import get_multi_server, get_scanner_server63from lp.codehosting.vfs import get_multi_server, get_scanner_server
64from lp.registry.interfaces.person import IPersonSet
61from lp.services.job.model.job import Job65from lp.services.job.model.job import Job
62from lp.services.job.interfaces.job import IRunnableJob66from lp.services.job.interfaces.job import IRunnableJob
63from lp.services.job.runner import BaseRunnableJob67from lp.services.job.runner import BaseRunnableJob
@@ -86,6 +90,13 @@
86 reviewers.90 reviewers.
87 """)91 """)
8892
93 REVIEW_REQUEST_EMAIL = DBItem(3, """
94 Send the review request email to the requested reviewer.
95
96 This job sends an email to the requested reviewer, or members of the
97 requested reviewer team asking them to review the proposal.
98 """)
99
89100
90class BranchMergeProposalJob(Storm):101class BranchMergeProposalJob(Storm):
91 """Base class for jobs related to branch merge proposals."""102 """Base class for jobs related to branch merge proposals."""
@@ -418,3 +429,65 @@
418 """Return a list of email-ids to notify about user errors."""429 """Return a list of email-ids to notify about user errors."""
419 commenter = self.code_review_comment.message.owner430 commenter = self.code_review_comment.message.owner
420 return [commenter.preferredemail]431 return [commenter.preferredemail]
432
433
434class ReviewRequestedEmailJob(BranchMergeProposalJobDerived):
435 """Send email to the reviewer telling them to review the proposal.
436
437 Provides class methods to create and retrieve such jobs.
438 """
439
440 implements(IReviewRequestedEmailJob)
441
442 classProvides(IReviewRequestedEmailJobSource)
443
444 class_job_type = BranchMergeProposalJobType.REVIEW_REQUEST_EMAIL
445
446 def run(self):
447 """See `IRunnableJob`."""
448 reason = RecipientReason.forReviewer(
449 self.branch_merge_proposal, True, self.reviewer)
450 mailer = BMPMailer.forReviewRequest(
451 reason, self.branch_merge_proposal, self.requester)
452 mailer.sendAll()
453
454 @classmethod
455 def create(cls, review_request):
456 """See `IReviewRequestedEmailJobSource`."""
457 metadata = cls.getMetadata(review_request)
458 bmp = review_request.branch_merge_proposal
459 job = BranchMergeProposalJob(bmp, cls.class_job_type, metadata)
460 return cls(job)
461
462 @staticmethod
463 def getMetadata(review_request):
464 return {
465 'reviewer': review_request.reviewer.name,
466 'requester': review_request.registrant.name,
467 }
468
469 @property
470 def reviewer(self):
471 """The person or team who has been asked to review."""
472 return getUtility(IPersonSet).getByName(self.metadata['reviewer'])
473
474 @property
475 def requester(self):
476 """The person who requested the review to be done."""
477 return getUtility(IPersonSet).getByName(self.metadata['requester'])
478
479 def getOopsVars(self):
480 """See `IRunnableJob`."""
481 vars = BranchMergeProposalJobDerived.getOopsVars(self)
482 vars.extend([
483 ('reviewer', self.metadata['reviewer']),
484 ('requester', self.metadata['requester']),
485 ])
486 return vars
487
488 def getErrorRecipients(self):
489 """Return a list of email-ids to notify about user errors."""
490 recipients = []
491 if self.requester is not None:
492 recipients.append(self.requester.preferredemail)
493 return recipients