Launchpad itself

Merge lp:~cjohnston/launchpad/ic-approx-time into lp:launchpad

  1. ic-approx-time
  2. Merge into devel
Proposed by Chris Johnston
Status: Merged
Merged at revision: 17101
Proposed branch: lp:~cjohnston/launchpad/ic-approx-time
Merge into: lp:launchpad
Diff against target: 238 lines (+151/-17)
6 files modified
lib/lp/app/javascript/date.js (+46/-0)
lib/lp/app/javascript/tests/test_date.html (+37/-0)
lib/lp/app/javascript/tests/test_date.js (+37/-0)
lib/lp/code/javascript/branchmergeproposal.inlinecomments.js (+8/-4)
lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.html (+2/-0)
lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js (+21/-13)
To merge this branch: bzr merge lp:~cjohnston/launchpad/ic-approx-time
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+225420@code.launchpad.net

Commit message

Switch ICs to use relative time stamps similar to other comment sections of LP.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) wrote :

Date.now and the ISO 8601 support in Date.parse are relatively new, and we probably want to fall back if they don't work. This will fail at least in IE8, and quite possibly on the Lucid buildbots which use an old WebKit. We probably don't care about IE8 at this point, but buildbot will be a problem.

Also, your bzr whoami is wrong; consider correcting it and recommitting.

And there are a few comments inline.

review: Needs Fixing (code)
Revision history for this message
William Grant (wgrant) wrote :

approximatedate should probably live in lib/lp/app/javascript somewhere, and it needs unit tests of its own. Also, a few more inline comments.

review: Needs Fixing (code)
Revision history for this message
William Grant (wgrant) wrote :

Thanks for the fixes. Just a few more trivialities and this will be good to land!

review: Needs Fixing (code)
Revision history for this message
William Grant (wgrant) wrote :

Thanks for the fixes. Have you tested this on Lucid?

Also, lib/lp/app/templates/base-layout-macros.pt still has the unnecessary added dependency.

Revision history for this message
William Grant (wgrant) :
review: Needs Fixing
Revision history for this message
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'lib/lp/app/javascript/date.js'
2--- lib/lp/app/javascript/date.js 1970-01-01 00:00:00 +0000
3+++ lib/lp/app/javascript/date.js 2014-07-08 12:33:30 +0000
4@@ -0,0 +1,46 @@
5+/* Copyright 2014 Canonical Ltd. This software is licensed under the
6+ * GNU Affero General Public License version 3 (see the file LICENSE). */
7+
8+YUI.add('lp.app.date', function(Y) {
9+
10+var namespace = Y.namespace('lp.app.date');
11+
12+namespace.parse_date = function(str) {
13+ // Parse an ISO-8601 date
14+ var re = /^(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2}):(\d{2})(?:\.(\d+))?(Z|\+00:00)$/
15+ var bits = re.exec(str).slice(1, 8).map(Number)
16+ // Adjusting for the fact that Date.UTC uses 0-11 for months
17+ bits[1] -= 1
18+ return new Date(Date.UTC.apply(null, bits))
19+};
20+
21+namespace.approximatedate = function(date) {
22+ // Display approximate time an event happened when it happened less than 1
23+ // day ago.
24+ var now = (new Date).valueOf();
25+ var timedelta = now - date;
26+ var days = timedelta / 86400000
27+ var hours = timedelta / 3600000
28+ var minutes = timedelta / 60000
29+ var amount = 0
30+ var unit = ""
31+ if (days > 1) {
32+ return 'on ' + Y.Date.format(
33+ new Date(date), {format: '%Y-%m-%d'});
34+ } else {
35+ if (hours >= 1) {
36+ amount = hours
37+ unit = "hour"
38+ } else if (minutes >= 1) {
39+ amount = minutes
40+ unit = "minute"
41+ } else {
42+ return "a moment ago"
43+ }
44+ if (Math.floor(amount) > 1) {
45+ unit = unit + 's'
46+ }
47+ return Math.floor(amount) + ' ' + unit + ' ago'
48+ }
49+};
50+}, "0.1", {'requires': ['datatype-date']});
51
52=== added file 'lib/lp/app/javascript/tests/test_date.html'
53--- lib/lp/app/javascript/tests/test_date.html 1970-01-01 00:00:00 +0000
54+++ lib/lp/app/javascript/tests/test_date.html 2014-07-08 12:33:30 +0000
55@@ -0,0 +1,37 @@
56+<!DOCTYPE html>
57+<!--
58+Copyright 2014 Canonical Ltd. This software is licensed under the
59+GNU Affero General Public License version 3 (see the file LICENSE).
60+-->
61+<html>
62+ <head>
63+ <title>Test lp-names</title>
64+ <!-- YUI and test setup -->
65+ <script type="text/javascript"
66+ src="../../../../../build/js/yui/yui/yui.js">
67+ </script>
68+ <link rel="stylesheet"
69+ href="../../../../../build/js/yui/console/assets/console-core.css" />
70+ <link rel="stylesheet"
71+ href="../../../../../build/js/yui/test-console/assets/skins/sam/test-console.css" />
72+ <link rel="stylesheet"
73+ href="../../../../../build/js/yui/test/assets/skins/sam/test.css" />
74+
75+ <script type="text/javascript"
76+ src="../../../../../build/js/lp/app/testing/testrunner.js"></script>
77+ <link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
78+
79+ <!-- The module under test. -->
80+ <script type="text/javascript" src="../date.js"></script>
81+
82+ <!-- The test suite. -->
83+ <script type="text/javascript" src="test_date.js"></script>
84+
85+ </head>
86+ <body class="yui3-skin-sam">
87+ <ul id="suites">
88+ <li>date.test</li>
89+ </ul>
90+
91+ </body>
92+</html>
93
94=== added file 'lib/lp/app/javascript/tests/test_date.js'
95--- lib/lp/app/javascript/tests/test_date.js 1970-01-01 00:00:00 +0000
96+++ lib/lp/app/javascript/tests/test_date.js 2014-07-08 12:33:30 +0000
97@@ -0,0 +1,37 @@
98+YUI.add('date.test', function (Y) {
99+
100+ var tests = Y.namespace('date.test');
101+ tests.suite = new Y.Test.Suite("date tests");
102+
103+ var now = (new Date).valueOf()
104+ tests.suite.add(new Y.Test.Case({
105+ name: 'test_approximatedate',
106+
107+ test_return_moment_ago: function () {
108+ Y.Assert.areEqual(
109+ 'a moment ago',
110+ Y.lp.app.date.approximatedate(new Date(now - 150)));
111+ },
112+
113+ test_return_minute_ago: function () {
114+ Y.Assert.areEqual(
115+ '1 minute ago',
116+ Y.lp.app.date.approximatedate(new Date(now - 65000)));
117+ },
118+
119+ test_return_hours_ago: function () {
120+ Y.Assert.areEqual(
121+ '3 hours ago',
122+ Y.lp.app.date.approximatedate(new Date(now - 12600000)));
123+ },
124+ test_return_days_ago: function () {
125+ Y.Assert.areEqual(
126+ 'on 2012-08-12', Y.lp.app.date.approximatedate(
127+ Y.lp.app.date.parse_date(
128+ '2012-08-12T10:00:00.00001+00:00')));
129+ },
130+ }));
131+
132+}, '0.1', {
133+ requires: ['lp.testing.runner', 'test', 'lp', 'lp.app.date']
134+});
135
136=== modified file 'lib/lp/code/javascript/branchmergeproposal.inlinecomments.js'
137--- lib/lp/code/javascript/branchmergeproposal.inlinecomments.js 2014-05-29 16:30:42 +0000
138+++ lib/lp/code/javascript/branchmergeproposal.inlinecomments.js 2014-07-08 12:33:30 +0000
139@@ -184,9 +184,13 @@
140 // XXX cprov 20140226: the date should be formatted according to
141 // the user locale/timezone similar to fmt:displaydate.
142 // We should leverage Y.Intl features.
143- var fmt_date = 'on ' + Y.Date.format(
144- new Date(comment.date), {format: '%Y-%m-%d'});
145- headerspan.one('span').set('text', fmt_date);
146+ if (typeof comment.date === "string") {
147+ var comment_date = Y.lp.app.date.parse_date(comment.date);
148+ } else {
149+ var comment_date = comment.date
150+ }
151+ var date = Y.lp.app.date.approximatedate(comment_date);
152+ headerspan.one('span').set('text', date);
153 newrow.one('.boardCommentDetails').append(headerspan);
154 newrow.one('.boardCommentBody div').append('<span></span>');
155 newrow.one('.boardCommentBody').one('span').set('text', comment.text);
156@@ -701,4 +705,4 @@
157 namespace.DiffNav = DiffNav;
158
159 }, '0.1', {requires: ['datatype-date', 'event', 'io', 'node', 'widget',
160- 'lp.client', 'lp.ui.editor']});
161+ 'lp.client', 'lp.ui.editor', 'lp.app.date']});
162
163=== modified file 'lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.html'
164--- lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.html 2014-05-19 21:56:02 +0000
165+++ lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.html 2014-07-08 12:33:30 +0000
166@@ -32,6 +32,8 @@
167 <script type="text/javascript"
168 src="../../../../../build/js/lp/app/client.js"></script>
169 <script type="text/javascript"
170+ src="../../../../../build/js/lp/app/date.js"></script>
171+ <script type="text/javascript"
172 src="../../../../../build/js/lp/app/testing/mockio.js"></script>
173 <script type="text/javascript"
174 src="../../../../../build/js/lp/app/ui/ui.js"></script>
175
176=== modified file 'lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js'
177--- lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js 2014-05-26 12:44:57 +0000
178+++ lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js 2014-07-08 12:33:30 +0000
179@@ -70,25 +70,28 @@
180 "ws.op=getInlineComments&previewdiff_id=1",
181 mockio.requests[0].config.data);
182 mockio.last_request = mockio.requests[0];
183+ var now = (new Date).valueOf()
184 published_comments = [
185 {'line_number': '2',
186 'person': person_obj,
187 'text': 'This is preloaded.',
188- 'date': '2012-08-12T10:00:00.00001+00:00'}
189+ 'date': '2012-08-12T10:00:00.00001+00:00'},
190+ {'line_number': '3',
191+ 'person': person_obj,
192+ 'text': 'This is great.',
193+ 'date': (new Date(now - 12600000)),
194+ },
195 ];
196 mockio.success({
197 responseText: Y.JSON.stringify(published_comments),
198 responseHeaders: {'Content-Type': 'application/json'}});
199
200 // Published comment is displayed.
201- var comments = Y.one('#diff-line-2').next().one('div');
202- // XXX cprov 20140226: test disabled due to ES4 lack of
203- // ISO8601 support. Although the vast majority of production
204- // clients run ES5.
205- //Y.Assert.areEqual(
206- // 'Foo Bar (name16) wrote on 2012-08-12',
207- // header.get('text'));
208- var first = comments.one('div:first-child')
209+ var first_comments = Y.one('#diff-line-2').next().one('div');
210+ var first = first_comments.one('div:first-child')
211+ Y.Assert.areEqual(
212+ 'Foo Bar (name16) wrote on 2012-08-12:',
213+ first.one('.boardCommentDetails').get('text'));
214 Y.Assert.areEqual(
215 'This is preloaded.',
216 first.one('.boardCommentBody').get('text'));
217@@ -103,12 +106,17 @@
218 'Boing!', second.one('.boardCommentBody').get('text'));
219
220 // Draft comment for line 3 is displayed.
221- var third = Y.one('#diff-line-3').next();
222+ var second_comments = Y.one('#diff-line-3').next().one('div');
223+ var third = second_comments.one('div:first-child')
224 Y.Assert.areEqual(
225- 'Unsaved comment',
226+ 'Foo Bar (name16) wrote 3 hours ago:',
227 third.one('.boardCommentDetails').get('text'));
228- Y.Assert.areEqual(
229- 'Zoing!', third.one('.boardCommentBody').get('text'));
230+ var fourth = third.next()
231+ Y.Assert.areEqual(
232+ 'Unsaved comment',
233+ fourth.one('.boardCommentDetails').get('text'));
234+ Y.Assert.areEqual(
235+ 'Zoing!', fourth.one('.boardCommentBody').get('text'));
236 },
237
238 test_draft_handler: function() {