launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-754089 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #754089 in Launchpad itself: "launchpad only remembers alternate landing targets in merge proposal creation page for one day"
  https://bugs.launchpad.net/launchpad/+bug/754089

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-754089/+merge/61070

https://code.launchpad.net/~abentley/launchpad/stale-targets/+merge/45418 intended to ignore proposals older than 90 days when suggesting branch merge targets. But it actually ignored anything older than a single day.

This branch fixes that, and addresses two test deficiencies: the request user was not set, making the vocabulary always empty; and there was no test for branches less than 90 days old.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-754089/+merge/61070
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-754089 into lp:launchpad.

=== modified file 'lib/lp/app/widgets/suggestion.py'
--- lib/lp/app/widgets/suggestion.py	2011-03-03 01:13:47 +0000
+++ lib/lp/app/widgets/suggestion.py	2011-05-16 07:31:49 +0000
@@ -227,7 +227,7 @@
         """
         default_target = branch.target.default_merge_target
         logged_in_user = getUtility(ILaunchBag).user
-        since = datetime.now(utc) - timedelta(days=1)
+        since = datetime.now(utc) - timedelta(days=90)
         collection = branch.target.collection.targetedBy(logged_in_user,
             since)
         collection = collection.visibleByUser(logged_in_user)

=== modified file 'lib/lp/app/widgets/tests/test_suggestion.py'
--- lib/lp/app/widgets/tests/test_suggestion.py	2011-04-18 02:30:08 +0000
+++ lib/lp/app/widgets/tests/test_suggestion.py	2011-05-16 07:31:49 +0000
@@ -32,6 +32,7 @@
     TargetBranchWidget,
     )
 from lp.testing import (
+    login_person,
     person_logged_in,
     TestCaseWithFactory,
     )
@@ -152,15 +153,24 @@
 
     layer = DatabaseFunctionalLayer
 
+    def makeBranchAndOldMergeProposal(self, timedelta):
+        """Make an old  merge proposal and a branch with the same target."""
+        bmp = self.factory.makeBranchMergeProposal(
+            date_created=datetime.now(utc) - timedelta)
+        login_person(bmp.registrant)
+        target = bmp.target_branch
+        return target, self.factory.makeBranchTargetBranch(target.target)
+
+    def test_recent_target(self):
+        """Targets for proposals newer than 90 days are included."""
+        target, source = self.makeBranchAndOldMergeProposal(
+            timedelta(days=89))
+        widget = make_target_branch_widget(source)
+        self.assertIn(target, widget.suggestion_vocab)
+
     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)
+        target, source = self.makeBranchAndOldMergeProposal(
+            timedelta(days=91))
+        widget = make_target_branch_widget(source)
         self.assertNotIn(target, widget.suggestion_vocab)