Merge lp:~sinzui/launchpad/announcement-heading into lp:launchpad

[Curtis Hovey]

[Curtis Hovey (sinzui)]

This is my branch to fix announcement headings.

    lp:~sinzui/launchpad/announcement-heading
    Diff size: 346
    Launchpad bug: https://bugs.launchpad.net/bugs/434851
    Test command: ./bin/test -vvt "announcement-views|xx-announcements"
    Pre-implementation: barry
    Target release: 3.1.10

= Fix announcement headings =

Fix for bug 297877 is not working as expected. None of these urls have the
announcement name visible anywhere on the page:
 - https://launchpad.dev/mozilla/+announcement/22/+retarget
 - https://launchpad.dev/mozilla/+announcement/22/+publish
 - https://launchpad.dev/mozilla/+announcement/22/+delete

== Rules ==

These were not fully converted.
    * Register TitleBreadcrumb for IAnnouncement to make the breadcrumbs
      render.
    * Move pagetitles into the view.
    * Update the announcement view mixin to provide a sane label and
      add a simple page_title to each view.

== QA ==

    * Visit https://edge.launchpad.net/launchpad-project/+announcement/3451
      * Verify it had breadcrumbs that list pillar
      * Verify the title contains the announcement title
    * Choose Modify
      * Verify it had breadcrumbs that list pillar
      * Verify the title contains the announcement title
    * Visit https://edge.launchpad.net/+announcements
      * Verify its title is
        Announcements from all projects hosted in Launchpad

== Lint ==

Linting changed files:
  lib/canonical/launchpad/pagetitles.py
  lib/lp/registry/browser/announcement.py
  lib/lp/registry/browser/configure.zcml
  lib/lp/registry/browser/tests/announcement-views.txt
  lib/lp/registry/stories/announcements/xx-announcements.txt
  lib/lp/registry/templates/announcement-index.pt
  lib/lp/testing/views.py

== Test ==

    * lib/lp/registry/browser/tests/announcement-views.txt
      * Added a new module to test that the announcement generates
        expect breadcrumbs (and titles).
      * Added a test for the menus because I wanted to make sure they
        worked since I saw errors as I was making changes. They were
        fine, my code was not.
    * lib/lp/registry/stories/announcements/xx-announcements.txt
      * Updated the tests to verify the page title changes.

== Implementation ==

    * lib/canonical/launchpad/pagetitles.py
      * Removed the old page titles.
    * lib/lp/registry/browser/announcement.py
      * Updated the Announcement mixing to provide consistent heading, added
        unique page titles for that last breadcrumb.
    * lib/lp/registry/browser/configure.zcml
      * Corrected IHasAnnouncements +announcements. It was using the Set view
        instead of the HasAnnouncementsView view. The Set view extends the
        HasAnnouncementsView and its page_title broke the tests, I realised
        the broken tests were only testing IHasAnnouncements objects.
      * Registered TitleBreadcrumb as an adapter for IAnnouncement.
    * lib/lp/registry/templates/announcement-index.pt
      * Removed the heading because the that is now managed by the header
        macros and tales.
    * lib/lp/testing/views.py
      * Updated create_view and create_initialized_view to allow me to
        pass a fake request for testing.

[Brad Crittenden (bac)]

Very nice fix, Curtis.

review: Approve (code)

=== modified file 'lib/canonical/launchpad/pagetitles.py'
--- lib/canonical/launchpad/pagetitles.py	2009-09-28 13:56:17 +0000
+++ lib/canonical/launchpad/pagetitles.py	2009-10-06 02:10:27 +0000
@@ -487,23 +487,6 @@
 
 milestone_delete = ContextTitle('Delete %s')
 
-announcement_add = 'Make an announcement'
-
-announcement_delete = 'Permanently delete this announcement'
-
-announcement_edit = 'Modify this announcement'
-
-def announcement_index(context, view):
-    return '%s announcement' % context.target.displayname
-
-announcement_publish = 'Publish this announcement'
-
-announcement_retarget = 'Move this announcement to a different project'
-
-announcement_retract = 'Retract this announcement'
-
-announcements_all = 'Announcements from all projects hosted in Launchpad'
-
 oauth_authorize = 'Authorize application to access Launchpad on your behalf'
 
 def object_driver(context, view):

=== modified file 'lib/lp/registry/browser/announcement.py'
--- lib/lp/registry/browser/announcement.py	2009-09-10 20:49:25 +0000
+++ lib/lp/registry/browser/announcement.py	2009-10-06 02:10:27 +0000
@@ -102,9 +102,8 @@
     """A mixin to provide the common form features."""
 
     @property
-    def page_title(self):
-        """The html page title."""
-        return self.label
+    def label(self):
+        return self.context.title
 
     @property
     def cancel_url(self):
@@ -127,6 +126,7 @@
 
     schema = AddAnnouncementForm
     label = "Make an announcement"
+    page_title = label
 
     custom_widget('publication_date', AnnouncementDateWidget)
 
@@ -157,7 +157,7 @@
 
     schema = AddAnnouncementForm
     field_names = ['title', 'summary', 'url', ]
-    label = _('Modify this announcement')
+    page_title = 'Modify announcement'
 
     @property
     def initial_values(self):
@@ -189,7 +189,7 @@
 
     schema = AnnouncementRetargetForm
     field_names = ['target']
-    label = _('Move this announcement to a different project')
+    page_title = 'Move announcement'
 
     def validate(self, data):
         """Ensure that the person can publish announcement at the new
@@ -224,7 +224,7 @@
 
     schema = AddAnnouncementForm
     field_names = ['publication_date']
-    label = _('Publish this announcement')
+    page_title = 'Publish announcement'
 
     custom_widget('publication_date', AnnouncementDateWidget)
 
@@ -239,7 +239,7 @@
     """A view to unpublish an announcement."""
 
     schema = IAnnouncement
-    label = _('Retract this announcement')
+    page_title = 'Retract announcement'
 
     @action(_('Retract'), name='retract')
     def retract_action(self, action, data):
@@ -251,7 +251,7 @@
     """A view to delete an annoucement."""
 
     schema = IAnnouncement
-    label = _('Delete this announcement')
+    page_title = 'Delete announcement'
 
     @action(_("Delete"), name="delete", validator='validate_cancel')
     def action_delete(self, action, data):
@@ -309,6 +309,13 @@
         RootAnnouncementsFeedLink,
         )
 
+    page_title = 'Announcements from all projects hosted in Launchpad'
+    label = page_title
+
 
 class AnnouncementView(LaunchpadView):
     """A view class for a single announcement."""
+
+    @property
+    def label(self):
+        return self.context.title

=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2009-09-22 13:11:13 +0000
+++ lib/lp/registry/browser/configure.zcml	2009-10-06 02:10:27 +0000
@@ -689,7 +689,7 @@
         template="../templates/announcement-delete.pt"/>
     <browser:pages
         for="lp.registry.interfaces.announcement.IHasAnnouncements"
-        class="lp.registry.browser.announcement.AnnouncementSetView"
+        class="lp.registry.browser.announcement.HasAnnouncementsView"
         facet="overview"
         permission="zope.Public">
         <browser:page
@@ -725,6 +725,11 @@
         name="+announcement-macros"
         permission="zope.Public"
         template="../templates/announcement-macros.pt" />
+    <adapter
+        provides="canonical.launchpad.webapp.interfaces.IBreadcrumb"
+        for="lp.registry.interfaces.announcement.IAnnouncement"
+        factory="canonical.launchpad.webapp.breadcrumb.TitleBreadcrumb"
+        permission="zope.Public"/>
     <browser:menus
         module="lp.registry.browser.announcement"
         classes="

=== added file 'lib/lp/registry/browser/tests/announcement-views.txt'
--- lib/lp/registry/browser/tests/announcement-views.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/browser/tests/announcement-views.txt	2009-10-06 02:10:27 +0000
@@ -0,0 +1,47 @@
+Announcement Pages
+==================
+
+Tests for breadcrumbs, menus, and pages.
+
+
+Breadcrumbs
+-----------
+
+Announcement breadcrumbs use the announcement title.
+
+    >>> from canonical.lazr.testing.menus import make_fake_request
+
+    >>> product = factory.makeProduct(name='cube')
+    >>> owner = product.owner
+    >>> announcement = product.announce(
+    ...     user=owner, title='A title', summary='A summary')
+    >>> view = create_initialized_view(announcement, '+index')
+
+    >>> request = make_fake_request(
+    ...     'http://launchpad.dev/joy-of-cooking/spam',
+    ...     [product, announcement, view])
+    >>> hierarchy = create_initialized_view(
+    ...     announcement, '+hierarchy', request=request)
+    >>> hierarchy.items
+    [<DisplaynameBreadcrumb ... text='Cube'>,
+     <TitleBreadcrumb ... text='A title'>]
+
+
+Menus
+-----
+
+The announcement module provides two menus. There is an edit menu for an
+announcement.
+
+    >>> from lp.registry.browser.announcement import (
+    ...     AnnouncementEditNavigationMenu, AnnouncementCreateNavigationMenu)
+    >>> from lp.testing.menu import check_menu_links
+
+    >>> check_menu_links(AnnouncementEditNavigationMenu(announcement))
+    True
+
+There is a menu for views for IHasAnnouncements objects to list there
+announcements. The product implements IHasAnnouncements.
+
+    >>> check_menu_links(AnnouncementCreateNavigationMenu(product))
+    True

=== modified file 'lib/lp/registry/stories/announcements/xx-announcements.txt'
--- lib/lp/registry/stories/announcements/xx-announcements.txt	2009-09-18 15:24:30 +0000
+++ lib/lp/registry/stories/announcements/xx-announcements.txt	2009-10-06 02:10:27 +0000
@@ -263,7 +263,7 @@
     >>> anon_browser.open('http://launchpad.dev/apache/+announcements')
     >>> anon_browser.getLink('Derby announcement headline').click()
     >>> print anon_browser.title
-    Derby announcement
+    Derby announcement headline : Derby
 
 The page shows the announcement and it has a link back to the announcements
 page that any user can navigate.
@@ -365,7 +365,7 @@
     >>> priv_browser.getLink('Kubuntu announcement headline').click()
     >>> priv_browser.getLink('Publish announcement').click()
     >>> print priv_browser.title
-    Publish this announcement...
+    Publish announcement : Kubuntu announcement headline : Kubuntu
     >>> print priv_browser.url
     http://launchpad.dev/kubuntu/+announceme.../+publish
     >>> radio = priv_browser.getControl(name="field.publication_date.action")
@@ -475,7 +475,7 @@
     >>> priv_browser.getLink('Apache announcement headline').click()
     >>> priv_browser.getLink('Modify announcement').click()
     >>> print priv_browser.title
-    Modify this announcement...
+    Modify announcement : Apache announcement headline : Apache
     >>> headline = priv_browser.getControl('Headline')
     >>> print headline.value
     Apache announcement headline
@@ -515,7 +515,7 @@
     >>> priv_browser.getLink('Delete announcement').click()
     >>> priv_browser.getLink('retracting the announcement').click()
     >>> print priv_browser.title
-    Retract this announcement...
+    Retract announcement : Kubuntu announcement headline : Kubuntu
 
 Actually clicking "Retract" takes us back to the listing page. The item
 is shown as having been retracted if you are a privileged user.
@@ -547,7 +547,7 @@
     >>> priv_browser.getLink('Kubuntu announcement headline').click()
     >>> priv_browser.getLink('Publish announcement').click()
     >>> print priv_browser.title
-    Publish this announcement...
+    Publish announcement : Kubuntu announcement headline : Kubuntu
     >>> radio = priv_browser.getControl(name="field.publication_date.action")
     >>> radio.value = ['immediately']
     >>> priv_browser.getControl(
@@ -576,7 +576,7 @@
     >>> priv_browser.getLink('Kubuntu announcement headline').click()
     >>> priv_browser.getLink('Move announcement').click()
     >>> print priv_browser.title
-    Move this announcement to a different project...
+    Move announcement : Kubuntu announcement headline : Kubuntu
     >>> priv_browser.getControl('For').value = 'guadalinex'
     >>> priv_browser.getControl('Retarget').click()
     >>> print priv_browser.title
@@ -593,13 +593,13 @@
     >>> kamion_browser.getLink('Kubuntu announcement headline').click()
     >>> kamion_browser.getLink('Move announcement').click()
     >>> print kamion_browser.title
-    Move this announcement to a different project...
+    Move announcement : Kubuntu announcement headline : GuadaLinex
     >>> kamion_browser.getControl('For').value = 'kubuntu'
     >>> kamion_browser.getControl('Retarget').click()
     >>> "don't have permission" in extract_text(find_main_content(kamion_browser.contents))
     True
     >>> print kamion_browser.title
-    Move this announcement to a different project...
+    Move announcement : Kubuntu announcement headline : GuadaLinex
 
 
 Atom/RSS Feeds
@@ -665,7 +665,7 @@
     >>> priv_browser.getLink('Delete announcement').click()
     >>> priv_browser.getLink('retracting the announcement').click()
     >>> print priv_browser.title
-    Retract this announcement...
+    Retract announcement : Kubuntu announcement headline : GuadaLinex
     >>> priv_browser.getControl('Retract').click()
     >>> nopriv_browser.reload()
     >>> 'Kubuntu announcement ' in nopriv_browser.contents
@@ -758,7 +758,7 @@
     >>> kamion_browser.getLink('Kubuntu announcement headline').click()
     >>> kamion_browser.getLink('Delete announcement').click()
     >>> print kamion_browser.title
-    Delete this announcement...
+    Delete announcement : Kubuntu announcement headline : GuadaLinex
     >>> kamion_browser.getControl('Delete').click()
     >>> print priv_browser.title
     GuadaLinex news and announcements...

=== modified file 'lib/lp/registry/templates/announcement-index.pt'
--- lib/lp/registry/templates/announcement-index.pt	2009-08-31 13:37:42 +0000
+++ lib/lp/registry/templates/announcement-index.pt	2009-10-06 02:10:27 +0000
@@ -8,10 +8,6 @@
 >
 
 <body>
-  <tal:heading metal:fill-slot="heading">
-    <h1 tal:content="context/title">Announcement headline</h1>
-  </tal:heading>
-
   <div metal:fill-slot="main">
     <tal:body define="announcement context;
                       show_title nothing">

=== modified file 'lib/lp/testing/views.py'
--- lib/lp/testing/views.py	2009-09-24 02:24:23 +0000
+++ lib/lp/testing/views.py	2009-10-06 02:10:27 +0000
@@ -20,7 +20,7 @@
 
 def create_view(context, name, form=None, layer=None, server_url=None,
                 method='GET', principal=None, query_string='', cookie='',
-                path_info='/', current_request=False, **kwargs):
+                request=None, path_info='/', current_request=False, **kwargs):
     """Return a view based on the given arguments.
 
     :param context: The context for the view.
@@ -33,15 +33,17 @@
         unauthenticated principal.
     :param query_string: The query string for the request.
     :patam cookie: The HTTP_COOKIE value for the request.
+    :param request: Use this request instead of creating a new one.
     :param path_info: The PATH_INFO value for the request.
     :param current_request: If True, the request will be set as the current
         interaction.
     :param **kwargs: Any other parameter for the request.
     :return: The view class for the given context and the name.
     """
-    request = LaunchpadTestRequest(
-        form=form, SERVER_URL=server_url, QUERY_STRING=query_string,
-        HTTP_COOKIE=cookie, method=method, **kwargs)
+    if request is None:
+        request = LaunchpadTestRequest(
+            form=form, SERVER_URL=server_url, QUERY_STRING=query_string,
+            HTTP_COOKIE=cookie, method=method, **kwargs)
     if principal is not None:
         request.setPrincipal(principal)
     else:
@@ -57,7 +59,7 @@
 
 def create_initialized_view(context, name, form=None, layer=None,
                             server_url=None, method=None, principal=None,
-                            query_string=None, cookie=None):
+                            query_string=None, cookie=None, request=None):
     """Return a view that has already been initialized."""
     if method is None:
         if form is None:
@@ -66,6 +68,6 @@
             method = 'POST'
     view = create_view(
         context, name, form, layer, server_url, method, principal,
-        query_string, cookie)
+        query_string, cookie, request)
     view.initialize()
     return view