launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #16319
[Merge] lp:~cprov/launchpad/cric-api-take2 into lp:launchpad
Celso Providelo has proposed merging lp:~cprov/launchpad/cric-api-take2 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cprov/launchpad/cric-api-take2/+merge/200563
--
https://code.launchpad.net/~cprov/launchpad/cric-api-take2/+merge/200563
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cprov/launchpad/cric-api-take2 into lp:launchpad.
=== modified file 'lib/lp/app/javascript/client.js'
--- lib/lp/app/javascript/client.js 2013-12-12 04:47:30 +0000
+++ lib/lp/app/javascript/client.js 2014-01-06 17:56:12 +0000
@@ -183,8 +183,9 @@
for (index = 0; index < value.length; index++) {
elems.push(enc(key) + "=" + enc(value[index]));
}
- }
- else {
+ } else if (Y.Lang.isObject(value)) {
+ elems.push(enc(key) + "=" + Y.JSON.stringify(value));
+ } else {
elems.push(enc(key) + "=" + enc(value));
}
return elems.join("&");
@@ -1256,5 +1257,5 @@
}
});
}, "0.1", {
- requires: ["base", "plugin", "dump", "lp.client"]
+ requires: ["base", "plugin", "dump", "json", "lp.client"]
});
=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py 2013-09-05 03:57:28 +0000
+++ lib/lp/code/browser/branchmergeproposal.py 2014-01-06 17:56:12 +0000
@@ -61,6 +61,7 @@
SimpleTerm,
SimpleVocabulary,
)
+from zope.security.proxy import removeSecurityProxy
from lp import _
from lp.app.browser.launchpadform import (
@@ -92,6 +93,9 @@
)
from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
from lp.code.interfaces.codereviewcomment import ICodeReviewComment
+from lp.code.interfaces.codereviewinlinecomment import (
+ ICodeReviewInlineCommentSet,
+ )
from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference
from lp.code.interfaces.diff import IPreviewDiff
from lp.registry.interfaces.person import IPersonSet
@@ -627,6 +631,11 @@
self.context).event_key
if getFeatureFlag("code.inline_diff_comments.enabled"):
cache.objects['inline_diff_comments'] = True
+ cache.objects['preview_diff_timestamps'] = [
+ pd.date_created for pd in self.context.preview_diffs]
+ cache.objects['published_inline_comments'] = (
+ getUtility(ICodeReviewInlineCommentSet).findByPreviewDiff(
+ removeSecurityProxy(self.context.preview_diff), self.user))
@action('Claim', name='claim')
def claim_action(self, action, data):
@@ -713,9 +722,7 @@
If no preview is available, try using the review diff.
"""
- if self.context.preview_diff is not None:
- return self.context.preview_diff
- return None
+ return self.context.preview_diff
@property
def has_bug_or_spec(self):
@@ -1462,8 +1469,7 @@
else:
review_type = self.users_vote_ref.review_type
# We'll be positive here and default the vote to approve.
- return {'vote': CodeReviewVote.APPROVE,
- 'review_type': review_type}
+ return {'vote': CodeReviewVote.APPROVE, 'review_type': review_type}
def initialize(self):
"""Get the users existing vote reference."""
@@ -1478,10 +1484,10 @@
if self.user is None:
# Anonymous users are not valid voters.
raise AssertionError('Invalid voter')
- LaunchpadFormView.initialize(self)
+ super(BranchMergeProposalAddVoteView, self).initialize()
def setUpFields(self):
- LaunchpadFormView.setUpFields(self)
+ super(BranchMergeProposalAddVoteView, self).setUpFields()
self.reviewer = self.user.name
# claim_review is set in situations where a user is reviewing on
# behalf of a team.
@@ -1512,8 +1518,7 @@
# the data dict. If this is the case, get the review_type from the
# hidden field that we so cunningly added to the form.
review_type = data.get(
- 'review_type',
- self.request.form.get('review_type'))
+ 'review_type', self.request.form.get('review_type'))
# Translate the request parameter back into what our object model
# needs.
if review_type == '':
=== modified file 'lib/lp/code/browser/codereviewcomment.py'
--- lib/lp/code/browser/codereviewcomment.py 2013-04-11 04:54:04 +0000
+++ lib/lp/code/browser/codereviewcomment.py 2014-01-06 17:56:12 +0000
@@ -13,6 +13,7 @@
from lazr.delegates import delegates
from lazr.restful.interface import copy_field
+from zope.component import getUtility
from zope.formlib.widgets import (
DropdownWidget,
TextAreaWidget,
@@ -21,7 +22,10 @@
implements,
Interface,
)
-from zope.schema import Text
+from zope.schema import (
+ Bool,
+ Text,
+ )
from lp import _
from lp.app.browser.launchpadform import (
@@ -30,11 +34,15 @@
LaunchpadFormView,
)
from lp.code.interfaces.codereviewcomment import ICodeReviewComment
+from lp.code.interfaces.codereviewinlinecomment import (
+ ICodeReviewInlineCommentSet,
+ )
from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference
from lp.services.comments.browser.comment import download_body
from lp.services.comments.browser.messagecomment import MessageComment
from lp.services.comments.interfaces.conversation import IComment
from lp.services.config import config
+from lp.services.features import getFeatureFlag
from lp.services.librarian.interfaces import ILibraryFileAlias
from lp.services.propertycache import (
cachedproperty,
@@ -214,6 +222,9 @@
comment = Text(title=_('Comment'), required=False)
+ publish_inline_comments = Bool(
+ title=_("Publish draft inline comments"), required=False)
+
class CodeReviewCommentAddView(LaunchpadFormView):
"""View for adding a CodeReviewComment."""
@@ -242,6 +253,11 @@
comment = ''
return {'comment': comment}
+ def setUpFields(self):
+ super(CodeReviewCommentAddView, self).setUpFields()
+ if not getFeatureFlag('code.inline_diff_comments.enabled'):
+ self.form_fields.omit('publish_inline_comments')
+
@property
def is_reply(self):
"""True if this comment is a reply to another comment, else False."""
@@ -268,9 +284,14 @@
"""Create the comment..."""
vote = data.get('vote')
review_type = data.get('review_type')
- self.branch_merge_proposal.createComment(
+ comment = self.branch_merge_proposal.createComment(
self.user, subject=None, content=data['comment'],
parent=self.reply_to, vote=vote, review_type=review_type)
+ if (
+ getFeatureFlag('code.inline_diff_comments.enabled') and
+ data.get('publish_inline_comments')):
+ getUtility(ICodeReviewInlineCommentSet).publishDraft(
+ self.branch_merge_proposal.preview_diff, self.user, comment)
@property
def next_url(self):
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2013-11-21 03:42:38 +0000
+++ lib/lp/code/configure.zcml 2014-01-06 17:56:12 +0000
@@ -586,6 +586,23 @@
provides="lp.services.webapp.interfaces.IPrimaryContext"
factory="lp.code.browser.codereviewcomment.CodeReviewCommentPrimaryContext"/>
+ <!-- CodeReviewInlineComment -->
+
+ <class class="lp.code.model.codereviewinlinecomment.CodeReviewInlineComment">
+ <allow interface="lp.code.interfaces.codereviewinlinecomment.ICodeReviewInlineComment"/>
+ </class>
+
+ <!-- CodeReviewInlineCommentSet -->
+
+ <securedutility
+ class="lp.code.model.codereviewinlinecomment.CodeReviewInlineCommentSet"
+ provides="lp.code.interfaces.codereviewinlinecomment.ICodeReviewInlineCommentSet">
+ <allow interface="lp.code.interfaces.codereviewinlinecomment.ICodeReviewInlineCommentSet"/>
+ </securedutility>
+ <class class="lp.code.model.codereviewinlinecomment.CodeReviewInlineCommentSet">
+ <allow interface="lp.code.interfaces.codereviewinlinecomment.ICodeReviewInlineCommentSet"/>
+ </class>
+
<!-- hierarchy -->
<class class="lp.code.model.branchjob.BranchJob">
=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py 2012-08-28 00:07:47 +0000
+++ lib/lp/code/errors.py 2014-01-06 17:56:12 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Errors used in the lp/code modules."""
@@ -27,6 +27,7 @@
'CodeImportAlreadyRunning',
'CodeImportNotInReviewedState',
'ClaimReviewFailed',
+ 'DiffNotFound',
'InvalidBranchMergeProposal',
'InvalidMergeQueueConfig',
'InvalidNamespace',
@@ -384,3 +385,8 @@
def __init__(self):
message = ('The configuration specified is not a valid JSON string.')
Exception.__init__(self, message)
+
+
+@error_status(httplib.BAD_REQUEST)
+class DiffNotFound(Exception):
+ """A `IPreviewDiff` with the timestamp was not found."""
=== modified file 'lib/lp/code/interfaces/branchmergeproposal.py'
--- lib/lp/code/interfaces/branchmergeproposal.py 2013-12-16 05:04:43 +0000
+++ lib/lp/code/interfaces/branchmergeproposal.py 2014-01-06 17:56:12 +0000
@@ -56,6 +56,7 @@
Bool,
Choice,
Datetime,
+ Dict,
Int,
Object,
Text,
@@ -569,6 +570,20 @@
:param original_email: Original email message.
"""
+ @export_write_operation()
+ @operation_parameters(
+ diff_timestamp=Datetime(), comments=Dict(key_type=TextLine()))
+ @call_with(person=REQUEST_USER)
+ @operation_for_version('devel')
+ def saveDraftInlineComment(diff_timestamp, person, comments):
+ """Save `ICodeReviewInlineCommentDraft`
+
+ :param diff_timestamp: The timestamp of the `PreviewDiff` the comments
+ are on.
+ :param person: The `IPerson` making the comments.
+ :param comments: The comments.
+ """
+
class IBranchMergeProposal(IBranchMergeProposalPublic,
IBranchMergeProposalView, IBranchMergeProposalEdit,
=== added file 'lib/lp/code/interfaces/codereviewinlinecomment.py'
--- lib/lp/code/interfaces/codereviewinlinecomment.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/interfaces/codereviewinlinecomment.py 2014-01-06 17:56:12 +0000
@@ -0,0 +1,67 @@
+# Copyright 2013 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Interfaces for inline comments with preview diffs."""
+
+__metaclass__ = type
+__all__ = [
+ 'ICodeReviewInlineComment',
+ 'ICodeReviewInlineCommentSet',
+ ]
+
+from lazr.restful.fields import Reference
+from zope.interface import Interface
+from zope.schema import (
+ Datetime,
+ TextLine,
+ )
+
+from lp import _
+from lp.code.interfaces.codereviewcomment import ICodeReviewComment
+from lp.code.interfaces.diff import IPreviewDiff
+from lp.registry.interfaces.person import IPerson
+
+
+class ICodeReviewInlineComment(Interface):
+ previewdiff = Reference(
+ title=_('The preview diff'), schema=IPreviewDiff, required=True,
+ readonly=True)
+ person = Reference(
+ title=_('Person'), schema=IPerson, required=True, readonly=True)
+ comment = Reference(
+ title=_('The branch merge proposal comment'),
+ schema=ICodeReviewComment, required=True, readonly=True)
+ date_created = Datetime(
+ title=_('The date on which the comments were published'),
+ required=True, readonly=True)
+ comments = TextLine(
+ title=_('Inline comments'), required=True, readonly=True)
+
+
+class ICodeReviewInlineCommentSet(Interface):
+ def findDraft(previewdiff, person):
+ """Find the draft comments for a given diff and person.
+
+ :param previewdiff: The `PreviewDiff` these comments are for.
+ :param person: The `Person` making the comments.
+ """
+
+ def ensureDraft(previewdiff, person, comments):
+ """Ensure a `ICodeReviewInlineCommentDraft` is up to date. This method
+ will also delete an existing draft if the comments are empty.
+
+ :param previewdiff: The `PreviewDiff` these comments are for.
+ :param person: The `Person` making the comments.
+ :param comments: The comments themselves.
+ """
+
+ def publishDraft(previewdiff, person, comment):
+ """Publish code review inline comments so other people can view them.
+
+ :param previewdiff: The `PreviewDiff` these comments are for.
+ :param person: The `Person` making the comments.
+ :param comment: The `CodeReviewComment` linked to the comments.
+ """
+
+ def findByPreviewDiff(previewdiff):
+ """Find all comments for a given `PreviewDiff`."""
=== modified file 'lib/lp/code/javascript/branchmergeproposal.inlinecomments.js'
--- lib/lp/code/javascript/branchmergeproposal.inlinecomments.js 2013-09-17 06:14:49 +0000
+++ lib/lp/code/javascript/branchmergeproposal.inlinecomments.js 2014-01-06 17:56:12 +0000
@@ -11,9 +11,9 @@
// Grab the namespace in order to be able to expose the connect methods.
var namespace = Y.namespace('lp.code.branchmergeproposal.inlinecomments');
+var inlinecomments = {};
namespace.add_doubleclick_handler = function() {
- var inlinecomments = {};
var rows = Y.one('.diff').all('tr');
var handling_request = false;
handler = function(e) {
@@ -39,9 +39,20 @@
inlinecomments[rownumber] = comment;
}
if (comment === '') {
+ delete inlinecomments[rownumber];
headerrow.remove(true);
newrow.remove(true);
}
+ var diff_timestamp = LP.cache.preview_diff_timestamps.slice(-1)[0];
+ var config = {
+ parameters: {
+ diff_timestamp: diff_timestamp,
+ comments: inlinecomments
+ }
+ };
+ lp_client = new Y.lp.client.Launchpad();
+ lp_client.named_post(
+ LP.cache.context.self_link, 'saveDraftInlineComment', config);
handling_request = false;
};
widget.editor.on('save', function() {
@@ -90,6 +101,10 @@
'<div class="yui3-editable_text-trigger"></div>' +
'</div></td></tr>').set('id', 'ict-' + middle);
newrow.one('td>div').set('id', 'inlinecomment-' + middle);
+ if (comment !== null) {
+ inlinecomments[rownumber] = comment;
+ //newrow.one('span').set('text', comment);
+ }
} else {
newrow = Y.Node.create('<tr><td></td><td><span></span></td></tr>');
newrow.one('span').set('text', comment);
=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py 2013-06-20 05:50:00 +0000
+++ lib/lp/code/model/branchmergeproposal.py 2014-01-06 17:56:12 +0000
@@ -45,6 +45,7 @@
BadBranchMergeProposalSearchContext,
BadStateTransition,
BranchMergeProposalExists,
+ DiffNotFound,
UserNotBranchReviewer,
WrongBranchMergeProposal,
)
@@ -63,6 +64,9 @@
)
from lp.code.interfaces.branchrevision import IBranchRevision
from lp.code.interfaces.branchtarget import IHasBranchTarget
+from lp.code.interfaces.codereviewinlinecomment import (
+ ICodeReviewInlineCommentSet,
+ )
from lp.code.mail.branch import RecipientReason
from lp.code.model.branchrevision import BranchRevision
from lp.code.model.codereviewcomment import CodeReviewComment
@@ -869,6 +873,19 @@
code_review_message, original_email))
return code_review_message
+ def saveDraftInlineComment(self, diff_timestamp, person, comments):
+ """See `IBranchMergeProposal`."""
+ previewdiff = IStore(PreviewDiff).find(
+ PreviewDiff,
+ PreviewDiff.branch_merge_proposal_id == self.id,
+ PreviewDiff.date_created == diff_timestamp).one()
+ if not previewdiff:
+ raise DiffNotFound(
+ "Could not locate a preview diff with a timestamp of %s" % (
+ diff_timestamp))
+ getUtility(ICodeReviewInlineCommentSet).ensureDraft(
+ previewdiff, person, comments)
+
def updatePreviewDiff(self, diff_content, source_revision_id,
target_revision_id, prerequisite_revision_id=None,
conflicts=None):
=== added file 'lib/lp/code/model/codereviewinlinecomment.py'
--- lib/lp/code/model/codereviewinlinecomment.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/model/codereviewinlinecomment.py 2014-01-06 17:56:12 +0000
@@ -0,0 +1,123 @@
+# Copyright 2013 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Model classes for inline comments for preview diffs."""
+
+__metaclass__ = type
+__all__ = [
+ 'CodeReviewInlineComment',
+ 'CodeReviewInlineCommentDraft',
+ 'CodeReviewInlineCommentSet',
+ ]
+
+import simplejson
+from storm.locals import (
+ Int,
+ Reference,
+ Unicode,
+ )
+from zope.component import getUtility
+from zope.interface import implements
+
+from lp.code.interfaces.codereviewinlinecomment import (
+ ICodeReviewInlineComment,
+ ICodeReviewInlineCommentSet,
+ )
+from lp.code.model.codereviewcomment import CodeReviewComment
+from lp.registry.interfaces.person import IPersonSet
+from lp.services.database.bulk import load_related
+from lp.services.database.interfaces import IStore
+from lp.services.database.stormbase import StormBase
+
+
+class CodeReviewInlineComment(StormBase):
+ __storm_table__ = 'CodeReviewInlineComment'
+
+ implements(ICodeReviewInlineComment)
+
+ previewdiff_id = Int(name='previewdiff')
+ previewdiff = Reference(previewdiff_id, 'PreviewDiff.id')
+ person_id = Int(name='person')
+ person = Reference(person_id, 'Person.id')
+ comment_id = Int(name='comment', primary=True)
+ comment = Reference(comment_id, 'CodeReviewComment.id')
+ comments = Unicode()
+
+
+class CodeReviewInlineCommentDraft(StormBase):
+ __storm_table__ = 'CodeReviewInlineCommentDraft'
+ __storm_primary__ = ('previewdiff_id', 'person_id')
+
+ previewdiff_id = Int(name='previewdiff')
+ previewdiff = Reference(previewdiff_id, 'PreviewDiff.id')
+ person_id = Int(name='person')
+ person = Reference(person_id, 'Person.id')
+ comments = Unicode()
+
+
+class CodeReviewInlineCommentSet:
+
+ implements(ICodeReviewInlineCommentSet)
+
+ def _findDraftObject(self, previewdiff, person):
+ return IStore(CodeReviewInlineCommentDraft).find(
+ CodeReviewInlineCommentDraft,
+ CodeReviewInlineCommentDraft.previewdiff_id == previewdiff.id,
+ CodeReviewInlineCommentDraft.person_id == person.id).one()
+
+ def findDraft(self, previewdiff, person):
+ cricd = self._findDraftObject(previewdiff, person)
+ if cricd:
+ return simplejson.loads(cricd.comments)
+ return {}
+
+ def ensureDraft(self, previewdiff, person, comments):
+ cricd = self._findDraftObject(previewdiff, person)
+ if not comments:
+ if cricd:
+ IStore(CodeReviewInlineCommentDraft).remove(cricd)
+ else:
+ if type(comments) == dict:
+ comments = simplejson.dumps(comments).decode('utf-8')
+ if cricd:
+ cricd.comments = comments
+ else:
+ cricd = CodeReviewInlineCommentDraft()
+ cricd.previewdiff = previewdiff
+ cricd.person = person
+ cricd.comments = comments
+ IStore(CodeReviewInlineCommentDraft).add(cricd)
+
+ def publishDraft(self, previewdiff, person, comment):
+ cricd = self._findDraftObject(previewdiff, person)
+ assert cricd is not None
+ cric = CodeReviewInlineComment()
+ cric.previewdiff = cricd.previewdiff
+ cric.person = cricd.person
+ cric.comment = comment
+ cric.comments = cricd.comments
+ IStore(CodeReviewInlineComment).add(cric)
+ IStore(CodeReviewInlineComment).remove(cricd)
+ return cric
+
+ def findByPreviewDiff(self, previewdiff, person):
+ crics = IStore(CodeReviewInlineComment).find(
+ CodeReviewInlineComment,
+ CodeReviewInlineComment.previewdiff_id == previewdiff.id)
+ getUtility(IPersonSet).getPrecachedPersonsFromIDs(
+ [cric.person_id for cric in crics])
+ load_related(CodeReviewComment, crics, ['comment_id'])
+ sorted_crics = sorted(
+ list(crics), key=lambda c: c.comment.date_created)
+ inline_comments = []
+ for cric in sorted_crics:
+ comments = simplejson.loads(cric.comments)
+ for lineno in comments:
+ inline_comments.append(
+ [lineno, cric.person, comments[lineno],
+ cric.comment.date_created])
+ draft_comments = self.findDraft(previewdiff, person)
+ for draft_lineno in draft_comments:
+ inline_comments.append(
+ [draft_lineno, None, draft_comments[draft_lineno], None])
+ return inline_comments
=== modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py'
--- lib/lp/code/model/tests/test_branchmergeproposal.py 2013-05-14 14:30:09 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposal.py 2014-01-06 17:56:12 +0000
@@ -47,6 +47,9 @@
IBranchMergeProposalGetter,
notify_modified,
)
+from lp.code.interfaces.codereviewinlinecomment import (
+ ICodeReviewInlineCommentSet,
+ )
from lp.code.model.branchmergeproposal import (
BranchMergeProposalGetter,
is_valid_transition,
@@ -2087,3 +2090,22 @@
user = previewdiff.branch_merge_proposal.target_branch.owner
ws_previewdiff = self.wsObject(previewdiff, user=user)
self.assertIsNone(ws_previewdiff.diffstat)
+
+ def test_saveDraftInlineComment_with_no_previewdiff(self):
+ bmp = self.factory.makeBranchMergeProposal()
+ ws_bmp = self.wsObject(bmp, user=bmp.target_branch.owner)
+ self.assertRaises(
+ BadRequest, ws_bmp.saveDraftInlineComment,
+ diff_timestamp=datetime(2001, 1, 1, 12, tzinfo=UTC), comments={})
+
+ def test_saveDraftInlineComment(self):
+ previewdiff = self.factory.makePreviewDiff()
+ user = previewdiff.branch_merge_proposal.target_branch.owner
+ ws_bmp = self.wsObject(previewdiff.branch_merge_proposal, user=user)
+ comments = {u'2': u'foo'}
+ ws_bmp.saveDraftInlineComment(
+ diff_timestamp=previewdiff.date_created, comments=comments)
+ transaction.commit()
+ draft_comments = getUtility(ICodeReviewInlineCommentSet).findDraft(
+ previewdiff, user)
+ self.assertEqual(comments, draft_comments)
=== added file 'lib/lp/code/model/tests/test_codereviewinlinecomment.py'
--- lib/lp/code/model/tests/test_codereviewinlinecomment.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/model/tests/test_codereviewinlinecomment.py 2014-01-06 17:56:12 +0000
@@ -0,0 +1,100 @@
+# Copyright 2013 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for CodeReviewInlineComment{,Draft,Set}"""
+
+__metaclass__ = type
+
+from datetime import datetime
+
+from pytz import UTC
+from zope.component import getUtility
+
+from lp.code.interfaces.codereviewinlinecomment import (
+ ICodeReviewInlineCommentSet,
+ )
+from lp.testing import TestCaseWithFactory
+from lp.testing.layers import LaunchpadFunctionalLayer
+
+
+class TestCodeReviewInlineComment(TestCaseWithFactory):
+ layer = LaunchpadFunctionalLayer
+
+ def _makeCRICD(self):
+ previewdiff = self.factory.makePreviewDiff()
+ person = self.factory.makePerson()
+ self.factory.makeCodeReviewInlineCommentDraft(
+ previewdiff=previewdiff, person=person)
+ return previewdiff, person
+
+ def test_ensure_creates(self):
+ # ICodeReviewInlineCommentSet.ensureDraft() will create one if it
+ # does not exist.
+ interface = getUtility(ICodeReviewInlineCommentSet)
+ previewdiff, person = self._makeCRICD()
+ self.assertEqual(
+ {'2': 'foobar'}, interface.findDraft(previewdiff, person))
+
+ def test_ensure_deletes(self):
+ # ICodeReviewInlineCommentSet.ensureDraft() will delete a draft if
+ # no comments are provided.
+ interface = getUtility(ICodeReviewInlineCommentSet)
+ previewdiff, person = self._makeCRICD()
+ interface.ensureDraft(previewdiff, person, {})
+ self.assertEqual({}, interface.findDraft(previewdiff, person))
+
+ def test_ensure_deletes_with_no_draft(self):
+ # ICodeReviewInlineCommentSet.ensureDraft() will cope with a draft
+ # that does not exist when called with no comments.
+ interface = getUtility(ICodeReviewInlineCommentSet)
+ previewdiff = self.factory.makePreviewDiff()
+ person = self.factory.makePerson()
+ interface.ensureDraft(previewdiff, person, {})
+ self.assertEqual({}, interface.findDraft(previewdiff, person))
+
+ def test_ensure_updates(self):
+ # ICodeReviewInlineCommentSet.ensureDraft() will update the draft when
+ # the comments change.
+ interface = getUtility(ICodeReviewInlineCommentSet)
+ previewdiff, person = self._makeCRICD()
+ interface.ensureDraft(previewdiff, person, {'1': 'bar'})
+ comment = interface.findDraft(previewdiff, person)
+ self.assertEqual({'1': 'bar'}, comment)
+
+ def test_publishDraft(self):
+ # ICodeReviewInlineCommentSet.publishDraft() will publish draft
+ # comments.
+ interface = getUtility(ICodeReviewInlineCommentSet)
+ previewdiff, person = self._makeCRICD()
+ comment = self.factory.makeCodeReviewComment(
+ merge_proposal=previewdiff.branch_merge_proposal, sender=person)
+ cric = interface.publishDraft(previewdiff, person, comment)
+ self.assertIsNot(None, cric)
+ self.assertEqual({}, interface.findDraft(previewdiff, person))
+
+ def test_findByPreviewDiff_draft(self):
+ # ICodeReviewInlineCommentSet.findByPreviewDiff() will return a draft
+ # so it can be rendered.
+ interface = getUtility(ICodeReviewInlineCommentSet)
+ previewdiff, person = self._makeCRICD()
+ results = interface.findByPreviewDiff(previewdiff, person)
+ self.assertEqual([[u'2', None, u'foobar', None]], results)
+
+ def test_findByPreviewDiff_sorted(self):
+ # ICodeReviewInlineCommentSet.findByPreviewDiff() will return a sorted
+ # list.
+ interface = getUtility(ICodeReviewInlineCommentSet)
+ previewdiff = self.factory.makePreviewDiff()
+ person = self.factory.makePerson()
+ comment = self.factory.makeCodeReviewComment()
+ self.factory.makeCodeReviewInlineComment(
+ previewdiff=previewdiff, person=person, comment=comment)
+ old_comment = self.factory.makeCodeReviewComment(
+ date_created=datetime(2001, 1, 1, 12, tzinfo=UTC))
+ self.factory.makeCodeReviewInlineComment(
+ previewdiff=previewdiff, person=person, comment=old_comment,
+ comments={'8': 'baz'})
+ self.assertEqual(
+ [[u'8', person, u'baz', old_comment.date_created],
+ [u'2', person, u'foobar', comment.date_created]],
+ interface.findByPreviewDiff(previewdiff, person))
=== modified file 'lib/lp/code/templates/branchmergeproposal-index.pt'
--- lib/lp/code/templates/branchmergeproposal-index.pt 2013-09-05 03:57:28 +0000
+++ lib/lp/code/templates/branchmergeproposal-index.pt 2014-01-06 17:56:12 +0000
@@ -144,6 +144,7 @@
<div id="add-comment-review-fields">
Review: <tal:review replace="structure comment_form/widgets/vote"/>
Review type: <tal:review replace="structure comment_form/widgets/review_type"/>
+ <tal:publish_inline_comments condition="features/code.inline_diff_comments.enabled">Publish inline comments: <tal:review replace="structure comment_form/widgets/publish_inline_comments" tal:condition="features/code.inline_diff_comments.enabled"/></tal:publish_inline_comments>
<div class="actions"
tal:content="structure comment_form/actions/field.actions.add/render" />
</div>
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2013-11-30 00:12:15 +0000
+++ lib/lp/testing/factory.py 2014-01-06 17:56:12 +0000
@@ -116,6 +116,9 @@
from lp.code.interfaces.codeimportevent import ICodeImportEventSet
from lp.code.interfaces.codeimportmachine import ICodeImportMachineSet
from lp.code.interfaces.codeimportresult import ICodeImportResultSet
+from lp.code.interfaces.codereviewinlinecomment import (
+ ICodeReviewInlineCommentSet,
+ )
from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
from lp.code.interfaces.revision import IRevisionSet
from lp.code.interfaces.sourcepackagerecipe import (
@@ -4316,6 +4319,27 @@
product.redeemSubscriptionVoucher(
self.getUniqueString(), person, person, months)
+ def makeCodeReviewInlineCommentDraft(self, previewdiff=None, person=None,
+ comments={'2': 'foobar'}):
+ if previewdiff is None:
+ previewdiff = self.makePreviewDiff()
+ if person is None:
+ person = self.makePerson()
+ getUtility(ICodeReviewInlineCommentSet).ensureDraft(
+ previewdiff, person, comments)
+
+ def makeCodeReviewInlineComment(self, previewdiff=None, person=None,
+ comment=None, comments={'2': 'foobar'}):
+ if previewdiff is None:
+ previewdiff = self.makePreviewDiff()
+ if person is None:
+ person = self.makePerson()
+ if comment is None:
+ comment = self.makeCodeReviewComment()
+ self.makeCodeReviewInlineCommentDraft(previewdiff, person, comments)
+ return getUtility(ICodeReviewInlineCommentSet).publishDraft(
+ previewdiff, person, comment)
+
# Some factory methods return simple Python types. We don't add
# security wrappers for them, as well as for objects created by
Follow ups
