Merge lp:~gz/launchpad/root_blog_feature_flag_939055 into lp:launchpad

Proposed by Martin Packman
Status: Merged
Approved by: Martin Packman
Approved revision: no longer in the source branch.
Merged at revision: 15805
Proposed branch: lp:~gz/launchpad/root_blog_feature_flag_939055
Merge into: lp:launchpad
Diff against target: 194 lines (+90/-8)
5 files modified
lib/lp/app/browser/root.py (+9/-0)
lib/lp/app/browser/tests/test_launchpadroot.py (+64/-0)
lib/lp/app/stories/launchpad-root/front-pages.txt (+4/-2)
lib/lp/app/templates/root-index.pt (+7/-6)
lib/lp/services/features/flags.py (+6/-0)
To merge this branch: bzr merge lp:~gz/launchpad/root_blog_feature_flag_939055
Reviewer Review Type Date Requested Status
Benji York (community) code Approve
Review via email: mp+119399@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

The branch looks good.

The boolean expression in show_whatslaunchpad is sufficiently complex
that a comment giving the prose translation (as well as the motivation
for including the "what is" section when the blog is disabled) would be
a good idea.

review: Approve (code)
Revision history for this message
Martin Packman (gz) wrote :

Added explanation to the docstring on that suggestion, also suppressed a few of the links shown when logged in that are duplicated from the intro if that's being displayed.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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-14 14:33:25 +0000
@@ -43,6 +43,7 @@
43 GoogleResponseError,43 GoogleResponseError,
44 ISearchService,44 ISearchService,
45 )45 )
46from lp.services.features import getFeatureFlag
46from lp.services.propertycache import cachedproperty47from lp.services.propertycache import cachedproperty
47from lp.services.statistics.interfaces.statistic import ILaunchpadStatisticSet48from lp.services.statistics.interfaces.statistic import ILaunchpadStatisticSet
48from lp.services.timeout import urlfetch49from lp.services.timeout import urlfetch
@@ -136,6 +137,14 @@
136 """The total blueprint count in all of Launchpad."""137 """The total blueprint count in all of Launchpad."""
137 return getUtility(ILaunchpadStatisticSet).value('question_count')138 return getUtility(ILaunchpadStatisticSet).value('question_count')
138139
140 @property
141 def show_whatslaunchpad(self):
142 """True if introduction to Launchpad should be displayed.
143
144 Shown when not logged in or if blog is disabled.
145 """
146 return self.user is None or not getFeatureFlag("app.root_blog.enabled")
147
139 def getRecentBlogPosts(self):148 def getRecentBlogPosts(self):
140 """Return the parsed feed of the most recent blog posts.149 """Return the parsed feed of the most recent blog posts.
141150
142151
=== 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-14 14:33:25 +0000
@@ -16,9 +16,11 @@
1616
17from lp.app.interfaces.launchpad import ILaunchpadCelebrities17from lp.app.interfaces.launchpad import ILaunchpadCelebrities
18from lp.registry.interfaces.person import IPersonSet18from lp.registry.interfaces.person import IPersonSet
19from lp.services.features.testing import FeatureFixture
19from lp.services.webapp.authorization import check_permission20from lp.services.webapp.authorization import check_permission
20from lp.services.webapp.interfaces import ILaunchpadRoot21from lp.services.webapp.interfaces import ILaunchpadRoot
21from lp.testing import (22from lp.testing import (
23 anonymous_logged_in,
22 login_person,24 login_person,
23 TestCaseWithFactory,25 TestCaseWithFactory,
24 )26 )
@@ -138,3 +140,65 @@
138 logo = markup.find(True, id='launchpad-logo-and-name')140 logo = markup.find(True, id='launchpad-logo-and-name')
139 self.assertIsNot(None, logo)141 self.assertIsNot(None, logo)
140 self.assertEqual('/@@/launchpad-logo-and-name.png', logo['src'])142 self.assertEqual('/@@/launchpad-logo-and-name.png', logo['src'])
143
144 @staticmethod
145 def _make_blog_post(linkid, title, body, date):
146 return {
147 'title': title,
148 'description': body,
149 'link': "http://blog.invalid/%s" % (linkid,),
150 'date': date,
151 }
152
153 def test_blog_posts(self):
154 """Posts from the launchpad blog are shown when feature is enabled"""
155 self.useFixture(FeatureFixture({'app.root_blog.enabled': True}))
156 posts = [
157 self._make_blog_post(1, "A post", "Post contents.", "2002"),
158 self._make_blog_post(2, "Another post", "More contents.", "2003"),
159 ]
160 calls = []
161
162 def _get_blog_posts():
163 calls.append('called')
164 return posts
165
166 root = getUtility(ILaunchpadRoot)
167 with anonymous_logged_in() as user:
168 view = create_initialized_view(root, 'index.html', principle=user)
169 view.getRecentBlogPosts = _get_blog_posts
170 result = view()
171 markup = BeautifulSoup(result,
172 parseOnlyThese=SoupStrainer(id='homepage-blogposts'))
173 self.assertEqual(['called'], calls)
174 items = markup.findAll('li', 'news')
175 # Notice about launchpad being opened is always added at the end
176 self.assertEqual(3, len(items))
177 a = items[-1].find("a")
178 self.assertEqual("Launchpad now open source", a.string.strip())
179 for post, item in zip(posts, items):
180 a = item.find("a")
181 self.assertEqual(post['link'], a["href"])
182 self.assertEqual(post['title'], a.string)
183
184 def test_blog_disabled(self):
185 """Launchpad blog not queried for display without feature"""
186 calls = []
187
188 def _get_blog_posts():
189 calls.append('called')
190 return []
191
192 root = getUtility(ILaunchpadRoot)
193 user = self.factory.makePerson()
194 login_person(user)
195 view = create_initialized_view(root, 'index.html', principal=user)
196 view.getRecentBlogPosts = _get_blog_posts
197 markup = BeautifulSoup(
198 view(), parseOnlyThese=SoupStrainer(id='homepage'))
199 self.assertEqual([], calls)
200 self.assertIs(None, markup.find(True, id='homepage-blogposts'))
201 # Even logged in users should get the launchpad intro text in the left
202 # column rather than blank space when the blog is not being displayed.
203 self.assertTrue(view.show_whatslaunchpad)
204 self.assertTrue(markup.find(True, 'homepage-whatslaunchpad'))
141205
=== modified file 'lib/lp/app/stories/launchpad-root/front-pages.txt'
--- lib/lp/app/stories/launchpad-root/front-pages.txt 2010-07-19 19:49:22 +0000
+++ lib/lp/app/stories/launchpad-root/front-pages.txt 2012-08-14 14:33:25 +0000
@@ -1,9 +1,11 @@
1Launchpad front pages1Launchpad front pages
2---------------------2---------------------
33
4Visit our home page:4Visit our home page with the typical configuration:
55
6 >>> browser.open('http://launchpad.dev/')6 >>> from lp.services.features.testing import FeatureFixture
7 >>> with FeatureFixture({"app.root_blog.enabled": True}):
8 ... browser.open('http://launchpad.dev/')
7 >>> browser.url9 >>> browser.url
8 'http://launchpad.dev/'10 'http://launchpad.dev/'
911
1012
=== 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-14 14:33:25 +0000
@@ -67,7 +67,7 @@
67 <div class="yui-g">67 <div class="yui-g">
68 <div class="yui-u first" style="margin-top: 1.5em;">68 <div class="yui-u first" style="margin-top: 1.5em;">
69 <div class="homepage-whatslaunchpad"69 <div class="homepage-whatslaunchpad"
70 tal:condition="not:view/user">70 tal:condition="view/show_whatslaunchpad">
71 <h2><span class="launchpad-gold">Launchpad</span> is a software collaboration platform that provides:</h2>71 <h2><span class="launchpad-gold">Launchpad</span> is a software collaboration platform that provides:</h2>
72 <ul tal:define="apphomes view/apphomes">72 <ul tal:define="apphomes view/apphomes">
73 <li>73 <li>
@@ -109,7 +109,8 @@
109 </div>109 </div>
110 </div>110 </div>
111111
112 <div id="homepage-blogposts" class="homepage-portlet">112 <div id="homepage-blogposts" class="homepage-portlet"
113 tal:condition="features/app.root_blog.enabled">
113 <h2>Recent Launchpad blog posts</h2>114 <h2>Recent Launchpad blog posts</h2>
114 <ul tal:define="posts view/getRecentBlogPosts">115 <ul tal:define="posts view/getRecentBlogPosts">
115 <li class="news"116 <li class="news"
@@ -188,15 +189,15 @@
188 <a class="sprite add"189 <a class="sprite add"
189 href="/people/+newteam">Register a team</a>190 href="/people/+newteam">Register a team</a>
190 </li>191 </li>
191 <li>192 <li tal:condition="not:view/show_whatslaunchpad">
192 <a class="sprite bug"193 <a class="sprite bug"
193 tal:attributes="href apphomes/bugs">Browse bugs</a>194 tal:attributes="href apphomes/bugs">Browse bugs</a>
194 </li>195 </li>
195 <li>196 <li tal:condition="not:view/show_whatslaunchpad">
196 <a class="sprite translate-icon"197 <a class="sprite translate-icon"
197 tal:attributes="href apphomes/translations">Help translate</a>198 tal:attributes="href apphomes/translations">Help translate</a>
198 </li>199 </li>
199 <li>200 <li tal:condition="not:view/show_whatslaunchpad">
200 <a class="sprite question"201 <a class="sprite question"
201 tal:attributes="href apphomes/answers">Find answers</a>202 tal:attributes="href apphomes/answers">Find answers</a>
202 </li>203 </li>
@@ -204,7 +205,7 @@
204 <a class="sprite ppa-icon"205 <a class="sprite ppa-icon"
205 href="/ubuntu/+ppas">Browse Ubuntu PPAs</a>206 href="/ubuntu/+ppas">Browse Ubuntu PPAs</a>
206 </li>207 </li>
207 <li>208 <li tal:condition="not:view/show_whatslaunchpad">
208 <a class="sprite tour" href="/+tour">Take the tour</a>209 <a class="sprite tour" href="/+tour">Take the tour</a>
209 </li>210 </li>
210 </ul>211 </ul>
211212
=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py 2012-08-14 01:57:17 +0000
+++ lib/lp/services/features/flags.py 2012-08-14 14:33:25 +0000
@@ -280,6 +280,12 @@
280 '',280 '',
281 '',281 '',
282 ''),282 ''),
283 ('app.root_blog.enabled',
284 'boolean',
285 'If true, load posts from the Launchpad blog to show on the root page.',
286 '',
287 '',
288 ''),
283 ])289 ])
284290
285# The set of all flag names that are documented.291# The set of all flag names that are documented.