← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~abentley/launchpad/fix-expirable-bugs into lp:launchpad

 

Aaron Bentley has proposed merging lp:~abentley/launchpad/fix-expirable-bugs into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #900398 in Launchpad itself: "KeyError: 'mustache_model' in +expirable-bugs page"
  https://bugs.launchpad.net/launchpad/+bug/900398

For more details, see:
https://code.launchpad.net/~abentley/launchpad/fix-expirable-bugs/+merge/85181

= Summary =
Fix bug #900398: KeyError: 'mustache_model' in +expirable-bugs page

== Proposed fix ==
Make BugTaskExpirableListingView inherit from BugTaskSearchListingView

== Pre-implementation notes ==
Discussed with deryck

== Implementation details ==
In addition to fixing the bug, this branch also makes +expirable-bugs useful again.  Previously, it had listed those bugs which were past the expiry date but not yet expired.  This is not very useful, since bugs past the expiry date will be expired by a script within 24 hours and disappear from the listing.  It also contradicts the text, and made local testing hard.


== Tests ==
bin/test -t BugTaskExpirableListingView -t test_bugtask -t xx-incomplete-bugs.txt

== Demo and Q/A ==
Ensure you are a member of custom-buglisting-demo

Go to https://bugs.qastaging.launchpad.net/bzr and ensure there's at least one incomplete bug.  Click "Incomplete bugs".  You should see a list of incomplete bugs in the new style.  Log out.  You should see a list of incomplete bugs in the old style.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/browser/tests/test_bugtask.py
  lib/lp/bugs/browser/bugtask.py
  lib/lp/bugs/stories/bugs/xx-incomplete-bugs.txt

./lib/lp/bugs/stories/bugs/xx-incomplete-bugs.txt
       1: narrative uses a moin header.
      81: narrative uses a moin header.
     129: narrative uses a moin header.
     221: narrative uses a moin header.
     254: narrative uses a moin header.
     284: want exceeds 78 characters.
-- 
https://code.launchpad.net/~abentley/launchpad/fix-expirable-bugs/+merge/85181
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/fix-expirable-bugs into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-12-09 09:23:38 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-12-09 19:28:28 +0000
@@ -2067,11 +2067,9 @@
         The bugtarget may be an `IDistribution`, `IDistroSeries`, `IProduct`,
         or `IProductSeries`.
         """
-        days_old = config.malone.days_before_expiration
-
         if target_has_expirable_bugs_listing(self.context):
             return getUtility(IBugTaskSet).findExpirableBugTasks(
-                days_old, user=self.user, target=self.context).count()
+                0, user=self.user, target=self.context).count()
         else:
             return None
 
@@ -4305,7 +4303,7 @@
     page_title = label
 
 
-class BugTaskExpirableListingView(LaunchpadView):
+class BugTaskExpirableListingView(BugTaskSearchListingView):
     """View for listing Incomplete bugs that can expire."""
 
     @property
@@ -4328,13 +4326,11 @@
         else:
             return ['id', 'summary', 'date_last_updated', 'heat']
 
-    @property
     def search(self):
         """Return an `ITableBatchNavigator` for the expirable bugtasks."""
-        days_old = config.malone.days_before_expiration
         bugtaskset = getUtility(IBugTaskSet)
         bugtasks = bugtaskset.findExpirableBugTasks(
-            days_old, user=self.user, target=self.context)
+            user=self.user, target=self.context, min_days_old=0)
         return BugListingBatchNavigator(
             bugtasks, self.request, columns_to_show=self.columns_to_show,
             size=config.malone.buglist_batch_size)

=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py	2011-12-08 13:23:00 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py	2011-12-09 19:28:28 +0000
@@ -1842,6 +1842,14 @@
         target_context=bugtask.target)
 
 
+@contextmanager
+def dynamic_listings():
+    """Context manager to enable new bug listings."""
+    with feature_flags():
+        set_feature_flag(u'bugs.dynamic_bug_listings.enabled', u'on')
+        yield
+
+
 class TestBugTaskSearchListingView(BrowserTestCase):
 
     layer = DatabaseFunctionalLayer
@@ -1881,13 +1889,6 @@
         view.initialize()
         return view
 
-    @contextmanager
-    def dynamic_listings(self):
-        """Context manager to enable new bug listings."""
-        with feature_flags():
-            set_feature_flag(u'bugs.dynamic_bug_listings.enabled', u'on')
-            yield
-
     def test_mustache_model_missing_if_no_flag(self):
         """The IJSONRequestCache should contain mustache_model."""
         view = self.makeView()
@@ -1902,7 +1903,7 @@
         """
         owner, item = make_bug_task_listing_item(self.factory)
         self.useContext(person_logged_in(owner))
-        with self.dynamic_listings():
+        with dynamic_listings():
             view = self.makeView(item.bugtask)
         cache = IJSONRequestCache(view.request)
         bugtasks = cache.objects['mustache_model']['bugtasks']
@@ -1919,7 +1920,7 @@
         """
         owner, item = make_bug_task_listing_item(self.factory)
         self.useContext(person_logged_in(owner))
-        with self.dynamic_listings():
+        with dynamic_listings():
             view = self.makeView(item.bugtask)
         cache = IJSONRequestCache(view.request)
         self.assertIs(None, cache.objects.get('next'))
@@ -1933,7 +1934,7 @@
         """
         task = self.factory.makeBugTask()
         self.factory.makeBugTask(target=task.target)
-        with self.dynamic_listings():
+        with dynamic_listings():
             view = self.makeView(task, size=1)
         cache = IJSONRequestCache(view.request)
         self.assertEqual({'memo': '1', 'start': 1}, cache.objects.get('next'))
@@ -1946,14 +1947,14 @@
         """
         task = self.factory.makeBugTask()
         task2 = self.factory.makeBugTask(target=task.target)
-        with self.dynamic_listings():
+        with dynamic_listings():
             view = self.makeView(task2, size=1, memo=1)
         cache = IJSONRequestCache(view.request)
         self.assertEqual({'memo': '1', 'start': 0}, cache.objects.get('prev'))
 
     def test_provides_view_name(self):
         """The IJSONRequestCache should provide the view's name."""
-        self.useContext(self.dynamic_listings())
+        self.useContext(dynamic_listings())
         view = self.makeView()
         cache = IJSONRequestCache(view.request)
         self.assertEqual('+bugs', cache.objects['view_name'])
@@ -1967,7 +1968,7 @@
     def test_default_order_by(self):
         """order_by defaults to '-importance in JSONRequestCache"""
         task = self.factory.makeBugTask()
-        with self.dynamic_listings():
+        with dynamic_listings():
             view = self.makeView(task)
         cache = IJSONRequestCache(view.request)
         self.assertEqual('-importance', cache.objects['order_by'])
@@ -1975,7 +1976,7 @@
     def test_order_by_importance(self):
         """order_by follows query params in JSONRequestCache"""
         task = self.factory.makeBugTask()
-        with self.dynamic_listings():
+        with dynamic_listings():
             view = self.makeView(task, orderby='importance')
         cache = IJSONRequestCache(view.request)
         self.assertEqual('importance', cache.objects['order_by'])
@@ -1986,7 +1987,7 @@
         order_by, memo, start, forwards.  These default to sane values.
         """
         task = self.factory.makeBugTask()
-        with self.dynamic_listings():
+        with dynamic_listings():
             view = self.makeView(task)
         cache = IJSONRequestCache(view.request)
         self.assertEqual('-importance', cache.objects['order_by'])
@@ -2001,7 +2002,7 @@
         order_by, memo, start, forwards.  These are calculated appropriately.
         """
         task = self.factory.makeBugTask()
-        with self.dynamic_listings():
+        with dynamic_listings():
             view = self.makeView(task, memo=1, forwards=False, size=1)
         cache = IJSONRequestCache(view.request)
         self.assertEqual('1', cache.objects['memo'])
@@ -2012,7 +2013,7 @@
     def test_cache_field_visibility(self):
         """Cache contains sane-looking field_visibility values."""
         task = self.factory.makeBugTask()
-        with self.dynamic_listings():
+        with dynamic_listings():
             view = self.makeView(task, memo=1, forwards=False, size=1)
         cache = IJSONRequestCache(view.request)
         field_visibility = cache.objects['field_visibility']
@@ -2021,7 +2022,7 @@
     def test_cache_cookie_name(self):
         """The cookie name should be in cache for js code access."""
         task = self.factory.makeBugTask()
-        with self.dynamic_listings():
+        with dynamic_listings():
             view = self.makeView(task, memo=1, forwards=False, size=1)
         cache = IJSONRequestCache(view.request)
         cookie_name = cache.objects['cbl_cookie_name']
@@ -2036,7 +2037,7 @@
             '&show_milestone_name=true&show_date_last_updated=true'
             '&show_assignee=true&show_heat=true&show_tag=true'
             '&show_importance=true&show_status=true')
-        with self.dynamic_listings():
+        with dynamic_listings():
             view = self.makeView(
                 task, memo=1, forwards=False, size=1, cookie=cookie)
         cache = IJSONRequestCache(view.request)
@@ -2052,7 +2053,7 @@
             '&show_milestone_name=true&show_date_last_updated=true'
             '&show_assignee=true&show_heat=true&show_tag=true'
             '&show_importance=true&show_status=true&show_title=true')
-        with self.dynamic_listings():
+        with dynamic_listings():
             view = self.makeView(
                 task, memo=1, forwards=False, size=1, cookie=cookie)
         cache = IJSONRequestCache(view.request)
@@ -2068,7 +2069,7 @@
             '&show_milestone_name=true&show_date_last_updated=true'
             '&show_assignee=true&show_heat=true&show_tag=true'
             '&show_importance=true&show_title=true')
-        with self.dynamic_listings():
+        with dynamic_listings():
             view = self.makeView(
                 task, memo=1, forwards=False, size=1, cookie=cookie)
         cache = IJSONRequestCache(view.request)
@@ -2078,7 +2079,7 @@
     def test_cache_field_visibility_defaults(self):
         """Cache contains sane-looking field_visibility_defaults values."""
         task = self.factory.makeBugTask()
-        with self.dynamic_listings():
+        with dynamic_listings():
             view = self.makeView(task, memo=1, forwards=False, size=1)
         cache = IJSONRequestCache(view.request)
         field_visibility_defaults = cache.objects['field_visibility_defaults']
@@ -2117,14 +2118,14 @@
 
     def test_mustache_rendering(self):
         """If the flag is present, then all mustache features appear."""
-        with self.dynamic_listings():
+        with dynamic_listings():
             bug_task, browser = self.getBugtaskBrowser()
         bug_number = self.getBugNumberTag(bug_task)
         self.assertHTML(browser, self.client_listing, bug_number)
 
     def test_mustache_rendering_obfuscation(self):
         """For anonymous users, email addresses are obfuscated."""
-        with self.dynamic_listings():
+        with dynamic_listings():
             bug_task, browser = self.getBugtaskBrowser(title='a@xxxxxxxxxxx',
                 no_login=True)
         self.assertNotIn('a@xxxxxxxxxxx', browser.contents)
@@ -2246,6 +2247,25 @@
         self.assertIn('Last updated updated1', navigator.mustache)
 
 
+class TestBugTaskExpirableListingView(BrowserTestCase):
+    """Test BugTaskExpirableListingView."""
+
+    layer = LaunchpadFunctionalLayer
+
+    def test_dynamic_bugs_expirable(self):
+        """With dyamic listings enabled, expirable bugs listing works."""
+        product = self.factory.makeProduct(official_malone=True)
+        with person_logged_in(product.owner):
+            product.enable_bug_expiration = True
+        bug = self.factory.makeBug(
+            product=product, status=BugTaskStatus.INCOMPLETE)
+        title = bug.title
+        with dynamic_listings():
+            content = self.getMainContent(
+                bug.default_bugtask.target, "+expirable-bugs")
+            self.assertIn(title, str(content))
+
+
 class TestBugListingBatchNavigator(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer

=== modified file 'lib/lp/bugs/stories/bugs/xx-incomplete-bugs.txt'
--- lib/lp/bugs/stories/bugs/xx-incomplete-bugs.txt	2011-11-25 16:51:29 +0000
+++ lib/lp/bugs/stories/bugs/xx-incomplete-bugs.txt	2011-12-09 19:28:28 +0000
@@ -130,18 +130,11 @@
 
 Users can view a list of expirable bugs via a link on the project's
 bug page. To see the behaviour of the bug listing, we need another
-expirable bug. No Privileges Person marks another bug as Incomplete 
-and does so some time ago so it appears as expirable.
+expirable bug. No Privileges Person marks another bug as Incomplete
 
     >>> user_browser.open('http://bugs.launchpad.dev/jokosher/+bug/12')
     >>> user_browser.getControl('Status').value = ['Incomplete']
     >>> user_browser.getControl('Save Changes', index=0).click()
-    >>> login('test@xxxxxxxxxxxxx')
-    >>> bug_12 = getUtility(IBugSet).get(12)
-    >>> time_delta = timedelta(days=60)
-    >>> bug_12.date_last_updated = bug_12.date_last_updated - time_delta
-    >>> flush_database_updates()
-    >>> logout()
 
 The project's bug page reports the number of bugs that will expire if
 they are not confirmed. No Privileges Person sees that Jokosher has 2
@@ -187,15 +180,12 @@
 
 The listing is sorted in order of most inactive to least inactive. The
 bugs at the top of the list will expire before the ones at the bottom.
-When bug 12's date_last_updated is modified to be older than bug 11's it
-appears as more inactive than bug 11 and is pushed to the top of the list. 
+When No Privileges Person adds a comment to the oldest bug, it is
+pushed to the bottom of the list.
 
-    >>> login('test@xxxxxxxxxxxxx')
-    >>> bug_12 = getUtility(IBugSet).get(12)
-    >>> time_delta = timedelta(days=2)
-    >>> bug_12.date_last_updated = bug_12.date_last_updated - time_delta
-    >>> flush_database_updates()
-    >>> logout()
+    >>> user_browser.getLink('Make Jokosher use autoaudiosink').click()
+    >>> user_browser.getControl(name='field.comment').value = "bump"
+    >>> user_browser.getControl('Post Comment').click()
     >>> user_browser.getLink('Bugs').click()
     >>> user_browser.getLink('Incomplete bugs').click()
     >>> contents = find_main_content(user_browser.contents)