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
=== added file 'lib/lp/app/javascript/date.js'
--- lib/lp/app/javascript/date.js 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/date.js 2014-07-08 12:33:30 +0000
@@ -0,0 +1,46 @@
1/* Copyright 2014 Canonical Ltd. This software is licensed under the
2 * GNU Affero General Public License version 3 (see the file LICENSE). */
3
4YUI.add('lp.app.date', function(Y) {
5
6var namespace = Y.namespace('lp.app.date');
7
8namespace.parse_date = function(str) {
9 // Parse an ISO-8601 date
10 var re = /^(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2}):(\d{2})(?:\.(\d+))?(Z|\+00:00)$/
11 var bits = re.exec(str).slice(1, 8).map(Number)
12 // Adjusting for the fact that Date.UTC uses 0-11 for months
13 bits[1] -= 1
14 return new Date(Date.UTC.apply(null, bits))
15};
16
17namespace.approximatedate = function(date) {
18 // Display approximate time an event happened when it happened less than 1
19 // day ago.
20 var now = (new Date).valueOf();
21 var timedelta = now - date;
22 var days = timedelta / 86400000
23 var hours = timedelta / 3600000
24 var minutes = timedelta / 60000
25 var amount = 0
26 var unit = ""
27 if (days > 1) {
28 return 'on ' + Y.Date.format(
29 new Date(date), {format: '%Y-%m-%d'});
30 } else {
31 if (hours >= 1) {
32 amount = hours
33 unit = "hour"
34 } else if (minutes >= 1) {
35 amount = minutes
36 unit = "minute"
37 } else {
38 return "a moment ago"
39 }
40 if (Math.floor(amount) > 1) {
41 unit = unit + 's'
42 }
43 return Math.floor(amount) + ' ' + unit + ' ago'
44 }
45};
46}, "0.1", {'requires': ['datatype-date']});
047
=== added file 'lib/lp/app/javascript/tests/test_date.html'
--- lib/lp/app/javascript/tests/test_date.html 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/tests/test_date.html 2014-07-08 12:33:30 +0000
@@ -0,0 +1,37 @@
1<!DOCTYPE html>
2<!--
3Copyright 2014 Canonical Ltd. This software is licensed under the
4GNU Affero General Public License version 3 (see the file LICENSE).
5-->
6<html>
7 <head>
8 <title>Test lp-names</title>
9 <!-- YUI and test setup -->
10 <script type="text/javascript"
11 src="../../../../../build/js/yui/yui/yui.js">
12 </script>
13 <link rel="stylesheet"
14 href="../../../../../build/js/yui/console/assets/console-core.css" />
15 <link rel="stylesheet"
16 href="../../../../../build/js/yui/test-console/assets/skins/sam/test-console.css" />
17 <link rel="stylesheet"
18 href="../../../../../build/js/yui/test/assets/skins/sam/test.css" />
19
20 <script type="text/javascript"
21 src="../../../../../build/js/lp/app/testing/testrunner.js"></script>
22 <link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
23
24 <!-- The module under test. -->
25 <script type="text/javascript" src="../date.js"></script>
26
27 <!-- The test suite. -->
28 <script type="text/javascript" src="test_date.js"></script>
29
30 </head>
31 <body class="yui3-skin-sam">
32 <ul id="suites">
33 <li>date.test</li>
34 </ul>
35
36 </body>
37</html>
038
=== added file 'lib/lp/app/javascript/tests/test_date.js'
--- lib/lp/app/javascript/tests/test_date.js 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/tests/test_date.js 2014-07-08 12:33:30 +0000
@@ -0,0 +1,37 @@
1YUI.add('date.test', function (Y) {
2
3 var tests = Y.namespace('date.test');
4 tests.suite = new Y.Test.Suite("date tests");
5
6 var now = (new Date).valueOf()
7 tests.suite.add(new Y.Test.Case({
8 name: 'test_approximatedate',
9
10 test_return_moment_ago: function () {
11 Y.Assert.areEqual(
12 'a moment ago',
13 Y.lp.app.date.approximatedate(new Date(now - 150)));
14 },
15
16 test_return_minute_ago: function () {
17 Y.Assert.areEqual(
18 '1 minute ago',
19 Y.lp.app.date.approximatedate(new Date(now - 65000)));
20 },
21
22 test_return_hours_ago: function () {
23 Y.Assert.areEqual(
24 '3 hours ago',
25 Y.lp.app.date.approximatedate(new Date(now - 12600000)));
26 },
27 test_return_days_ago: function () {
28 Y.Assert.areEqual(
29 'on 2012-08-12', Y.lp.app.date.approximatedate(
30 Y.lp.app.date.parse_date(
31 '2012-08-12T10:00:00.00001+00:00')));
32 },
33 }));
34
35}, '0.1', {
36 requires: ['lp.testing.runner', 'test', 'lp', 'lp.app.date']
37});
038
=== modified file 'lib/lp/code/javascript/branchmergeproposal.inlinecomments.js'
--- lib/lp/code/javascript/branchmergeproposal.inlinecomments.js 2014-05-29 16:30:42 +0000
+++ lib/lp/code/javascript/branchmergeproposal.inlinecomments.js 2014-07-08 12:33:30 +0000
@@ -184,9 +184,13 @@
184 // XXX cprov 20140226: the date should be formatted according to184 // XXX cprov 20140226: the date should be formatted according to
185 // the user locale/timezone similar to fmt:displaydate.185 // the user locale/timezone similar to fmt:displaydate.
186 // We should leverage Y.Intl features.186 // We should leverage Y.Intl features.
187 var fmt_date = 'on ' + Y.Date.format(187 if (typeof comment.date === "string") {
188 new Date(comment.date), {format: '%Y-%m-%d'});188 var comment_date = Y.lp.app.date.parse_date(comment.date);
189 headerspan.one('span').set('text', fmt_date);189 } else {
190 var comment_date = comment.date
191 }
192 var date = Y.lp.app.date.approximatedate(comment_date);
193 headerspan.one('span').set('text', date);
190 newrow.one('.boardCommentDetails').append(headerspan);194 newrow.one('.boardCommentDetails').append(headerspan);
191 newrow.one('.boardCommentBody div').append('<span></span>');195 newrow.one('.boardCommentBody div').append('<span></span>');
192 newrow.one('.boardCommentBody').one('span').set('text', comment.text);196 newrow.one('.boardCommentBody').one('span').set('text', comment.text);
@@ -701,4 +705,4 @@
701namespace.DiffNav = DiffNav;705namespace.DiffNav = DiffNav;
702706
703}, '0.1', {requires: ['datatype-date', 'event', 'io', 'node', 'widget',707}, '0.1', {requires: ['datatype-date', 'event', 'io', 'node', 'widget',
704 'lp.client', 'lp.ui.editor']});708 'lp.client', 'lp.ui.editor', 'lp.app.date']});
705709
=== modified file 'lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.html'
--- lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.html 2014-05-19 21:56:02 +0000
+++ lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.html 2014-07-08 12:33:30 +0000
@@ -32,6 +32,8 @@
32 <script type="text/javascript"32 <script type="text/javascript"
33 src="../../../../../build/js/lp/app/client.js"></script>33 src="../../../../../build/js/lp/app/client.js"></script>
34 <script type="text/javascript"34 <script type="text/javascript"
35 src="../../../../../build/js/lp/app/date.js"></script>
36 <script type="text/javascript"
35 src="../../../../../build/js/lp/app/testing/mockio.js"></script>37 src="../../../../../build/js/lp/app/testing/mockio.js"></script>
36 <script type="text/javascript"38 <script type="text/javascript"
37 src="../../../../../build/js/lp/app/ui/ui.js"></script>39 src="../../../../../build/js/lp/app/ui/ui.js"></script>
3840
=== modified file 'lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js'
--- lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js 2014-05-26 12:44:57 +0000
+++ lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js 2014-07-08 12:33:30 +0000
@@ -70,25 +70,28 @@
70 "ws.op=getInlineComments&previewdiff_id=1",70 "ws.op=getInlineComments&previewdiff_id=1",
71 mockio.requests[0].config.data);71 mockio.requests[0].config.data);
72 mockio.last_request = mockio.requests[0];72 mockio.last_request = mockio.requests[0];
73 var now = (new Date).valueOf()
73 published_comments = [74 published_comments = [
74 {'line_number': '2',75 {'line_number': '2',
75 'person': person_obj,76 'person': person_obj,
76 'text': 'This is preloaded.',77 'text': 'This is preloaded.',
77 'date': '2012-08-12T10:00:00.00001+00:00'}78 'date': '2012-08-12T10:00:00.00001+00:00'},
79 {'line_number': '3',
80 'person': person_obj,
81 'text': 'This is great.',
82 'date': (new Date(now - 12600000)),
83 },
78 ];84 ];
79 mockio.success({85 mockio.success({
80 responseText: Y.JSON.stringify(published_comments),86 responseText: Y.JSON.stringify(published_comments),
81 responseHeaders: {'Content-Type': 'application/json'}});87 responseHeaders: {'Content-Type': 'application/json'}});
8288
83 // Published comment is displayed.89 // Published comment is displayed.
84 var comments = Y.one('#diff-line-2').next().one('div');90 var first_comments = Y.one('#diff-line-2').next().one('div');
85 // XXX cprov 20140226: test disabled due to ES4 lack of91 var first = first_comments.one('div:first-child')
86 // ISO8601 support. Although the vast majority of production92 Y.Assert.areEqual(
87 // clients run ES5.93 'Foo Bar (name16) wrote on 2012-08-12:',
88 //Y.Assert.areEqual(94 first.one('.boardCommentDetails').get('text'));
89 // 'Foo Bar (name16) wrote on 2012-08-12',
90 // header.get('text'));
91 var first = comments.one('div:first-child')
92 Y.Assert.areEqual(95 Y.Assert.areEqual(
93 'This is preloaded.',96 'This is preloaded.',
94 first.one('.boardCommentBody').get('text'));97 first.one('.boardCommentBody').get('text'));
@@ -103,12 +106,17 @@
103 'Boing!', second.one('.boardCommentBody').get('text'));106 'Boing!', second.one('.boardCommentBody').get('text'));
104107
105 // Draft comment for line 3 is displayed.108 // Draft comment for line 3 is displayed.
106 var third = Y.one('#diff-line-3').next();109 var second_comments = Y.one('#diff-line-3').next().one('div');
110 var third = second_comments.one('div:first-child')
107 Y.Assert.areEqual(111 Y.Assert.areEqual(
108 'Unsaved comment',112 'Foo Bar (name16) wrote 3 hours ago:',
109 third.one('.boardCommentDetails').get('text'));113 third.one('.boardCommentDetails').get('text'));
110 Y.Assert.areEqual(114 var fourth = third.next()
111 'Zoing!', third.one('.boardCommentBody').get('text'));115 Y.Assert.areEqual(
116 'Unsaved comment',
117 fourth.one('.boardCommentDetails').get('text'));
118 Y.Assert.areEqual(
119 'Zoing!', fourth.one('.boardCommentBody').get('text'));
112 },120 },
113121
114 test_draft_handler: function() {122 test_draft_handler: function() {