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

[Paul Hummer]

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

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.

[Gavin Panella (allenap)]

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

[Gavin Panella (allenap)]

> [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.

=== 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 @@
     links = [
         'add_subscriber', 'browse_revisions', 'create_recipe', 'link_bug',
         'link_blueprint', 'register_merge', 'source', 'subscription',
-        'edit_status', 'edit_import', 'upgrade_branch', 'view_recipes']
+        'edit_status', 'edit_import', 'upgrade_branch', 'view_recipes',
+        'create_queue']
 
     @enabled_with_permission('launchpad.Edit')
     def edit_status(self):
@@ -388,6 +389,10 @@
         text = 'Create packaging recipe'
         return Link('+new-recipe', text, enabled=enabled, icon='add')
 
+    @enabled_with_permission('launchpad.Edit')
+    def create_queue(self):
+        return Link('+create-queue', 'Create a new queue', icon='add')
+
 
 class BranchMirrorMixin:
     """Provide mirror_location property.

=== 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 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""SourcePackageRecipe views."""
+
+__metaclass__ = type
+
+__all__ = [
+    'BranchMergeQueueContextMenu',
+    'BranchMergeQueueView',
+    ]
+
+from lazr.restful.interface import copy_field
+from zope.component import getUtility
+from zope.interface import Interface
+
+from canonical.launchpad.webapp import (
+    action,
+    canonical_url,
+    ContextMenu,
+    LaunchpadFormView,
+    LaunchpadView,
+    )
+from lp.code.interfaces.branchmergequeue import (
+    IBranchMergeQueue,
+    IBranchMergeQueueSource,
+    )
+
+
+class BranchMergeQueueContextMenu(ContextMenu):
+    """Context menu for sourcepackage recipes."""
+
+    usedfor = IBranchMergeQueue
+
+    facet = 'branches'
+
+    links = ()
+
+
+class BranchMergeQueueView(LaunchpadView):
+    """Default view of a SourcePackageRecipe."""
+
+    @property
+    def page_title(self):
+        return "%(queue_name)s queue owned by %(name)s" % {
+            'name': self.context.owner.displayname,
+            'queue_name': self.context.name}
+
+    label = page_title
+
+
+class BranchMergeQueueAddView(LaunchpadFormView):
+
+    title = label = 'Create a new branch merge queue'
+
+    class schema(Interface):
+        name = copy_field(IBranchMergeQueue['name'], readonly=False)
+        owner = copy_field(IBranchMergeQueue['owner'], readonly=False)
+        description = copy_field(IBranchMergeQueue['description'],
+            readonly=False)
+
+    def initialize(self):
+        super(BranchMergeQueueAddView, self).initialize()
+
+    @property
+    def initial_values(self):
+        return {}
+
+    @property
+    def cancel_url(self):
+        return canonical_url(self.context)
+
+    @action('Create Queue', name='create')
+    def request_action(self, action, data):
+        merge_queue = getUtility(IBranchMergeQueueSource).new(
+            data['name'], data['owner'], self.user, data['description'])
+        self.context.addToQueue(merge_queue)
+
+        self.next_url = canonical_url(merge_queue)

=== 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 @@
         path_expression="string:+merge-queues/${name}"
         rootsite="code" />
 
+    <browser:menus
+        classes="BranchMergeQueueContextMenu"
+        module="lp.code.browser.branchmergequeue"/>
+
+    <facet facet="branches">
+        <browser:defaultView
+            for="lp.code.interfaces.branchmergequeue.IBranchMergeQueue"
+            name="+index"
+            layer="lp.code.publisher.CodeLayer" />
+        <browser:page
+            for="lp.code.interfaces.branchmergequeue.IBranchMergeQueue"
+            layer="lp.code.publisher.CodeLayer"
+            class="lp.code.browser.branchmergequeue.BranchMergeQueueView"
+            name="+index"
+            template="../templates/branchmergequeue-index.pt"
+            permission="zope.Public" />
+        <browser:page
+            for="lp.code.interfaces.branch.IBranch"
+            layer="lp.code.publisher.CodeLayer"
+            class="lp.code.browser.branchmergequeue.BranchMergeQueueAddView"
+            name="+create-queue"
+            template="../../app/templates/generic-edit.pt"
+            permission="launchpad.Edit" />
+
+    </facet>
+
 </configure>

=== 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 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the branch merge queue view classes and templates."""
+
+from __future__ import with_statement
+
+__metaclass__ = type
+
+from mechanize import LinkNotFoundError
+import re
+
+import soupmatchers
+
+from canonical.launchpad.webapp import canonical_url
+from canonical.testing.layers import (
+    DatabaseFunctionalLayer,
+    )
+from lp.services.features.model import FeatureFlag, getFeatureStore
+from lp.testing import (
+    ANONYMOUS,
+    BrowserTestCase,
+    person_logged_in,
+    )
+
+
+class TestBranchMergeQueue(BrowserTestCase):
+    """Test the Branch Merge Queue index page."""
+
+    layer = DatabaseFunctionalLayer
+
+    def enable_queue_flag(self):
+        getFeatureStore().add(FeatureFlag(
+            scope=u'default', flag=u'code.branchmergequeue',
+            value=u'on', priority=1))
+
+    def test_index(self):
+        """Test the index page of a branch merge queue."""
+        with person_logged_in(ANONYMOUS):
+            queue = self.factory.makeBranchMergeQueue()
+            queue_owner = queue.owner.displayname
+            queue_registrant = queue.registrant.displayname
+            queue_description = queue.description
+            queue_url = canonical_url(queue)
+
+            branch = self.factory.makeBranch()
+            branch_name = branch.bzr_identity
+            with person_logged_in(branch.owner):
+                branch.addToQueue(queue)
+
+        # XXX: rockstar - bug #666979 - The text argument should really ignore
+        # whitespace, but it currently doesn't.  Now I have two problems.
+        queue_matcher = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Page title', 'h1',
+                text=re.compile('\w*%s queue owned by %s\w*' % (
+                    queue.name, queue.owner.displayname))),
+            soupmatchers.Tag(
+                'Description Label', 'dt',
+                text=re.compile('\w*Description\w*')),
+            soupmatchers.Tag(
+                'Description Value', 'dd',
+                text=re.compile('\w*%s\w*' % queue.description)),
+            soupmatchers.Tag(
+                'Branch link', 'a',
+                text=re.compile('\w*%s\w*' % branch.bzr_identity)))
+
+        browser = self.getUserBrowser(canonical_url(queue), user=queue.owner)
+
+        self.assertThat(browser.contents, queue_matcher)
+
+    def test_create(self):
+        """Test that branch merge queues can be created from a branch."""
+        self.enable_queue_flag()
+        with person_logged_in(ANONYMOUS):
+            rockstar = self.factory.makePerson(name='rockstar')
+            branch = self.factory.makeBranch(owner=rockstar)
+            self.factory.makeBranch(product=branch.product)
+            owner_name = branch.owner.name
+
+        browser = self.getUserBrowser(canonical_url(branch), user=rockstar)
+
+        # There shouldn't be a merge queue linked here.
+        noqueue_matcher = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Not managed', 'div',
+                text=re.compile(
+                    '\w*This branch is not managed by a queue.\w*')))
+        self.assertThat(browser.contents, noqueue_matcher)
+
+        browser.getLink('Create a new queue').click()
+
+        browser.getControl('Name').value = 'libbob-queue'
+        browser.getControl('Description').value = (
+            'This is a queue for the libbob projects.')
+        browser.getControl('Create Queue').click()
+
+        self.assertEqual(
+            'http://code.launchpad.dev/~rockstar/+merge-queues/libbob-queue',
+            browser.url)
+
+    def test_create_unauthorized(self):
+        """Test that queues can't be created by unauthorized users."""
+        self.enable_queue_flag()
+        with person_logged_in(ANONYMOUS):
+            branch = self.factory.makeBranch()
+            self.factory.makeBranch(product=branch.product)
+
+        browser = self.getUserBrowser(canonical_url(branch))
+        self.assertRaises(
+            LinkNotFoundError,
+            browser.getLink,
+            'Create a new queue')
+
+    def test_create_featureflag(self):
+        """Test that the feature flag hides the "create" link."""
+        with person_logged_in(ANONYMOUS):
+            rockstar = self.factory.makePerson(name='rockstar')
+            branch = self.factory.makeBranch(owner=rockstar)
+            self.factory.makeBranch(product=branch.product)
+            owner_name = branch.owner.name
+
+        browser = self.getUserBrowser(canonical_url(branch), user=rockstar)
+        self.assertRaises(
+            LinkNotFoundError,
+            browser.getLink,
+            'Create a new queue')

=== 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 @@
         """See `IBranchMergeQueueSource`."""
         store = IMasterStore(BranchMergeQueue)
 
+        if configuration is None:
+            configuration = unicode(simplejson.dumps({}))
+
         queue = cls()
         queue.name = name
         queue.owner = owner

=== 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 @@
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  tal:define="context_menu view/context/menu:context"
+  tal:define="
+      context_menu view/context/menu:context;
+      features request/features"
   tal:condition="view/show_merge_links">
 
   <h3>Branch merges</h3>
@@ -46,6 +48,18 @@
       tal:condition="link/enabled"
       tal:replace="structure link/render"
       />
+
+    <div tal:condition="features/code.branchmergequeue">
+      <div tal:condition="not: context/merge_queue">
+        <h4>Merge Queue</h4>
+        This branch is not managed by a queue.
+        <div
+          tal:define="link context_menu/create_queue"
+          tal:condition="link/enabled"
+          tal:content="structure link/render"
+          />
+      </div>
+    </div>
   </div>
 
 </div>

=== 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 @@
+<html
+  xmlns="http://www.w3.org/1999/xhtml"
+  xmlns:tal="http://xml.zope.org/namespaces/tal"
+  xmlns:metal="http://xml.zope.org/namespaces/metal"
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
+  metal:use-macro="view/macro:page/main_side"
+  i18n:domain="launchpad"
+>
+
+<metal:side fill-slot="side">
+  <div tal:replace="structure context/@@+global-actions" />
+</metal:side>
+
+<tal:registering metal:fill-slot="registering">
+  Created by
+    <tal:registrant replace="structure context/registrant/fmt:link" />
+  on
+    <tal:created-on replace="structure context/date_created/fmt:date" />
+</tal:registering>
+
+<div metal:fill-slot="main">
+  <dl>
+    <dt>
+      Description
+    </dt>
+    <dd tal:content="context/description"></dd>
+  </dl>
+  <div tal:condition="context/branches">
+    The following branches are managed by this queue:
+    <ul>
+      <tal:branches repeat="branch context/branches">
+      <li>
+        <a tal:content="structure branch/fmt:link" />
+      </li>
+      </tal:branches>
+    </ul>
+  </div>
+</div>
+</html>

=== 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 @@
             user = self.factory.makePerson(password=password)
         naked_user = removeSecurityProxy(user)
         email = naked_user.preferredemail.email
+        if hasattr(naked_user, '_password_cleartext_cached'):
+            password = naked_user._password_cleartext_cached
         logout()
         browser = setupBrowser(
             auth="Basic %s:%s" % (str(email), password))

=== 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 @@
                 self.getUniqueString('key'): self.getUniqueString('value')}))
 
         queue = getUtility(IBranchMergeQueueSource).new(
-            name, registrant, owner, description, configuration)
+            name, owner, registrant, description, configuration)
         return queue
 
     def enableDefaultStackingForProduct(self, product, branch=None):

=== 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 @@
     # used in zcml.
     install_requires=[
         'ampoule',
+        'BeautifulSoup',
         'bzr',
         'chameleon.core',
         'chameleon.zpt',
@@ -63,6 +64,7 @@
         'RestrictedPython',
         'setproctitle',
         'setuptools',
+        'soupmatchers',
         'sourcecodegen',
         'storm',
         'testtools',

=== 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 @@
 # Alphabetical, case-insensitive, please! :-)
 
 ampoule = 0.2.0
+BeautifulSoup = 3.1.0.1
 bzr = 2.2.0
 chameleon.core = 1.0b35
 chameleon.zpt = 1.0b17
@@ -64,6 +65,7 @@
 simplejson = 2.0.9
 simplesettings = 0.4
 SimpleTal = 4.1
+soupmatchers = 0.1r53
 sourcecodegen = 0.6.9
 storm = 0.18
 testtools = 0.9.6