Launchpad itself

Merge lp:~abentley/launchpad/fix-inline-reply into lp:launchpad/db-devel

  1. fix-inline-reply
  2. Merge into db-devel
Proposed by Aaron Bentley
Status: Merged
Approved by: Tim Penhey
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~abentley/launchpad/fix-inline-reply
Merge into: lp:launchpad/db-devel
Diff against target: 95 lines (+35/-12)
2 files modified
lib/canonical/launchpad/javascript/lp/comment.js (+5/-4)
lib/lp/code/windmill/tests/test_merge_proposal_commenting.py (+30/-8)
To merge this branch: bzr merge lp:~abentley/launchpad/fix-inline-reply
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Review via email: mp+15295@code.launchpad.net

Commit message

Fix and test replying to code review comments.

To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

= Summary =
Fix bug 487209

== Proposed fix ==
Remove whitespace that was causing a lookup failure. Add windmill test of replying to code review comments.

== Pre-implementation notes ==
Tim pointed out to me that the bug was caused by whitespace and asked me to
write a windmill test.

== Implementation details ==
None

== Tests ==
bin/test -t test_merge_proposal_replying --layer=CodeWindmillLayer

== Demo and Q/A ==
Create a merge proposal and make a comment. Reload the page. Click the reply
link and create a reply. If this works, you've succeeded.

Revision history for this message
Tim Penhey (thumper) wrote :

While I think that the new comment text should not be a guid, I think this is fine.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/javascript/lp/comment.js'
2--- lib/canonical/launchpad/javascript/lp/comment.js 2009-11-24 09:30:01 +0000
3+++ lib/canonical/launchpad/javascript/lp/comment.js 2009-11-27 15:11:11 +0000
4@@ -275,11 +275,12 @@
5 content: this.comment_input.get('value'),
6 subject: '',
7 review_type: this.review_type.get('value'),
8- vote: this.get_vote(),
9+ vote: this.get_vote()
10 }
11 };
12- if (this.in_reply_to !== null)
13- config['parameters']['parent'] = this.in_reply_to.get('self_link')
14+ if (this.in_reply_to !== null) {
15+ config.parameters.parent = this.in_reply_to.get('self_link');
16+ }
17 this.lp_client.named_post(
18 LP.client.cache.context.self_link, 'createComment', config);
19 },
20@@ -313,7 +314,7 @@
21 reply_link.length - '+reply'.length);
22 var object_url = '/api/beta' + root_url;
23 this.activateProgressUI('Loading...');
24- window.scrollTo(0, Y.one('label [for=field.vote]').getY());
25+ window.scrollTo(0, Y.one('label[for=field.vote]').getY());
26 this.lp_client.get(object_url, {
27 on: {
28 success: bind(function(comment){
29
30=== modified file 'lib/lp/code/windmill/tests/test_merge_proposal_commenting.py'
31--- lib/lp/code/windmill/tests/test_merge_proposal_commenting.py 2009-10-09 20:27:35 +0000
32+++ lib/lp/code/windmill/tests/test_merge_proposal_commenting.py 2009-11-27 15:11:11 +0000
33@@ -14,7 +14,7 @@
34 from canonical.launchpad.windmill.testing import lpuser
35 from canonical.uuid import generate_uuid
36 from lp.code.windmill.testing import canonical_url, CodeWindmillLayer
37-from lp.testing import TestCaseWithFactory
38+from lp.testing import login_person, TestCaseWithFactory
39
40 WAIT_PAGELOAD = u'30000'
41 WAIT_ELEMENT_COMPLETE = u'30000'
42@@ -28,17 +28,19 @@
43 layer = CodeWindmillLayer
44
45
46- def test_merge_proposal_commenting(self):
47- """Test commenting on bugs."""
48- client = WindmillTestClient('Bug commenting')
49- lpuser.NO_PRIV.ensure_login(client)
50-
51- proposal = self.factory.makeBranchMergeProposal()
52+ def open_proposal_page(self, client, proposal):
53 transaction.commit()
54 client.open(url=canonical_url(proposal))
55 client.waits.forPageLoad(timeout=WAIT_PAGELOAD)
56+
57+ def test_merge_proposal_commenting(self):
58+ """Test commenting on merge proposals."""
59+ client = WindmillTestClient('Code review commenting')
60+ lpuser.NO_PRIV.ensure_login(client)
61+
62+ proposal = self.factory.makeBranchMergeProposal()
63+ self.open_proposal_page(client, proposal)
64 client.waits.forElement(xpath=ADD_COMMENT_BUTTON)
65-
66 # Generate a unique piece of text, so we can run the test multiple
67 # times, without resetting the db.
68 new_comment_text = generate_uuid()
69@@ -50,6 +52,26 @@
70 xpath='//div[@id="conversation"]//div[@class="boardCommentBody"]'
71 '/pre[contains(., "%s")]' % new_comment_text)
72
73+ def test_merge_proposal_replying(self):
74+ """Replying to code review comments works."""
75+ client = WindmillTestClient('Code review commenting')
76+ lpuser.NO_PRIV.ensure_login(client)
77+ proposal = self.factory.makeBranchMergeProposal()
78+ login_person(proposal.registrant)
79+ proposal.createComment(proposal.registrant, 'hello', 'content')
80+ self.open_proposal_page(client, proposal)
81+ REPLY_LINK_XPATH = '//a[contains(., "Reply")]'
82+ client.waits.forElement(xpath=REPLY_LINK_XPATH)
83+ client.click(xpath=REPLY_LINK_XPATH)
84+ client.waits.sleep(milliseconds=10)
85+ client.asserts.assertValue(id="field.comment", validator="> content")
86+ new_comment_text = "My reply"
87+ client.type(text=new_comment_text, id="field.comment")
88+ client.click(xpath=ADD_COMMENT_BUTTON)
89+ client.waits.forElement(
90+ xpath='//div[@id="conversation"]//div[@class="boardCommentBody"]'
91+ '/pre[contains(., "%s")]' % new_comment_text)
92+
93
94 def test_suite():
95 return unittest.TestLoader().loadTestsFromName(__name__)

Subscribers

People subscribed via source and target branches

to status/vote changes: