Launchpad itself

Merge lp:~rockstar/launchpad/merge-queue-index into lp:launchpad/db-devel

  1. merge-queue-index
  2. Merge into db-devel
Proposed by Paul Hummer
Status: Merged
Approved by: Gavin Panella
Approved revision: no longer in the source branch.
Merged at revision: 9943
Proposed branch: lp:~rockstar/launchpad/merge-queue-index
Merge into: lp:launchpad/db-devel
Diff against target: 433 lines (+302/-3)
11 files modified
lib/lp/code/browser/branch.py (+6/-1)
lib/lp/code/browser/branchmergequeue.py (+79/-0)
lib/lp/code/browser/configure.zcml (+26/-0)
lib/lp/code/browser/tests/test_branchmergequeue.py (+127/-0)
lib/lp/code/model/branchmergequeue.py (+3/-0)
lib/lp/code/templates/branch-pending-merges.pt (+15/-1)
lib/lp/code/templates/branchmergequeue-index.pt (+39/-0)
lib/lp/testing/__init__.py (+2/-0)
lib/lp/testing/factory.py (+1/-1)
setup.py (+2/-0)
versions.cfg (+2/-0)
To merge this branch: bzr merge lp:~rockstar/launchpad/merge-queue-index
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Review via email: mp+39488@code.launchpad.net

Commit message

Add branch merge queue index page. Add soupmatchers library dependency.

Description of the change

This branch adds a (really basic) index page for branch merge queues, as long as a flow for creating a merge queue and linking a branch to it. This branch also adds some more things, including:

 - The addition of james_w's soupmatchers library for HTML page matchers. This makes pagetest-type unit tests that we write in code a little nicer.
 - Fixed a bug in getUserBrowser where the authentication didn't support any passwords but 'test'. I fixed this by setting the password for auth to the sooper sekrit attribute that holds the cleartext password.
 - I also fixed a bug in the factory where the branch merge queues owner and registrant were being specified backwards.

This work is hiding behind a feature flag, and the UI is really raw, but I need to get this (and the next branch) landed so that others can also start working on branch merge queues.

To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) wrote :

Merge queues are cool. A few pretty minor comments.

[1]

+ def enable_queue_flag(self):
+ getFeatureStore().add(FeatureFlag(
+ scope=u'default', flag=u'code.branchmergequeue',
+ value=u'on', priority=1))

There's a neat context manager in lp.testing that you might like. I
think the flag only needs to be enabled on the branch index page, so
something like this might work:

    from lp.testing import feature_flags, set_feature_flag

    with feature_flags():
        set_feature_flag(u'code.branchmergequeue', u'on')
        browser = self.getUserBrowser(
            canonical_url(branch), user=rockstar)

Okay, that's not actually much clearer. Anyway, it's there.

[2]

I think you should ad a short test to show that the +create-queue link
is not available when the feature flag is not set.

[3]

+ if configuration is None:
+ configuration = unicode(simplejson.dumps({}))

The docstring for unicode says:

  Create a new Unicode object from the given encoded string.
  encoding defaults to the current default string encoding.

So I think this would be more correct as:

  configuration = simplejson.dumps({}).decode('ascii')

Or:

  configuration = simplejson.dumps({}, ensure_ascii=False)

In practice it makes bugger all difference though because I doubt
Launchpad will ever run with a default encoding that isn't a superset
of ASCII.

I should just shut up :)

[4]

+ <h4>Merge Queue</h4>
+ This branch is not managed by a queue.

Maybe a test for this too.

[5]

+ password = naked_user._password_cleartext_cached

This clobbers the password argument. Can you remove the argument? (In
a follow-on branch is fine if you need to get this landed.)

review: Approve
Revision history for this message
Gavin Panella (allenap) wrote :

> [5]
>
> + password = naked_user._password_cleartext_cached
>
> This clobbers the password argument. Can you remove the argument? (In
> a follow-on branch is fine if you need to get this landed.)

