launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-ref-more-encoding into lp:launchpad.

Commit message:
Fix GitRef:+index rendering for non-ASCII ref names.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-ref-more-encoding/+merge/353637
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-ref-more-encoding into lp:launchpad.

=== modified file 'lib/lp/code/browser/tests/test_gitref.py'
--- lib/lp/code/browser/tests/test_gitref.py	2018-08-23 09:30:24 +0000
+++ lib/lp/code/browser/tests/test_gitref.py	2018-08-23 12:41:08 +0000
@@ -89,7 +89,7 @@
         super(TestGitRefView, self).setUp()
         self.hosting_fixture = self.useFixture(GitHostingFixture())
 
-    def test_rendering(self):
+    def _test_rendering(self, branch_name):
         repository = self.factory.makeGitRepository(
             owner=self.factory.makePerson(name="person"),
             target=self.factory.makeProduct(name="target"),
@@ -97,7 +97,7 @@
         getUtility(IGitRepositorySet).setDefaultRepositoryForOwner(
             repository.owner, repository.target, repository, repository.owner)
         [ref] = self.factory.makeGitRefs(
-            repository=repository, paths=["refs/heads/master"])
+            repository=repository, paths=["refs/heads/%s" % branch_name])
         view = create_view(ref, "+index")
         # To test the breadcrumbs we need a correct traversal stack.
         view.request.traversed_objects = [repository, ref, view]
@@ -124,7 +124,13 @@
                     breadcrumbs_tag,
                     soupmatchers.Tag(
                         'git ref breadcrumb', 'li',
-                        text=re.compile(r'\smaster\s')))))
+                        text=re.compile(r'\s%s\s' % branch_name)))))
+
+    def test_rendering(self):
+        self._test_rendering("master")
+
+    def test_rendering_non_ascii(self):
+        self._test_rendering("\N{SNOWMAN}")
 
     def test_clone_instructions(self):
         [ref] = self.factory.makeGitRefs(paths=["refs/heads/branch"])

=== modified file 'lib/lp/code/model/gitref.py'
--- lib/lp/code/model/gitref.py	2018-08-23 09:30:24 +0000
+++ lib/lp/code/model/gitref.py	2018-08-23 12:41:08 +0000
@@ -145,7 +145,8 @@
     def getCodebrowseUrl(self):
         """See `IGitRef`."""
         return "%s?h=%s" % (
-            self.repository.getCodebrowseUrl(), quote_plus(self.name))
+            self.repository.getCodebrowseUrl(),
+            quote_plus(self.name.encode("UTF-8")))
 
     def getCodebrowseUrlForRevision(self, commit):
         """See `IGitRef`."""

=== modified file 'lib/lp/code/model/tests/test_gitref.py'
--- lib/lp/code/model/tests/test_gitref.py	2018-07-13 09:17:40 +0000
+++ lib/lp/code/model/tests/test_gitref.py	2018-08-23 12:41:08 +0000
@@ -37,6 +37,7 @@
     GitRepositoryScanFault,
     InvalidBranchMergeProposal,
     )
+from lp.code.interfaces.gitrepository import IGitRepositorySet
 from lp.code.tests.helpers import GitHostingFixture
 from lp.services.config import config
 from lp.services.features.testing import FeatureFixture
@@ -71,6 +72,27 @@
             ["%s:master" % repo_path, "%s:people/foo/bar" % repo_path],
             [ref.display_name for ref in (master, personal)])
 
+    def test_codebrowse_url(self):
+        [ref] = self.factory.makeGitRefs(paths=["refs/heads/name"])
+        expected_url = "%s?h=name" % urlutils.join(
+            config.codehosting.git_browse_root, ref.repository.shortened_path)
+        self.assertEqual(expected_url, ref.getCodebrowseUrl())
+
+    def test_codebrowse_url_for_default(self):
+        [ref] = self.factory.makeGitRefs(paths=["refs/heads/name"])
+        with person_logged_in(ref.repository.target.owner):
+            getUtility(IGitRepositorySet).setDefaultRepository(
+                ref.repository.target, ref.repository)
+        expected_url = "%s?h=name" % urlutils.join(
+            config.codehosting.git_browse_root, ref.repository.shortened_path)
+        self.assertEqual(expected_url, ref.getCodebrowseUrl())
+
+    def test_codebrowse_url_non_ascii(self):
+        [ref] = self.factory.makeGitRefs(paths=["refs/heads/\N{ANCHOR}"])
+        expected_url = "%s?h=%%E2%%9A%%93" % urlutils.join(
+            config.codehosting.git_browse_root, ref.repository.shortened_path)
+        self.assertEqual(expected_url, ref.getCodebrowseUrl())
+
     def test_getMergeProposals(self):
         [target_ref] = self.factory.makeGitRefs()
         bmp = self.factory.makeBranchMergeProposalForGit(target_ref=target_ref)