launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Inline comments:

> --- lib/lp/code/interfaces/codereviewinlinecomment.py	2014-04-04 04:30:48 +0000
> +++ lib/lp/code/interfaces/codereviewinlinecomment.py	2014-05-21 02:32:55 +0000
> @@ -89,3 +89,12 @@
>          :return: a dictionary containing the given `CodeReviewComment.id`s
>              and the corresponding `PreviewDiff.id` or None.
>          """
> +
> +    def removeInlineComments(previewdiff_ids):
> +        """Remove inline comments for the given `PreviewDiff` ids.
> +
> +        Remove `CodeReviewInlineComment`s and `CodeReviewInlineCommentDraft`s
> +        associated with a given list of `PreviewDiff` IDs.
> +
> +        :param comments: a list of `PreviewDiff` IDs.
> +        """
> 
> === modified file 'lib/lp/code/model/branchmergeproposal.py'
> --- lib/lp/code/model/branchmergeproposal.py	2014-02-25 19:40:56 +0000
> +++ lib/lp/code/model/branchmergeproposal.py	2014-05-21 02:32:55 +0000
> @@ -719,6 +719,9 @@
>          # Delete the related CodeReviewVoteReferences.
>          for vote in self.votes:
>              vote.destroySelf()
> +        # Delete published and draft inline comments related to this MP.
> +        getUtility(ICodeReviewInlineCommentSet).removeInlineComments(
> +            [pd.id for pd in self._preview_diffs])
>          # Delete the related CodeReviewComments.
>          for comment in self.all_comments:
>              comment.destroySelf()
> 
> === modified file 'lib/lp/code/model/codereviewinlinecomment.py'
> --- lib/lp/code/model/codereviewinlinecomment.py	2014-04-04 04:30:48 +0000
> +++ lib/lp/code/model/codereviewinlinecomment.py	2014-05-21 02:32:55 +0000
> @@ -143,3 +143,16 @@
>               CodeReviewInlineComment.previewdiff_id),
>              CodeReviewComment.id.is_in(c.id for c in comments))
>          return dict(relations)
> +
> +    def removeInlineComments(self, previewdiff_ids):
> +        """See `ICodeReviewInlineCommentSet`."""

removeFromDiffs, maybe? This is on CodeReviewInlineCommentSet, after all.

> +        IStore(CodeReviewInlineComment).find(
> +            CodeReviewInlineComment,
> +            CodeReviewInlineComment.previewdiff_id.is_in(
> +                previewdiff_ids)
> +        ).remove()
> +        IStore(CodeReviewInlineCommentDraft).find(
> +            CodeReviewInlineCommentDraft,
> +            CodeReviewInlineCommentDraft.previewdiff_id.is_in(
> +                previewdiff_ids)
> +        ).remove()
> 
> === modified file 'lib/lp/code/model/tests/test_branch.py'
> --- lib/lp/code/model/tests/test_branch.py	2014-01-15 01:25:27 +0000
> +++ lib/lp/code/model/tests/test_branch.py	2014-05-21 02:32:55 +0000
> @@ -1408,6 +1408,40 @@
>              branches=[branch1, branch2])
>          branch2.destroySelf(break_references=True)
>  
> +    def test_destroySelf_with_inline_comments_draft(self):
> +        # Draft inline comments related to a deleted branch (source
> +        # or target MP branch) also get removed.
> +        merge_proposal = self.factory.makeBranchMergeProposal(
> +            registrant=self.user, target_branch=self.branch)
> +        preview_diff = self.factory.makePreviewDiff(
> +            merge_proposal=merge_proposal)
> +        transaction.commit()
> +        self.useFixture(FeatureFixture({
> +            'code.inline_diff_comments.enabled': 'enabled'}))
> +        merge_proposal.saveDraftInlineComment(
> +            previewdiff_id=preview_diff.id,
> +            person=self.user,
> +            comments={'1': 'Should vanish.'})
> +        self.branch.destroySelf(break_references=True)
> +
> +    def test_destroySelf_with_inline_comments_published(self):
> +        # Published inline comments related to a deleted branch (source
> +        # or target MP branch) also get removed.
> +        merge_proposal = self.factory.makeBranchMergeProposal(
> +            registrant=self.user, target_branch=self.branch)
> +        preview_diff = self.factory.makePreviewDiff(
> +            merge_proposal=merge_proposal)
> +        transaction.commit()
> +        self.useFixture(FeatureFixture({
> +            'code.inline_diff_comments.enabled': 'enabled'}))
> +        merge_proposal.createComment(
> +            owner=self.user,
> +            subject='Delete me!',
> +            previewdiff_id=preview_diff.id,
> +            inline_comments={'1': 'Must disappear.'},
> +        )
> +        self.branch.destroySelf(break_references=True)
> +
>  
>  class TestBranchDeletionConsequences(TestCase):
>      """Test determination and application of branch deletion consequences."""
> 


-- 
https://code.launchpad.net/~cprov/launchpad/delete-branches-with-ics/+merge/220317
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.