Merge lp:~dev-nigelj/launchpad/bug-810551 into lp:launchpad

Proposed by Nigel Jones
Status: Merged
Approved by: Gavin Panella
Approved revision: no longer in the source branch.
Merged at revision: 13846
Proposed branch: lp:~dev-nigelj/launchpad/bug-810551
Merge into: lp:launchpad
Diff against target: 88 lines (+54/-4)
3 files modified
lib/lp/registry/browser/announcement.py (+2/-0)
lib/lp/registry/browser/tests/test_announcements.py (+47/-0)
lib/lp/registry/templates/announcement-macros.pt (+5/-4)
To merge this branch: bzr merge lp:~dev-nigelj/launchpad/bug-810551
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Nigel Jones (community) Needs Resubmitting
Review via email: mp+73673@code.launchpad.net

Commit message

[r=allenap][bug=810551] Show announcement date alongside the target and author information.

Description of the change

Summary

Bug 810551 describes an observation that the announcement date is not easily obtainable when reading the full announcement, and is a request for the date to be put in a logical place.

Proposed/Implemented Fix

The proposed fix was to modify the format the date is shown on the announcements list, and instead include it in with the announcement location and author.

It now displays as:

"Written for (product|distribution) by (person) on (date)"

Tests

bin/test -t registry
and
bin/test -t announcement

Both ran clean.

Demo QA

with sample data, https://launchpad.dev/ubuntu/+announcement/23 & https://launchpad.dev/ubuntu/+announcements should show the dates in a logical manner.

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

This is a nice improvement. You could even go further and remove the leading date from the titles on +announcements, but this is good on its own.

It does, however, need a test, even though there wasn't one before. I haven't done any coding in a while, and found myself too tempted to resist writing one myself - http://paste.ubuntu.com/679849/ - if you'd like to use it.

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

> You could even go further and remove the leading date from the titles
> on +announcements, but this is good on its own.

This is rubbish; it's already been done. My mistake!

Revision history for this message
Nigel Jones (dev-nigelj) wrote :

> It does, however, need a test, even though there wasn't one before. I haven't
> done any coding in a while, and found myself too tempted to resist writing one
> myself - http://paste.ubuntu.com/679849/ - if you'd like to use it.

I don't think I could have written the test better myself, so I'd be more than happy to include it (why double up on work too?).

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

Looks good :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/registry/browser/announcement.py'
--- lib/lp/registry/browser/announcement.py 2011-04-11 02:32:50 +0000
+++ lib/lp/registry/browser/announcement.py 2011-09-01 16:15:28 +0000
@@ -340,3 +340,5 @@
340 @property340 @property
341 def label(self):341 def label(self):
342 return self.context.title342 return self.context.title
343
344 page_title = label
343345
=== added file 'lib/lp/registry/browser/tests/test_announcements.py'
--- lib/lp/registry/browser/tests/test_announcements.py 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/browser/tests/test_announcements.py 2011-09-01 16:15:28 +0000
@@ -0,0 +1,47 @@
1# Copyright 2011 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Tests for +announcement views."""
5
6__metaclass__ = type
7
8from datetime import datetime
9
10from lxml import html
11from pytz import utc
12
13from canonical.testing.layers import LaunchpadFunctionalLayer
14from lp.testing import (
15 normalize_whitespace,
16 TestCaseWithFactory,
17 )
18from lp.testing.views import create_initialized_view
19
20
21class TestAnnouncement(TestCaseWithFactory):
22
23 layer = LaunchpadFunctionalLayer
24
25 def test_announcement_info(self):
26 product = self.factory.makeProduct(displayname=u"Foo")
27 announcer = self.factory.makePerson(displayname=u"Bar Baz")
28 announcement = product.announce(announcer, "Hello World")
29 view = create_initialized_view(announcement, "+index")
30 root = html.fromstring(view())
31 [reg_para] = root.cssselect("p.registered")
32 self.assertEqual(
33 "Written for Foo by Bar Baz",
34 normalize_whitespace(reg_para.text_content()))
35
36 def test_announcement_info_with_publication_date(self):
37 product = self.factory.makeProduct(displayname=u"Foo")
38 announcer = self.factory.makePerson(displayname=u"Bar Baz")
39 announced = datetime(2007, 01, 12, tzinfo=utc)
40 announcement = product.announce(
41 announcer, "Hello World", publication_date=announced)
42 view = create_initialized_view(announcement, "+index")
43 root = html.fromstring(view())
44 [reg_para] = root.cssselect("p.registered")
45 self.assertEqual(
46 "Written for Foo by Bar Baz on 2007-01-12",
47 normalize_whitespace(reg_para.text_content()))
048
=== modified file 'lib/lp/registry/templates/announcement-macros.pt'
--- lib/lp/registry/templates/announcement-macros.pt 2009-08-20 01:17:15 +0000
+++ lib/lp/registry/templates/announcement-macros.pt 2011-09-01 16:15:28 +0000
@@ -14,9 +14,6 @@
14 tal:attributes="id id_string/fmt:css-id">14 tal:attributes="id id_string/fmt:css-id">
15 <h2 tal:condition="show_title">15 <h2 tal:condition="show_title">
16 <a tal:attributes="href string:${announcement/fmt:url}">16 <a tal:attributes="href string:${announcement/fmt:url}">
17 <tal:date tal:condition="announcement/date_announced">
18 <span tal:replace="announcement/date_announced/fmt:date"/>:
19 </tal:date>
20 <tal:title content="announcement/title">News item title</tal:title>17 <tal:title content="announcement/title">News item title</tal:title>
21 </a>18 </a>
22 </h2>19 </h2>
@@ -25,7 +22,11 @@
25 Written for22 Written for
26 <tal:pillar replace="structure announcement/target/fmt:link" />23 <tal:pillar replace="structure announcement/target/fmt:link" />
27 by24 by
28 <tal:registrant replace="structure announcement/registrant/fmt:link" />.25 <tal:registrant replace="structure announcement/registrant/fmt:link" />
26 <tal:date tal:condition="announcement/date_announced">
27 on
28 <span tal:replace="announcement/date_announced/fmt:date" />
29 </tal:date>
29 </p>30 </p>
3031
31 <div tal:condition="announcement/summary"32 <div tal:condition="announcement/summary"