Merge lp:~bac/launchpad/bug-674897 into lp:launchpad

[Brad Crittenden]

[r=gmb][ui=none][bug=674897] Remove intermediate breadcrumb from name blacklist edit page as it is not traversable.

= Summary =

The breadcrumbs for editing a NameBlackList regex have a link to a
non-traversable element.

== Proposed fix ==

Remove that part of the breadcrumb chain.

== Pre-implementation notes ==

Chat with Curtis.

== Implementation details ==

Easy fix; remove the adapter and the breadcrumb isn't created for that
portion of the traversal.

== Tests ==

bin/test -vvm lp.registry -t test_breadcrumbs

== Demo and Q/A ==

Go to https://launchpad.dev/+nameblacklist, edit an existing one and
verify the breadcrumb does not include the regex.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/browser/configure.zcml
  lib/canonical/launchpad/pagetests/REFERENCE.txt
  lib/lp/registry/browser/tests/test_breadcrumbs.py
  lib/lp/registry/browser/nameblacklist.py

[Graham Binns (gmb)]

review: Approve (code)

=== modified file 'lib/canonical/launchpad/pagetests/REFERENCE.txt'
--- lib/canonical/launchpad/pagetests/REFERENCE.txt	2008-09-23 13:02:41 +0000
+++ lib/canonical/launchpad/pagetests/REFERENCE.txt	2010-11-30 15:31:40 +0000
@@ -1,4 +1,5 @@
-= Kiko's wonderfully incomplete testbrowser reference manual for dummies =
+Kiko's wonderfully incomplete testbrowser reference manual for dummies
+======================================================================
 
 Opening pages:
 
@@ -31,7 +32,8 @@
     True
 
 
-== Debugging ==
+Debugging
+---------
 
 If you need to debug a pagetest, you can use the convenient 'stop()' method
 exported into pagetest globals.  This is essentially the same as importing pdb
@@ -49,7 +51,8 @@
 appserver will be unhappy.
 
 
-== Parsing content ==
+Parsing content
+---------------
 
 Parsing specific sections of pages:
 
@@ -73,7 +76,7 @@
     >>> print extract_text(html_string, extract_image_text=True)
     Here's some text with an image.
 
-    >>> html_string = "<img src="/@@/bar" alt="Alt text works too." />"
+    >>> html_string = """<img src="/@@/bar" alt="Alt text works too." />"""
     >>> print extract_text(html_string, extract_image_text=True)
     Alt text works too.
 
@@ -110,21 +113,22 @@
 
 Using BeautifulSoup in all its glory:
 
-  >>> from BeautifulSoup import BeautifulSoup, SoupStrainer
-  >>> anon_browser.open("http://launchpad.dev/~name16")
-  >>> strainer = SoupStrainer(['link'], {'type': ['application/rdf+xml']})
-  >>> soup = BeautifulSoup(anon_browser.contents, parseOnlyThese=strainer)
-  >>> print soup.renderContents()
-  <link rel="meta" type="application/rdf+xml" title="FOAF" href="+rdf" />
-
-  >>> strainer = SoupStrainer({'class': ['error']})
-  >>> soup = BeautifulSoup(anon_browser.contents, parseOnlyThese=strainer)
-  >>> for tag in soup:
-  ...     print extract_text(tag)
-  Quantities must be greater than or equal 0.
-
-
-== Links ==
+    >>> from BeautifulSoup import BeautifulSoup, SoupStrainer
+    >>> anon_browser.open("http://launchpad.dev/~name16")
+    >>> strainer = SoupStrainer(['link'], {'type': ['application/rdf+xml']})
+    >>> soup = BeautifulSoup(anon_browser.contents, parseOnlyThese=strainer)
+    >>> print soup.renderContents()
+    <link rel="meta" type="application/rdf+xml" title="FOAF" href="+rdf" />
+
+    >>> strainer = SoupStrainer({'class': ['error']})
+    >>> soup = BeautifulSoup(anon_browser.contents, parseOnlyThese=strainer)
+    >>> for tag in soup:
+    ...     print extract_text(tag)
+    Quantities must be greater than or equal 0.
+
+
+Links
+-----
 
 Manipulating links:
 
@@ -163,7 +167,8 @@
     u'http://launchpad.dev/foo'
 
 
-== Form Controls ==
+Form Controls
+-------------
 
 Basics:
 
@@ -202,7 +207,8 @@
     'http://where-you-went-after-posting/'
 
 
-== Committing transactions ==
+Committing transactions
+-----------------------
 
 The transaction module is already imported into your pagetest's globals, so
 just do:
@@ -213,24 +219,24 @@
 You do not need to be logged in to do this.
 
 
-== Gotchas ==
+Gotchas
+-------
 
 If you get:
 
-    AttributeError: 'zope.thread.local' object has no attribute 'interaction'
+    AttributeError: 'thread._local' object has no attribute 'interaction'
 
 This means you are trying to getUtility() and aren't logged in.
 Use something like this:
 
-If you get:
-
     >>> from canonical.launchpad.ftests import ANONYMOUS, login, logout
     >>> login(ANONYMOUS)
 
+If you get:
+
     AssertionError: newInteraction called while another interaction is
     active.
 
 This means you forgot to log out:
 
-   >>> logout()
-
+    >>> logout()

=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2010-11-22 23:08:45 +0000
+++ lib/lp/registry/browser/configure.zcml	2010-11-30 15:31:40 +0000
@@ -1607,10 +1607,6 @@
     <adapter
         factory="lp.registry.browser.nameblacklist.NameBlacklistSetBreadcrumb"
         />
-    <adapter
-        factory="lp.registry.browser.nameblacklist.NameBlacklistBreadcrumb"
-        />
-
     <browser:page
         name="+addseries"
         for="lp.registry.interfaces.product.IProduct"

=== modified file 'lib/lp/registry/browser/nameblacklist.py'
--- lib/lp/registry/browser/nameblacklist.py	2010-11-23 23:22:27 +0000
+++ lib/lp/registry/browser/nameblacklist.py	2010-11-30 15:31:40 +0000
@@ -162,14 +162,3 @@
     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

=== modified file 'lib/lp/registry/browser/tests/test_breadcrumbs.py'
--- lib/lp/registry/browser/tests/test_breadcrumbs.py	2010-08-20 20:31:18 +0000
+++ lib/lp/registry/browser/tests/test_breadcrumbs.py	2010-11-30 15:31:40 +0000
@@ -125,6 +125,32 @@
         last_crumb = crumbs[-1]
         self.assertEqual(self.poll.title, last_crumb.text)
 
+from lp.registry.interfaces.nameblacklist import INameBlacklistSet
+
+
+class TestNameblacklistBreadcrumb(BaseBreadcrumbTestCase):
+    """Test breadcrumbs for +nameblacklist."""
+
+    def setUp(self):
+        super(TestNameblacklistBreadcrumb, self).setUp()
+        self.name_blacklist_set = getUtility(INameBlacklistSet)
+        self.registry_expert = self.factory.makeRegistryExpert()
+        login_person(self.registry_expert)
+
+    def test_nameblacklist(self):
+        expected = [('Name Blacklist', 'http://launchpad.dev/+nameblacklist')]
+        self.assertBreadcrumbs(expected, self.name_blacklist_set)
+
+    def test_nameblacklist_edit(self):
+        blacklist = self.name_blacklist_set.getByRegExp(u'blacklist')
+        expected = [
+            ('Name Blacklist',
+             'http://launchpad.dev/+nameblacklist'),
+            ('Edit a blacklist expression',
+             'http://launchpad.dev/+nameblacklist/1/+edit'),
+            ]
+        self.assertBreadcrumbs(expected, blacklist, view_name='+edit')
+
 
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)