Launchpad itself

Merge lp:~thumper/launchpad/revisions-in-conversation into lp:launchpad

  1. revisions-in-conversation
  2. Merge into devel
Proposed by Tim Penhey
Status: Merged
Merged at revision: 10022
Proposed branch: lp:~thumper/launchpad/revisions-in-conversation
Merge into: lp:launchpad
Diff against target: 676 lines (+350/-59)
15 files modified
lib/lp/code/browser/branch.py (+1/-1)
lib/lp/code/browser/branchmergeproposal.py (+68/-1)
lib/lp/code/browser/configure.zcml (+12/-0)
lib/lp/code/browser/tests/test_branchmergeproposal.py (+59/-0)
lib/lp/code/configure.zcml (+1/-1)
lib/lp/code/doc/branch.txt (+3/-3)
lib/lp/code/interfaces/branch.py (+14/-1)
lib/lp/code/model/branch.py (+21/-10)
lib/lp/code/model/revision.py (+10/-6)
lib/lp/code/model/tests/test_branch.py (+70/-31)
lib/lp/code/stories/branches/xx-code-review-comments.txt (+46/-3)
lib/lp/code/templates/codereviewnewrevisions-footer.pt (+11/-0)
lib/lp/code/templates/codereviewnewrevisions-header.pt (+10/-0)
lib/lp/code/tests/helpers.py (+21/-0)
lib/lp/testing/factory.py (+3/-2)
To merge this branch: bzr merge lp:~thumper/launchpad/revisions-in-conversation
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+15910@code.launchpad.net

Commit message

Show revisions committed in the code review conversation.

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

This branch adds in grouped revisions on the merge proposal page.

When the scanner scans a branch, all the revisions added at that time have the same date_created. We use this time to work out where in the conversation to put the revisions.

The revision rendering is exactly the same as where we render revisions elsewhere, including the date the revision created at the remote end. The revision numbers are hyperlinked to codebrowse.

To play locally, you can do what I did and enter the following into 'make harness'

import pytz
from canonical.launchpad.webapp import canonical_url
from lp.code.tests.helpers import add_revision_to_branch

bmp = factory.makeBranchMergeProposal()
review_date = datetime(2009, 9, 10, tzinfo=pytz.UTC)
bmp.requestReview(review_date)

revision_date = review_date + timedelta(days=1)
revisions = []
for date in range(2):
    revisions.append(
        add_revision_to_branch(
            factory, bmp.source_branch, revision_date))
    revisions.append(
        add_revision_to_branch(
            factory, bmp.source_branch, revision_date))
    revision_date += timedelta(days=1)

print canonical_url(bmp)

Then open up that url to see the joy.

Revision history for this message
Curtis Hovey (sinzui) wrote :

This is very nice. I have no remarks about your code. SO I read it again and to see if I missed something. I still had no remarks about your code. So I think this is fine to land.

There are two spelling mistakes in code you changed. You can correct these if you wish.

> === modified file 'lib/lp/code/doc/branch.txt'
> --- lib/lp/code/doc/branch.txt 2009-08-02 23:59:31 +0000
> +++ lib/lp/code/doc/branch.txt 2009-12-10 01:54:21 +0000
> @@ -426,14 +426,14 @@
> >>> list(junk.latest_revisions(3)) == three_latest
> True
>
> -Branch.revisions_since gives all the BranchRevisions for revisions comitted
> +Branch.getRevisionsSince gives all the BranchRevisions for revisions comitted
> since a given timestamp. It may give suprising results if some committers had a

