launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/git-fixes into lp:launchpad.

Commit message:
Fix Git merge proposal diff JSON rendering, Unicode diffs, and comment rendering.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/git-fixes/+merge/257977

Fix a few trivial issues with Git MPs:

 - Fix Git PreviewDiff JSON rendering.

 - Fix non-ASCII git diffs.

 - Fix codereviewcomment rendering OOPS for git MPs.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/git-fixes into lp:launchpad.

=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2015-04-22 16:41:41 +0000
+++ lib/lp/code/interfaces/branch.py	2015-04-30 22:13:56 +0000
@@ -358,11 +358,12 @@
              description=_("Unique name of the branch, including the "
                            "owner and project names.")))
 
-    displayname = exported(
+    display_name = exported(
         Text(title=_('Display name'), readonly=True,
              description=_(
-                "The branch unique_name.")),
-        exported_as='display_name')
+                "The branch unique_name.")))
+
+    displayname = Attribute("Display name (deprecated)")
 
     code_reviewer = Attribute(
         "The reviewer if set, otherwise the owner of the branch.")

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2015-04-22 16:41:41 +0000
+++ lib/lp/code/model/branch.py	2015-04-30 22:13:56 +0000
@@ -675,10 +675,12 @@
         return safe_open('lp-internal', self.getInternalBzrUrl())
 
     @property
-    def displayname(self):
+    def display_name(self):
         """See `IBranch`."""
         return self.bzr_identity
 
+    displayname = display_name
+
     @property
     def code_reviewer(self):
         """See `IBranch`."""

=== modified file 'lib/lp/code/model/codereviewcomment.py'
--- lib/lp/code/model/codereviewcomment.py	2015-04-15 13:41:13 +0000
+++ lib/lp/code/model/codereviewcomment.py	2015-04-30 22:13:56 +0000
@@ -95,8 +95,8 @@
     @property
     def title(self):
         return ('Comment on proposed merge of %(source)s into %(target)s' %
-            {'source': self.branch_merge_proposal.source_branch.displayname,
-             'target': self.branch_merge_proposal.target_branch.displayname,
+            {'source': self.branch_merge_proposal.merge_source.display_name,
+             'target': self.branch_merge_proposal.merge_source.display_name,
             })
 
     @property

=== modified file 'lib/lp/code/model/diff.py'
--- lib/lp/code/model/diff.py	2015-04-30 19:11:18 +0000
+++ lib/lp/code/model/diff.py	2015-04-30 22:13:56 +0000
@@ -12,6 +12,7 @@
 
 from contextlib import nested
 from cStringIO import StringIO
+from operator import attrgetter
 import sys
 from uuid import uuid1
 
@@ -471,8 +472,9 @@
             conflicts = u"".join(
                 u"Conflict in %s\n" % path for path in response['conflicts'])
             preview = cls.create(
-                bmp, response['patch'], source_revision_id, target_revision_id,
-                prerequisite_revision_id, conflicts, strip_prefix_segments=1)
+                bmp, response['patch'].encode('utf-8'), source_revision_id,
+                target_revision_id, prerequisite_revision_id, conflicts,
+                strip_prefix_segments=1)
         del get_property_cache(bmp).preview_diffs
         del get_property_cache(bmp).preview_diff
         return preview
@@ -513,18 +515,14 @@
         # A preview diff is stale if the revision ids used to make the diff
         # are different from the tips of the source or target branches.
         bmp = self.branch_merge_proposal
-        if (self.source_revision_id != bmp.source_branch.last_scanned_id or
-            self.target_revision_id != bmp.target_branch.last_scanned_id):
-            # This is the simple frequent case.
-            return True
-
-        # More complex involves the prerequisite branch too.
-        if (bmp.prerequisite_branch is not None and
-            (self.prerequisite_revision_id !=
-             bmp.prerequisite_branch.last_scanned_id)):
-            return True
-        else:
-            return False
+        get_id = attrgetter(
+            'last_scanned_id' if bmp.source_branch else 'commit_sha1')
+        if (self.source_revision_id != get_id(bmp.merge_source) or
+            self.target_revision_id != get_id(bmp.merge_target)):
+            return True
+        return (
+            bmp.merge_prerequisite is not None and
+            self.prerequisite_revision_id != get_id(bmp.merge_prerequisite))
 
     def getFileByName(self, filename):
         """See `IPreviewDiff`."""