launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/stale-targets into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #541713 Show only recent merge targets
  https://bugs.launchpad.net/bugs/541713

For more details, see:
https://code.launchpad.net/~abentley/launchpad/stale-targets/+merge/45418

= Summary =
Fix bug #541713: Show only recent merge targets

== Proposed fix ==
Change the merge proposal target branch widget to suggest only branches
targeted for merging in the past 90 days

== Pre-implementation notes ==
None

== Implementation details ==
I believe this is our first unit test of a widget.  Fun!

== Tests ==
bin/test -t test_stale_target -t test_targetedBy_since

== Demo and Q/A ==
Since it's impossible to create a merge proposal dated 90+ days ago, this
cannot be tested.

= Launchpad lint =

I don't understand the complaint at test_suggestion.py line 50

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/widgets/ftests/test_suggestion.py
  lib/lp/code/model/branchcollection.py
  lib/lp/code/interfaces/branchcollection.py
  lib/lp/code/model/tests/test_branchcollection.py
  lib/canonical/widgets/suggestion.py

./lib/lp/code/interfaces/branchcollection.py
      50: E301 expected 1 blank line, found 2
-- 
https://code.launchpad.net/~abentley/launchpad/stale-targets/+merge/45418
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/stale-targets into lp:launchpad.

=== added file 'lib/canonical/widgets/ftests/test_suggestion.py'
--- lib/canonical/widgets/ftests/test_suggestion.py	1970-01-01 00:00:00 +0000
+++ lib/canonical/widgets/ftests/test_suggestion.py	2011-01-06 17:22:57 +0000
@@ -0,0 +1,48 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+
+from datetime import (
+    datetime,
+    timedelta,
+    )
+
+from pytz import utc
+from zope.schema import Choice
+from zope.security.proxy import removeSecurityProxy
+
+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
+from canonical.testing.layers import DatabaseFunctionalLayer
+from canonical.widgets.suggestion import TargetBranchWidget
+from lp.testing import (
+    person_logged_in,
+    TestCaseWithFactory,
+    )
+
+
+def make_target_branch_widget(branch):
+    """Given a branch, return a widget for selecting where to land it."""
+    choice = Choice(vocabulary='Branch').bind(branch)
+    request = LaunchpadTestRequest()
+    return TargetBranchWidget(choice, None, request)
+
+
+class TestTargetBranchWidget(TestCaseWithFactory):
+    """Test the TargetBranchWidget class."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_stale_target(self):
+        """Targets for proposals older than 90 days are not considered."""
+        bmp = self.factory.makeBranchMergeProposal()
+        target = bmp.target_branch
+        source = self.factory.makeBranchTargetBranch(target.target)
+        with person_logged_in(bmp.registrant):
+            widget = make_target_branch_widget(source)
+            self.assertIn(target, widget.suggestion_vocab)
+            stale_date = datetime.now(utc) - timedelta(days=91)
+            removeSecurityProxy(bmp).date_created = stale_date
+            widget = make_target_branch_widget(source)
+        self.assertNotIn(target, widget.suggestion_vocab)

=== modified file 'lib/canonical/widgets/suggestion.py'
--- lib/canonical/widgets/suggestion.py	2010-11-19 00:59:48 +0000
+++ lib/canonical/widgets/suggestion.py	2011-01-06 17:22:57 +0000
@@ -11,7 +11,12 @@
 
 
 import cgi
+from datetime import (
+    datetime,
+    timedelta,
+    )
 
+from pytz import utc
 from zope.app.form.browser.widget import renderElement
 from zope.app.form.interfaces import IInputWidget, InputErrors
 from zope.app.form.utility import setUpWidget
@@ -62,8 +67,7 @@
         suggestions = cls._getSuggestions(context)
         terms = [
             term for term in full_vocabulary
-            if term.value in suggestions
-            ]
+            if term.value in suggestions]
         return SimpleVocabulary(terms)
 
     def _shouldRenderSuggestions(self):
@@ -209,7 +213,9 @@
         """
         default_target = branch.target.default_merge_target
         logged_in_user = getUtility(ILaunchBag).user
-        collection = branch.target.collection.targetedBy(logged_in_user)
+        since = datetime.now(utc) - timedelta(days=1)
+        collection = branch.target.collection.targetedBy(logged_in_user,
+            since)
         collection = collection.visibleByUser(logged_in_user)
         branches = collection.getBranches().config(distinct=True)
         target_branches = list(branches.config(limit=5))

=== modified file 'lib/lp/code/interfaces/branchcollection.py'
--- lib/lp/code/interfaces/branchcollection.py	2009-10-05 03:19:15 +0000
+++ lib/lp/code/interfaces/branchcollection.py	2011-01-06 17:22:57 +0000
@@ -180,8 +180,10 @@
 
         A branch is targeted by a person if that person has registered a merge
         proposal with the branch as the target.
+
+        :param since: If supplied, ignore merge proposals before this date.
         """
 
 
 class IAllBranches(IBranchCollection):
-    """An `IBranchCollection` representing all branches in Launchpad."""
+    """A `IBranchCollection` representing all branches in Launchpad."""

=== modified file 'lib/lp/code/model/branchcollection.py'
--- lib/lp/code/model/branchcollection.py	2010-11-18 16:13:32 +0000
+++ lib/lp/code/model/branchcollection.py	2011-01-06 17:22:57 +0000
@@ -364,10 +364,13 @@
             join=Join(BranchSubscription,
                       BranchSubscription.branch == Branch.id))
 
-    def targetedBy(self, person):
+    def targetedBy(self, person, since=None):
         """See `IBranchCollection`."""
+        clauses = [BranchMergeProposal.registrant == person]
+        if since is not None:
+            clauses.append(BranchMergeProposal.date_created >= since)
         return self._filterBy(
-            [BranchMergeProposal.registrant == person],
+            clauses,
             table=BranchMergeProposal,
             join=Join(BranchMergeProposal,
                       BranchMergeProposal.target_branch == Branch.id))

=== modified file 'lib/lp/code/model/tests/test_branchcollection.py'
--- lib/lp/code/model/tests/test_branchcollection.py	2010-11-18 16:34:35 +0000
+++ lib/lp/code/model/tests/test_branchcollection.py	2011-01-06 17:22:57 +0000
@@ -465,6 +465,19 @@
         branches = self.all_branches.targetedBy(registrant)
         self.assertEqual([target_branch], list(branches.getBranches()))
 
+    def test_targetedBy_since(self):
+        # Ignore proposals created before 'since'.
+        all_branches = self.all_branches
+        bmp = self.factory.makeBranchMergeProposal()
+        date_created = self.factory.getUniqueDate()
+        removeSecurityProxy(bmp).date_created = date_created
+        registrant = bmp.registrant
+        branches = all_branches.targetedBy(registrant, since=date_created)
+        self.assertEqual([bmp.target_branch], list(branches.getBranches()))
+        since = self.factory.getUniqueDate()
+        branches = all_branches.targetedBy(registrant, since=since)
+        self.assertEqual([], list(branches.getBranches()))
+
 
 class TestGenericBranchCollectionVisibleFilter(TestCaseWithFactory):