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
1=== modified file 'lib/lp/registry/browser/announcement.py'
2--- lib/lp/registry/browser/announcement.py 2011-04-11 02:32:50 +0000
3+++ lib/lp/registry/browser/announcement.py 2011-09-01 16:15:28 +0000
4@@ -340,3 +340,5 @@
5 @property
6 def label(self):
7 return self.context.title
8+
9+ page_title = label
10
11=== added file 'lib/lp/registry/browser/tests/test_announcements.py'
12--- lib/lp/registry/browser/tests/test_announcements.py 1970-01-01 00:00:00 +0000
13+++ lib/lp/registry/browser/tests/test_announcements.py 2011-09-01 16:15:28 +0000
14@@ -0,0 +1,47 @@
15+# Copyright 2011 Canonical Ltd. This software is licensed under the
16+# GNU Affero General Public License version 3 (see the file LICENSE).
17+
18+"""Tests for +announcement views."""
19+
20+__metaclass__ = type
21+
22+from datetime import datetime
23+
24+from lxml import html
25+from pytz import utc
26+
27+from canonical.testing.layers import LaunchpadFunctionalLayer
28+from lp.testing import (
29+ normalize_whitespace,
30+ TestCaseWithFactory,
31+ )
32+from lp.testing.views import create_initialized_view
33+
34+
35+class TestAnnouncement(TestCaseWithFactory):
36+
37+ layer = LaunchpadFunctionalLayer
38+
39+ def test_announcement_info(self):
40+ product = self.factory.makeProduct(displayname=u"Foo")
41+ announcer = self.factory.makePerson(displayname=u"Bar Baz")
42+ announcement = product.announce(announcer, "Hello World")
43+ view = create_initialized_view(announcement, "+index")
44+ root = html.fromstring(view())
45+ [reg_para] = root.cssselect("p.registered")
46+ self.assertEqual(
47+ "Written for Foo by Bar Baz",
48+ normalize_whitespace(reg_para.text_content()))
49+
50+ def test_announcement_info_with_publication_date(self):
51+ product = self.factory.makeProduct(displayname=u"Foo")
52+ announcer = self.factory.makePerson(displayname=u"Bar Baz")
53+ announced = datetime(2007, 01, 12, tzinfo=utc)
54+ announcement = product.announce(
55+ announcer, "Hello World", publication_date=announced)
56+ view = create_initialized_view(announcement, "+index")
57+ root = html.fromstring(view())
58+ [reg_para] = root.cssselect("p.registered")
59+ self.assertEqual(
60+ "Written for Foo by Bar Baz on 2007-01-12",
61+ normalize_whitespace(reg_para.text_content()))
62
63=== modified file 'lib/lp/registry/templates/announcement-macros.pt'
64--- lib/lp/registry/templates/announcement-macros.pt 2009-08-20 01:17:15 +0000
65+++ lib/lp/registry/templates/announcement-macros.pt 2011-09-01 16:15:28 +0000
66@@ -14,9 +14,6 @@
67 tal:attributes="id id_string/fmt:css-id">
68 <h2 tal:condition="show_title">
69 <a tal:attributes="href string:${announcement/fmt:url}">
70- <tal:date tal:condition="announcement/date_announced">
71- <span tal:replace="announcement/date_announced/fmt:date"/>:
72- </tal:date>
73 <tal:title content="announcement/title">News item title</tal:title>
74 </a>
75 </h2>
76@@ -25,7 +22,11 @@
77 Written for
78 <tal:pillar replace="structure announcement/target/fmt:link" />
79 by
80- <tal:registrant replace="structure announcement/registrant/fmt:link" />.
81+ <tal:registrant replace="structure announcement/registrant/fmt:link" />
82+ <tal:date tal:condition="announcement/date_announced">
83+ on
84+ <span tal:replace="announcement/date_announced/fmt:date" />
85+ </tal:date>
86 </p>
87
88 <div tal:condition="announcement/summary"