Launchpad itself

Merge lp:~thumper/launchpad/bug-branch-proposal-view into lp:launchpad

  1. bug-branch-proposal-view
  2. Merge into devel
Proposed by Tim Penhey
Status: Merged
Merged at revision: not available
Proposed branch: lp:~thumper/launchpad/bug-branch-proposal-view
Merge into: lp:launchpad
Prerequisite: lp:~thumper/launchpad/project-sourcecode
Diff against target: 640 lines (+233/-57)
10 files modified
lib/lp/code/browser/branchmergeproposal.py (+19/-1)
lib/lp/code/browser/configure.zcml (+10/-5)
lib/lp/code/browser/tests/test_branchmergeproposal.py (+55/-17)
lib/lp/code/model/branchmergeproposal.py (+11/-6)
lib/lp/code/stories/branches/xx-branch-merge-proposals.txt (+40/-27)
lib/lp/code/templates/branch-index.pt (+4/-0)
lib/lp/code/templates/branch-pending-merges.pt (+1/-0)
lib/lp/code/templates/branchmergeproposal-link-summary.pt (+2/-1)
lib/lp/code/templates/branchmergeproposal-vote-summary.pt (+49/-0)
lib/lp/code/tests/helpers.py (+42/-0)
To merge this branch: bzr merge lp:~thumper/launchpad/bug-branch-proposal-view
Reviewer Review Type Date Requested Status
Barry Warsaw (community) ui* Approve
Paul Hummer (community) ui code Approve
Review via email: mp+14736@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

Trying the prerequisite branch bit.

This branch shows more merge proposal details on the branch page.

 browser/branch.py | 1

Drive by pep-8 cleanup

 browser/branchmergeproposal.py | 20 ++++++

Define a title for the status link based on the approver/rejecter.

 browser/configure.zcml | 15 +++--

New view, and renamed an old one to be more consistent.

 browser/tests/test_branchmergeproposal.py | 72 +++++++++++++++++++------

Cleaned up a test before I realised it was the wrong one.
Added a test for the status title in the view above.

 model/branchmergeproposal.py | 17 +++--

This change is just adding the ability to pass the review date through to the function for better testing.

 stories/branches/xx-branch-merge-proposals.txt | 67 +++++++++++++----------

Move some object creation out into a helper function.
Change the explicit BMP urls to use a recorded value.
Add a section at the end to show how merge proposals look on branch pages.

 templates/branch-index.pt | 4 +

Some page based styles.

 templates/branch-pending-merges.pt | 1

Add in the vote summary.

 templates/branchmergeproposal-link-summary.pt | 3 -

Add the title to the status anchor.

 templates/branchmergeproposal-vote-summary.pt | 49 +++++++++++++++++

Render the details.

 tests/helpers.py | 42 ++++++++++++++

Create some standard people/projects for the tests.
 11 files changed, 234 insertions(+), 57 deletions(-)

This branch only has the change on the branch page, but I'm expecting to have it on the bug page as well in a follow on branch.

Revision history for this message
Paul Hummer (rockstar) :
review: Approve (ui code)
Revision history for this message
Barry Warsaw (barry) :
review: Approve (ui*)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py 2009-11-10 22:57:42 +0000
+++ lib/lp/code/browser/branchmergeproposal.py 2009-11-18 00:28:15 +0000
@@ -60,7 +60,8 @@
60from canonical.launchpad.webapp.breadcrumb import Breadcrumb60from canonical.launchpad.webapp.breadcrumb import Breadcrumb
61from canonical.launchpad.webapp.interfaces import IPrimaryContext61from canonical.launchpad.webapp.interfaces import IPrimaryContext
62from canonical.launchpad.webapp.menu import NavigationMenu62from canonical.launchpad.webapp.menu import NavigationMenu
63from canonical.launchpad.webapp.tales import FormattersAPI63from canonical.launchpad.webapp.tales import (
64 DateTimeFormatterAPI, FormattersAPI)
64from canonical.widgets.lazrjs import (65from canonical.widgets.lazrjs import (
65 TextAreaEditorWidget, vocabulary_to_choice_edit_items)66 TextAreaEditorWidget, vocabulary_to_choice_edit_items)
6667
@@ -138,6 +139,23 @@
138 }139 }
139 return friendly_texts[self.context.queue_status]140 return friendly_texts[self.context.queue_status]
140141
142 @property
143 def status_title(self):
144 """The title for the status text.
145
146 Only set if the status is approved or rejected.
147 """
148 result = ''
149 if self.context.queue_status in (
150 BranchMergeProposalStatus.CODE_APPROVED,
151 BranchMergeProposalStatus.REJECTED
152 ):
153 formatter = DateTimeFormatterAPI(self.context.date_reviewed)
154 result = '%s %s' % (
155 self.context.reviewer.displayname,
156 formatter.displaydate())
157 return result
158
141159
142class BranchMergeProposalMenuMixin:160class BranchMergeProposalMenuMixin:
143 """Mixin class for merge proposal menus."""161 """Mixin class for merge proposal menus."""
144162
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2009-11-13 20:12:17 +0000
+++ lib/lp/code/browser/configure.zcml 2009-11-18 00:28:15 +0000
@@ -184,13 +184,18 @@
184 name="+pagelet-summary"184 name="+pagelet-summary"
185 template="../templates/branchmergeproposal-pagelet-summary.pt"/>185 template="../templates/branchmergeproposal-pagelet-summary.pt"/>
186 </browser:pages>186 </browser:pages>
187 <browser:page187 <browser:pages
188 name="+votes"
189 for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"188 for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"
190 class="lp.code.browser.branchmergeproposal.BranchMergeProposalVoteView"189 class="lp.code.browser.branchmergeproposal.BranchMergeProposalVoteView"
191 facet="branches"190 facet="branches"
192 permission="launchpad.View"191 permission="launchpad.View">
193 template="../templates/branchmergeproposal-votes.pt"/>192 <browser:page
193 name="+votes"
194 template="../templates/branchmergeproposal-votes.pt"/>
195 <browser:page
196 name="+vote-summary"
197 template="../templates/branchmergeproposal-vote-summary.pt"/>
198 </browser:pages>
194 <browser:pages199 <browser:pages
195 for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"200 for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"
196 class="lp.code.browser.branchmergeproposal.BranchMergeProposalEditView"201 class="lp.code.browser.branchmergeproposal.BranchMergeProposalEditView"
@@ -276,7 +281,7 @@
276 class="lp.code.browser.branchmergeproposal.BranchMergeCandidateView"281 class="lp.code.browser.branchmergeproposal.BranchMergeCandidateView"
277 facet="branches"282 facet="branches"
278 permission="zope.Public"283 permission="zope.Public"
279 template="../templates/branch-merge-link-summary.pt"/>284 template="../templates/branchmergeproposal-link-summary.pt"/>
280 <browser:page285 <browser:page
281 name="+pagelet-subscribers"286 name="+pagelet-subscribers"
282 for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"287 for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"
283288
=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py 2009-11-01 23:13:09 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py 2009-11-18 00:28:15 +0000
@@ -7,10 +7,11 @@
77
8__metaclass__ = type8__metaclass__ = type
99
10from datetime import timedelta10from datetime import datetime, timedelta
11from difflib import unified_diff11from difflib import unified_diff
12import unittest12import unittest
1313
14import pytz
14import transaction15import transaction
15from zope.component import getMultiAdapter16from zope.component import getMultiAdapter
16from zope.security.interfaces import Unauthorized17from zope.security.interfaces import Unauthorized
@@ -66,6 +67,7 @@
66 link = menu.add_comment()67 link = menu.add_comment()
67 self.assertTrue(menu.add_comment().enabled)68 self.assertTrue(menu.add_comment().enabled)
6869
70
69class TestDecoratedCodeReviewVoteReference(TestCaseWithFactory):71class TestDecoratedCodeReviewVoteReference(TestCaseWithFactory):
7072
71 layer = DatabaseFunctionalLayer73 layer = DatabaseFunctionalLayer
@@ -459,13 +461,6 @@
459 self.bmp = self.factory.makeBranchMergeProposal(registrant=self.user)461 self.bmp = self.factory.makeBranchMergeProposal(registrant=self.user)
460 login_person(self.user)462 login_person(self.user)
461463
462 def _createView(self):
463 # Construct the view and initialize it.
464 view = BranchMergeProposalView(
465 self.bmp, LaunchpadTestRequest())
466 view.initialize()
467 return view
468
469 def makeTeamReview(self):464 def makeTeamReview(self):
470 owner = self.bmp.source_branch.owner465 owner = self.bmp.source_branch.owner
471 review_team = self.factory.makeTeam()466 review_team = self.factory.makeTeam()
@@ -477,7 +472,7 @@
477 albert = self.factory.makePerson()472 albert = self.factory.makePerson()
478 albert.join(review.reviewer)473 albert.join(review.reviewer)
479 login_person(albert)474 login_person(albert)
480 view = self._createView()475 view = create_initialized_view(self.bmp, '+index')
481 view.claim_action.success({'review_id': review.id})476 view.claim_action.success({'review_id': review.id})
482 self.assertEqual(albert, review.reviewer)477 self.assertEqual(albert, review.reviewer)
483478
@@ -486,13 +481,13 @@
486 review = self.makeTeamReview()481 review = self.makeTeamReview()
487 albert = self.factory.makePerson()482 albert = self.factory.makePerson()
488 login_person(albert)483 login_person(albert)
489 view = self._createView()484 view = create_initialized_view(self.bmp, '+index')
490 self.assertRaises(Unauthorized, view.claim_action.success,485 self.assertRaises(Unauthorized, view.claim_action.success,
491 {'review_id': review.id})486 {'review_id': review.id})
492487
493 def test_preview_diff_text_with_no_diff(self):488 def test_preview_diff_text_with_no_diff(self):
494 """review_diff should be None when there is no context.review_diff."""489 """review_diff should be None when there is no context.review_diff."""
495 view = self._createView()490 view = create_initialized_view(self.bmp, '+index')
496 self.assertIs(None, view.preview_diff_text)491 self.assertIs(None, view.preview_diff_text)
497492
498 def test_review_diff_utf8(self):493 def test_review_diff_utf8(self):
@@ -502,8 +497,9 @@
502 diff = StaticDiff.acquireFromText('x', 'y', diff_bytes)497 diff = StaticDiff.acquireFromText('x', 'y', diff_bytes)
503 transaction.commit()498 transaction.commit()
504 self.bmp.review_diff = diff499 self.bmp.review_diff = diff
500 view = create_initialized_view(self.bmp, '+index')
505 self.assertEqual(diff_bytes.decode('utf-8'),501 self.assertEqual(diff_bytes.decode('utf-8'),
506 self._createView().preview_diff_text)502 view.preview_diff_text)
507503
508 def test_review_diff_all_chars(self):504 def test_review_diff_all_chars(self):
509 """review_diff should work on diffs containing all possible bytes."""505 """review_diff should work on diffs containing all possible bytes."""
@@ -512,8 +508,9 @@
512 diff = StaticDiff.acquireFromText('x', 'y', diff_bytes)508 diff = StaticDiff.acquireFromText('x', 'y', diff_bytes)
513 transaction.commit()509 transaction.commit()
514 self.bmp.review_diff = diff510 self.bmp.review_diff = diff
511 view = create_initialized_view(self.bmp, '+index')
515 self.assertEqual(diff_bytes.decode('windows-1252', 'replace'),512 self.assertEqual(diff_bytes.decode('windows-1252', 'replace'),
516 self._createView().preview_diff_text)513 view.preview_diff_text)
517514
518 def addReviewDiff(self):515 def addReviewDiff(self):
519 review_diff_bytes = ''.join(unified_diff('', 'review'))516 review_diff_bytes = ''.join(unified_diff('', 'review'))
@@ -532,27 +529,31 @@
532 def test_preview_diff_prefers_preview_diff(self):529 def test_preview_diff_prefers_preview_diff(self):
533 """The preview will be used for BMP with both a review and preview."""530 """The preview will be used for BMP with both a review and preview."""
534 preview_diff = self.addBothDiffs()531 preview_diff = self.addBothDiffs()
535 self.assertEqual(preview_diff, self._createView().preview_diff)532 view = create_initialized_view(self.bmp, '+index')
533 self.assertEqual(preview_diff, view.preview_diff)
536534
537 def test_preview_diff_uses_review_diff(self):535 def test_preview_diff_uses_review_diff(self):
538 """The review diff will be used if there is no preview."""536 """The review diff will be used if there is no preview."""
539 review_diff = self.addReviewDiff()537 review_diff = self.addReviewDiff()
538 view = create_initialized_view(self.bmp, '+index')
540 self.assertEqual(review_diff.diff,539 self.assertEqual(review_diff.diff,
541 self._createView().preview_diff)540 view.preview_diff)
542541
543 def test_review_diff_text_prefers_preview_diff(self):542 def test_review_diff_text_prefers_preview_diff(self):
544 """The preview will be used for BMP with both a review and preview."""543 """The preview will be used for BMP with both a review and preview."""
545 preview_diff = self.addBothDiffs()544 preview_diff = self.addBothDiffs()
546 transaction.commit()545 transaction.commit()
546 view = create_initialized_view(self.bmp, '+index')
547 self.assertEqual(547 self.assertEqual(
548 preview_diff.text, self._createView().preview_diff_text)548 preview_diff.text, view.preview_diff_text)
549549
550 def test_linked_bugs_excludes_mutual_bugs(self):550 def test_linked_bugs_excludes_mutual_bugs(self):
551 """List bugs that are linked to the source only."""551 """List bugs that are linked to the source only."""
552 bug = self.factory.makeBug()552 bug = self.factory.makeBug()
553 self.bmp.source_branch.linkBug(bug, self.bmp.registrant)553 self.bmp.source_branch.linkBug(bug, self.bmp.registrant)
554 self.bmp.target_branch.linkBug(bug, self.bmp.registrant)554 self.bmp.target_branch.linkBug(bug, self.bmp.registrant)
555 self.assertEqual([], self._createView().linked_bugs)555 view = create_initialized_view(self.bmp, '+index')
556 self.assertEqual([], view.linked_bugs)
556557
557558
558class TestBranchMergeProposalChangeStatusOptions(TestCaseWithFactory):559class TestBranchMergeProposalChangeStatusOptions(TestCaseWithFactory):
@@ -692,5 +693,42 @@
692 self.assertEqual(u'\u2615', diff_attachment.diff_text)693 self.assertEqual(u'\u2615', diff_attachment.diff_text)
693694
694695
696class TestBranchMergeCandidateView(TestCaseWithFactory):
697 """Test the status title for the view."""
698
699 layer = DatabaseFunctionalLayer
700
701 def test_needs_review_title(self):
702 # No title is set for a proposal needing review.
703 bmp = self.factory.makeBranchMergeProposal(
704 set_state=BranchMergeProposalStatus.NEEDS_REVIEW)
705 view = create_initialized_view(bmp, '+link-summary')
706 self.assertEqual('', view.status_title)
707
708 def test_approved_shows_reviewer(self):
709 # If the proposal is approved, the approver is shown in the title
710 # along with when they approved it.
711 bmp = self.factory.makeBranchMergeProposal()
712 owner = bmp.target_branch.owner
713 login_person(bmp.target_branch.owner)
714 owner.displayname = 'Eric'
715 bmp.approveBranch(owner, 'some-rev', datetime(
716 year=2008, month=9, day=10, tzinfo=pytz.UTC))
717 view = create_initialized_view(bmp, '+link-summary')
718 self.assertEqual('Eric on 2008-09-10', view.status_title)
719
720 def test_rejected_shows_reviewer(self):
721 # If the proposal is rejected, the approver is shown in the title
722 # along with when they approved it.
723 bmp = self.factory.makeBranchMergeProposal()
724 owner = bmp.target_branch.owner
725 login_person(bmp.target_branch.owner)
726 owner.displayname = 'Eric'
727 bmp.rejectBranch(owner, 'some-rev', datetime(
728 year=2008, month=9, day=10, tzinfo=pytz.UTC))
729 view = create_initialized_view(bmp, '+link-summary')
730 self.assertEqual('Eric on 2008-09-10', view.status_title)
731
732
695def test_suite():733def test_suite():
696 return unittest.TestLoader().loadTestsFromName(__name__)734 return unittest.TestLoader().loadTestsFromName(__name__)
697735
=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py 2009-10-05 14:17:48 +0000
+++ lib/lp/code/model/branchmergeproposal.py 2009-11-18 00:28:15 +0000
@@ -361,7 +361,8 @@
361 # or superseded, then it is valid to be merged.361 # or superseded, then it is valid to be merged.
362 return (self.queue_status not in FINAL_STATES)362 return (self.queue_status not in FINAL_STATES)
363363
364 def _reviewProposal(self, reviewer, next_state, revision_id):364 def _reviewProposal(self, reviewer, next_state, revision_id,
365 _date_reviewed=None):
365 """Set the proposal to one of the two review statuses."""366 """Set the proposal to one of the two review statuses."""
366 # Check the reviewer can review the code for the target branch.367 # Check the reviewer can review the code for the target branch.
367 old_state = self.queue_status368 old_state = self.queue_status
@@ -371,21 +372,25 @@
371 self._transitionToState(next_state, reviewer)372 self._transitionToState(next_state, reviewer)
372 # Record the reviewer373 # Record the reviewer
373 self.reviewer = reviewer374 self.reviewer = reviewer
374 self.date_reviewed = UTC_NOW375 if _date_reviewed is None:
376 _date_reviewed = UTC_NOW
377 self.date_reviewed = _date_reviewed
375 # Record the reviewed revision id378 # Record the reviewed revision id
376 self.reviewed_revision_id = revision_id379 self.reviewed_revision_id = revision_id
377 notify(BranchMergeProposalStatusChangeEvent(380 notify(BranchMergeProposalStatusChangeEvent(
378 self, reviewer, old_state, next_state))381 self, reviewer, old_state, next_state))
379382
380 def approveBranch(self, reviewer, revision_id):383 def approveBranch(self, reviewer, revision_id, _date_reviewed=None):
381 """See `IBranchMergeProposal`."""384 """See `IBranchMergeProposal`."""
382 self._reviewProposal(385 self._reviewProposal(
383 reviewer, BranchMergeProposalStatus.CODE_APPROVED, revision_id)386 reviewer, BranchMergeProposalStatus.CODE_APPROVED, revision_id,
387 _date_reviewed)
384388
385 def rejectBranch(self, reviewer, revision_id):389 def rejectBranch(self, reviewer, revision_id, _date_reviewed=None):
386 """See `IBranchMergeProposal`."""390 """See `IBranchMergeProposal`."""
387 self._reviewProposal(391 self._reviewProposal(
388 reviewer, BranchMergeProposalStatus.REJECTED, revision_id)392 reviewer, BranchMergeProposalStatus.REJECTED, revision_id,
393 _date_reviewed)
389394
390 def enqueue(self, queuer, revision_id):395 def enqueue(self, queuer, revision_id):
391 """See `IBranchMergeProposal`."""396 """See `IBranchMergeProposal`."""
392397
=== modified file 'lib/lp/code/stories/branches/xx-branch-merge-proposals.txt'
--- lib/lp/code/stories/branches/xx-branch-merge-proposals.txt 2009-10-28 19:31:47 +0000
+++ lib/lp/code/stories/branches/xx-branch-merge-proposals.txt 2009-11-18 00:28:15 +0000
@@ -29,6 +29,8 @@
29 >>> _unused = branch.subscribe(subscriber,29 >>> _unused = branch.subscribe(subscriber,
30 ... BranchSubscriptionNotificationLevel.NOEMAIL, None,30 ... BranchSubscriptionNotificationLevel.NOEMAIL, None,
31 ... CodeReviewNotificationLevel.FULL)31 ... CodeReviewNotificationLevel.FULL)
32 >>> from lp.code.tests.helpers import make_erics_fooix_project
33 >>> locals().update(make_erics_fooix_project(factory))
32 >>> logout()34 >>> logout()
3335
34Any logged in user can register a merge proposal. Registering36Any logged in user can register a merge proposal. Registering
@@ -68,7 +70,8 @@
6870
69Registering the merge proposal takes the user to the new merge proposal.71Registering the merge proposal takes the user to the new merge proposal.
7072
71 >>> print nopriv_browser.url73 >>> klingon_proposal = nopriv_browser.url
74 >>> print klingon_proposal
72 http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/...75 http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/...
7376
74The summary reflects the selected target and prerequisite.77The summary reflects the selected target and prerequisite.
@@ -97,9 +100,6 @@
97 ... 'Add more <b>mojo</b>')100 ... 'Add more <b>mojo</b>')
98 >>> nopriv_browser.getControl('Update').click()101 >>> nopriv_browser.getControl('Update').click()
99102
100 >>> print nopriv_browser.url
101 http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/1
102
103 >>> print_tag_with_id(nopriv_browser.contents, 'edit-description')103 >>> print_tag_with_id(nopriv_browser.contents, 'edit-description')
104 Commit Message104 Commit Message
105 Add more &lt;b&gt;mojo&lt;/b&gt;105 Add more &lt;b&gt;mojo&lt;/b&gt;
@@ -113,7 +113,6 @@
113for the source_branch.113for the source_branch.
114114
115 >>> login('foo.bar@canonical.com')115 >>> login('foo.bar@canonical.com')
116 >>> eric = factory.makePerson(email="eric@example.com", password="test")
117 >>> bmp = factory.makeBranchMergeProposal(registrant=eric)116 >>> bmp = factory.makeBranchMergeProposal(registrant=eric)
118 >>> bmp_url = canonical_url(bmp)117 >>> bmp_url = canonical_url(bmp)
119 >>> branch_url = canonical_url(bmp.source_branch)118 >>> branch_url = canonical_url(bmp.source_branch)
@@ -128,14 +127,13 @@
128127
129 >>> sample_browser = setupBrowser(auth="Basic test@canonical.com:test")128 >>> sample_browser = setupBrowser(auth="Basic test@canonical.com:test")
130129
130
131Requesting reviews131Requesting reviews
132------------------132------------------
133133
134You can request a review of a merge proposal.134You can request a review of a merge proposal.
135135
136 >>> sample_browser.open(136 >>> sample_browser.open(klingon_proposal)
137 ... 'http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/1'
138 ... )
139 >>> sample_browser.getLink('Request a review').click()137 >>> sample_browser.getLink('Request a review').click()
140 >>> sample_browser.getControl('Reviewer').value = 'mark'138 >>> sample_browser.getControl('Reviewer').value = 'mark'
141 >>> sample_browser.getControl('Review type').value = 'first'139 >>> sample_browser.getControl('Review type').value = 'first'
@@ -169,7 +167,7 @@
169 Reviewer Review Type Date Requested Status167 Reviewer Review Type Date Requested Status
170 Sample Person Pending [Review]168 Sample Person Pending [Review]
171 Mark Shuttleworth second ... ago Pending169 Mark Shuttleworth second ... ago Pending
172 Review via email: mp+1@code.launchpad.dev170 Review via email: mp+...@code.launchpad.dev
173 Request another review171 Request another review
174172
175173
@@ -178,16 +176,14 @@
178176
179People not logged in cannot perform reviews.177People not logged in cannot perform reviews.
180178
181 >>> anon_browser.open('http://code.launchpad.dev/~name12/gnome-terminal'179 >>> anon_browser.open(klingon_proposal)
182 ... '/klingon/+merge/1')
183 >>> link = anon_browser.getLink('[Review]')180 >>> link = anon_browser.getLink('[Review]')
184 Traceback (most recent call last):181 Traceback (most recent call last):
185 LinkNotFoundError182 LinkNotFoundError
186183
187People who are logged in can perform reviews.184People who are logged in can perform reviews.
188185
189 >>> nopriv_browser.open('http://code.launchpad.dev/~name12/gnome-terminal'186 >>> nopriv_browser.open(klingon_proposal)
190 ... '/klingon/+merge/1')
191 >>> nopriv_browser.getLink('Add a review or comment').click()187 >>> nopriv_browser.getLink('Add a review or comment').click()
192 >>> nopriv_browser.getControl(name='field.comment').value = "Don't like it"188 >>> nopriv_browser.getControl(name='field.comment').value = "Don't like it"
193 >>> nopriv_browser.getControl(name='field.vote').getControl(189 >>> nopriv_browser.getControl(name='field.vote').getControl(
@@ -201,8 +197,7 @@
201197
202People can claim reviews for teams of which they are a member.198People can claim reviews for teams of which they are a member.
203199
204 >>> sample_browser.open('http://code.launchpad.dev/~name12/gnome-terminal'200 >>> sample_browser.open(klingon_proposal)
205 ... '/klingon/+merge/1')
206 >>> sample_browser.getLink('Request another review').click()201 >>> sample_browser.getLink('Request another review').click()
207 >>> sample_browser.getControl('Reviewer').value = 'hwdb-team'202 >>> sample_browser.getControl('Reviewer').value = 'hwdb-team'
208 >>> sample_browser.getControl('Review type').value = 'claimable'203 >>> sample_browser.getControl('Review type').value = 'claimable'
@@ -213,8 +208,7 @@
213 Reviewer Review Type Date Requested Status...208 Reviewer Review Type Date Requested Status...
214 HWDB Team claimable ... ago Pending ...209 HWDB Team claimable ... ago Pending ...
215 >>> foobar_browser = setupBrowser(auth="Basic foo.bar@canonical.com:test")210 >>> foobar_browser = setupBrowser(auth="Basic foo.bar@canonical.com:test")
216 >>> foobar_browser.open('http://code.launchpad.dev/~name12/gnome-terminal'211 >>> foobar_browser.open(klingon_proposal)
217 ... '/klingon/+merge/1')
218 >>> foobar_browser.getControl('Claim review').click()212 >>> foobar_browser.getControl('Claim review').click()
219 >>> pending = find_tag_by_id(213 >>> pending = find_tag_by_id(
220 ... foobar_browser.contents, 'code-review-votes')214 ... foobar_browser.contents, 'code-review-votes')
@@ -230,8 +224,7 @@
230 >>> foobar_browser.getLink('Reassign').click()224 >>> foobar_browser.getLink('Reassign').click()
231 >>> foobar_browser.getControl('Reviewer').value = 'hwdb-team'225 >>> foobar_browser.getControl('Reviewer').value = 'hwdb-team'
232 >>> foobar_browser.getControl('Reassign').click()226 >>> foobar_browser.getControl('Reassign').click()
233 >>> foobar_browser.open('http://code.launchpad.dev/~name12/gnome-terminal'227 >>> foobar_browser.open(klingon_proposal)
234 ... '/klingon/+merge/1')
235 >>> pending = find_tag_by_id(228 >>> pending = find_tag_by_id(
236 ... foobar_browser.contents, 'code-review-votes')229 ... foobar_browser.contents, 'code-review-votes')
237230
@@ -248,8 +241,7 @@
248When a branch has been merged into the target branch, the proposal should241When a branch has been merged into the target branch, the proposal should
249be marked as merged.242be marked as merged.
250243
251 >>> nopriv_browser.open(244 >>> nopriv_browser.open(klingon_proposal)
252 ... 'http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/1')
253245
254The edit icon at the end of the status allows the user to edit the status.246The edit icon at the end of the status allows the user to edit the status.
255247
@@ -264,8 +256,6 @@
264 >>> nopriv_browser.getControl(name='field.queue_status').displayValue = (256 >>> nopriv_browser.getControl(name='field.queue_status').displayValue = (
265 ... ['Merged'])257 ... ['Merged'])
266 >>> nopriv_browser.getControl('Change Status').click()258 >>> nopriv_browser.getControl('Change Status').click()
267 >>> print nopriv_browser.url
268 http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/1
269259
270The proposal is now shown as have being merged.260The proposal is now shown as have being merged.
271261
@@ -282,6 +272,7 @@
282 >>> print extract_text(find_tag_by_id(272 >>> print extract_text(find_tag_by_id(
283 ... sample_browser.contents, 'landing-targets'))273 ... sample_browser.contents, 'landing-targets'))
284 Merged into lp://dev/~name12/gnome-terminal/main274 Merged into lp://dev/~name12/gnome-terminal/main
275 ...
285 >>> sample_browser.getLink('Merged').click()276 >>> sample_browser.getLink('Merged').click()
286 >>> print_summary(sample_browser)277 >>> print_summary(sample_browser)
287 Status: Merged278 Status: Merged
@@ -303,6 +294,7 @@
303 >>> print extract_text(find_tag_by_id(294 >>> print extract_text(find_tag_by_id(
304 ... sample_browser.contents, 'landing-targets'))295 ... sample_browser.contents, 'landing-targets'))
305 Merged into lp://dev/~name12/gnome-terminal/main at revision 42296 Merged into lp://dev/~name12/gnome-terminal/main at revision 42
297 ...
306298
307299
308Resubmitting proposals300Resubmitting proposals
@@ -315,9 +307,7 @@
315branches but with the state set to work-in-progress.307branches but with the state set to work-in-progress.
316308
317 >>> login('foo.bar@canonical.com')309 >>> login('foo.bar@canonical.com')
318 >>> fooix = factory.makeProduct(name='fooix')310 >>> bmp = factory.makeBranchMergeProposal(target_branch=trunk)
319 >>> target = factory.makeProductBranch(product=fooix, owner=eric)
320 >>> bmp = factory.makeBranchMergeProposal(target_branch=target)
321 >>> bmp_url = canonical_url(bmp)311 >>> bmp_url = canonical_url(bmp)
322 >>> logout()312 >>> logout()
323 >>> eric_browser.open(bmp_url)313 >>> eric_browser.open(bmp_url)
@@ -538,7 +528,6 @@
538 Bug #...: Bug for linking (Undecided &ndash; New)528 Bug #...: Bug for linking (Undecided &ndash; New)
539529
540530
541
542Target branch edge cases531Target branch edge cases
543------------------------532------------------------
544533
@@ -626,3 +615,27 @@
626 >>> print extract_text(get_review_diff())615 >>> print extract_text(get_review_diff())
627 Preview Diff616 Preview Diff
628 Empty617 Empty
618
619
620Merge proposal details shown on the branch page
621-----------------------------------------------
622
623A branch that has a merge proposal, but no requested reviews shows this on the
624branch page.
625
626 >>> nopriv_browser.open('http://code.launchpad.dev/~fred/fooix/feature')
627 >>> nopriv_browser.getLink('Propose for merging').click()
628 >>> nopriv_browser.getControl('Reviewer').value = ''
629 >>> nopriv_browser.getControl('Propose Merge').click()
630 >>> nopriv_browser.getLink('lp://dev/~fred/fooix/feature').click()
631 >>> print_tag_with_id(nopriv_browser.contents, 'landing-targets')
632 Ready for review for merging into lp://dev/fooix
633 No reviews requested
634
635If there are reviews either pending or completed these are also shown.
636
637 >>> nopriv_browser.open('http://code.launchpad.dev/~fred/fooix/proposed')
638 >>> print_tag_with_id(nopriv_browser.contents, 'landing-targets')
639 Ready for review for merging into lp://dev/fooix
640 Eric the Viking: Pending (code) requested ... ago
641 Diff: 47 lines
629642
=== modified file 'lib/lp/code/templates/branch-index.pt'
--- lib/lp/code/templates/branch-index.pt 2009-10-11 02:33:30 +0000
+++ lib/lp/code/templates/branch-index.pt 2009-11-18 00:28:15 +0000
@@ -25,6 +25,10 @@
25 #download-url dd span.branch-url, #upload-url dd span.branch-url {25 #download-url dd span.branch-url, #upload-url dd span.branch-url {
26 font-weight: normal;26 font-weight: normal;
27 }27 }
28 dl.reviews dd {
29 padding-left: 2em;
30 margin-bottom: 0;
31 }
2832
29 </style>33 </style>
30 <script type="text/javascript"34 <script type="text/javascript"
3135
=== modified file 'lib/lp/code/templates/branch-pending-merges.pt'
--- lib/lp/code/templates/branch-pending-merges.pt 2009-09-18 21:20:42 +0000
+++ lib/lp/code/templates/branch-pending-merges.pt 2009-11-18 00:28:15 +0000
@@ -36,6 +36,7 @@
36 <div>36 <div>
37 <tal:merge-fragment tal:replace="structure mergeproposal/@@+link-summary" />37 <tal:merge-fragment tal:replace="structure mergeproposal/@@+link-summary" />
38 </div>38 </div>
39 <tal:merge-fragment tal:replace="structure mergeproposal/@@+vote-summary" />
39 </tal:landing-candidates>40 </tal:landing-candidates>
40 </div>41 </div>
4142
4243
=== renamed file 'lib/lp/code/templates/branch-merge-link-summary.pt' => 'lib/lp/code/templates/branchmergeproposal-link-summary.pt'
--- lib/lp/code/templates/branch-merge-link-summary.pt 2009-09-22 16:31:42 +0000
+++ lib/lp/code/templates/branchmergeproposal-link-summary.pt 2009-11-18 00:28:15 +0000
@@ -4,7 +4,8 @@
4 xmlns:i18n="http://xml.zope.org/namespaces/i18n">4 xmlns:i18n="http://xml.zope.org/namespaces/i18n">
55
6 <img src="/@@/merge-proposal-icon" />6 <img src="/@@/merge-proposal-icon" />
7 <a tal:attributes="href context/fmt:url"7 <a tal:attributes="href context/fmt:url;
8 title view/status_title"
8 tal:content="view/friendly_text">Approved</a>9 tal:content="view/friendly_text">Approved</a>
9 <tal:for-merging condition="not: context/queue_status/enumvalue:MERGED">10 <tal:for-merging condition="not: context/queue_status/enumvalue:MERGED">
10 for merging11 for merging
1112
=== added file 'lib/lp/code/templates/branchmergeproposal-vote-summary.pt'
--- lib/lp/code/templates/branchmergeproposal-vote-summary.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/branchmergeproposal-vote-summary.pt 2009-11-18 00:28:15 +0000
@@ -0,0 +1,49 @@
1<tal:root
2 xmlns:tal="http://xml.zope.org/namespaces/tal"
3 xmlns:metal="http://xml.zope.org/namespaces/metal"
4 xmlns:i18n="http://xml.zope.org/namespaces/i18n">
5
6 <tal:comment condition="nothing">
7 <!--
8 Yet again we are bitten by white space issues, so some tags in this
9 template have closing brackets on following lines, and not breaks
10 between some tags.
11 -->
12 </tal:comment>
13
14<dl class="reviews">
15 <dd tal:condition="not: view/reviews">
16 No reviews requested
17 </dd>
18 <dd tal:repeat="review view/current_reviews">
19 <tal:reviewer replace="structure review/reviewer/fmt:link:mainsite">
20 Eric the Reviewer</tal:reviewer
21 ><tal:community condition="not: review/trusted">
22 (community)</tal:community>:
23 <span tal:attributes="class string:vote${review/comment/vote/name}"
24 tal:content="review/status_text">
25 Approved
26 </span>
27 <tal:vote-tags condition="review/review_type_str">
28 (<tal:tag replace="review/review_type_str"/>)
29 </tal:vote-tags>
30 <tal:date replace="review/date_of_comment/fmt:displaydate" />
31 </dd>
32 <dd tal:repeat="review view/requested_reviews"
33 tal:attributes="id string:review-${review/reviewer/name}">
34 <tal:reviewer
35 tal:replace="structure review/reviewer/fmt:link:mainsite" />:
36
37 <span class="votePENDING">Pending</span>
38 <tal:vote-tags condition="review/review_type_str">
39 (<tal:tag replace="review/review_type_str"/>)
40 </tal:vote-tags>
41 requested
42 <tal:date replace="review/date_requested/fmt:approximatedate"/>
43 </dd>
44 <dd tal:condition="context/preview_diff">
45 Diff: <tal:diff replace="structure context/preview_diff/fmt:link"/>
46 </dd>
47
48</dl>
49</tal:root>
050
=== modified file 'lib/lp/code/tests/helpers.py'
--- lib/lp/code/tests/helpers.py 2009-10-26 18:40:04 +0000
+++ lib/lp/code/tests/helpers.py 2009-11-18 00:28:15 +0000
@@ -6,10 +6,12 @@
6__metaclass__ = type6__metaclass__ = type
7__all__ = [7__all__ = [
8 'make_linked_package_branch',8 'make_linked_package_branch',
9 'make_erics_fooix_project',
9 ]10 ]
1011
1112
12from datetime import timedelta13from datetime import timedelta
14from difflib import unified_diff
13from itertools import count15from itertools import count
1416
15from zope.component import getUtility17from zope.component import getUtility
@@ -23,6 +25,46 @@
23from lp.testing import time_counter25from lp.testing import time_counter
2426
2527
28def make_erics_fooix_project(factory):
29 """Make Eric, the Fooix project, and some branches.
30
31 :return: a dict of objects to put into local scope.
32 """
33 result = {}
34 eric = factory.makePerson(
35 name='eric', displayname='Eric the Viking',
36 email='eric@example.com', password='test')
37 fooix = factory.makeProduct(
38 name='fooix', displayname='Fooix', owner=eric)
39 trunk = factory.makeProductBranch(
40 owner=eric, product=fooix, name='trunk')
41 removeSecurityProxy(fooix.development_focus).branch = trunk
42 # Development is done by Fred.
43 fred = factory.makePerson(
44 name='fred', displayname='Fred Flintstone',
45 email='fred@example.com', password='test')
46 feature = factory.makeProductBranch(
47 owner=fred, product=fooix, name='feature')
48 proposed = factory.makeProductBranch(
49 owner=fred, product=fooix, name='proposed')
50 bmp = proposed.addLandingTarget(
51 registrant=fred, target_branch=trunk, needs_review=True,
52 review_requests=[(eric, 'code')])
53 # And fake a diff.
54 naked_bmp = removeSecurityProxy(bmp)
55 preview = removeSecurityProxy(bmp.updatePreviewDiff(
56 ''.join(unified_diff('', 'random content')), u'rev-a', u'rev-b'))
57 naked_bmp.source_branch.last_scanned_id = preview.source_revision_id
58 naked_bmp.target_branch.last_scanned_id = preview.target_revision_id
59 preview.diff_lines_count = 47
60 preview.added_lines_count = 7
61 preview.remvoed_lines_count = 13
62 preview.diffstat = {'file1': (3, 8), 'file2': (4, 5)}
63 return {
64 'eric': eric, 'fooix': fooix, 'trunk':trunk, 'feature': feature,
65 'proposed': proposed, 'fred': fred}
66
67
26def make_linked_package_branch(factory, distribution=None,68def make_linked_package_branch(factory, distribution=None,
27 sourcepackagename=None):69 sourcepackagename=None):
28 """Make a new package branch and make it official."""70 """Make a new package branch and make it official."""