grammar: s/comitted/committed/
grammar: s/suprising/surprising/

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py 2009-11-25 23:25:51 +0000
+++ lib/lp/code/browser/branch.py 2009-12-10 20:07:22 +0000
@@ -353,7 +353,7 @@
353 def recent_revision_count(self, days=30):353 def recent_revision_count(self, days=30):
354 """Number of revisions committed during the last N days."""354 """Number of revisions committed during the last N days."""
355 timestamp = datetime.now(pytz.UTC) - timedelta(days=days)355 timestamp = datetime.now(pytz.UTC) - timedelta(days=days)
356 return self.context.revisions_since(timestamp).count()356 return self.context.getRevisionsSince(timestamp).count()
357357
358 def owner_is_registrant(self):358 def owner_is_registrant(self):
359 """Is the branch owner the registrant?"""359 """Is the branch owner the registrant?"""
360360
=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py 2009-11-24 01:53:32 +0000
+++ lib/lp/code/browser/branchmergeproposal.py 2009-12-10 20:07:22 +0000
@@ -31,6 +31,7 @@
31 'latest_proposals_for_each_branch',31 'latest_proposals_for_each_branch',
32 ]32 ]
3333
34from collections import defaultdict
34import operator35import operator
3536
36import simplejson37import simplejson
@@ -78,7 +79,8 @@
78 ICodeReviewVoteReference)79 ICodeReviewVoteReference)
79from lp.code.interfaces.diff import IPreviewDiff80from lp.code.interfaces.diff import IPreviewDiff
80from lp.registry.interfaces.person import IPersonSet81from lp.registry.interfaces.person import IPersonSet
81from lp.services.comments.interfaces.conversation import IConversation82from lp.services.comments.interfaces.conversation import (
83 IComment, IConversation)
8284
83from lazr.delegates import delegates85from lazr.delegates import delegates
84from lazr.restful.interface import copy_field86from lazr.restful.interface import copy_field
@@ -503,6 +505,36 @@
503 return diff_text.count('\n') >= config.diff.max_format_lines505 return diff_text.count('\n') >= config.diff.max_format_lines
504506
505507
508class ICodeReviewNewRevisions(Interface):
509 """Marker interface used to register views for CodeReviewNewRevisions."""
510
511
512class CodeReviewNewRevisions:
513 """Represents a logical grouping of revisions.
514
515 Each object instance represents a number of revisions scanned at a
516 particular time.
517 """
518 implements(IComment, ICodeReviewNewRevisions)
519
520 def __init__(self, revisions, date, branch):
521 self.revisions = revisions
522 self.branch = branch
523 self.has_body = False
524 self.has_footer = True
525 # The date attribute is used to sort the comments in the conversation.
526 self.date = date
527
528
529class CodeReviewNewRevisionsView(LaunchpadView):
530 """The view for rendering the new revisions."""
531
532 @property
533 def codebrowse_url(self):
534 """Return the link to codebrowse for this branch."""
535 return self.context.branch.codebrowse_url()
536
537
506class BranchMergeProposalView(LaunchpadFormView, UnmergedRevisionsMixin,538class BranchMergeProposalView(LaunchpadFormView, UnmergedRevisionsMixin,
507 BranchMergeProposalRevisionIdMixin,539 BranchMergeProposalRevisionIdMixin,
508 BranchMergeProposalStatusMixin,540 BranchMergeProposalStatusMixin,
@@ -529,6 +561,40 @@
529 """Location of page for commenting on this proposal."""561 """Location of page for commenting on this proposal."""
530 return canonical_url(self.context, view_name='+comment')562 return canonical_url(self.context, view_name='+comment')
531563
564 @property
565 def revision_end_date(self):
566 """The cutoff date for showing revisions.
567
568 If the proposal has been merged, then we stop at the merged date. If
569 it is rejected, we stop at the reviewed date. For superseded
570 proposals, it should ideally use the non-existant date_last_modified,
571 but could use the last comment date.
572 """
573 status = self.context.queue_status
574 if status == BranchMergeProposalStatus.MERGED:
575 return self.context.date_merged
576 if status == BranchMergeProposalStatus.REJECTED:
577 return self.context.date_reviewed
578 # Otherwise return None representing an open end date.
579 return None
580
581 def _getRevisionsSinceReviewStart(self):
582 """Get the grouped revisions since the review started."""
583 # Work out the start of the review.
584 start_date = self.context.date_review_requested
585 if start_date is None:
586 start_date = self.context.date_created
587 source = self.context.source_branch
588 resultset = source.getMainlineBranchRevisions(
589 start_date, self.revision_end_date, oldest_first=True)
590 # Now group by date created.
591 groups = defaultdict(list)
592 for branch_revision, revision, revision_author in resultset:
593 groups[revision.date_created].append(branch_revision)
594 return [
595 CodeReviewNewRevisions(revisions, date, source)
596 for date, revisions in groups.iteritems()]
597
532 @cachedproperty598 @cachedproperty
533 def conversation(self):599 def conversation(self):
534 """Return a conversation that is to be rendered."""600 """Return a conversation that is to be rendered."""
@@ -536,6 +602,7 @@
536 comments = [602 comments = [
537 CodeReviewDisplayComment(comment)603 CodeReviewDisplayComment(comment)
538 for comment in self.context.all_comments]604 for comment in self.context.all_comments]
605 comments.extend(self._getRevisionsSinceReviewStart())
539 comments = sorted(comments, key=operator.attrgetter('date'))606 comments = sorted(comments, key=operator.attrgetter('date'))
540 return CodeReviewConversation(comments)607 return CodeReviewConversation(comments)
541608
542609
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2009-11-18 00:22:49 +0000
+++ lib/lp/code/browser/configure.zcml 2009-12-10 20:07:22 +0000
@@ -632,6 +632,18 @@
632 name="+fragment"632 name="+fragment"
633 template="../templates/codereviewcomment-fragment.pt"/>633 template="../templates/codereviewcomment-fragment.pt"/>
634 </browser:pages>634 </browser:pages>
635 <browser:pages
636 facet="branches"
637 for="lp.code.browser.branchmergeproposal.ICodeReviewNewRevisions"
638 class="lp.code.browser.branchmergeproposal.CodeReviewNewRevisionsView"
639 permission="zope.Public">
640 <browser:page
641 name="+comment-header"
642 template="../templates/codereviewnewrevisions-header.pt"/>
643 <browser:page
644 name="+comment-footer"
645 template="../templates/codereviewnewrevisions-footer.pt"/>
646 </browser:pages>
635 <browser:page647 <browser:page
636 name="+reply"648 name="+reply"
637 facet="branches"649 facet="branches"
638650
=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py 2009-11-13 02:12:38 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py 2009-12-10 20:07:22 +0000
@@ -9,6 +9,7 @@
99
10from datetime import datetime, timedelta10from datetime import datetime, timedelta
11from difflib import unified_diff11from difflib import unified_diff
12import operator
12import unittest13import unittest
1314
14import pytz15import pytz
@@ -24,6 +25,7 @@
24 BranchMergeProposalVoteView, DecoratedCodeReviewVoteReference,25 BranchMergeProposalVoteView, DecoratedCodeReviewVoteReference,
25 latest_proposals_for_each_branch)26 latest_proposals_for_each_branch)
26from lp.code.enums import BranchMergeProposalStatus, CodeReviewVote27from lp.code.enums import BranchMergeProposalStatus, CodeReviewVote
28from lp.code.tests.helpers import add_revision_to_branch
27from lp.testing import (29from lp.testing import (
28 login_person, TestCaseWithFactory, time_counter)30 login_person, TestCaseWithFactory, time_counter)
29from lp.testing.views import create_initialized_view31from lp.testing.views import create_initialized_view
@@ -555,6 +557,63 @@
555 view = create_initialized_view(self.bmp, '+index')557 view = create_initialized_view(self.bmp, '+index')
556 self.assertEqual([], view.linked_bugs)558 self.assertEqual([], view.linked_bugs)
557559
560 def test_revision_end_date_active(self):
561 # An active merge proposal will have None as an end date.
562 bmp = self.factory.makeBranchMergeProposal()
563 view = create_initialized_view(bmp, '+index')
564 self.assertIs(None, view.revision_end_date)
565
566 def test_revision_end_date_merged(self):
567 # An merged proposal will have the date merged as an end date.
568 bmp = self.factory.makeBranchMergeProposal(
569 set_state=BranchMergeProposalStatus.MERGED)
570 view = create_initialized_view(bmp, '+index')
571 self.assertEqual(bmp.date_merged, view.revision_end_date)
572
573 def test_revision_end_date_rejected(self):
574 # An rejected proposal will have the date reviewed as an end date.
575 bmp = self.factory.makeBranchMergeProposal(
576 set_state=BranchMergeProposalStatus.REJECTED)
577 view = create_initialized_view(bmp, '+index')
578 self.assertEqual(bmp.date_reviewed, view.revision_end_date)
579
580 def assertRevisionGroups(self, bmp, expected_groups):
581 """Get the groups for the merge proposal and check them."""
582 view = create_initialized_view(bmp, '+index')
583 groups = view._getRevisionsSinceReviewStart()
584 view_groups = [
585 obj.revisions for obj in sorted(
586 groups, key=operator.attrgetter('date'))]
587 self.assertEqual(expected_groups, view_groups)
588
589 def test_getRevisionsSinceReviewStart_no_revisions(self):
590 # If there have been no revisions pushed since the start of the
591 # review, the method returns an empty list.
592 self.assertRevisionGroups(self.bmp, [])
593
594 def test_getRevisionsSinceReviewStart_groups(self):
595 # Revisions that were scanned at the same time have the same
596 # date_created. These revisions are grouped together.
597 review_date = datetime(2009, 9, 10, tzinfo=pytz.UTC)
598 bmp = self.factory.makeBranchMergeProposal(
599 date_created=review_date)
600 login_person(bmp.registrant)
601 bmp.requestReview(review_date)
602 revision_date = review_date + timedelta(days=1)
603 revisions = []
604 for date in range(2):
605 revisions.append(
606 add_revision_to_branch(
607 self.factory, bmp.source_branch, revision_date))
608 revisions.append(
609 add_revision_to_branch(
610 self.factory, bmp.source_branch, revision_date))
611 revision_date += timedelta(days=1)
612 expected_groups = [
613 [revisions[0], revisions[1]],
614 [revisions[2], revisions[3]]]
615 self.assertRevisionGroups(bmp, expected_groups)
616
558617
559class TestBranchMergeProposalChangeStatusOptions(TestCaseWithFactory):618class TestBranchMergeProposalChangeStatusOptions(TestCaseWithFactory):
560 """Test the status vocabulary generated for then +edit-status view."""619 """Test the status vocabulary generated for then +edit-status view."""
561620
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2009-11-16 22:53:42 +0000
+++ lib/lp/code/configure.zcml 2009-12-10 20:07:22 +0000
@@ -438,7 +438,7 @@
438 addLandingTarget438 addLandingTarget
439 scheduleDiffUpdates439 scheduleDiffUpdates
440 getMergeQueue440 getMergeQueue
441 revisions_since441 getRevisionsSince
442 code_is_browseable442 code_is_browseable
443 browse_source_url443 browse_source_url
444 code_import444 code_import
445445
=== modified file 'lib/lp/code/doc/branch.txt'
--- lib/lp/code/doc/branch.txt 2009-08-02 23:59:31 +0000
+++ lib/lp/code/doc/branch.txt 2009-12-10 20:07:22 +0000
@@ -426,14 +426,14 @@
426 >>> list(junk.latest_revisions(3)) == three_latest426 >>> list(junk.latest_revisions(3)) == three_latest
427 True427 True
428428
429Branch.revisions_since gives all the BranchRevisions for revisions comitted429Branch.getRevisionsSince gives all the BranchRevisions for revisions committed
430since a given timestamp. It may give suprising results if some committers had a430since a given timestamp. It may give surprising results if some committers had a
431skewed clock.431skewed clock.
432432
433 >>> from datetime import datetime433 >>> from datetime import datetime
434 >>> timestamp = datetime(2005, 10, 31, 12, 00, 00)434 >>> timestamp = datetime(2005, 10, 31, 12, 00, 00)
435 >>> two_latest = list(junk.revision_history)[:2]435 >>> two_latest = list(junk.revision_history)[:2]
436 >>> list(junk.revisions_since(timestamp)) == two_latest436 >>> list(junk.getRevisionsSince(timestamp)) == two_latest
437 True437 True
438438
439439
440440
=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py 2009-11-25 23:24:11 +0000
+++ lib/lp/code/interfaces/branch.py 2009-12-10 20:07:22 +0000
@@ -809,7 +809,20 @@
809 def getMergeQueue():809 def getMergeQueue():
810 """The proposals that are QUEUED to land on this branch."""810 """The proposals that are QUEUED to land on this branch."""
811811
812 def revisions_since(timestamp):812 def getMainlineBranchRevisions(start_date, end_date=None,
813 oldest_first=False):
814 """Return the matching mainline branch revision objects.
815
816 :param start_date: Return revisions that were committed after the
817 start_date.
818 :param end_date: Return revisions that were committed before the
819 end_date
820 :param oldest_first: Defines the ordering of the result set.
821 :returns: A resultset of tuples for
822 (BranchRevision, Revision, RevisionAuthor)
823 """
824
825 def getRevisionsSince(timestamp):
813 """Revisions in the history that are more recent than timestamp."""826 """Revisions in the history that are more recent than timestamp."""
814827
815 code_is_browseable = Attribute(828 code_is_browseable = Attribute(
816829
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py 2009-11-26 22:55:06 +0000
+++ lib/lp/code/model/branch.py 2009-12-10 20:07:22 +0000
@@ -52,7 +52,7 @@
52 BranchMergeProposal, BranchMergeProposalGetter)52 BranchMergeProposal, BranchMergeProposalGetter)
53from lp.code.model.branchrevision import BranchRevision53from lp.code.model.branchrevision import BranchRevision
54from lp.code.model.branchsubscription import BranchSubscription54from lp.code.model.branchsubscription import BranchSubscription
55from lp.code.model.revision import Revision55from lp.code.model.revision import Revision, RevisionAuthor
56from lp.code.model.seriessourcepackagebranch import SeriesSourcePackageBranch56from lp.code.model.seriessourcepackagebranch import SeriesSourcePackageBranch
57from lp.code.event.branchmergeproposal import NewBranchMergeProposalEvent57from lp.code.event.branchmergeproposal import NewBranchMergeProposalEvent
58from lp.code.interfaces.branch import (58from lp.code.interfaces.branch import (
@@ -518,7 +518,26 @@
518 """See `IBranch`."""518 """See `IBranch`."""
519 return self.revision_history.limit(quantity)519 return self.revision_history.limit(quantity)
520520
521 def revisions_since(self, timestamp):521 def getMainlineBranchRevisions(self, start_date, end_date=None,
522 oldest_first=False):
523 """See `IBranch`."""
524 date_clause = Revision.revision_date >= start_date
525 if end_date is not None:
526 date_clause = And(date_clause, Revision.revision_date <= end_date)
527 result = Store.of(self).find(
528 (BranchRevision, Revision, RevisionAuthor),
529 BranchRevision.branch == self,
530 BranchRevision.sequence != None,
531 BranchRevision.revision == Revision.id,
532 Revision.revision_author == RevisionAuthor.id,
533 date_clause)
534 if oldest_first:
535 result = result.order_by(BranchRevision.sequence)
536 else:
537 result = result.order_by(Desc(BranchRevision.sequence))
538 return result
539
540 def getRevisionsSince(self, timestamp):
522 """See `IBranch`."""541 """See `IBranch`."""
523 return BranchRevision.select(542 return BranchRevision.select(
524 'Revision.id=BranchRevision.revision AND '543 'Revision.id=BranchRevision.revision AND '
@@ -706,14 +725,6 @@
706 BranchSubscription.delete(subscription.id)725 BranchSubscription.delete(subscription.id)
707 store.flush()726 store.flush()
708727
709 def getMainlineBranchRevisions(self, revision_ids):
710 return Store.of(self).find(
711 BranchRevision,
712 BranchRevision.branch == self,
713 BranchRevision.sequence != None,
714 BranchRevision.revision == Revision.id,
715 Revision.revision_id.is_in(revision_ids))
716
717 def getBranchRevision(self, sequence=None, revision=None,728 def getBranchRevision(self, sequence=None, revision=None,
718 revision_id=None):729 revision_id=None):
719 """See `IBranch`."""730 """See `IBranch`."""
720731
=== modified file 'lib/lp/code/model/revision.py'
--- lib/lp/code/model/revision.py 2009-11-17 17:39:15 +0000
+++ lib/lp/code/model/revision.py 2009-12-10 20:07:22 +0000
@@ -26,7 +26,7 @@
26 BoolCol, ForeignKey, IntCol, StringCol, SQLObjectNotFound,26 BoolCol, ForeignKey, IntCol, StringCol, SQLObjectNotFound,
27 SQLMultipleJoin)27 SQLMultipleJoin)
2828
29from canonical.database.constants import DEFAULT29from canonical.database.constants import DEFAULT, UTC_NOW
30from canonical.database.datetimecol import UtcDateTimeCol30from canonical.database.datetimecol import UtcDateTimeCol
31from canonical.database.sqlbase import quote, SQLBase, sqlvalues31from canonical.database.sqlbase import quote, SQLBase, sqlvalues
3232
@@ -218,16 +218,20 @@
218 return author218 return author
219219
220 def new(self, revision_id, log_body, revision_date, revision_author,220 def new(self, revision_id, log_body, revision_date, revision_author,
221 parent_ids, properties):221 parent_ids, properties, _date_created=None):
222 """See IRevisionSet.new()"""222 """See IRevisionSet.new()"""
223 if properties is None:223 if properties is None:
224 properties = {}224 properties = {}
225 if _date_created is None:
226 _date_created = UTC_NOW
225 author = self.acquireRevisionAuthor(revision_author)227 author = self.acquireRevisionAuthor(revision_author)
226228
227 revision = Revision(revision_id=revision_id,229 revision = Revision(
228 log_body=log_body,230 revision_id=revision_id,
229 revision_date=revision_date,231 log_body=log_body,
230 revision_author=author)232 revision_date=revision_date,
233 revision_author=author,
234 date_created=_date_created)
231 # Don't create future revisions.235 # Don't create future revisions.
232 if revision.revision_date > revision.date_created:236 if revision.revision_date > revision.date_created:
233 revision.revision_date = revision.date_created237 revision.revision_date = revision.date_created
234238
=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py 2009-11-25 23:24:58 +0000
+++ lib/lp/code/model/tests/test_branch.py 2009-12-10 20:07:22 +0000
@@ -60,6 +60,7 @@
60 BranchMergeProposal)60 BranchMergeProposal)
61from lp.code.model.codeimport import CodeImport, CodeImportSet61from lp.code.model.codeimport import CodeImport, CodeImportSet
62from lp.code.model.codereviewcomment import CodeReviewComment62from lp.code.model.codereviewcomment import CodeReviewComment
63from lp.code.tests.helpers import add_revision_to_branch
63from lp.registry.interfaces.person import IPersonSet64from lp.registry.interfaces.person import IPersonSet
64from lp.registry.model.product import ProductSet65from lp.registry.model.product import ProductSet
65from lp.registry.model.sourcepackage import SourcePackage66from lp.registry.model.sourcepackage import SourcePackage
@@ -142,37 +143,6 @@
142 sequence=1, revision_id=rev1.revision_id)143 sequence=1, revision_id=rev1.revision_id)
143144
144145
145class TestGetMainlineBranchRevisions(TestCaseWithFactory):
146
147 layer = DatabaseFunctionalLayer
148
149 def test_getMainlineBranchRevisions(self):
150 """Only gets the mainline revisions, ignoring the others."""
151 branch = self.factory.makeBranch()
152 self.factory.makeBranchRevision(branch, 'rev1', 1)
153 self.factory.makeBranchRevision(branch, 'rev2', 2)
154 self.factory.makeBranchRevision(branch, 'rev2b', None)
155 result_set = branch.getMainlineBranchRevisions(
156 ['rev1', 'rev2', 'rev3'])
157 revid_set = set(
158 branch_revision.revision.revision_id for
159 branch_revision in result_set)
160 self.assertEqual(set(['rev1', 'rev2']), revid_set)
161
162 def test_getMainlineBranchRevisionsWrongBranch(self):
163 """Only gets the revisions for this branch, ignoring the others."""
164 branch = self.factory.makeBranch()
165 other_branch = self.factory.makeBranch()
166 self.factory.makeBranchRevision(branch, 'rev1', 1)
167 self.factory.makeBranchRevision(other_branch, 'rev1b', 2)
168 result_set = branch.getMainlineBranchRevisions(
169 ['rev1', 'rev1b'])
170 revid_set = set(
171 branch_revision.revision.revision_id for
172 branch_revision in result_set)
173 self.assertEqual(set(['rev1']), revid_set)
174
175
176class TestBranch(TestCaseWithFactory):146class TestBranch(TestCaseWithFactory):
177 """Test basic properties about Launchpad database branches."""147 """Test basic properties about Launchpad database branches."""
178148
@@ -1947,5 +1917,74 @@
1947 self.assertEqual(0, len(jobs))1917 self.assertEqual(0, len(jobs))
19481918
19491919
1920class TestBranchGetMainlineBranchRevisions(TestCaseWithFactory):
1921 """Tests for Branch.getMainlineBranchRevisions."""
1922
1923 layer = DatabaseFunctionalLayer
1924
1925 def test_start_date(self):
1926 # Revisions created before the start date are not returned.
1927 branch = self.factory.makeAnyBranch()
1928 epoch = datetime(2009, 9, 10, tzinfo=UTC)
1929 old = add_revision_to_branch(
1930 self.factory, branch, epoch - timedelta(days=1))
1931 new = add_revision_to_branch(
1932 self.factory, branch, epoch + timedelta(days=1))
1933 result = branch.getMainlineBranchRevisions(epoch)
1934 branch_revisions = [br for br, rev, ra in result]
1935 self.assertEqual([new], branch_revisions)
1936
1937 def test_end_date(self):
1938 # Revisions created after the end date are not returned.
1939 branch = self.factory.makeAnyBranch()
1940 epoch = datetime(2009, 9, 10, tzinfo=UTC)
1941 end_date = epoch + timedelta(days=2)
1942 in_range = add_revision_to_branch(
1943 self.factory, branch, end_date - timedelta(days=1))
1944 too_new = add_revision_to_branch(
1945 self.factory, branch, end_date + timedelta(days=1))
1946 result = branch.getMainlineBranchRevisions(epoch, end_date)
1947 branch_revisions = [br for br, rev, ra in result]
1948 self.assertEqual([in_range], branch_revisions)
1949
1950 def test_newest_first(self):
1951 # If oldest_first is False, the newest are returned first.
1952 branch = self.factory.makeAnyBranch()
1953 epoch = datetime(2009, 9, 10, tzinfo=UTC)
1954 old = add_revision_to_branch(
1955 self.factory, branch, epoch + timedelta(days=1))
1956 new = add_revision_to_branch(
1957 self.factory, branch, epoch + timedelta(days=2))
1958 result = branch.getMainlineBranchRevisions(epoch, oldest_first=False)
1959 branch_revisions = [br for br, rev, ra in result]
1960 self.assertEqual([new, old], branch_revisions)
1961
1962 def test_oldest_first(self):
1963 # If oldest_first is True, the oldest are returned first.
1964 branch = self.factory.makeAnyBranch()
1965 epoch = datetime(2009, 9, 10, tzinfo=UTC)
1966 old = add_revision_to_branch(
1967 self.factory, branch, epoch + timedelta(days=1))
1968 new = add_revision_to_branch(
1969 self.factory, branch, epoch + timedelta(days=2))
1970 result = branch.getMainlineBranchRevisions(epoch, oldest_first=True)
1971 branch_revisions = [br for br, rev, ra in result]
1972 self.assertEqual([old, new], branch_revisions)
1973
1974 def test_only_mainline_revisions(self):
1975 # Only mainline revisions are returned.
1976 branch = self.factory.makeAnyBranch()
1977 epoch = datetime(2009, 9, 10, tzinfo=UTC)
1978 old = add_revision_to_branch(
1979 self.factory, branch, epoch + timedelta(days=1))
1980 merged = add_revision_to_branch(
1981 self.factory, branch, epoch + timedelta(days=2), mainline=False)
1982 new = add_revision_to_branch(
1983 self.factory, branch, epoch + timedelta(days=3))
1984 result = branch.getMainlineBranchRevisions(epoch)
1985 branch_revisions = [br for br, rev, ra in result]
1986 self.assertEqual([new, old], branch_revisions)
1987
1988
1950def test_suite():1989def test_suite():
1951 return TestLoader().loadTestsFromName(__name__)1990 return TestLoader().loadTestsFromName(__name__)
19521991
=== modified file 'lib/lp/code/stories/branches/xx-code-review-comments.txt'
--- lib/lp/code/stories/branches/xx-code-review-comments.txt 2009-07-01 13:16:44 +0000
+++ lib/lp/code/stories/branches/xx-code-review-comments.txt 2009-12-10 20:07:22 +0000
@@ -1,4 +1,5 @@
1= Code Review Comments =1Code Review Comments
2====================
23
3Set up required objects4Set up required objects
45
@@ -91,7 +92,8 @@
91 Email me at &lt;email address hidden&gt; for more details92 Email me at &lt;email address hidden&gt; for more details
92 Reply93 Reply
9394
94== Reviewing ==95Reviewing
96---------
9597
96If the user wants to review the branch, they click on the 'Add a review or98If the user wants to review the branch, they click on the 'Add a review or
97comment' link.99comment' link.
@@ -108,7 +110,8 @@
108 >>> print_comments('boardCommentBody', eric_browser, index=2)110 >>> print_comments('boardCommentBody', eric_browser, index=2)
109111
110112
111== Vote summaries ==113Vote summaries
114--------------
112115
113The summary of the votes that have been made for a code review are shown116The summary of the votes that have been made for a code review are shown
114in a table at the top of the page.117in a table at the top of the page.
@@ -119,3 +122,43 @@
119 Eric the Viking (community) timeless ... Abstain...122 Eric the Viking (community) timeless ... Abstain...
120 second...ago123 second...ago
121 Review via email: mp+1@code.launchpad.dev124 Review via email: mp+1@code.launchpad.dev
125
126
127Commits shown in the conversation
128---------------------------------
129
130If the source branch is updated during the review process, the commits are
131shown as part of the conversation at the time they were pushed to Launchpad.
132
133 >>> login('admin@canonical.com')
134 >>> from lp.code.tests.helpers import add_revision_to_branch
135 >>> bmp = factory.makeBranchMergeProposal()
136 >>> from datetime import datetime, timedelta
137 >>> import pytz
138 >>> review_date = datetime(2009, 9, 10, tzinfo=pytz.UTC)
139 >>> bmp.requestReview(review_date)
140 >>> revision_date = review_date + timedelta(days=1)
141 >>> for date in range(2):
142 ... ignored = add_revision_to_branch(
143 ... factory, bmp.source_branch, revision_date,
144 ... commit_msg='Testing commits in conversation')
145 ... ignored = add_revision_to_branch(
146 ... factory, bmp.source_branch, revision_date,
147 ... commit_msg='and it works!')
148 ... revision_date += timedelta(days=1)
149 >>> url = canonical_url(bmp)
150 >>> logout()
151
152 >>> browser.open(url)
153 >>> print_tag_with_id(browser.contents, 'conversation')
154 lp://dev/... updated on 2009-09-11
155 1. By ... on 2009-09-11
156 Testing commits in conversation
157 2. By ... on 2009-09-11
158 and it works!
159 lp://dev/... updated on 2009-09-12
160 3. By ... on 2009-09-12
161 Testing commits in conversation
162 4. By ... on 2009-09-12
163 and it works!
164
122165
=== added file 'lib/lp/code/templates/codereviewnewrevisions-footer.pt'
--- lib/lp/code/templates/codereviewnewrevisions-footer.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/codereviewnewrevisions-footer.pt 2009-12-10 20:07:22 +0000
@@ -0,0 +1,11 @@
1<tal:root
2 xmlns:tal="http://xml.zope.org/namespaces/tal"
3 xmlns:metal="http://xml.zope.org/namespaces/metal"
4 omit-tag="">
5
6 <tal:revisions define="branch context/branch;
7 revisions context/revisions">
8 <metal:landing-target use-macro="branch/@@+macros/branch-revisions"/>
9 </tal:revisions>
10
11</tal:root>
012
=== added file 'lib/lp/code/templates/codereviewnewrevisions-header.pt'
--- lib/lp/code/templates/codereviewnewrevisions-header.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/codereviewnewrevisions-header.pt 2009-12-10 20:07:22 +0000
@@ -0,0 +1,10 @@
1<tal:root
2 xmlns:tal="http://xml.zope.org/namespaces/tal"
3 xmlns:metal="http://xml.zope.org/namespaces/metal"
4 omit-tag="">
5
6 <tal:branch replace="structure context/branch/fmt:link"/>
7 updated
8 <tal:date replace="context/date/fmt:displaydate" />
9
10</tal:root>
011
=== modified file 'lib/lp/code/tests/helpers.py'
--- lib/lp/code/tests/helpers.py 2009-11-12 02:27:19 +0000
+++ lib/lp/code/tests/helpers.py 2009-12-10 20:07:22 +0000
@@ -5,6 +5,7 @@
55
6__metaclass__ = type6__metaclass__ = type
7__all__ = [7__all__ = [
8 'add_revision_to_branch',
8 'make_linked_package_branch',9 'make_linked_package_branch',
9 'make_erics_fooix_project',10 'make_erics_fooix_project',
10 ]11 ]
@@ -25,6 +26,26 @@
25from lp.testing import time_counter26from lp.testing import time_counter
2627
2728
29def add_revision_to_branch(factory, branch, revision_date, date_created=None,
30 mainline=True, commit_msg=None):
31 """Add a new revision to the branch with the specified revision date.
32
33 If date_created is None, it gets set to the revision_date.
34 """
35 if date_created is None:
36 date_created = revision_date
37 revision = factory.makeRevision(
38 revision_date=revision_date, date_created=date_created,
39 log_body=commit_msg)
40 if mainline:
41 sequence = branch.revision_count + 1
42 branch_revision = branch.createBranchRevision(sequence, revision)
43 branch.updateScannedDetails(revision, sequence)
44 else:
45 branch_revision = branch.createBranchRevision(None, revision)
46 return branch_revision
47
48
28def make_erics_fooix_project(factory):49def make_erics_fooix_project(factory):
29 """Make Eric, the Fooix project, and some branches.50 """Make Eric, the Fooix project, and some branches.
3051
3152
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2009-12-08 14:50:07 +0000
+++ lib/lp/testing/factory.py 2009-12-10 20:07:22 +0000
@@ -897,7 +897,7 @@
897 CodeReviewNotificationLevel.NOEMAIL)897 CodeReviewNotificationLevel.NOEMAIL)
898898
899 def makeRevision(self, author=None, revision_date=None, parent_ids=None,899 def makeRevision(self, author=None, revision_date=None, parent_ids=None,
900 rev_id=None, log_body=None):900 rev_id=None, log_body=None, date_created=None):
901 """Create a single `Revision`."""901 """Create a single `Revision`."""
902 if author is None:902 if author is None:
903 author = self.getUniqueString('author')903 author = self.getUniqueString('author')
@@ -912,7 +912,8 @@
912 return getUtility(IRevisionSet).new(912 return getUtility(IRevisionSet).new(
913 revision_id=rev_id, log_body=log_body,913 revision_id=rev_id, log_body=log_body,
914 revision_date=revision_date, revision_author=author,914 revision_date=revision_date, revision_author=author,
915 parent_ids=parent_ids, properties={})915 parent_ids=parent_ids, properties={},
916 _date_created=date_created)
916917
917 def makeRevisionsForBranch(self, branch, count=5, author=None,918 def makeRevisionsForBranch(self, branch, count=5, author=None,
918 date_generator=None):919 date_generator=None):