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
1=== modified file 'lib/lp/code/browser/branch.py'
2--- lib/lp/code/browser/branch.py 2009-11-25 23:25:51 +0000
3+++ lib/lp/code/browser/branch.py 2009-12-10 20:07:22 +0000
4@@ -353,7 +353,7 @@
5 def recent_revision_count(self, days=30):
6 """Number of revisions committed during the last N days."""
7 timestamp = datetime.now(pytz.UTC) - timedelta(days=days)
8- return self.context.revisions_since(timestamp).count()
9+ return self.context.getRevisionsSince(timestamp).count()
10
11 def owner_is_registrant(self):
12 """Is the branch owner the registrant?"""
13
14=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
15--- lib/lp/code/browser/branchmergeproposal.py 2009-11-24 01:53:32 +0000
16+++ lib/lp/code/browser/branchmergeproposal.py 2009-12-10 20:07:22 +0000
17@@ -31,6 +31,7 @@
18 'latest_proposals_for_each_branch',
19 ]
20
21+from collections import defaultdict
22 import operator
23
24 import simplejson
25@@ -78,7 +79,8 @@
26 ICodeReviewVoteReference)
27 from lp.code.interfaces.diff import IPreviewDiff
28 from lp.registry.interfaces.person import IPersonSet
29-from lp.services.comments.interfaces.conversation import IConversation
30+from lp.services.comments.interfaces.conversation import (
31+ IComment, IConversation)
32
33 from lazr.delegates import delegates
34 from lazr.restful.interface import copy_field
35@@ -503,6 +505,36 @@
36 return diff_text.count('\n') >= config.diff.max_format_lines
37
38
39+class ICodeReviewNewRevisions(Interface):
40+ """Marker interface used to register views for CodeReviewNewRevisions."""
41+
42+
43+class CodeReviewNewRevisions:
44+ """Represents a logical grouping of revisions.
45+
46+ Each object instance represents a number of revisions scanned at a
47+ particular time.
48+ """
49+ implements(IComment, ICodeReviewNewRevisions)
50+
51+ def __init__(self, revisions, date, branch):
52+ self.revisions = revisions
53+ self.branch = branch
54+ self.has_body = False
55+ self.has_footer = True
56+ # The date attribute is used to sort the comments in the conversation.
57+ self.date = date
58+
59+
60+class CodeReviewNewRevisionsView(LaunchpadView):
61+ """The view for rendering the new revisions."""
62+
63+ @property
64+ def codebrowse_url(self):
65+ """Return the link to codebrowse for this branch."""
66+ return self.context.branch.codebrowse_url()
67+
68+
69 class BranchMergeProposalView(LaunchpadFormView, UnmergedRevisionsMixin,
70 BranchMergeProposalRevisionIdMixin,
71 BranchMergeProposalStatusMixin,
72@@ -529,6 +561,40 @@
73 """Location of page for commenting on this proposal."""
74 return canonical_url(self.context, view_name='+comment')
75
76+ @property
77+ def revision_end_date(self):
78+ """The cutoff date for showing revisions.
79+
80+ If the proposal has been merged, then we stop at the merged date. If
81+ it is rejected, we stop at the reviewed date. For superseded
82+ proposals, it should ideally use the non-existant date_last_modified,
83+ but could use the last comment date.
84+ """
85+ status = self.context.queue_status
86+ if status == BranchMergeProposalStatus.MERGED:
87+ return self.context.date_merged
88+ if status == BranchMergeProposalStatus.REJECTED:
89+ return self.context.date_reviewed
90+ # Otherwise return None representing an open end date.
91+ return None
92+
93+ def _getRevisionsSinceReviewStart(self):
94+ """Get the grouped revisions since the review started."""
95+ # Work out the start of the review.
96+ start_date = self.context.date_review_requested
97+ if start_date is None:
98+ start_date = self.context.date_created
99+ source = self.context.source_branch
100+ resultset = source.getMainlineBranchRevisions(
101+ start_date, self.revision_end_date, oldest_first=True)
102+ # Now group by date created.
103+ groups = defaultdict(list)
104+ for branch_revision, revision, revision_author in resultset:
105+ groups[revision.date_created].append(branch_revision)
106+ return [
107+ CodeReviewNewRevisions(revisions, date, source)
108+ for date, revisions in groups.iteritems()]
109+
110 @cachedproperty
111 def conversation(self):
112 """Return a conversation that is to be rendered."""
113@@ -536,6 +602,7 @@
114 comments = [
115 CodeReviewDisplayComment(comment)
116 for comment in self.context.all_comments]
117+ comments.extend(self._getRevisionsSinceReviewStart())
118 comments = sorted(comments, key=operator.attrgetter('date'))
119 return CodeReviewConversation(comments)
120
121
122=== modified file 'lib/lp/code/browser/configure.zcml'
123--- lib/lp/code/browser/configure.zcml 2009-11-18 00:22:49 +0000
124+++ lib/lp/code/browser/configure.zcml 2009-12-10 20:07:22 +0000
125@@ -632,6 +632,18 @@
126 name="+fragment"
127 template="../templates/codereviewcomment-fragment.pt"/>
128 </browser:pages>
129+ <browser:pages
130+ facet="branches"
131+ for="lp.code.browser.branchmergeproposal.ICodeReviewNewRevisions"
132+ class="lp.code.browser.branchmergeproposal.CodeReviewNewRevisionsView"
133+ permission="zope.Public">
134+ <browser:page
135+ name="+comment-header"
136+ template="../templates/codereviewnewrevisions-header.pt"/>
137+ <browser:page
138+ name="+comment-footer"
139+ template="../templates/codereviewnewrevisions-footer.pt"/>
140+ </browser:pages>
141 <browser:page
142 name="+reply"
143 facet="branches"
144
145=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
146--- lib/lp/code/browser/tests/test_branchmergeproposal.py 2009-11-13 02:12:38 +0000
147+++ lib/lp/code/browser/tests/test_branchmergeproposal.py 2009-12-10 20:07:22 +0000
148@@ -9,6 +9,7 @@
149
150 from datetime import datetime, timedelta
151 from difflib import unified_diff
152+import operator
153 import unittest
154
155 import pytz
156@@ -24,6 +25,7 @@
157 BranchMergeProposalVoteView, DecoratedCodeReviewVoteReference,
158 latest_proposals_for_each_branch)
159 from lp.code.enums import BranchMergeProposalStatus, CodeReviewVote
160+from lp.code.tests.helpers import add_revision_to_branch
161 from lp.testing import (
162 login_person, TestCaseWithFactory, time_counter)
163 from lp.testing.views import create_initialized_view
164@@ -555,6 +557,63 @@
165 view = create_initialized_view(self.bmp, '+index')
166 self.assertEqual([], view.linked_bugs)
167
168+ def test_revision_end_date_active(self):
169+ # An active merge proposal will have None as an end date.
170+ bmp = self.factory.makeBranchMergeProposal()
171+ view = create_initialized_view(bmp, '+index')
172+ self.assertIs(None, view.revision_end_date)
173+
174+ def test_revision_end_date_merged(self):
175+ # An merged proposal will have the date merged as an end date.
176+ bmp = self.factory.makeBranchMergeProposal(
177+ set_state=BranchMergeProposalStatus.MERGED)
178+ view = create_initialized_view(bmp, '+index')
179+ self.assertEqual(bmp.date_merged, view.revision_end_date)
180+
181+ def test_revision_end_date_rejected(self):
182+ # An rejected proposal will have the date reviewed as an end date.
183+ bmp = self.factory.makeBranchMergeProposal(
184+ set_state=BranchMergeProposalStatus.REJECTED)
185+ view = create_initialized_view(bmp, '+index')
186+ self.assertEqual(bmp.date_reviewed, view.revision_end_date)
187+
188+ def assertRevisionGroups(self, bmp, expected_groups):
189+ """Get the groups for the merge proposal and check them."""
190+ view = create_initialized_view(bmp, '+index')
191+ groups = view._getRevisionsSinceReviewStart()
192+ view_groups = [
193+ obj.revisions for obj in sorted(
194+ groups, key=operator.attrgetter('date'))]
195+ self.assertEqual(expected_groups, view_groups)
196+
197+ def test_getRevisionsSinceReviewStart_no_revisions(self):
198+ # If there have been no revisions pushed since the start of the
199+ # review, the method returns an empty list.
200+ self.assertRevisionGroups(self.bmp, [])
201+
202+ def test_getRevisionsSinceReviewStart_groups(self):
203+ # Revisions that were scanned at the same time have the same
204+ # date_created. These revisions are grouped together.
205+ review_date = datetime(2009, 9, 10, tzinfo=pytz.UTC)
206+ bmp = self.factory.makeBranchMergeProposal(
207+ date_created=review_date)
208+ login_person(bmp.registrant)
209+ bmp.requestReview(review_date)
210+ revision_date = review_date + timedelta(days=1)
211+ revisions = []
212+ for date in range(2):
213+ revisions.append(
214+ add_revision_to_branch(
215+ self.factory, bmp.source_branch, revision_date))
216+ revisions.append(
217+ add_revision_to_branch(
218+ self.factory, bmp.source_branch, revision_date))
219+ revision_date += timedelta(days=1)
220+ expected_groups = [
221+ [revisions[0], revisions[1]],
222+ [revisions[2], revisions[3]]]
223+ self.assertRevisionGroups(bmp, expected_groups)
224+
225
226 class TestBranchMergeProposalChangeStatusOptions(TestCaseWithFactory):
227 """Test the status vocabulary generated for then +edit-status view."""
228
229=== modified file 'lib/lp/code/configure.zcml'
230--- lib/lp/code/configure.zcml 2009-11-16 22:53:42 +0000
231+++ lib/lp/code/configure.zcml 2009-12-10 20:07:22 +0000
232@@ -438,7 +438,7 @@
233 addLandingTarget
234 scheduleDiffUpdates
235 getMergeQueue
236- revisions_since
237+ getRevisionsSince
238 code_is_browseable
239 browse_source_url
240 code_import
241
242=== modified file 'lib/lp/code/doc/branch.txt'
243--- lib/lp/code/doc/branch.txt 2009-08-02 23:59:31 +0000
244+++ lib/lp/code/doc/branch.txt 2009-12-10 20:07:22 +0000
245@@ -426,14 +426,14 @@
246 >>> list(junk.latest_revisions(3)) == three_latest
247 True
248
249-Branch.revisions_since gives all the BranchRevisions for revisions comitted
250-since a given timestamp. It may give suprising results if some committers had a
251+Branch.getRevisionsSince gives all the BranchRevisions for revisions committed
252+since a given timestamp. It may give surprising results if some committers had a
253 skewed clock.
254
255 >>> from datetime import datetime
256 >>> timestamp = datetime(2005, 10, 31, 12, 00, 00)
257 >>> two_latest = list(junk.revision_history)[:2]
258- >>> list(junk.revisions_since(timestamp)) == two_latest
259+ >>> list(junk.getRevisionsSince(timestamp)) == two_latest
260 True
261
262
263
264=== modified file 'lib/lp/code/interfaces/branch.py'
265--- lib/lp/code/interfaces/branch.py 2009-11-25 23:24:11 +0000
266+++ lib/lp/code/interfaces/branch.py 2009-12-10 20:07:22 +0000
267@@ -809,7 +809,20 @@
268 def getMergeQueue():
269 """The proposals that are QUEUED to land on this branch."""
270
271- def revisions_since(timestamp):
272+ def getMainlineBranchRevisions(start_date, end_date=None,
273+ oldest_first=False):
274+ """Return the matching mainline branch revision objects.
275+
276+ :param start_date: Return revisions that were committed after the
277+ start_date.
278+ :param end_date: Return revisions that were committed before the
279+ end_date
280+ :param oldest_first: Defines the ordering of the result set.
281+ :returns: A resultset of tuples for
282+ (BranchRevision, Revision, RevisionAuthor)
283+ """
284+
285+ def getRevisionsSince(timestamp):
286 """Revisions in the history that are more recent than timestamp."""
287
288 code_is_browseable = Attribute(
289
290=== modified file 'lib/lp/code/model/branch.py'
291--- lib/lp/code/model/branch.py 2009-11-26 22:55:06 +0000
292+++ lib/lp/code/model/branch.py 2009-12-10 20:07:22 +0000
293@@ -52,7 +52,7 @@
294 BranchMergeProposal, BranchMergeProposalGetter)
295 from lp.code.model.branchrevision import BranchRevision
296 from lp.code.model.branchsubscription import BranchSubscription
297-from lp.code.model.revision import Revision
298+from lp.code.model.revision import Revision, RevisionAuthor
299 from lp.code.model.seriessourcepackagebranch import SeriesSourcePackageBranch
300 from lp.code.event.branchmergeproposal import NewBranchMergeProposalEvent
301 from lp.code.interfaces.branch import (
302@@ -518,7 +518,26 @@
303 """See `IBranch`."""
304 return self.revision_history.limit(quantity)
305
306- def revisions_since(self, timestamp):
307+ def getMainlineBranchRevisions(self, start_date, end_date=None,
308+ oldest_first=False):
309+ """See `IBranch`."""
310+ date_clause = Revision.revision_date >= start_date
311+ if end_date is not None:
312+ date_clause = And(date_clause, Revision.revision_date <= end_date)
313+ result = Store.of(self).find(
314+ (BranchRevision, Revision, RevisionAuthor),
315+ BranchRevision.branch == self,
316+ BranchRevision.sequence != None,
317+ BranchRevision.revision == Revision.id,
318+ Revision.revision_author == RevisionAuthor.id,
319+ date_clause)
320+ if oldest_first:
321+ result = result.order_by(BranchRevision.sequence)
322+ else:
323+ result = result.order_by(Desc(BranchRevision.sequence))
324+ return result
325+
326+ def getRevisionsSince(self, timestamp):
327 """See `IBranch`."""
328 return BranchRevision.select(
329 'Revision.id=BranchRevision.revision AND '
330@@ -706,14 +725,6 @@
331 BranchSubscription.delete(subscription.id)
332 store.flush()
333
334- def getMainlineBranchRevisions(self, revision_ids):
335- return Store.of(self).find(
336- BranchRevision,
337- BranchRevision.branch == self,
338- BranchRevision.sequence != None,
339- BranchRevision.revision == Revision.id,
340- Revision.revision_id.is_in(revision_ids))
341-
342 def getBranchRevision(self, sequence=None, revision=None,
343 revision_id=None):
344 """See `IBranch`."""
345
346=== modified file 'lib/lp/code/model/revision.py'
347--- lib/lp/code/model/revision.py 2009-11-17 17:39:15 +0000
348+++ lib/lp/code/model/revision.py 2009-12-10 20:07:22 +0000
349@@ -26,7 +26,7 @@
350 BoolCol, ForeignKey, IntCol, StringCol, SQLObjectNotFound,
351 SQLMultipleJoin)
352
353-from canonical.database.constants import DEFAULT
354+from canonical.database.constants import DEFAULT, UTC_NOW
355 from canonical.database.datetimecol import UtcDateTimeCol
356 from canonical.database.sqlbase import quote, SQLBase, sqlvalues
357
358@@ -218,16 +218,20 @@
359 return author
360
361 def new(self, revision_id, log_body, revision_date, revision_author,
362- parent_ids, properties):
363+ parent_ids, properties, _date_created=None):
364 """See IRevisionSet.new()"""
365 if properties is None:
366 properties = {}
367+ if _date_created is None:
368+ _date_created = UTC_NOW
369 author = self.acquireRevisionAuthor(revision_author)
370
371- revision = Revision(revision_id=revision_id,
372- log_body=log_body,
373- revision_date=revision_date,
374- revision_author=author)
375+ revision = Revision(
376+ revision_id=revision_id,
377+ log_body=log_body,
378+ revision_date=revision_date,
379+ revision_author=author,
380+ date_created=_date_created)
381 # Don't create future revisions.
382 if revision.revision_date > revision.date_created:
383 revision.revision_date = revision.date_created
384
385=== modified file 'lib/lp/code/model/tests/test_branch.py'
386--- lib/lp/code/model/tests/test_branch.py 2009-11-25 23:24:58 +0000
387+++ lib/lp/code/model/tests/test_branch.py 2009-12-10 20:07:22 +0000
388@@ -60,6 +60,7 @@
389 BranchMergeProposal)
390 from lp.code.model.codeimport import CodeImport, CodeImportSet
391 from lp.code.model.codereviewcomment import CodeReviewComment
392+from lp.code.tests.helpers import add_revision_to_branch
393 from lp.registry.interfaces.person import IPersonSet
394 from lp.registry.model.product import ProductSet
395 from lp.registry.model.sourcepackage import SourcePackage
396@@ -142,37 +143,6 @@
397 sequence=1, revision_id=rev1.revision_id)
398
399
400-class TestGetMainlineBranchRevisions(TestCaseWithFactory):
401-
402- layer = DatabaseFunctionalLayer
403-
404- def test_getMainlineBranchRevisions(self):
405- """Only gets the mainline revisions, ignoring the others."""
406- branch = self.factory.makeBranch()
407- self.factory.makeBranchRevision(branch, 'rev1', 1)
408- self.factory.makeBranchRevision(branch, 'rev2', 2)
409- self.factory.makeBranchRevision(branch, 'rev2b', None)
410- result_set = branch.getMainlineBranchRevisions(
411- ['rev1', 'rev2', 'rev3'])
412- revid_set = set(
413- branch_revision.revision.revision_id for
414- branch_revision in result_set)
415- self.assertEqual(set(['rev1', 'rev2']), revid_set)
416-
417- def test_getMainlineBranchRevisionsWrongBranch(self):
418- """Only gets the revisions for this branch, ignoring the others."""
419- branch = self.factory.makeBranch()
420- other_branch = self.factory.makeBranch()
421- self.factory.makeBranchRevision(branch, 'rev1', 1)
422- self.factory.makeBranchRevision(other_branch, 'rev1b', 2)
423- result_set = branch.getMainlineBranchRevisions(
424- ['rev1', 'rev1b'])
425- revid_set = set(
426- branch_revision.revision.revision_id for
427- branch_revision in result_set)
428- self.assertEqual(set(['rev1']), revid_set)
429-
430-
431 class TestBranch(TestCaseWithFactory):
432 """Test basic properties about Launchpad database branches."""
433
434@@ -1947,5 +1917,74 @@
435 self.assertEqual(0, len(jobs))
436
437
438+class TestBranchGetMainlineBranchRevisions(TestCaseWithFactory):
439+ """Tests for Branch.getMainlineBranchRevisions."""
440+
441+ layer = DatabaseFunctionalLayer
442+
443+ def test_start_date(self):
444+ # Revisions created before the start date are not returned.
445+ branch = self.factory.makeAnyBranch()
446+ epoch = datetime(2009, 9, 10, tzinfo=UTC)
447+ old = add_revision_to_branch(
448+ self.factory, branch, epoch - timedelta(days=1))
449+ new = add_revision_to_branch(
450+ self.factory, branch, epoch + timedelta(days=1))
451+ result = branch.getMainlineBranchRevisions(epoch)
452+ branch_revisions = [br for br, rev, ra in result]
453+ self.assertEqual([new], branch_revisions)
454+
455+ def test_end_date(self):
456+ # Revisions created after the end date are not returned.
457+ branch = self.factory.makeAnyBranch()
458+ epoch = datetime(2009, 9, 10, tzinfo=UTC)
459+ end_date = epoch + timedelta(days=2)
460+ in_range = add_revision_to_branch(
461+ self.factory, branch, end_date - timedelta(days=1))
462+ too_new = add_revision_to_branch(
463+ self.factory, branch, end_date + timedelta(days=1))
464+ result = branch.getMainlineBranchRevisions(epoch, end_date)
465+ branch_revisions = [br for br, rev, ra in result]
466+ self.assertEqual([in_range], branch_revisions)
467+
468+ def test_newest_first(self):
469+ # If oldest_first is False, the newest are returned first.
470+ branch = self.factory.makeAnyBranch()
471+ epoch = datetime(2009, 9, 10, tzinfo=UTC)
472+ old = add_revision_to_branch(
473+ self.factory, branch, epoch + timedelta(days=1))
474+ new = add_revision_to_branch(
475+ self.factory, branch, epoch + timedelta(days=2))
476+ result = branch.getMainlineBranchRevisions(epoch, oldest_first=False)
477+ branch_revisions = [br for br, rev, ra in result]
478+ self.assertEqual([new, old], branch_revisions)
479+
480+ def test_oldest_first(self):
481+ # If oldest_first is True, the oldest are returned first.
482+ branch = self.factory.makeAnyBranch()
483+ epoch = datetime(2009, 9, 10, tzinfo=UTC)
484+ old = add_revision_to_branch(
485+ self.factory, branch, epoch + timedelta(days=1))
486+ new = add_revision_to_branch(
487+ self.factory, branch, epoch + timedelta(days=2))
488+ result = branch.getMainlineBranchRevisions(epoch, oldest_first=True)
489+ branch_revisions = [br for br, rev, ra in result]
490+ self.assertEqual([old, new], branch_revisions)
491+
492+ def test_only_mainline_revisions(self):
493+ # Only mainline revisions are returned.
494+ branch = self.factory.makeAnyBranch()
495+ epoch = datetime(2009, 9, 10, tzinfo=UTC)
496+ old = add_revision_to_branch(
497+ self.factory, branch, epoch + timedelta(days=1))
498+ merged = add_revision_to_branch(
499+ self.factory, branch, epoch + timedelta(days=2), mainline=False)
500+ new = add_revision_to_branch(
501+ self.factory, branch, epoch + timedelta(days=3))
502+ result = branch.getMainlineBranchRevisions(epoch)
503+ branch_revisions = [br for br, rev, ra in result]
504+ self.assertEqual([new, old], branch_revisions)
505+
506+
507 def test_suite():
508 return TestLoader().loadTestsFromName(__name__)
509
510=== modified file 'lib/lp/code/stories/branches/xx-code-review-comments.txt'
511--- lib/lp/code/stories/branches/xx-code-review-comments.txt 2009-07-01 13:16:44 +0000
512+++ lib/lp/code/stories/branches/xx-code-review-comments.txt 2009-12-10 20:07:22 +0000
513@@ -1,4 +1,5 @@
514-= Code Review Comments =
515+Code Review Comments
516+====================
517
518 Set up required objects
519
520@@ -91,7 +92,8 @@
521 Email me at &lt;email address hidden&gt; for more details
522 Reply
523
524-== Reviewing ==
525+Reviewing
526+---------
527
528 If the user wants to review the branch, they click on the 'Add a review or
529 comment' link.
530@@ -108,7 +110,8 @@
531 >>> print_comments('boardCommentBody', eric_browser, index=2)
532
533
534-== Vote summaries ==
535+Vote summaries
536+--------------
537
538 The summary of the votes that have been made for a code review are shown
539 in a table at the top of the page.
540@@ -119,3 +122,43 @@
541 Eric the Viking (community) timeless ... Abstain...
542 second...ago
543 Review via email: mp+1@code.launchpad.dev
544+
545+
546+Commits shown in the conversation
547+---------------------------------
548+
549+If the source branch is updated during the review process, the commits are
550+shown as part of the conversation at the time they were pushed to Launchpad.
551+
552+ >>> login('admin@canonical.com')
553+ >>> from lp.code.tests.helpers import add_revision_to_branch
554+ >>> bmp = factory.makeBranchMergeProposal()
555+ >>> from datetime import datetime, timedelta
556+ >>> import pytz
557+ >>> review_date = datetime(2009, 9, 10, tzinfo=pytz.UTC)
558+ >>> bmp.requestReview(review_date)
559+ >>> revision_date = review_date + timedelta(days=1)
560+ >>> for date in range(2):
561+ ... ignored = add_revision_to_branch(
562+ ... factory, bmp.source_branch, revision_date,
563+ ... commit_msg='Testing commits in conversation')
564+ ... ignored = add_revision_to_branch(
565+ ... factory, bmp.source_branch, revision_date,
566+ ... commit_msg='and it works!')
567+ ... revision_date += timedelta(days=1)
568+ >>> url = canonical_url(bmp)
569+ >>> logout()
570+
571+ >>> browser.open(url)
572+ >>> print_tag_with_id(browser.contents, 'conversation')
573+ lp://dev/... updated on 2009-09-11
574+ 1. By ... on 2009-09-11
575+ Testing commits in conversation
576+ 2. By ... on 2009-09-11
577+ and it works!
578+ lp://dev/... updated on 2009-09-12
579+ 3. By ... on 2009-09-12
580+ Testing commits in conversation
581+ 4. By ... on 2009-09-12
582+ and it works!
583+
584
585=== added file 'lib/lp/code/templates/codereviewnewrevisions-footer.pt'
586--- lib/lp/code/templates/codereviewnewrevisions-footer.pt 1970-01-01 00:00:00 +0000
587+++ lib/lp/code/templates/codereviewnewrevisions-footer.pt 2009-12-10 20:07:22 +0000
588@@ -0,0 +1,11 @@
589+<tal:root
590+ xmlns:tal="http://xml.zope.org/namespaces/tal"
591+ xmlns:metal="http://xml.zope.org/namespaces/metal"
592+ omit-tag="">
593+
594+ <tal:revisions define="branch context/branch;
595+ revisions context/revisions">
596+ <metal:landing-target use-macro="branch/@@+macros/branch-revisions"/>
597+ </tal:revisions>
598+
599+</tal:root>
600
601=== added file 'lib/lp/code/templates/codereviewnewrevisions-header.pt'
602--- lib/lp/code/templates/codereviewnewrevisions-header.pt 1970-01-01 00:00:00 +0000
603+++ lib/lp/code/templates/codereviewnewrevisions-header.pt 2009-12-10 20:07:22 +0000
604@@ -0,0 +1,10 @@
605+<tal:root
606+ xmlns:tal="http://xml.zope.org/namespaces/tal"
607+ xmlns:metal="http://xml.zope.org/namespaces/metal"
608+ omit-tag="">
609+
610+ <tal:branch replace="structure context/branch/fmt:link"/>
611+ updated
612+ <tal:date replace="context/date/fmt:displaydate" />
613+
614+</tal:root>
615
616=== modified file 'lib/lp/code/tests/helpers.py'
617--- lib/lp/code/tests/helpers.py 2009-11-12 02:27:19 +0000
618+++ lib/lp/code/tests/helpers.py 2009-12-10 20:07:22 +0000
619@@ -5,6 +5,7 @@
620
621 __metaclass__ = type
622 __all__ = [
623+ 'add_revision_to_branch',
624 'make_linked_package_branch',
625 'make_erics_fooix_project',
626 ]
627@@ -25,6 +26,26 @@
628 from lp.testing import time_counter
629
630
631+def add_revision_to_branch(factory, branch, revision_date, date_created=None,
632+ mainline=True, commit_msg=None):
633+ """Add a new revision to the branch with the specified revision date.
634+
635+ If date_created is None, it gets set to the revision_date.
636+ """
637+ if date_created is None:
638+ date_created = revision_date
639+ revision = factory.makeRevision(
640+ revision_date=revision_date, date_created=date_created,
641+ log_body=commit_msg)
642+ if mainline:
643+ sequence = branch.revision_count + 1
644+ branch_revision = branch.createBranchRevision(sequence, revision)
645+ branch.updateScannedDetails(revision, sequence)
646+ else:
647+ branch_revision = branch.createBranchRevision(None, revision)
648+ return branch_revision
649+
650+
651 def make_erics_fooix_project(factory):
652 """Make Eric, the Fooix project, and some branches.
653
654
655=== modified file 'lib/lp/testing/factory.py'
656--- lib/lp/testing/factory.py 2009-12-08 14:50:07 +0000
657+++ lib/lp/testing/factory.py 2009-12-10 20:07:22 +0000
658@@ -897,7 +897,7 @@
659 CodeReviewNotificationLevel.NOEMAIL)
660
661 def makeRevision(self, author=None, revision_date=None, parent_ids=None,
662- rev_id=None, log_body=None):
663+ rev_id=None, log_body=None, date_created=None):
664 """Create a single `Revision`."""
665 if author is None:
666 author = self.getUniqueString('author')
667@@ -912,7 +912,8 @@
668 return getUtility(IRevisionSet).new(
669 revision_id=rev_id, log_body=log_body,
670 revision_date=revision_date, revision_author=author,
671- parent_ids=parent_ids, properties={})
672+ parent_ids=parent_ids, properties={},
673+ _date_created=date_created)
674
675 def makeRevisionsForBranch(self, branch, count=5, author=None,
676 date_generator=None):