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
=== modified file 'lib/canonical/launchpad/javascript/lp/comment.js'
--- lib/canonical/launchpad/javascript/lp/comment.js 2009-11-24 09:30:01 +0000
+++ lib/canonical/launchpad/javascript/lp/comment.js 2009-11-27 15:11:11 +0000
@@ -275,11 +275,12 @@
275 content: this.comment_input.get('value'),275 content: this.comment_input.get('value'),
276 subject: '',276 subject: '',
277 review_type: this.review_type.get('value'),277 review_type: this.review_type.get('value'),
278 vote: this.get_vote(),278 vote: this.get_vote()
279 }279 }
280 };280 };
281 if (this.in_reply_to !== null)281 if (this.in_reply_to !== null) {
282 config['parameters']['parent'] = this.in_reply_to.get('self_link')282 config.parameters.parent = this.in_reply_to.get('self_link');
283 }
283 this.lp_client.named_post(284 this.lp_client.named_post(
284 LP.client.cache.context.self_link, 'createComment', config);285 LP.client.cache.context.self_link, 'createComment', config);
285 },286 },
@@ -313,7 +314,7 @@
313 reply_link.length - '+reply'.length);314 reply_link.length - '+reply'.length);
314 var object_url = '/api/beta' + root_url;315 var object_url = '/api/beta' + root_url;
315 this.activateProgressUI('Loading...');316 this.activateProgressUI('Loading...');
316 window.scrollTo(0, Y.one('label [for=field.vote]').getY());317 window.scrollTo(0, Y.one('label[for=field.vote]').getY());
317 this.lp_client.get(object_url, {318 this.lp_client.get(object_url, {
318 on: {319 on: {
319 success: bind(function(comment){320 success: bind(function(comment){
320321
=== modified file 'lib/lp/code/windmill/tests/test_merge_proposal_commenting.py'
--- lib/lp/code/windmill/tests/test_merge_proposal_commenting.py 2009-10-09 20:27:35 +0000
+++ lib/lp/code/windmill/tests/test_merge_proposal_commenting.py 2009-11-27 15:11:11 +0000
@@ -14,7 +14,7 @@
14from canonical.launchpad.windmill.testing import lpuser14from canonical.launchpad.windmill.testing import lpuser
15from canonical.uuid import generate_uuid15from canonical.uuid import generate_uuid
16from lp.code.windmill.testing import canonical_url, CodeWindmillLayer16from lp.code.windmill.testing import canonical_url, CodeWindmillLayer
17from lp.testing import TestCaseWithFactory17from lp.testing import login_person, TestCaseWithFactory
1818
19WAIT_PAGELOAD = u'30000'19WAIT_PAGELOAD = u'30000'
20WAIT_ELEMENT_COMPLETE = u'30000'20WAIT_ELEMENT_COMPLETE = u'30000'
@@ -28,17 +28,19 @@
28 layer = CodeWindmillLayer28 layer = CodeWindmillLayer
2929
3030
31 def test_merge_proposal_commenting(self):31 def open_proposal_page(self, client, proposal):
32 """Test commenting on bugs."""
33 client = WindmillTestClient('Bug commenting')
34 lpuser.NO_PRIV.ensure_login(client)
35
36 proposal = self.factory.makeBranchMergeProposal()
37 transaction.commit()32 transaction.commit()
38 client.open(url=canonical_url(proposal))33 client.open(url=canonical_url(proposal))
39 client.waits.forPageLoad(timeout=WAIT_PAGELOAD)34 client.waits.forPageLoad(timeout=WAIT_PAGELOAD)
35
36 def test_merge_proposal_commenting(self):
37 """Test commenting on merge proposals."""
38 client = WindmillTestClient('Code review commenting')
39 lpuser.NO_PRIV.ensure_login(client)
40
41 proposal = self.factory.makeBranchMergeProposal()
42 self.open_proposal_page(client, proposal)
40 client.waits.forElement(xpath=ADD_COMMENT_BUTTON)43 client.waits.forElement(xpath=ADD_COMMENT_BUTTON)
41
42 # Generate a unique piece of text, so we can run the test multiple44 # Generate a unique piece of text, so we can run the test multiple
43 # times, without resetting the db.45 # times, without resetting the db.
44 new_comment_text = generate_uuid()46 new_comment_text = generate_uuid()
@@ -50,6 +52,26 @@
50 xpath='//div[@id="conversation"]//div[@class="boardCommentBody"]'52 xpath='//div[@id="conversation"]//div[@class="boardCommentBody"]'
51 '/pre[contains(., "%s")]' % new_comment_text)53 '/pre[contains(., "%s")]' % new_comment_text)
5254
55 def test_merge_proposal_replying(self):
56 """Replying to code review comments works."""
57 client = WindmillTestClient('Code review commenting')
58 lpuser.NO_PRIV.ensure_login(client)
59 proposal = self.factory.makeBranchMergeProposal()
60 login_person(proposal.registrant)
61 proposal.createComment(proposal.registrant, 'hello', 'content')
62 self.open_proposal_page(client, proposal)
63 REPLY_LINK_XPATH = '//a[contains(., "Reply")]'
64 client.waits.forElement(xpath=REPLY_LINK_XPATH)
65 client.click(xpath=REPLY_LINK_XPATH)
66 client.waits.sleep(milliseconds=10)
67 client.asserts.assertValue(id="field.comment", validator="> content")
68 new_comment_text = "My reply"
69 client.type(text=new_comment_text, id="field.comment")
70 client.click(xpath=ADD_COMMENT_BUTTON)
71 client.waits.forElement(
72 xpath='//div[@id="conversation"]//div[@class="boardCommentBody"]'
73 '/pre[contains(., "%s")]' % new_comment_text)
74
5375
54def test_suite():76def test_suite():
55 return unittest.TestLoader().loadTestsFromName(__name__)77 return unittest.TestLoader().loadTestsFromName(__name__)

Subscribers

People subscribed via source and target branches

to status/vote changes: