Merge lp:~edwin-grubbs/launchpad/bug-446074-blacklist-form into lp:launchpad

[Edwin Grubbs]

Summary
-------

Added ability for the Registry Experts team to view, add, and edit the
NameBlacklist table.

Tests
-----

./bin/test -vv -t nameblacklist

Demo and Q/A
------------

* Open http://launchpad.dev/+nameblacklist
  * Click on "Add blacklist expression"
  * It should not be possible to add "(" as an expression.
* Open http://launchpad.dev/+nameblacklist
  * Click on a yellow edit icon.
  * It should not be possible to add "(" as an expression.

[Curtis Hovey (sinzui)]

Hi Edwin.

Thank you very much for providing this fine implementation. There are many
places where I discovered you had implemented the feature as I would hope
(using RegistryEditFormView, and making the table sortable). I think there
is a chance for an oops in the code, and there are some historic code that
I hope you can clean up.

The page titles in the browser are broken, breadcrumbs are missing. This is
because the there is no breadcrumb adapter This might help:

{{{

@adapter(INameBlackListSet)
class NameBlacklistSetBreadcrumb(Breadcrumb):
    """Return a breadcrumb for an `INameBlackListSet`."""
    text = "Name Blacklist"

@adapter(INameBlackList)
class NameBlacklistBreadcrumb(Breadcrumb):
    """Return a breadcrumb for an `INameBlackList`."""

    @property
    def text(self):
        return self.context.regexp

}}}

> === added file 'lib/lp/registry/browser/nameblacklist.py'
> --- lib/lp/registry/browser/nameblacklist.py 1970-01-01 00:00:00 +0000
> +++ lib/lp/registry/browser/nameblacklist.py 2010-11-09 05:26:23 +0000
...
> +class NameBlacklistAddView(NameBlacklistValidationMixin, LaunchpadFormView):
> + """View for adding a blacklist expression."""
> +
> + schema = INameBlacklist
> + field_names = ['regexp', 'comment']
> + label = "Add a new blacklist expression"
> +
> + custom_widget('regexp', TextWidget, displayWidth=60)
> +
> + @property
> + def page_title(self):
> + """The page title."""
> + return self.label

This could be
    page_title = label

...

> +class NameBlacklistSetView(LaunchpadView):
> + """View for /+nameblacklists top level collection."""
> +
> + page_title = (
> + 'Blacklist for names of Launchpad pillars, persons, and teams')
> + label = page_title

I think
    Blacklist for names of Launchpad pillars and persons
is good enough. I think user and team are really subclasses of IPerson.
One day we will also have agent for the bots we have impersonating users.

> +class NameBlacklistSetNavigation(Navigation):
> +
> + usedfor = INameBlacklistSet
> +
> + def traverse(self, name):
> + return self.context.get(int(name))

Do I get a 404 or a 500 for
    http://launchpad.net/+nameblacklist/word

A 404 is correct. The interface says None will be returned (404), but int()
will raise an Exception. Maybe int() should be pushed into get() or this
method chooses to raise the 404 itself when the name cannot be cast.

...

> === added file 'lib/lp/registry/model/nameblacklist.py'
> --- lib/lp/registry/model/nameblacklist.py 1970-01-01 00:00:00 +0000
> +++ lib/lp/registry/model/nameblacklist.py 2010-11-09 05:26:23 +0000
...
> + def get(self, id):
> + """See `INameBlacklistSet`."""
> + store = IStore(NameBlacklist)
> + return store.find(NameBlacklist, NameBlacklist.id == id).one()

I think this method could cast the id to an int and return None when an
exception is raised to avoid a query that will always return None.

...

> === added file 'lib/lp/registry/templates/nameblacklists-index.pt'
> --- lib/lp/registry/templates/nameblacklists-index.pt 1970-01-01 00:00:00 +0000
> +++ lib/lp/registry/templates/nameblacklists-index.pt 2010-11-09 05:2...

review: Needs Fixing (code)

[Edwin Grubbs (edwin-grubbs)]

On Wed, Nov 10, 2010 at 9:42 AM, Curtis Hovey
<email address hidden> wrote:
> Review: Needs Fixing code
> Hi Edwin.
>
> Thank you very much for providing this fine implementation. There are many
> places where I discovered you had implemented the feature as I would hope
> (using RegistryEditFormView, and making the table sortable). I think there
> is a chance for an oops in the code, and there are some historic code that
> I hope you can clean up.
>
> The page titles in the browser are broken, breadcrumbs are missing. This is
> because the there is no breadcrumb adapter This might help:
>
> {{{
>
> @adapter(INameBlackListSet)
> class NameBlacklistSetBreadcrumb(Breadcrumb):
>    """Return a breadcrumb for an `INameBlackListSet`."""
>    text = "Name Blacklist"
>
> @adapter(INameBlackList)
> class NameBlacklistBreadcrumb(Breadcrumb):
>    """Return a breadcrumb for an `INameBlackList`."""
>
>    @property
>    def text(self):
>        return self.context.regexp
>
> }}}

Fixed.

>> === added file 'lib/lp/registry/browser/nameblacklist.py'
>> --- lib/lp/registry/browser/nameblacklist.py  1970-01-01 00:00:00 +0000
>> +++ lib/lp/registry/browser/nameblacklist.py  2010-11-09 05:26:23 +0000
> ...
>> +class NameBlacklistAddView(NameBlacklistValidationMixin, LaunchpadFormView):
>> +    """View for adding a blacklist expression."""
>> +
>> +    schema = INameBlacklist
>> +    field_names = ['regexp', 'comment']
>> +    label = "Add a new blacklist expression"
>> +
>> +    custom_widget('regexp', TextWidget, displayWidth=60)
>> +
>> +    @property
>> +    def page_title(self):
>> +        """The page title."""
>> +        return self.label
>
> This could be
>    page_title = label

Fixed.

> ...
>
>> +class NameBlacklistSetView(LaunchpadView):
>> +    """View for /+nameblacklists top level collection."""
>> +
>> +    page_title = (
>> +        'Blacklist for names of Launchpad pillars, persons, and teams')
>> +    label = page_title
>
> I think
>    Blacklist for names of Launchpad pillars and persons
> is good enough. I think user and team are really subclasses of IPerson.
> One day we will also have agent for the bots we have impersonating users.

Fixed.

>> +class NameBlacklistSetNavigation(Navigation):
>> +
>> +    usedfor = INameBlacklistSet
>> +
>> +    def traverse(self, name):
>> +        return self.context.get(int(name))
>
> Do I get a 404 or a 500 for
>    http://launchpad.net/+nameblacklist/word
>
> A 404 is correct. The interface says None will be returned (404), but int()
> will raise an Exception. Maybe int() should be pushed into get() or this
> method chooses to raise the 404 itself when the name cannot be cast.

Fixed.

> ...
>
>> === added file 'lib/lp/registry/model/nameblacklist.py'
>> --- lib/lp/registry/model/nameblacklist.py    1970-01-01 00:00:00 +0000
>> +++ lib/lp/registry/model/nameblacklist.py    2010-11-09 05:26:23 +0000
> ...
>> +    def get(self, id):
>> +        """See `INameBlacklistSet`."""
>> +        store = IStore(NameBlacklist)
>> +        return store.find(NameBlacklist, NameBlacklist.id == id).one()
>
> I think this method could cast the id to an int and return None when an
> exception is raised to av...

[Curtis Hovey (sinzui)]

Thanks for addressing my concerns.

review: Approve (code)

[Robert Collins (lifeless)]

>> + self.failUnlessEqual(
>> + self.name_blacklist_match("barfoo"),
>> + self.foo_exp.id)
>
> from https://dev.launchpad.net/TestsStyleGuide:
> When asserting for equality use the form assertEqual
> Can you update all these old tests to assertEqual?

This might be nicer still as

self.assertThat('barfoo', IsBlacklisted())

(with a trivial IsBlacklisted matcher).

-Rob

=== modified file 'lib/canonical/launchpad/browser/launchpad.py'
--- lib/canonical/launchpad/browser/launchpad.py	2010-11-02 20:10:56 +0000
+++ lib/canonical/launchpad/browser/launchpad.py	2010-11-12 06:04:05 +0000
@@ -89,6 +89,14 @@
     INavigationMenu,
     )
 from canonical.launchpad.webapp.publisher import RedirectionView
+from canonical.launchpad.webapp.url import urlappend
+from canonical.launchpad.webapp.vhosts import allvhosts
+from canonical.lazr import (
+    ExportedFolder,
+    ExportedImageFolder,
+    )
+from canonical.widgets.project import ProjectScopeWidget
+from lp.answers.interfaces.questioncollection import IQuestionSet
 # XXX SteveAlexander 2005-09-22: this is imported here because there is no
 #     general timedelta to duration format adapter available.  This should
 #     be factored out into a generally available adapter for both this
@@ -99,14 +107,6 @@
     MenuAPI,
     PageTemplateContextsAPI,
     )
-from canonical.launchpad.webapp.url import urlappend
-from canonical.launchpad.webapp.vhosts import allvhosts
-from canonical.lazr import (
-    ExportedFolder,
-    ExportedImageFolder,
-    )
-from canonical.widgets.project import ProjectScopeWidget
-from lp.answers.interfaces.questioncollection import IQuestionSet
 from lp.app.errors import (
     GoneError,
     NotFoundError,
@@ -131,6 +131,7 @@
 from lp.registry.interfaces.codeofconduct import ICodeOfConductSet
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.karma import IKarmaActionSet
+from lp.registry.interfaces.nameblacklist import INameBlacklistSet
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.pillar import IPillarNameSet
 from lp.registry.interfaces.product import (
@@ -583,6 +584,7 @@
         'karmaaction': IKarmaActionSet,
         '+imports': ITranslationImportQueue,
         '+languages': ILanguageSet,
+        '+nameblacklist': INameBlacklistSet,
         'package-sets': IPackagesetSet,
         'people': IPersonSet,
         'pillars': IPillarNameSet,

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2010-10-21 01:42:14 +0000
+++ lib/canonical/launchpad/security.py	2010-11-12 06:04:05 +0000
@@ -51,6 +51,7 @@
     )
 from lp.blueprints.interfaces.sprint import ISprint
 from lp.blueprints.interfaces.sprintspecification import ISprintSpecification
+from lp.bugs.interfaces.bugtarget import IOfficialBugTagTargetRestricted
 from lp.buildmaster.interfaces.builder import (
     IBuilder,
     IBuilderSet,
@@ -61,7 +62,6 @@
     IBuildFarmJobOld,
     )
 from lp.buildmaster.interfaces.packagebuild import IPackageBuild
-from lp.bugs.interfaces.bugtarget import IOfficialBugTagTargetRestricted
 from lp.code.interfaces.branch import (
     IBranch,
     user_has_special_branch_access,
@@ -114,6 +114,10 @@
     IMilestone,
     IProjectGroupMilestone,
     )
+from lp.registry.interfaces.nameblacklist import (
+    INameBlacklist,
+    INameBlacklistSet,
+    )
 from lp.registry.interfaces.packaging import IPackaging
 from lp.registry.interfaces.person import (
     IPerson,
@@ -1664,6 +1668,26 @@
         return team in user.person.getAdministratedTeams()
 
 
+class ViewNameBlacklist(EditByRegistryExpertsOrAdmins):
+    permission = 'launchpad.View'
+    usedfor = INameBlacklist
+
+
+class EditNameBlacklist(EditByRegistryExpertsOrAdmins):
+    permission = 'launchpad.Edit'
+    usedfor = INameBlacklist
+
+
+class ViewNameBlacklistSet(EditByRegistryExpertsOrAdmins):
+    permission = 'launchpad.View'
+    usedfor = INameBlacklistSet
+
+
+class EditNameBlacklistSet(EditByRegistryExpertsOrAdmins):
+    permission = 'launchpad.Edit'
+    usedfor = INameBlacklistSet
+
+
 class ViewLanguageSet(AnonymousAuthorization):
     """Anyone can view an ILangaugeSet."""
     usedfor = ILanguageSet

=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2010-10-11 09:35:29 +0000
+++ lib/lp/registry/browser/configure.zcml	2010-11-12 06:04:05 +0000
@@ -1560,6 +1560,57 @@
         class="lp.registry.browser.product.ProductReviewLicenseView"
         permission="launchpad.Moderate"
         template="../templates/product-review-license.pt"/>
+
+    <browser:page
+        for="lp.registry.interfaces.nameblacklist.INameBlacklist"
+        permission="launchpad.Edit"
+        facet="overview"
+        class="lp.registry.browser.nameblacklist.NameBlacklistEditView"
+        name="+edit"
+        template="../../app/templates/generic-edit.pt"/>
+    <browser:url
+        for="lp.registry.interfaces.nameblacklist.INameBlacklistSet"
+        path_expression="string:+nameblacklist"
+        parent_utility="canonical.launchpad.webapp.interfaces.ILaunchpadRoot"
+        />
+    <browser:url
+        for="lp.registry.interfaces.nameblacklist.INameBlacklist"
+        path_expression="string:${id}"
+        parent_utility="lp.registry.interfaces.nameblacklist.INameBlacklistSet"
+        />
+    <browser:defaultView
+        for="lp.registry.interfaces.nameblacklist.INameBlacklistSet"
+        name="+index"/>
+    <browser:page
+        for="lp.registry.interfaces.nameblacklist.INameBlacklistSet"
+        permission="launchpad.View"
+        facet="overview"
+        class="lp.registry.browser.nameblacklist.NameBlacklistSetView"
+        name="+index"
+        template="../templates/nameblacklists-index.pt"/>
+    <browser:page
+        for="lp.registry.interfaces.nameblacklist.INameBlacklistSet"
+        permission="launchpad.Edit"
+        facet="overview"
+        class="lp.registry.browser.nameblacklist.NameBlacklistAddView"
+        name="+add"
+        template="../../app/templates/generic-edit.pt"/>
+    <browser:navigation
+        module="lp.registry.browser.nameblacklist"
+        classes="NameBlacklistSetNavigation"/>
+    <browser:menus
+        classes="
+            NameBlacklistNavigationMenu
+            NameBlacklistSetNavigationMenu
+            "
+        module="lp.registry.browser.nameblacklist"/>
+    <adapter
+        factory="lp.registry.browser.nameblacklist.NameBlacklistSetBreadcrumb"
+        />
+    <adapter
+        factory="lp.registry.browser.nameblacklist.NameBlacklistBreadcrumb"
+        />
+
     <browser:page
         name="+addseries"
         for="lp.registry.interfaces.product.IProduct"

=== added file 'lib/lp/registry/browser/nameblacklist.py'
--- lib/lp/registry/browser/nameblacklist.py	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/browser/nameblacklist.py	2010-11-12 06:04:05 +0000
@@ -0,0 +1,175 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+__all__ = [
+    'NameBlacklistAddView',
+    'NameBlacklistEditView',
+    'NameBlacklistNavigationMenu',
+    'NameBlacklistSetNavigationMenu',
+    'NameBlacklistSetView',
+    ]
+
+import re
+
+from zope.app.form.browser import TextWidget
+from zope.component import (
+    adapter,
+    getUtility,
+    )
+from zope.interface import implements
+
+from canonical.launchpad.webapp import action
+from canonical.launchpad.webapp.breadcrumb import Breadcrumb
+from canonical.launchpad.webapp.interfaces import IBreadcrumb
+from canonical.launchpad.webapp.launchpadform import (
+    custom_widget,
+    LaunchpadFormView,
+    )
+from canonical.launchpad.webapp.menu import (
+    ApplicationMenu,
+    enabled_with_permission,
+    Link,
+    NavigationMenu,
+    )
+from canonical.launchpad.webapp.publisher import (
+    canonical_url,
+    LaunchpadView,
+    Navigation,
+    )
+from lp.registry.browser import RegistryEditFormView
+from lp.registry.interfaces.nameblacklist import (
+    INameBlacklist,
+    INameBlacklistSet,
+    )
+
+
+class NameBlacklistValidationMixin:
+    """Validate regular expression when adding or editing."""
+
+    def validate(self, data):
+        """Validate regular expression."""
+        regexp = data['regexp']
+        try:
+            re.compile(regexp)
+            name_blacklist_set = getUtility(INameBlacklistSet)
+            if (INameBlacklistSet.providedBy(self.context)
+                or self.context.regexp != regexp):
+                # Check if the regular expression already exists if a
+                # new expression is being created or if an existing
+                # regular expression has been modified.
+                if name_blacklist_set.getByRegExp(regexp) is not None:
+                    self.setFieldError(
+                        'regexp',
+                        'This regular expression already exists.')
+        except re.error, e:
+            self.setFieldError(
+                'regexp',
+                'Invalid regular expression: %s' % e)
+
+
+class NameBlacklistEditView(NameBlacklistValidationMixin,
+                            RegistryEditFormView):
+    """View for editing a blacklist expression."""
+
+    schema = INameBlacklist
+    field_names = ['regexp', 'comment']
+    label = "Edit a blacklist expression"
+    page_title = label
+
+    @property
+    def cancel_url(self):
+        return canonical_url(getUtility(INameBlacklistSet))
+
+    next_url = cancel_url
+
+
+class NameBlacklistAddView(NameBlacklistValidationMixin, LaunchpadFormView):
+    """View for adding a blacklist expression."""
+
+    schema = INameBlacklist
+    field_names = ['regexp', 'comment']
+    label = "Add a new blacklist expression"
+    page_title = label
+
+    custom_widget('regexp', TextWidget, displayWidth=60)
+
+    @property
+    def cancel_url(self):
+        """See `LaunchpadFormView`."""
+        return canonical_url(self.context)
+
+    next_url = cancel_url
+
+    @action("Add to blacklist", name='add')
+    def add_action(self, action, data):
+        name_blacklist_set = getUtility(INameBlacklistSet)
+        name_blacklist_set.create(
+            regexp=data['regexp'],
+            comment=data['comment'],
+            )
+        self.request.response.addInfoNotification(
+            'Regular expression "%s" has been added to the name blacklist.'
+            % data['regexp'])
+
+
+class NameBlacklistSetView(LaunchpadView):
+    """View for /+nameblacklists top level collection."""
+
+    label = (
+        'Blacklist for names of Launchpad pillars and persons')
+    page_title = label
+
+
+class NameBlacklistSetNavigation(Navigation):
+
+    usedfor = INameBlacklistSet
+
+    def traverse(self, name):
+        return self.context.get(name)
+
+
+class NameBlacklistSetNavigationMenu(NavigationMenu):
+    """Action menu for NameBlacklistSet."""
+    usedfor = INameBlacklistSet
+    facet = 'overview'
+    links = [
+        'add_blacklist_expression',
+        ]
+
+    @enabled_with_permission('launchpad.Edit')
+    def add_blacklist_expression(self):
+        return Link('+add', 'Add blacklist expression', icon='add')
+
+
+class NameBlacklistNavigationMenu(ApplicationMenu):
+    """Action menu for NameBlacklist."""
+    usedfor = INameBlacklist
+    facet = 'overview'
+    links = [
+        'edit_blacklist_expression',
+        ]
+
+    @enabled_with_permission('launchpad.Edit')
+    def edit_blacklist_expression(self):
+        return Link('+edit', 'Edit blacklist expression', icon='edit')
+
+
+@adapter(INameBlacklistSet)
+class NameBlacklistSetBreadcrumb(Breadcrumb):
+    """Return a breadcrumb for an `INameBlackListSet`."""
+
+    implements(IBreadcrumb)
+
+    text = "Name Blacklist"
+
+
+@adapter(INameBlacklist)
+class NameBlacklistBreadcrumb(Breadcrumb):
+    """Return a breadcrumb for an `INameBlackList`."""
+
+    implements(IBreadcrumb)
+
+    @property
+    def text(self):
+        return self.context.regexp

=== added file 'lib/lp/registry/browser/tests/nameblacklist-views.txt'
--- lib/lp/registry/browser/tests/nameblacklist-views.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/browser/tests/nameblacklist-views.txt	2010-11-12 06:04:05 +0000
@@ -0,0 +1,96 @@
+NameBlacklist pages
+===================
+
+    >>> from zope.component import getUtility
+    >>> from lp.testing.sampledata import ADMIN_EMAIL
+    >>> from canonical.launchpad.interfaces.launchpad import (
+    ...     ILaunchpadCelebrities)
+    >>> from lp.registry.interfaces.nameblacklist import INameBlacklistSet
+    >>> name_blacklist_set = getUtility(INameBlacklistSet)
+    >>> from canonical.launchpad.testing.pages import (
+    ...     extract_text, find_tag_by_id)
+    >>> registry_experts = getUtility(ILaunchpadCelebrities).registry_experts
+    >>> registry_expert = factory.makePerson()
+    >>> login(ADMIN_EMAIL)
+    >>> ignore = registry_experts.addMember(registry_expert, registry_expert)
+
+
+View all
+--------
+
+All the blacklisted regular expressions that filter pillar names and
+person names can be seen on the /+nameblacklist page.
+
+    >>> login_person(registry_expert)
+    >>> view = create_initialized_view(name_blacklist_set, '+index',
+    ...                                principal=registry_expert)
+    >>> print extract_text(find_tag_by_id(view.render(), 'blacklist'))
+    Regular Expression                      Comment
+    ^admin Edit blacklist expression
+    blacklist Edit blacklist expression     For testing purposes
+
+
+Add expression to blacklist
+---------------------------
+
+An invalid regular expression cannot be added.
+
+    >>> form = {
+    ...     'field.regexp': u'(',
+    ...     'field.comment': u'old-comment',
+    ...     'field.actions.add': 'Add to blacklist',
+    ...     }
+    >>> view = create_initialized_view(name_blacklist_set, '+add', form=form)
+    >>> for error in view.errors:
+    ...     print error
+    Invalid regular expression: unbalanced parenthesis
+
+A duplicate regular expression cannot be added.
+
+    >>> form['field.regexp'] = u'blacklist'
+    >>> view = create_initialized_view(name_blacklist_set, '+add', form=form)
+    >>> for error in view.errors:
+    ...     print error
+    This regular expression already exists.
+
+After adding a regular expression, a notification will be displayed.
+
+    >>> form['field.regexp'] = u'foo'
+    >>> view = create_initialized_view(name_blacklist_set, '+add', form=form)
+    >>> for notification in view.request.response.notifications:
+    ...     print notification.message
+    Regular expression "foo" has been added to the name blacklist.
+
+
+Edit expression in blacklist
+----------------------------
+
+When a regular expression is edited, it still must be valid.
+
+    >>> import transaction
+    >>> transaction.commit()
+    >>> foo_exp = name_blacklist_set.getByRegExp(u'foo')
+    >>> form = {
+    ...     'field.regexp': u'(',
+    ...     'field.comment': u'new-comment',
+    ...     'field.actions.change': 'Change',
+    ...     }
+    >>> view = create_initialized_view(foo_exp, '+edit', form=form)
+    >>> for error in view.errors:
+    ...     print error
+    Invalid regular expression: unbalanced parenthesis
+
+It cannot changed to conflict with another regular expression.
+
+    >>> form['field.regexp'] = u'blacklist'
+    >>> view = create_initialized_view(foo_exp, '+edit', form=form)
+    >>> for error in view.errors:
+    ...     print error
+    This regular expression already exists.
+
+Otherwise, the change will be successful.
+
+    >>> form['field.regexp'] = u'bar'
+    >>> view = create_initialized_view(foo_exp, '+edit', form=form)
+    >>> print foo_exp.regexp, foo_exp.comment
+    bar new-comment

=== modified file 'lib/lp/registry/browser/tests/test_peoplemerge.py'
--- lib/lp/registry/browser/tests/test_peoplemerge.py	2010-11-01 20:39:17 +0000
+++ lib/lp/registry/browser/tests/test_peoplemerge.py	2010-11-12 06:04:05 +0000
@@ -128,7 +128,9 @@
         # Super team memberships are removed.
         self.target_team = getUtility(ILaunchpadCelebrities).registry_experts
         super_team = self.factory.makeTeam()
+        login_celebrity('admin')
         self.dupe_team.join(super_team, self.dupe_team.teamowner)
+        login_celebrity('registry_experts')
         view = self.getView()
         self.assertEqual([], view.errors)
         self.assertEqual(self.target_team, self.dupe_team.merged)

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2010-11-09 16:26:11 +0000
+++ lib/lp/registry/configure.zcml	2010-11-12 06:04:05 +0000
@@ -102,6 +102,29 @@
             interface="lp.registry.interfaces.persontransferjob.IMembershipNotificationJob"/>
     </class>
 
+    <!-- INameBlacklist -->
+    <securedutility
+        class="lp.registry.model.nameblacklist.NameBlacklistSet"
+        provides="lp.registry.interfaces.nameblacklist.INameBlacklistSet">
+        <allow
+            interface="lp.registry.interfaces.nameblacklist.INameBlacklistSet"/>
+    </securedutility>
+
+    <class class="lp.registry.model.nameblacklist.NameBlacklistSet">
+        <require
+            permission="launchpad.Edit"
+            interface="lp.registry.interfaces.nameblacklist.INameBlacklistSet"/>
+    </class>
+
+    <class class="lp.registry.model.nameblacklist.NameBlacklist">
+        <require
+            permission="launchpad.View"
+            interface="lp.registry.interfaces.nameblacklist.INameBlacklist"/>
+        <require
+            permission="launchpad.Edit"
+            set_schema="lp.registry.interfaces.nameblacklist.INameBlacklist"/>
+    </class>
+
     <!-- Location -->
 
     <class

=== added file 'lib/lp/registry/interfaces/nameblacklist.py'
--- lib/lp/registry/interfaces/nameblacklist.py	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/interfaces/nameblacklist.py	2010-11-12 06:04:05 +0000
@@ -0,0 +1,44 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""NameBlacklist interfaces."""
+
+__metaclass__ = type
+
+__all__ = [
+    'INameBlacklist',
+    'INameBlacklistSet',
+    ]
+
+from zope.interface import Interface
+from zope.schema import (
+    Int,
+    Text,
+    TextLine,
+    )
+
+from canonical.launchpad import _
+
+
+class INameBlacklist(Interface):
+    """The interface for the NameBlacklist table."""
+
+    id = Int(title=_('ID'), required=True, readonly=True)
+    regexp = TextLine(title=_('Regular expression'), required=True)
+    comment = Text(title=_('Comment'), required=False)
+
+
+class INameBlacklistSet(Interface):
+    """The set of INameBlacklist objects."""
+
+    def getAll():
+        """Return all the name blacklist expressions."""
+
+    def create(regexp, comment=None):
+        """Create and return a new NameBlacklist with given arguments."""
+
+    def get(id):
+        """Return the NameBlacklist with the given id or None."""
+
+    def getByRegExp(regexp):
+        """Return the NameBlacklist with the given regexp or None."""

=== added file 'lib/lp/registry/model/nameblacklist.py'
--- lib/lp/registry/model/nameblacklist.py	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/model/nameblacklist.py	2010-11-12 06:04:05 +0000
@@ -0,0 +1,70 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Classes for managing the NameBlacklist table."""
+
+__metaclass__ = type
+__all__ = [
+    'NameBlacklist',
+    'NameBlacklistSet',
+    ]
+
+
+from storm.base import Storm
+from storm.locals import (
+    Int,
+    Unicode,
+    )
+from zope.interface import implements
+
+from canonical.launchpad.interfaces.lpstorm import IStore
+from lp.registry.interfaces.nameblacklist import (
+    INameBlacklist,
+    INameBlacklistSet,
+    )
+
+
+class NameBlacklist(Storm):
+    """Class for the NameBlacklist table."""
+
+    implements(INameBlacklist)
+
+    __storm_table__ = 'NameBlacklist'
+
+    id = Int(primary=True)
+    regexp = Unicode(name='regexp', allow_none=False)
+    comment = Unicode(name='comment', allow_none=True)
+
+
+class NameBlacklistSet:
+    """Class for creating and retrieving NameBlacklist objects."""
+
+    implements(INameBlacklistSet)
+
+    def getAll(self):
+        """See `INameBlacklistSet`."""
+        store = IStore(NameBlacklist)
+        return store.find(NameBlacklist).order_by(NameBlacklist.regexp)
+
+    def create(self, regexp, comment=None):
+        """See `INameBlacklistSet`."""
+        nameblacklist = NameBlacklist()
+        nameblacklist.regexp = regexp
+        nameblacklist.comment = comment
+        store = IStore(NameBlacklist)
+        store.add(nameblacklist)
+        return nameblacklist
+
+    def get(self, id):
+        """See `INameBlacklistSet`."""
+        try:
+            id = int(id)
+        except ValueError:
+            return None
+        store = IStore(NameBlacklist)
+        return store.find(NameBlacklist, NameBlacklist.id == id).one()
+
+    def getByRegExp(self, regexp):
+        """See `INameBlacklistSet`."""
+        store = IStore(NameBlacklist)
+        return store.find(NameBlacklist, NameBlacklist.regexp == regexp).one()

=== renamed file 'lib/canonical/launchpad/pagetests/standalone/xx-nameblacklist.txt' => 'lib/lp/registry/stories/object/xx-nameblacklist.txt'
=== added file 'lib/lp/registry/templates/nameblacklists-index.pt'
--- lib/lp/registry/templates/nameblacklists-index.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/templates/nameblacklists-index.pt	2010-11-12 06:04:05 +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"
+  xml:lang="en"
+  lang="en"
+  dir="ltr"
+  metal:use-macro="view/macro:page/main_side"
+  i18n:domain="launchpad">
+
+  <body>
+
+    <tal:side metal:fill-slot="side">
+      <tal:menu replace="structure context/@@+global-actions" />
+    </tal:side>
+
+    <div metal:fill-slot="main" class="main-portlet">
+      <table id="blacklist" class="listing sortable">
+        <thead>
+          <th>Regular Expression</th>
+          <th>Comment</th>
+        </thead>
+        <tbody>
+          <tr tal:repeat="item context/getAll">
+            <td>
+              <tt tal:content="item/regexp"/>
+              <tal:link replace="
+                structure
+                item/menu:overview/edit_blacklist_expression/fmt:icon"/>
+            </td>
+            <td tal:content="item/comment"/>
+          </tr>
+        </tbody>
+      </table>
+    </div>
+
+  </body>
+</html>

=== modified file 'lib/lp/registry/tests/test_nameblacklist.py'
--- lib/lp/registry/tests/test_nameblacklist.py	2010-10-04 19:50:45 +0000
+++ lib/lp/registry/tests/test_nameblacklist.py	2010-11-12 06:04:05 +0000
@@ -5,42 +5,52 @@
 
 __metaclass__ = type
 
-import unittest
-
-from canonical.testing.layers import LaunchpadLayer
-
-
-class TestNameBlacklist(unittest.TestCase):
-    layer = LaunchpadLayer
+
+from zope.component import getUtility
+from zope.interface.verify import verifyObject
+
+from canonical.launchpad.interfaces.lpstorm import IStore
+from canonical.launchpad.webapp.authorization import check_permission
+from canonical.testing.layers import (
+    DatabaseFunctionalLayer,
+    ZopelessDatabaseLayer,
+    )
+from lp.registry.interfaces.nameblacklist import (
+    INameBlacklist,
+    INameBlacklistSet,
+    )
+from lp.testing import (
+    ANONYMOUS,
+    login,
+    login_celebrity,
+    TestCaseWithFactory,
+    )
+
+
+class TestNameBlacklist(TestCaseWithFactory):
+    layer = ZopelessDatabaseLayer
 
     def setUp(self):
-        self.con = self.layer.connect()
-        self.cur = self.con.cursor()
-
-        # Create a couple of blacklist entries
-        self.cur.execute("""
-            INSERT INTO NameBlacklist(id, regexp) VALUES (-200, '^foo')
-            """)
-        self.cur.execute("""
-            INSERT INTO NameBlacklist(id, regexp) VALUES (-100, 'foo')
-            """)
-        self.cur.execute("""
-            INSERT INTO NameBlacklist(id, regexp) VALUES (-50, 'v e r b o s e')
-            """)
-
-    def tearDown(self):
-        self.con.close()
+        super(TestNameBlacklist, self).setUp()
+        self.name_blacklist_set = getUtility(INameBlacklistSet)
+        self.caret_foo_exp = self.name_blacklist_set.create(u'^foo')
+        self.foo_exp = self.name_blacklist_set.create(u'foo')
+        self.verbose_exp = self.name_blacklist_set.create(u'v e r b o s e')
+        self.store = IStore(self.foo_exp)
+        self.store.flush()
 
     def name_blacklist_match(self, name):
         '''Return the result of the name_blacklist_match stored procedure.'''
-        self.cur.execute("SELECT name_blacklist_match(%(name)s)", vars())
-        return self.cur.fetchone()[0]
+        result = self.store.execute(
+            "SELECT name_blacklist_match(%s)", (name,))
+        return result.get_one()[0]
 
     def is_blacklisted_name(self, name):
         '''Call the is_blacklisted_name stored procedure and return the result
         '''
-        self.cur.execute("SELECT is_blacklisted_name(%(name)s)", vars())
-        blacklisted = self.cur.fetchone()[0]
+        result = self.store.execute(
+            "SELECT is_blacklisted_name(%s)", (name,))
+        blacklisted = result.get_one()[0]
         self.failIf(blacklisted is None, 'is_blacklisted_name returned NULL')
         return bool(blacklisted)
 
@@ -52,21 +62,25 @@
         # A name that is blacklisted returns the id of the row in the
         # NameBlacklist table that matched. Rows are tried in order, and the
         # first match is returned.
-        self.failUnlessEqual(self.name_blacklist_match("foobar"), -200)
-        self.failUnlessEqual(self.name_blacklist_match("barfoo"), -100)
+        self.assertEqual(
+            self.name_blacklist_match("foobar"),
+            self.caret_foo_exp.id)
+        self.assertEqual(
+            self.name_blacklist_match("barfoo"),
+            self.foo_exp.id)
 
     def test_name_blacklist_match_cache(self):
         # If the blacklist is changed in the DB, these changes are noticed.
         # This test is needed because the stored procedure keeps a cache
         # of the compiled regular expressions.
-        self.failUnlessEqual(self.name_blacklist_match("foobar"), -200)
-        self.cur.execute(
-                "UPDATE NameBlacklist SET regexp='nomatch' where id=-200"
-                )
-        self.failUnlessEqual(self.name_blacklist_match("foobar"), -100)
-        self.cur.execute(
-                "UPDATE NameBlacklist SET regexp='nomatch2' where id=-100"
-                )
+        self.assertEqual(
+            self.name_blacklist_match("foobar"),
+            self.caret_foo_exp.id)
+        self.caret_foo_exp.regexp = u'nomatch'
+        self.assertEqual(
+            self.name_blacklist_match("foobar"),
+            self.foo_exp.id)
+        self.foo_exp.regexp = u'nomatch2'
         self.failUnless(self.name_blacklist_match("foobar") is None)
 
     def test_is_blacklisted_name(self):
@@ -74,7 +88,8 @@
         # that is friendlier to use in a boolean context.
         self.failUnless(self.is_blacklisted_name("bar") is False)
         self.failUnless(self.is_blacklisted_name("foo") is True)
-        self.cur.execute("UPDATE NameBlacklist SET regexp='bar' || regexp")
+        self.caret_foo_exp.regexp = u'bar'
+        self.foo_exp.regexp = u'bar2'
         self.failUnless(self.is_blacklisted_name("foo") is False)
 
     def test_case_insensitive(self):
@@ -85,5 +100,82 @@
         self.failUnless(self.is_blacklisted_name("verbose") is True)
 
 
-def test_suite():
-    return unittest.TestLoader().loadTestsFromName(__name__)
+class TestNameBlacklistSet(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestNameBlacklistSet, self).setUp()
+        login_celebrity('registry_experts')
+        self.name_blacklist_set = getUtility(INameBlacklistSet)
+
+    def test_create_with_one_arg(self):
+        # Test NameBlacklistSet.create(regexp).
+        name_blacklist = self.name_blacklist_set.create(u'foo')
+        self.assertTrue(verifyObject(INameBlacklist, name_blacklist))
+        self.assertEqual(u'foo', name_blacklist.regexp)
+        self.assertIs(None, name_blacklist.comment)
+
+    def test_create_with_two_args(self):
+        # Test NameBlacklistSet.create(regexp, comment).
+        name_blacklist = self.name_blacklist_set.create(u'foo', u'bar')
+        self.assertTrue(verifyObject(INameBlacklist, name_blacklist))
+        self.assertEqual(u'foo', name_blacklist.regexp)
+        self.assertEqual(u'bar', name_blacklist.comment)
+
+    def test_get_int(self):
+        # Test NameBlacklistSet.get() with int id.
+        name_blacklist = self.name_blacklist_set.create(u'foo', u'bar')
+        store = IStore(name_blacklist)
+        store.flush()
+        retrieved = self.name_blacklist_set.get(name_blacklist.id)
+        self.assertEqual(name_blacklist, retrieved)
+
+    def test_get_string(self):
+        # Test NameBlacklistSet.get() with string id.
+        name_blacklist = self.name_blacklist_set.create(u'foo', u'bar')
+        store = IStore(name_blacklist)
+        store.flush()
+        retrieved = self.name_blacklist_set.get(str(name_blacklist.id))
+        self.assertEqual(name_blacklist, retrieved)
+
+    def test_get_returns_None_instead_of_ValueError(self):
+        # Test that NameBlacklistSet.get() will return None instead of
+        # raising a ValueError when it tries to cast the id to an int,
+        # so that traversing an invalid url causes a Not Found error
+        # instead of an error that is recorded as an oops.
+        self.assertIs(None, self.name_blacklist_set.get('asdf'))
+
+    def test_getAll(self):
+        # Test NameBlacklistSet.getAll().
+        result = [
+            (item.regexp, item.comment)
+            for item in self.name_blacklist_set.getAll()]
+        expected = [
+            ('^admin', None),
+            ('blacklist', 'For testing purposes'),
+            ]
+        self.assertEqual(expected, result)
+
+    def test_NameBlacklistSet_permissions(self):
+        # Verify that non-registry-experts do not have permission to
+        # access the NameBlacklistSet.
+        self.assertTrue(
+            check_permission('launchpad.View', self.name_blacklist_set))
+        self.assertTrue(
+            check_permission('launchpad.Edit', self.name_blacklist_set))
+        login(ANONYMOUS)
+        self.assertFalse(
+            check_permission('launchpad.View', self.name_blacklist_set))
+        self.assertFalse(
+            check_permission('launchpad.Edit', self.name_blacklist_set))
+
+    def test_NameBlacklist_permissions(self):
+        # Verify that non-registry-experts do not have permission to
+        # access the NameBlacklist.
+        name_blacklist = self.name_blacklist_set.create(u'foo')
+        self.assertTrue(check_permission('launchpad.View', name_blacklist))
+        self.assertTrue(check_permission('launchpad.Edit', name_blacklist))
+        login(ANONYMOUS)
+        self.assertFalse(check_permission('launchpad.View', name_blacklist))
+        self.assertFalse(check_permission('launchpad.Edit', name_blacklist))

=== modified file 'lib/lp/soyuz/tests/test_archive_subscriptions.py'
--- lib/lp/soyuz/tests/test_archive_subscriptions.py	2010-10-26 15:47:24 +0000
+++ lib/lp/soyuz/tests/test_archive_subscriptions.py	2010-11-12 06:04:05 +0000
@@ -72,6 +72,11 @@
         with celebrity_logged_in('commercial_admin'):
             self.archive.commercial = True
 
+        # Logging in as a celebrity team causes an email to be sent
+        # because a person is added as a member of the team, so this
+        # needs to be cleared out before calling newSubscription().
+        pop_notifications()
+
         self.archive.newSubscription(
             self.subscriber, registrant=self.archive.owner)
 

=== modified file 'lib/lp/testing/_login.py'
--- lib/lp/testing/_login.py	2010-10-26 15:47:24 +0000
+++ lib/lp/testing/_login.py	2010-11-12 06:04:05 +0000
@@ -23,7 +23,6 @@
     ]
 
 from contextlib import contextmanager
-import random
 
 from zope.component import getUtility
 from zope.security.management import endInteraction
@@ -39,10 +38,12 @@
 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 from canonical.launchpad.webapp.vhosts import allvhosts
 from lp.services.utils import decorate_with
+from lp.testing.sampledata import ADMIN_EMAIL
 
 
 _logged_in = False
 
+
 def is_logged_in():
     global _logged_in
     return _logged_in
@@ -93,24 +94,15 @@
     setupInteractionForPerson(person, participation)
 
 
-def _get_arbitrary_team_member(team):
-    """Get an arbitrary member of 'team'.
-
-    :param team: An `ITeam`.
-    """
-    # Set up the interaction.
-    login(ANONYMOUS)
-    return random.choice(list(team.allmembers))
-
-
 def login_team(team, participation=None):
     """Login as a member of 'team'."""
-    # This check isn't strictly necessary (it depends on the implementation of
-    # _get_arbitrary_team_member), but this gives us a nice error message,
-    # which can save time when debugging.
+    # Prevent import loop.
+    from lp.testing.factory import LaunchpadObjectFactory
     if not team.is_team:
         raise ValueError("Got person, expected team: %r" % (team,))
-    person = _get_arbitrary_team_member(team)
+    login(ADMIN_EMAIL)
+    person = LaunchpadObjectFactory().makePerson()
+    team.addMember(person, person)
     login_person(person, participation=participation)
     return person