launchpad-reviewers team mailing list archive

[Tim Penhey <tim.penhey@xxxxxxxxxxxxx>]

Tim Penhey has proposed merging lp:~thumper/launchpad/needs-review-event into lp:launchpad/devel with lp:~thumper/launchpad/rename-created-job as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


This branch adds a new event for the needs review email.  No functionality change at all, and the behaviour is exactly the same for now.

test_branchmergeproposal
-- 
https://code.launchpad.net/~thumper/launchpad/needs-review-event/+merge/38686
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/needs-review-event into lp:launchpad/devel.

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2010-10-18 03:42:46 +0000
+++ lib/lp/code/configure.zcml	2010-10-18 03:42:47 +0000
@@ -353,6 +353,10 @@
       handler="lp.code.subscribers.branchmergeproposal.merge_proposal_created"/>
   <subscriber
       for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal
+           lp.code.interfaces.event.IBranchMergeProposalNeedsReviewEvent"
+      handler="lp.code.subscribers.branchmergeproposal.merge_proposal_needs_review"/>
+  <subscriber
+      for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal
            lazr.lifecycle.interfaces.IObjectModifiedEvent"
       handler="lp.code.subscribers.branchmergeproposal.merge_proposal_modified"/>
   <subscriber

=== modified file 'lib/lp/code/event/branchmergeproposal.py'
--- lib/lp/code/event/branchmergeproposal.py	2010-03-17 07:54:15 +0000
+++ lib/lp/code/event/branchmergeproposal.py	2010-10-18 03:42:47 +0000
@@ -7,6 +7,7 @@
 
 __metaclass__ = type
 __all__ = [
+    'BranchMergeProposalNeedsReviewEvent',
     'BranchMergeProposalStatusChangeEvent',
     'NewBranchMergeProposalEvent',
     'NewCodeReviewCommentEvent',
@@ -17,6 +18,7 @@
 from zope.interface import implements
 
 from lp.code.interfaces.event import (
+    IBranchMergeProposalNeedsReviewEvent,
     IBranchMergeProposalStatusChangeEvent,
     INewBranchMergeProposalEvent,
     INewCodeReviewCommentEvent,
@@ -41,6 +43,11 @@
     implements(INewBranchMergeProposalEvent)
 
 
+class BranchMergeProposalNeedsReviewEvent(ObjectEvent):
+    """The merge proposal has moved from work in progress to needs reivew."""
+    implements(IBranchMergeProposalNeedsReviewEvent)
+
+
 class ReviewerNominatedEvent(ObjectEvent):
     """A reviewer has been nominated."""
     implements(IReviewerNominatedEvent)

=== modified file 'lib/lp/code/interfaces/event.py'
--- lib/lp/code/interfaces/event.py	2010-03-17 07:54:15 +0000
+++ lib/lp/code/interfaces/event.py	2010-10-18 03:42:47 +0000
@@ -6,6 +6,7 @@
 __metaclass__ = type
 __all__ = [
     'IBranchMergeProposalStatusChangeEvent',
+    'IBranchMergeProposalNeedsReviewEvent',
     'INewBranchMergeProposalEvent',
     'INewCodeReviewCommentEvent',
     'IReviewerNominatedEvent',
@@ -34,3 +35,7 @@
 
 class INewCodeReviewCommentEvent(IObjectEvent):
     """A new comment has been added to the merge proposal."""
+
+
+class IBranchMergeProposalNeedsReviewEvent(IObjectEvent):
+    """The merge proposal has moved from work in progress to needs reivew."""

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2010-10-07 22:46:08 +0000
+++ lib/lp/code/model/branch.py	2010-10-18 03:42:47 +0000
@@ -83,7 +83,10 @@
     CannotDeleteBranch,
     InvalidBranchMergeProposal,
     )
-from lp.code.event.branchmergeproposal import NewBranchMergeProposalEvent
+from lp.code.event.branchmergeproposal import (
+    BranchMergeProposalNeedsReviewEvent,
+    NewBranchMergeProposalEvent,
+    )
 from lp.code.interfaces.branch import (
     BzrIdentityMixin,
     DEFAULT_BRANCH_STATUS_IN_LISTING,
@@ -434,6 +437,8 @@
                 reviewer, registrant, review_type, _notify_listeners=False)
 
         notify(NewBranchMergeProposalEvent(bmp))
+        notify(BranchMergeProposalNeedsReviewEvent(bmp))
+
         return bmp
 
     def _createMergeProposal(

=== modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py'
--- lib/lp/code/model/tests/test_branchmergeproposal.py	2010-10-18 03:42:46 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposal.py	2010-10-18 03:42:47 +0000
@@ -50,6 +50,7 @@
     WrongBranchMergeProposal,
     )
 from lp.code.event.branchmergeproposal import (
+    BranchMergeProposalNeedsReviewEvent,
     NewBranchMergeProposalEvent,
     NewCodeReviewCommentEvent,
     ReviewerNominatedEvent,
@@ -645,12 +646,12 @@
         merge_proposal = self.factory.makeBranchMergeProposal()
         commenter = self.factory.makePerson()
         login_person(commenter)
-        result, event = self.assertNotifies(
+        result, events = self.assertNotifies(
             NewCodeReviewCommentEvent,
             merge_proposal.createComment,
             owner=commenter,
             subject='A review.')
-        self.assertEqual(result, event.object)
+        self.assertEqual(result, events[0].object)
 
     def test_notify_on_nominate_suppressed_if_requested(self):
         # Ensure that the notification is supressed if the notify listeners
@@ -758,10 +759,11 @@
         target_branch = self.factory.makeProductBranch(
             product=source_branch.product)
         registrant = self.factory.makePerson()
-        result, event = self.assertNotifies(
-            NewBranchMergeProposalEvent,
+        result, events = self.assertNotifies(
+            [NewBranchMergeProposalEvent,
+             BranchMergeProposalNeedsReviewEvent],
             source_branch.addLandingTarget, registrant, target_branch)
-        self.assertEqual(result, event.object)
+        self.assertEqual(result, events[0].object)
 
     def test_getNotificationRecipients(self):
         """Ensure that recipients can be added/removed with subscribe"""
@@ -1333,12 +1335,12 @@
         merge_proposal = self.factory.makeBranchMergeProposal()
         login_person(merge_proposal.source_branch.owner)
         reviewer = self.factory.makePerson()
-        result, event = self.assertNotifies(
+        result, events = self.assertNotifies(
             ReviewerNominatedEvent,
             merge_proposal.nominateReviewer,
             reviewer=reviewer,
             registrant=merge_proposal.source_branch.owner)
-        self.assertEqual(result, event.object)
+        self.assertEqual(result, events[0].object)
 
     def test_notify_on_nominate_suppressed_if_requested(self):
         # Ensure that a notification is suppressed if notify listeners is set

=== modified file 'lib/lp/code/model/tests/test_codereviewcomment.py'
--- lib/lp/code/model/tests/test_codereviewcomment.py	2010-10-04 19:50:45 +0000
+++ lib/lp/code/model/tests/test_codereviewcomment.py	2010-10-18 03:42:47 +0000
@@ -83,7 +83,7 @@
         self.assertEqual('Re: Message subject', reply.message.subject)
 
     def test_createNoParentComment(self):
-        comment = self.bmp.createComment(
+        self.bmp.createComment(
             self.submitter, 'Message subject', 'Message content')
         new_comment = self.bmp.createComment(
             self.reviewer, 'New subject', 'New content',

=== modified file 'lib/lp/code/subscribers/branchmergeproposal.py'
--- lib/lp/code/subscribers/branchmergeproposal.py	2010-10-18 03:42:46 +0000
+++ lib/lp/code/subscribers/branchmergeproposal.py	2010-10-18 03:42:47 +0000
@@ -27,6 +27,14 @@
     Also create a job to email the subscribers about the new proposal.
     """
     getUtility(IUpdatePreviewDiffJobSource).create(merge_proposal)
+
+
+def merge_proposal_needs_review(merge_proposal, event):
+    """A new merge proposal needs a review.
+
+    This event is raised when the proposal moves from work in progress to
+    needs review.
+    """
     getUtility(IMergeProposalNeedsReviewEmailJobSource).create(
         merge_proposal)
 

=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py	2010-10-17 21:21:07 +0000
+++ lib/lp/testing/__init__.py	2010-10-18 03:42:47 +0000
@@ -357,27 +357,26 @@
             verifyClass(interface, cls),
             "%r does not correctly implement %r." % (cls, interface))
 
-    def assertNotifies(self, event_type, callable_obj, *args, **kwargs):
+    def assertNotifies(self, event_types, callable_obj, *args, **kwargs):
         """Assert that a callable performs a given notification.
 
-        :param event_type: The type of event that notification is expected
-            for.
+        :param event_type: One or more event types that notification is
+            expected for.
         :param callable_obj: The callable to call.
         :param *args: The arguments to pass to the callable.
         :param **kwargs: The keyword arguments to pass to the callable.
         :return: (result, event), where result was the return value of the
             callable, and event is the event emitted by the callable.
         """
+        if not isinstance(event_types, (list, tuple)):
+            event_types = [event_types]
         result, events = capture_events(callable_obj, *args, **kwargs)
         if len(events) == 0:
             raise AssertionError('No notification was performed.')
-        elif len(events) > 1:
-            raise AssertionError('Too many (%d) notifications performed.'
-                % len(events))
-        elif not isinstance(events[0], event_type):
-            raise AssertionError('Wrong event type: %r (expected %r).' %
-                (events[0], event_type))
-        return result, events[0]
+        self.assertEqual(len(event_types), len(events))
+        for index, expected in enumerate(event_types):
+            self.assertIsInstance(events[index], expected)
+        return result, events
 
     def assertNoNotification(self, callable_obj, *args, **kwargs):
         """Assert that no notifications are generated by the callable.