Launchpad itself

Merge lp:~mwhudson/launchpad/vostok-main-template into lp:launchpad

  1. vostok-main-template
  2. Merge into devel
Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: no longer in the source branch.
Merged at revision: 11299
Proposed branch: lp:~mwhudson/launchpad/vostok-main-template
Merge into: lp:launchpad
Prerequisite: lp:~mwhudson/launchpad/vostok-add-root
Diff against target: 324 lines (+155/-20)
11 files modified
Makefile (+1/-0)
lib/canonical/launchpad/doc/webapp-publication.txt (+5/-0)
lib/lp/vostok/browser/configure.zcml (+8/-0)
lib/lp/vostok/browser/root.py (+9/-0)
lib/lp/vostok/browser/tests/request.py (+5/-7)
lib/lp/vostok/browser/tests/test_main_template.py (+33/-0)
lib/lp/vostok/browser/tests/test_root.py (+41/-3)
lib/lp/vostok/publisher.py (+18/-1)
lib/lp/vostok/templates/main-template.pt (+15/-0)
lib/lp/vostok/templates/root.pt (+20/-3)
lib/lp/vostok/tests/test_publisher.py (+0/-6)
To merge this branch: bzr merge lp:~mwhudson/launchpad/vostok-main-template
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Review via email: mp+31240@code.launchpad.net

Commit message

Add the publication plumbing and very basic root page for the 'vostok' vhost

Description of the change

Hi,

This branch adds a very simple main template METAL macro for the vostok layer
and makes the view for the root object use it.

The macro is done in an old-school way compared to how it's done in Launchpad
today, mostly because we couldn't be bothered to figure out how to make the
macro: tales stuff be layer dependent and also because we should be able to get
away with all our pages using the same main template macro.

Cheers,
mwh

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

> class VostokTestRequest(VostokRequestMixin, LaunchpadTestRequest):
> pass

Instead of pass, how about a docstring?

The browser tests are missing docstrings too. I know they are (most likely)
boring, but should be there anyway.

> def test_distributions(self):
> # VostokRootView.distributions is an iterable of all registered
> # distributions.
> root_view = self.view()

This likes you are actually doing

     VostokRootView.__call__

Is that what you want? I wouldn't have thought so.

Are you familiar at all with the new lp.testing.BrowserTestCase?
It might be better than manually creating the view, initializing it, and
rendering it.

VostokBrowserRequest could benefit from a docstring rather than pass too.

I think that your main-template.pt macro should be a valid HTML file rather
than just <h1> and <div>. I'm pretty sure that most of the HTML in root.pt is
discarded given that it is using the master macro.

review: Needs Information
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

On Thu, 29 Jul 2010 23:38:41 -0000, Tim Penhey <email address hidden> wrote:
> Review: Needs Information
> > class VostokTestRequest(VostokRequestMixin, LaunchpadTestRequest):
> > pass
>
> Instead of pass, how about a docstring?

OK.

> The browser tests are missing docstrings too. I know they are (most likely)
> boring, but should be there anyway.

Added.

> > def test_distributions(self):
> > # VostokRootView.distributions is an iterable of all registered
> > # distributions.
> > root_view = self.view()
>
> This likes you are actually doing
>
> VostokRootView.__call__
>
> Is that what you want? I wouldn't have thought so.

view is a method, not a property.

> Are you familiar at all with the new lp.testing.BrowserTestCase?
> It might be better than manually creating the view, initializing it, and
> rendering it.

The basic reason I didn't use testbrowser was that I wanted to have the
view object around so I can check the list matches up with what's in the
template -- the idea being to test the template. I agree it's a bit
ugly.

> VostokBrowserRequest could benefit from a docstring rather than pass too.

I sprinkled a few docstrings around.

> I think that your main-template.pt macro should be a valid HTML file rather
> than just <h1> and <div>. I'm pretty sure that most of the HTML in root.pt is
> discarded given that it is using the master macro.

You were right. Fixed.

Cheers,
mwh

Revision history for this message
Tim Penhey (thumper) wrote :

