launchpad-reviewers team mailing list archive

[Martin Packman <martin.packman@xxxxxxxxxxxxx>]

Martin Packman has proposed merging lp:~gz/launchpad/root_blog_feature_flag_939055 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #939055 in Launchpad itself: "front page blog feed updates are done in-line with requests"
  https://bugs.launchpad.net/launchpad/+bug/939055

For more details, see:
https://code.launchpad.net/~gz/launchpad/root_blog_feature_flag_939055/+merge/119399

Only display posts from the launchpad blog on the main page when the new app.root_blog.enabled feature is set. The aim is to easy the datacentre migration by having the option of toggling off the lookup when blog.launchpad.net needs to taken down.

This is something of an abuse of the feature flags... feature, but Robert suggested it as the easiest method in the bug.

To prevent gaping whitespace in the left column, the logic for whether or not to display the launchpad introduction is adapted from always being off for logged in users to being on regardless if the blog isn't being shown.

The tests added are not exhaustive as they're expensive at this level, but should cover the details we care about.

= LOC =

I have some credit to burn...


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/browser/root.py
  lib/lp/app/browser/tests/test_launchpadroot.py
  lib/lp/app/templates/root-index.pt
  lib/lp/services/features/flags.py

-- 
https://code.launchpad.net/~gz/launchpad/root_blog_feature_flag_939055/+merge/119399
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gz/launchpad/root_blog_feature_flag_939055 into lp:launchpad.

=== modified file 'lib/lp/app/browser/root.py'
--- lib/lp/app/browser/root.py	2012-03-23 20:56:11 +0000
+++ lib/lp/app/browser/root.py	2012-08-13 17:49:26 +0000
@@ -43,6 +43,7 @@
     GoogleResponseError,
     ISearchService,
     )
+from lp.services.features import getFeatureFlag
 from lp.services.propertycache import cachedproperty
 from lp.services.statistics.interfaces.statistic import ILaunchpadStatisticSet
 from lp.services.timeout import urlfetch
@@ -136,6 +137,11 @@
         """The total blueprint count in all of Launchpad."""
         return getUtility(ILaunchpadStatisticSet).value('question_count')
 
+    @property
+    def show_whatslaunchpad(self):
+        """True if introduction to Launchpad should be displayed."""
+        return self.user is None or not getFeatureFlag("app.root_blog.enabled")
+
     def getRecentBlogPosts(self):
         """Return the parsed feed of the most recent blog posts.
 

=== modified file 'lib/lp/app/browser/tests/test_launchpadroot.py'
--- lib/lp/app/browser/tests/test_launchpadroot.py	2012-06-04 16:13:51 +0000
+++ lib/lp/app/browser/tests/test_launchpadroot.py	2012-08-13 17:49:26 +0000
@@ -16,9 +16,11 @@
 
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.registry.interfaces.person import IPersonSet
+from lp.services.features.testing import FeatureFixture
 from lp.services.webapp.authorization import check_permission
 from lp.services.webapp.interfaces import ILaunchpadRoot
 from lp.testing import (
+    anonymous_logged_in,
     login_person,
     TestCaseWithFactory,
     )
@@ -138,3 +140,65 @@
         logo = markup.find(True, id='launchpad-logo-and-name')
         self.assertIsNot(None, logo)
         self.assertEqual('/@@/launchpad-logo-and-name.png', logo['src'])
+
+    @staticmethod
+    def _make_blog_post(linkid, title, body, date):
+        return {
+            'title': title,
+            'description': body,
+            'link': "http://blog.invalid/%s"; % (linkid,),
+            'date': date,
+            }
+
+    def test_blog_posts(self):
+        """Posts from the launchpad blog are shown when feature is enabled"""
+        self.useFixture(FeatureFixture({'app.root_blog.enabled': True}))
+        posts = [
+            self._make_blog_post(1, "A post", "Post contents.", "2002"),
+            self._make_blog_post(2, "Another post", "More contents.", "2003"),
+            ]
+        calls = []
+
+        def _get_blog_posts():
+            calls.append('called')
+            return posts
+
+        root = getUtility(ILaunchpadRoot)
+        with anonymous_logged_in() as user:
+            view = create_initialized_view(root, 'index.html', principle=user)
+            view.getRecentBlogPosts = _get_blog_posts
+            result = view()
+        markup = BeautifulSoup(result,
+            parseOnlyThese=SoupStrainer(id='homepage-blogposts'))
+        self.assertEqual(['called'], calls)
+        items = markup.findAll('li', 'news')
+        # Notice about launchpad being opened is always added at the end
+        self.assertEqual(3, len(items))
+        a = items[-1].find("a")
+        self.assertEqual("Launchpad now open source", a.string.strip())
+        for post, item in zip(posts, items):
+            a = item.find("a")
+            self.assertEqual(post['link'], a["href"])
+            self.assertEqual(post['title'], a.string)
+
+    def test_blog_disabled(self):
+        """Launchpad blog not queried for display without feature"""
+        calls = []
+
+        def _get_blog_posts():
+            calls.append('called')
+            return []
+
+        root = getUtility(ILaunchpadRoot)
+        user = self.factory.makePerson()
+        login_person(user)
+        view = create_initialized_view(root, 'index.html', principal=user)
+        view.getRecentBlogPosts = _get_blog_posts
+        markup = BeautifulSoup(
+            view(), parseOnlyThese=SoupStrainer(id='homepage'))
+        self.assertEqual([], calls)
+        self.assertIs(None, markup.find(True, id='homepage-blogposts'))
+        # Even logged in users should get the launchpad intro text in the left
+        # column rather than blank space when the blog is not being displayed.
+        self.assertTrue(view.show_whatslaunchpad)
+        self.assertTrue(markup.find(True, 'homepage-whatslaunchpad'))

=== modified file 'lib/lp/app/templates/root-index.pt'
--- lib/lp/app/templates/root-index.pt	2012-06-11 00:47:38 +0000
+++ lib/lp/app/templates/root-index.pt	2012-08-13 17:49:26 +0000
@@ -67,7 +67,7 @@
         <div class="yui-g">
           <div class="yui-u first" style="margin-top: 1.5em;">
             <div class="homepage-whatslaunchpad"
-                 tal:condition="not:view/user">
+                 tal:condition="view/show_whatslaunchpad">
               <h2><span class="launchpad-gold">Launchpad</span> is a software collaboration platform that provides:</h2>
               <ul tal:define="apphomes view/apphomes">
               <li>
@@ -109,7 +109,8 @@
                </div>
             </div>
 
-            <div id="homepage-blogposts" class="homepage-portlet">
+            <div id="homepage-blogposts" class="homepage-portlet"
+                 tal:condition="features/app.root_blog.enabled">
               <h2>Recent Launchpad blog posts</h2>
               <ul tal:define="posts view/getRecentBlogPosts">
                 <li class="news"

=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	2012-08-10 06:38:56 +0000
+++ lib/lp/services/features/flags.py	2012-08-13 17:49:26 +0000
@@ -287,6 +287,12 @@
      '',
      '',
      ''),
+    ('app.root_blog.enabled',
+     'boolean',
+     'If true, load posts from the launchpad for display on the root page.',
+     '',
+     '',
+     ''),
     ])
 
 # The set of all flag names that are documented.