Review: Approve
Diff comments: > diff --git a/lib/lp/bugs/browser/bugcomment.py > b/lib/lp/bugs/browser/bugcomment.py > index 2b61552..e40967f 100644 > --- a/lib/lp/bugs/browser/bugcomment.py > +++ b/lib/lp/bugs/browser/bugcomment.py > @@ -72,7 +72,10 @@ def build_comments_from_chunks( > cache = get_property_cache(message) > if getattr(cache, 'chunks', None) is None: > cache.chunks = [] > - cache.chunks.append(removeSecurityProxy(chunk)) > + # Soft-deleted messages will have None chunk here. Skip cache > + # filling in this case. > + if chunk is not None: Ah, I see. Fair enough then. > + cache.chunks.append(removeSecurityProxy(chunk)) > bug_comment = comments.get(message.id) > if bug_comment is None: > if bugmessage.index == 0 and hide_first: > diff --git a/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt > b/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt > index 32f0373..58b86e7 100644 > --- a/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt > +++ b/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt > @@ -198,9 +198,13 @@ The comments on a branch merge proposal are exposed > through the API. > as_quoted_email: '> This is great work' > author_link: 'http://api.launchpad.test/devel/~...' > branch_merge_proposal_link: 'http://.../~source/fooix/fix-it/+merge/...' > + content: 'This is great work' If this is the plan, then we should deprecate `ICodeReviewComment.message_body` in this MP, but not remove it: that is, just change its description to indicate that users of that attribute should use `content` instead. > date_created: '...' > + date_deleted: None > + date_last_edited: None > id: ... > message_body: 'This is great work' > + owner_link: 'http://...' > resource_type_link: 'http://.../#code_review_comment' > self_link: 'http://.../~source/fooix/fix-it/+merge/.../comments/...' > title: 'Comment on proposed merge of lp://dev/~source/fooix/fix-it into > lp://dev/~target/fooix/trunk' > diff --git a/lib/lp/services/messages/browser/message.py > b/lib/lp/services/messages/browser/message.py > index 3b5a3c4..06800e6 100644 > --- a/lib/lp/services/messages/browser/message.py > +++ b/lib/lp/services/messages/browser/message.py > @@ -18,7 +18,7 @@ class QuestionMessageCanonicalUrlData: > > def __init__(self, question, message): > self.inside = question > - self.path = "messages/%d" % list(question.messages).index(message) > + self.path = "messages/%d" % message.display_index Interesting - OK then. And I checked lp-spam-utils and it doesn't in fact care about the message URL (it just does `message.question.setCommentVisibility(comment_number=message.index - 1, visible=False)` or similar), so I guess that won't be a problem. > > > @implementer(ICanonicalUrlData) -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402211 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-model. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