Changes look good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Makefile'
2--- Makefile 2010-08-05 01:32:59 +0000
3+++ Makefile 2010-08-05 01:33:00 +0000
4@@ -200,6 +200,7 @@
5 configuration:instance_name=${LPCONFIG} -c $(BUILDOUT_CFG)
6
7 compile: $(PY) $(BZR_VERSION_INFO)
8+ mkdir -p /var/tmp/vostok-archive
9 ${SHHH} $(MAKE) -C sourcecode build PYTHON=${PYTHON} \
10 LPCONFIG=${LPCONFIG}
11 ${SHHH} LPCONFIG=${LPCONFIG} ${PY} -t buildmailman.py
12
13=== modified file 'lib/canonical/launchpad/doc/webapp-publication.txt'
14--- lib/canonical/launchpad/doc/webapp-publication.txt 2010-07-23 08:50:49 +0000
15+++ lib/canonical/launchpad/doc/webapp-publication.txt 2010-08-05 01:33:00 +0000
16@@ -93,6 +93,10 @@
17 rooturl: http://ubuntu-openid.launchpad.dev/
18 althosts:
19 ----
20+ vostok @ vostok.dev
21+ rooturl: http://vostok.dev/
22+ althosts:
23+ ----
24 xmlrpc @ xmlrpc.launchpad.dev
25 rooturl: http://launchpad.dev/
26 althosts:
27@@ -124,6 +128,7 @@
28 testopenid.dev
29 translations.launchpad.dev
30 ubuntu-openid.launchpad.dev
31+ vostok.dev
32 xmlrpc-private.launchpad.dev
33 xmlrpc.launchpad.dev
34
35
36=== modified file 'lib/lp/vostok/browser/configure.zcml'
37--- lib/lp/vostok/browser/configure.zcml 2010-08-05 01:32:59 +0000
38+++ lib/lp/vostok/browser/configure.zcml 2010-08-05 01:33:00 +0000
39@@ -5,6 +5,14 @@
40 xmlns:xmlrpc="http://namespaces.zope.org/xmlrpc"
41 i18n_domain="launchpad">
42
43+ <browser:page
44+ for="*"
45+ name="main_template"
46+ template="../templates/main-template.pt"
47+ permission="zope.Public"
48+ layer="lp.vostok.publisher.VostokLayer"
49+ />
50+
51 <browser:defaultView
52 for="lp.vostok.publisher.IVostokRoot"
53 name="+index"
54
55=== modified file 'lib/lp/vostok/browser/root.py'
56--- lib/lp/vostok/browser/root.py 2010-08-05 01:32:59 +0000
57+++ lib/lp/vostok/browser/root.py 2010-08-05 01:33:00 +0000
58@@ -8,8 +8,17 @@
59 'VostokRootView',
60 ]
61
62+from zope.component import getUtility
63+
64 from canonical.launchpad.webapp import LaunchpadView
65
66+from lp.registry.interfaces.distribution import IDistributionSet
67+
68
69 class VostokRootView(LaunchpadView):
70 """The view for the Vostok root object."""
71+
72+ @property
73+ def distributions(self):
74+ """An iterable of all registered distributions."""
75+ return getUtility(IDistributionSet)
76
77=== modified file 'lib/lp/vostok/browser/tests/request.py'
78--- lib/lp/vostok/browser/tests/request.py 2010-08-05 01:32:59 +0000
79+++ lib/lp/vostok/browser/tests/request.py 2010-08-05 01:33:00 +0000
80@@ -8,12 +8,10 @@
81 'VostokTestRequest',
82 ]
83
84-from zope.interface import implements
85-
86 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
87
88-from lp.vostok.publisher import VostokLayer
89-
90-
91-class VostokTestRequest(LaunchpadTestRequest):
92- implements(VostokLayer)
93+from lp.vostok.publisher import VostokRequestMixin
94+
95+
96+class VostokTestRequest(VostokRequestMixin, LaunchpadTestRequest):
97+ """A test request for `VostokLayer`."""
98
99=== added file 'lib/lp/vostok/browser/tests/test_main_template.py'
100--- lib/lp/vostok/browser/tests/test_main_template.py 1970-01-01 00:00:00 +0000
101+++ lib/lp/vostok/browser/tests/test_main_template.py 2010-08-05 01:33:00 +0000
102@@ -0,0 +1,33 @@
103+# Copyright 2010 Canonical Ltd. This software is licensed under the
104+# GNU Affero General Public License version 3 (see the file LICENSE).
105+
106+"""Tests for the vostok 'main_template'."""
107+
108+__metaclass__ = type
109+
110+import unittest
111+
112+from zope.component import getMultiAdapter
113+
114+from canonical.testing.layers import FunctionalLayer
115+
116+from lp.testing import TestCase
117+from lp.vostok.browser.tests.request import VostokTestRequest
118+
119+
120+class TestMainTemplate(TestCase):
121+ """Tests for our main template."""
122+
123+ layer = FunctionalLayer
124+
125+ def test_main_template_defines_master_macro(self):
126+ # The main template, which is registered as a view for any object at
127+ # all when in the VostokLayer, defines a 'master' macro.
128+ adapter = getMultiAdapter(
129+ (None, VostokTestRequest()), name='main_template')
130+ self.assertEqual(['master'], adapter.index.macros.keys())
131+ self.assertIn('lp/vostok', adapter.index.filename)
132+
133+
134+def test_suite():
135+ return unittest.TestLoader().loadTestsFromName(__name__)
136
137=== modified file 'lib/lp/vostok/browser/tests/test_root.py'
138--- lib/lp/vostok/browser/tests/test_root.py 2010-08-05 01:32:59 +0000
139+++ lib/lp/vostok/browser/tests/test_root.py 2010-08-05 01:33:00 +0000
140@@ -9,16 +9,19 @@
141
142 from zope.app.publisher.browser import getDefaultViewName
143
144-from canonical.testing.layers import FunctionalLayer
145+from canonical.testing.layers import DatabaseFunctionalLayer, FunctionalLayer
146+from canonical.launchpad.testing.pages import extract_text, find_tag_by_id
147
148-from lp.testing import TestCase
149+from lp.testing import TestCase, TestCaseWithFactory
150 from lp.testing.views import create_initialized_view
151 from lp.vostok.browser.root import VostokRootView
152 from lp.vostok.browser.tests.request import VostokTestRequest
153 from lp.vostok.publisher import VostokLayer, VostokRoot
154
155
156-class TestBrowseRoot(TestCase):
157+
158+class TestRootRegistrations(TestCase):
159+ """Test the registration of views for `VostokRoot`."""
160
161 layer = FunctionalLayer
162
163@@ -34,5 +37,40 @@
164 self.assertIsInstance(view, VostokRootView)
165
166
167+class TestRootView(TestCaseWithFactory):
168+ """Tests for `VostokRootView`."""
169+
170+ layer = DatabaseFunctionalLayer
171+
172+ def view(self):
173+ return create_initialized_view(
174+ VostokRoot(), name='+index', layer=VostokLayer)
175+
176+ def test_distributions(self):
177+ # VostokRootView.distributions is an iterable of all registered
178+ # distributions.
179+ root_view = self.view()
180+ new_distro = self.factory.makeDistribution()
181+ self.assertIn(new_distro, list(root_view.distributions))
182+
183+
184+class TestRootTemplate(TestCaseWithFactory):
185+ """Tests for the templates used by views of `VostokRoot`."""
186+
187+ layer = DatabaseFunctionalLayer
188+
189+ def test_distribution_list(self):
190+ # The element with id 'distro-list' on the root page contains a list
191+ # of links to all registered distributions.
192+ v = create_initialized_view(
193+ VostokRoot(), name='+index', layer=VostokLayer)
194+ contents = v.render()
195+ link_list = find_tag_by_id(contents, 'distro-list')('a')
196+ distro_list = list(v.distributions)
197+ self.assertEqual(len(link_list), len(distro_list))
198+ for distro, link in zip(distro_list, link_list):
199+ self.assertEqual(distro.displayname, extract_text(link))
200+
201+
202 def test_suite():
203 return unittest.TestLoader().loadTestsFromName(__name__)
204
205=== modified file 'lib/lp/vostok/publisher.py'
206--- lib/lp/vostok/publisher.py 2010-08-05 01:32:59 +0000
207+++ lib/lp/vostok/publisher.py 2010-08-05 01:33:00 +0000
208@@ -18,21 +18,38 @@
209 from canonical.launchpad.webapp.publication import LaunchpadBrowserPublication
210 from canonical.launchpad.webapp.servers import (
211 LaunchpadBrowserRequest, VirtualHostRequestPublicationFactory)
212+from canonical.launchpad.webapp.vhosts import allvhosts
213
214
215 class VostokLayer(IBrowserRequest, IDefaultBrowserLayer):
216 """The Vostok layer."""
217
218
219-class VostokBrowserRequest(LaunchpadBrowserRequest):
220+class VostokRequestMixin:
221+ """This mixin defines behaviour for the real and test Vostok requests."""
222+
223 implements(VostokLayer)
224
225+ def getRootURL(self, rootsite):
226+ """See `IBasicLaunchpadRequest`."""
227+ return allvhosts.configs['vostok'].rooturl
228+
229+
230+class VostokBrowserRequest(VostokRequestMixin, LaunchpadBrowserRequest):
231+ """Request class for Vostok layer."""
232+
233
234 class IVostokRoot(Interface):
235 """Marker interface for the root vostok object."""
236
237
238 class VostokRoot:
239+ """The root object for the Vostok site.
240+
241+ No behaviour here, it just exists so it can have view and navigation
242+ registrations attached to it.
243+ """
244+
245 implements(IVostokRoot)
246
247
248
249=== added file 'lib/lp/vostok/templates/main-template.pt'
250--- lib/lp/vostok/templates/main-template.pt 1970-01-01 00:00:00 +0000
251+++ lib/lp/vostok/templates/main-template.pt 2010-08-05 01:33:00 +0000
252@@ -0,0 +1,15 @@
253+<metal:page
254+ xmlns:metal="http://xml.zope.org/namespaces/metal"
255+ xmlns:tal="http://xml.zope.org/namespaces/tal"
256+ define-macro="master"><!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
257+<html xmlns="http://www.w3.org/1999/xhtml">
258+ <head>
259+ <!-- Obviously, we'll need to do something better here. -->
260+ <title>Vostok page</title>
261+ </head>
262+ <body>
263+ <h1 metal:define-slot="heading" />
264+ <div metal:define-slot="content" />
265+ </body>
266+</html>
267+</metal:page>
268
269=== modified file 'lib/lp/vostok/templates/root.pt'
270--- lib/lp/vostok/templates/root.pt 2010-08-05 01:32:59 +0000
271+++ lib/lp/vostok/templates/root.pt 2010-08-05 01:33:00 +0000
272@@ -1,3 +1,20 @@
273-<big>
274-THIS IS VOSTOK
275-</big>
276+<html
277+ xmlns="http://www.w3.org/1999/xhtml"
278+ xmlns:tal="http://xml.zope.org/namespaces/tal"
279+ xmlns:metal="http://xml.zope.org/namespaces/metal"
280+ xmlns:i18n="http://xml.zope.org/namespaces/i18n"
281+ metal:use-macro="context/@@main_template/master"
282+ i18n:domain="vostok">
283+ <body>
284+ <tal:heading metal:fill-slot="heading">
285+ <h1>Vostok</h1>
286+ </tal:heading>
287+ <tal:content metal:fill-slot="content">
288+ <ul id="distro-list">
289+ <tal:loop tal:repeat="distro view/distributions">
290+ <li tal:content="structure distro/fmt:link" />
291+ </tal:loop>
292+ </ul>
293+ </tal:content>
294+ </body>
295+</html>
296
297=== modified file 'lib/lp/vostok/tests/test_publisher.py'
298--- lib/lp/vostok/tests/test_publisher.py 2010-08-05 01:32:59 +0000
299+++ lib/lp/vostok/tests/test_publisher.py 2010-08-05 01:33:00 +0000
300@@ -9,15 +9,12 @@
301
302 from canonical.config import config
303 from canonical.testing.layers import FunctionalLayer
304-from canonical.launchpad.webapp.interfaces import IOpenLaunchBag
305
306 from lp.testing import TestCase
307 from lp.testing.publication import get_request_and_publication
308
309 from lp.vostok.publisher import VostokLayer, VostokRoot
310
311-from zope.component import getUtility
312-
313
314 class TestRegistration(TestCase):
315 """Vostok's publication customizations are installed correctly."""
316@@ -37,9 +34,6 @@
317 request, publication = get_request_and_publication(
318 host=config.vhost.vostok.hostname)
319 self.assertProvides(request, VostokLayer)
320- # XXX getApplication caches the root object in the LaunchBag, so we
321- # need to set it up, or it crashes.
322- getUtility(IOpenLaunchBag).clear()
323 root = publication.getApplication(request)
324 self.assertIsInstance(root, VostokRoot)
325