Erm, not true, as rockstar just pointed out to me on IRC.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py 2010-09-30 00:37:25 +0000
+++ lib/lp/code/browser/branch.py 2010-11-03 19:19:49 +0000
@@ -296,7 +296,8 @@
296 links = [296 links = [
297 'add_subscriber', 'browse_revisions', 'create_recipe', 'link_bug',297 'add_subscriber', 'browse_revisions', 'create_recipe', 'link_bug',
298 'link_blueprint', 'register_merge', 'source', 'subscription',298 'link_blueprint', 'register_merge', 'source', 'subscription',
299 'edit_status', 'edit_import', 'upgrade_branch', 'view_recipes']299 'edit_status', 'edit_import', 'upgrade_branch', 'view_recipes',
300 'create_queue']
300301
301 @enabled_with_permission('launchpad.Edit')302 @enabled_with_permission('launchpad.Edit')
302 def edit_status(self):303 def edit_status(self):
@@ -388,6 +389,10 @@
388 text = 'Create packaging recipe'389 text = 'Create packaging recipe'
389 return Link('+new-recipe', text, enabled=enabled, icon='add')390 return Link('+new-recipe', text, enabled=enabled, icon='add')
390391
392 @enabled_with_permission('launchpad.Edit')
393 def create_queue(self):
394 return Link('+create-queue', 'Create a new queue', icon='add')
395
391396
392class BranchMirrorMixin:397class BranchMirrorMixin:
393 """Provide mirror_location property.398 """Provide mirror_location property.
394399
=== added file 'lib/lp/code/browser/branchmergequeue.py'
--- lib/lp/code/browser/branchmergequeue.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/browser/branchmergequeue.py 2010-11-03 19:19:49 +0000
@@ -0,0 +1,79 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""SourcePackageRecipe views."""
5
6__metaclass__ = type
7
8__all__ = [
9 'BranchMergeQueueContextMenu',
10 'BranchMergeQueueView',
11 ]
12
13from lazr.restful.interface import copy_field
14from zope.component import getUtility
15from zope.interface import Interface
16
17from canonical.launchpad.webapp import (
18 action,
19 canonical_url,
20 ContextMenu,
21 LaunchpadFormView,
22 LaunchpadView,
23 )
24from lp.code.interfaces.branchmergequeue import (
25 IBranchMergeQueue,
26 IBranchMergeQueueSource,
27 )
28
29
30class BranchMergeQueueContextMenu(ContextMenu):
31 """Context menu for sourcepackage recipes."""
32
33 usedfor = IBranchMergeQueue
34
35 facet = 'branches'
36
37 links = ()
38
39
40class BranchMergeQueueView(LaunchpadView):
41 """Default view of a SourcePackageRecipe."""
42
43 @property
44 def page_title(self):
45 return "%(queue_name)s queue owned by %(name)s" % {
46 'name': self.context.owner.displayname,
47 'queue_name': self.context.name}
48
49 label = page_title
50
51
52class BranchMergeQueueAddView(LaunchpadFormView):
53
54 title = label = 'Create a new branch merge queue'
55
56 class schema(Interface):
57 name = copy_field(IBranchMergeQueue['name'], readonly=False)
58 owner = copy_field(IBranchMergeQueue['owner'], readonly=False)
59 description = copy_field(IBranchMergeQueue['description'],
60 readonly=False)
61
62 def initialize(self):
63 super(BranchMergeQueueAddView, self).initialize()
64
65 @property
66 def initial_values(self):
67 return {}
68
69 @property
70 def cancel_url(self):
71 return canonical_url(self.context)
72
73 @action('Create Queue', name='create')
74 def request_action(self, action, data):
75 merge_queue = getUtility(IBranchMergeQueueSource).new(
76 data['name'], data['owner'], self.user, data['description'])
77 self.context.addToQueue(merge_queue)
78
79 self.next_url = canonical_url(merge_queue)
080
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2010-10-21 02:53:17 +0000
+++ lib/lp/code/browser/configure.zcml 2010-11-03 19:19:49 +0000
@@ -1326,4 +1326,30 @@
1326 path_expression="string:+merge-queues/${name}"1326 path_expression="string:+merge-queues/${name}"
1327 rootsite="code" />1327 rootsite="code" />
13281328
1329 <browser:menus
1330 classes="BranchMergeQueueContextMenu"
1331 module="lp.code.browser.branchmergequeue"/>
1332
1333 <facet facet="branches">
1334 <browser:defaultView
1335 for="lp.code.interfaces.branchmergequeue.IBranchMergeQueue"
1336 name="+index"
1337 layer="lp.code.publisher.CodeLayer" />
1338 <browser:page
1339 for="lp.code.interfaces.branchmergequeue.IBranchMergeQueue"
1340 layer="lp.code.publisher.CodeLayer"
1341 class="lp.code.browser.branchmergequeue.BranchMergeQueueView"
1342 name="+index"
1343 template="../templates/branchmergequeue-index.pt"
1344 permission="zope.Public" />
1345 <browser:page
1346 for="lp.code.interfaces.branch.IBranch"
1347 layer="lp.code.publisher.CodeLayer"
1348 class="lp.code.browser.branchmergequeue.BranchMergeQueueAddView"
1349 name="+create-queue"
1350 template="../../app/templates/generic-edit.pt"
1351 permission="launchpad.Edit" />
1352
1353 </facet>
1354
1329</configure>1355</configure>
13301356
=== added file 'lib/lp/code/browser/tests/test_branchmergequeue.py'
--- lib/lp/code/browser/tests/test_branchmergequeue.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/browser/tests/test_branchmergequeue.py 2010-11-03 19:19:49 +0000
@@ -0,0 +1,127 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Tests for the branch merge queue view classes and templates."""
5
6from __future__ import with_statement
7
8__metaclass__ = type
9
10from mechanize import LinkNotFoundError
11import re
12
13import soupmatchers
14
15from canonical.launchpad.webapp import canonical_url
16from canonical.testing.layers import (
17 DatabaseFunctionalLayer,
18 )
19from lp.services.features.model import FeatureFlag, getFeatureStore
20from lp.testing import (
21 ANONYMOUS,
22 BrowserTestCase,
23 person_logged_in,
24 )
25
26
27class TestBranchMergeQueue(BrowserTestCase):
28 """Test the Branch Merge Queue index page."""
29
30 layer = DatabaseFunctionalLayer
31
32 def enable_queue_flag(self):
33 getFeatureStore().add(FeatureFlag(
34 scope=u'default', flag=u'code.branchmergequeue',
35 value=u'on', priority=1))
36
37 def test_index(self):
38 """Test the index page of a branch merge queue."""
39 with person_logged_in(ANONYMOUS):
40 queue = self.factory.makeBranchMergeQueue()
41 queue_owner = queue.owner.displayname
42 queue_registrant = queue.registrant.displayname
43 queue_description = queue.description
44 queue_url = canonical_url(queue)
45
46 branch = self.factory.makeBranch()
47 branch_name = branch.bzr_identity
48 with person_logged_in(branch.owner):
49 branch.addToQueue(queue)
50
51 # XXX: rockstar - bug #666979 - The text argument should really ignore
52 # whitespace, but it currently doesn't. Now I have two problems.
53 queue_matcher = soupmatchers.HTMLContains(
54 soupmatchers.Tag(
55 'Page title', 'h1',
56 text=re.compile('\w*%s queue owned by %s\w*' % (
57 queue.name, queue.owner.displayname))),
58 soupmatchers.Tag(
59 'Description Label', 'dt',
60 text=re.compile('\w*Description\w*')),
61 soupmatchers.Tag(
62 'Description Value', 'dd',
63 text=re.compile('\w*%s\w*' % queue.description)),
64 soupmatchers.Tag(
65 'Branch link', 'a',
66 text=re.compile('\w*%s\w*' % branch.bzr_identity)))
67
68 browser = self.getUserBrowser(canonical_url(queue), user=queue.owner)
69
70 self.assertThat(browser.contents, queue_matcher)
71
72 def test_create(self):
73 """Test that branch merge queues can be created from a branch."""
74 self.enable_queue_flag()
75 with person_logged_in(ANONYMOUS):
76 rockstar = self.factory.makePerson(name='rockstar')
77 branch = self.factory.makeBranch(owner=rockstar)
78 self.factory.makeBranch(product=branch.product)
79 owner_name = branch.owner.name
80
81 browser = self.getUserBrowser(canonical_url(branch), user=rockstar)
82
83 # There shouldn't be a merge queue linked here.
84 noqueue_matcher = soupmatchers.HTMLContains(
85 soupmatchers.Tag(
86 'Not managed', 'div',
87 text=re.compile(
88 '\w*This branch is not managed by a queue.\w*')))
89 self.assertThat(browser.contents, noqueue_matcher)
90
91 browser.getLink('Create a new queue').click()
92
93 browser.getControl('Name').value = 'libbob-queue'
94 browser.getControl('Description').value = (
95 'This is a queue for the libbob projects.')
96 browser.getControl('Create Queue').click()
97
98 self.assertEqual(
99 'http://code.launchpad.dev/~rockstar/+merge-queues/libbob-queue',
100 browser.url)
101
102 def test_create_unauthorized(self):
103 """Test that queues can't be created by unauthorized users."""
104 self.enable_queue_flag()
105 with person_logged_in(ANONYMOUS):
106 branch = self.factory.makeBranch()
107 self.factory.makeBranch(product=branch.product)
108
109 browser = self.getUserBrowser(canonical_url(branch))
110 self.assertRaises(
111 LinkNotFoundError,
112 browser.getLink,
113 'Create a new queue')
114
115 def test_create_featureflag(self):
116 """Test that the feature flag hides the "create" link."""
117 with person_logged_in(ANONYMOUS):
118 rockstar = self.factory.makePerson(name='rockstar')
119 branch = self.factory.makeBranch(owner=rockstar)
120 self.factory.makeBranch(product=branch.product)
121 owner_name = branch.owner.name
122
123 browser = self.getUserBrowser(canonical_url(branch), user=rockstar)
124 self.assertRaises(
125 LinkNotFoundError,
126 browser.getLink,
127 'Create a new queue')
0128
=== modified file 'lib/lp/code/model/branchmergequeue.py'
--- lib/lp/code/model/branchmergequeue.py 2010-10-22 04:16:34 +0000
+++ lib/lp/code/model/branchmergequeue.py 2010-11-03 19:19:49 +0000
@@ -72,6 +72,9 @@
72 """See `IBranchMergeQueueSource`."""72 """See `IBranchMergeQueueSource`."""
73 store = IMasterStore(BranchMergeQueue)73 store = IMasterStore(BranchMergeQueue)
7474
75 if configuration is None:
76 configuration = unicode(simplejson.dumps({}))
77
75 queue = cls()78 queue = cls()
76 queue.name = name79 queue.name = name
77 queue.owner = owner80 queue.owner = owner
7881
=== modified file 'lib/lp/code/templates/branch-pending-merges.pt'
--- lib/lp/code/templates/branch-pending-merges.pt 2010-06-10 07:54:59 +0000
+++ lib/lp/code/templates/branch-pending-merges.pt 2010-11-03 19:19:49 +0000
@@ -2,7 +2,9 @@
2 xmlns:tal="http://xml.zope.org/namespaces/tal"2 xmlns:tal="http://xml.zope.org/namespaces/tal"
3 xmlns:metal="http://xml.zope.org/namespaces/metal"3 xmlns:metal="http://xml.zope.org/namespaces/metal"
4 xmlns:i18n="http://xml.zope.org/namespaces/i18n"4 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
5 tal:define="context_menu view/context/menu:context"5 tal:define="
6 context_menu view/context/menu:context;
7 features request/features"
6 tal:condition="view/show_merge_links">8 tal:condition="view/show_merge_links">
79
8 <h3>Branch merges</h3>10 <h3>Branch merges</h3>
@@ -46,6 +48,18 @@
46 tal:condition="link/enabled"48 tal:condition="link/enabled"
47 tal:replace="structure link/render"49 tal:replace="structure link/render"
48 />50 />
51
52 <div tal:condition="features/code.branchmergequeue">
53 <div tal:condition="not: context/merge_queue">
54 <h4>Merge Queue</h4>
55 This branch is not managed by a queue.
56 <div
57 tal:define="link context_menu/create_queue"
58 tal:condition="link/enabled"
59 tal:content="structure link/render"
60 />
61 </div>
62 </div>
49 </div>63 </div>
5064
51</div>65</div>
5266
=== added file 'lib/lp/code/templates/branchmergequeue-index.pt'
--- lib/lp/code/templates/branchmergequeue-index.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/branchmergequeue-index.pt 2010-11-03 19:19:49 +0000
@@ -0,0 +1,39 @@
1<html
2 xmlns="http://www.w3.org/1999/xhtml"
3 xmlns:tal="http://xml.zope.org/namespaces/tal"
4 xmlns:metal="http://xml.zope.org/namespaces/metal"
5 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
6 metal:use-macro="view/macro:page/main_side"
7 i18n:domain="launchpad"
8>
9
10<metal:side fill-slot="side">
11 <div tal:replace="structure context/@@+global-actions" />
12</metal:side>
13
14<tal:registering metal:fill-slot="registering">
15 Created by
16 <tal:registrant replace="structure context/registrant/fmt:link" />
17 on
18 <tal:created-on replace="structure context/date_created/fmt:date" />
19</tal:registering>
20
21<div metal:fill-slot="main">
22 <dl>
23 <dt>
24 Description
25 </dt>
26 <dd tal:content="context/description"></dd>
27 </dl>
28 <div tal:condition="context/branches">
29 The following branches are managed by this queue:
30 <ul>
31 <tal:branches repeat="branch context/branches">
32 <li>
33 <a tal:content="structure branch/fmt:link" />
34 </li>
35 </tal:branches>
36 </ul>
37 </div>
38</div>
39</html>
040
=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py 2010-10-27 14:20:21 +0000
+++ lib/lp/testing/__init__.py 2010-11-03 19:19:49 +0000
@@ -561,6 +561,8 @@
561 user = self.factory.makePerson(password=password)561 user = self.factory.makePerson(password=password)
562 naked_user = removeSecurityProxy(user)562 naked_user = removeSecurityProxy(user)
563 email = naked_user.preferredemail.email563 email = naked_user.preferredemail.email
564 if hasattr(naked_user, '_password_cleartext_cached'):
565 password = naked_user._password_cleartext_cached
564 logout()566 logout()
565 browser = setupBrowser(567 browser = setupBrowser(
566 auth="Basic %s:%s" % (str(email), password))568 auth="Basic %s:%s" % (str(email), password))
567569
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-10-29 19:28:14 +0000
+++ lib/lp/testing/factory.py 2010-11-03 19:19:49 +0000
@@ -1128,7 +1128,7 @@
1128 self.getUniqueString('key'): self.getUniqueString('value')}))1128 self.getUniqueString('key'): self.getUniqueString('value')}))
11291129
1130 queue = getUtility(IBranchMergeQueueSource).new(1130 queue = getUtility(IBranchMergeQueueSource).new(
1131 name, registrant, owner, description, configuration)1131 name, owner, registrant, description, configuration)
1132 return queue1132 return queue
11331133
1134 def enableDefaultStackingForProduct(self, product, branch=None):1134 def enableDefaultStackingForProduct(self, product, branch=None):
11351135
=== modified file 'setup.py'
--- setup.py 2010-09-18 08:00:27 +0000
+++ setup.py 2010-11-03 19:19:49 +0000
@@ -25,6 +25,7 @@
25 # used in zcml.25 # used in zcml.
26 install_requires=[26 install_requires=[
27 'ampoule',27 'ampoule',
28 'BeautifulSoup',
28 'bzr',29 'bzr',
29 'chameleon.core',30 'chameleon.core',
30 'chameleon.zpt',31 'chameleon.zpt',
@@ -63,6 +64,7 @@
63 'RestrictedPython',64 'RestrictedPython',
64 'setproctitle',65 'setproctitle',
65 'setuptools',66 'setuptools',
67 'soupmatchers',
66 'sourcecodegen',68 'sourcecodegen',
67 'storm',69 'storm',
68 'testtools',70 'testtools',
6971
=== modified file 'versions.cfg'
--- versions.cfg 2010-10-27 04:23:52 +0000
+++ versions.cfg 2010-11-03 19:19:49 +0000
@@ -5,6 +5,7 @@
5# Alphabetical, case-insensitive, please! :-)5# Alphabetical, case-insensitive, please! :-)
66
7ampoule = 0.2.07ampoule = 0.2.0
8BeautifulSoup = 3.1.0.1
8bzr = 2.2.09bzr = 2.2.0
9chameleon.core = 1.0b3510chameleon.core = 1.0b35
10chameleon.zpt = 1.0b1711chameleon.zpt = 1.0b17
@@ -64,6 +65,7 @@
64simplejson = 2.0.965simplejson = 2.0.9
65simplesettings = 0.466simplesettings = 0.4
66SimpleTal = 4.167SimpleTal = 4.1
68soupmatchers = 0.1r53
67sourcecodegen = 0.6.969sourcecodegen = 0.6.9
68storm = 0.1870storm = 0.18
69testtools = 0.9.671testtools = 0.9.6

Subscribers

People subscribed via source and target branches

to status/vote changes: