Merge lp:~cjwatson/launchpad/git-fix-clone-from-owner-default into lp:launchpad

[Colin Watson]

Fix code to get the owner default repository if there is no target default.

The code to find the owner default if there's no target default used the wrong IGitRepositorySet method. This fixes it and adds a test.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/code/xmlrpc/git.py'
--- lib/lp/code/xmlrpc/git.py	2015-04-28 04:48:55 +0000
+++ lib/lp/code/xmlrpc/git.py	2015-04-28 10:32:26 +0000
@@ -200,12 +200,15 @@
             # If repository has target_default, clone from default.
             target_path = None
             try:
-                target_default = self.repository_set.getDefaultRepository(
-                    repository.target) or (
-                        self.repository_set.getDefaultRepository(
-                    repository.owner))
-                if target_default and target_default.visibleByUser(requester):
-                    target_path = target_default.getInternalPath()
+                default = self.repository_set.getDefaultRepository(
+                    repository.target)
+                if default is not None and default.visibleByUser(requester):
+                    target_path = default.getInternalPath()
+                else:
+                    default = self.repository_set.getDefaultRepositoryForOwner(
+                        repository.owner, repository.target)
+                    if default is not None and default.visibleByUser(requester):
+                        target_path = default.getInternalPath()
             except GitTargetError:
                 pass  # Ignore Personal repositories.
 

=== modified file 'lib/lp/code/xmlrpc/tests/test_git.py'
--- lib/lp/code/xmlrpc/tests/test_git.py	2015-04-28 04:48:55 +0000
+++ lib/lp/code/xmlrpc/tests/test_git.py	2015-04-28 10:32:26 +0000
@@ -217,13 +217,12 @@
             self.git_api.hosting_client.calls[0][0:2])
         return repository
 
-    def assertCreatesFromClone(self, requester, path, can_authenticate=False):
-        repository = self.assertCreates(requester, path, can_authenticate)
-        target_default = removeSecurityProxy(
-            self.repository_set.getDefaultRepository(repository.target))
-        self.assertIsNotNone(target_default)
-        self.assertEqual(target_default.getInternalPath(),
-                         self.git_api.hosting_client.calls[0][2])
+    def assertCreatesFromClone(self, requester, path, cloned_from,
+                               can_authenticate=False):
+        self.assertCreates(requester, path, can_authenticate)
+        self.assertEqual(
+            cloned_from.getInternalPath(),
+            self.git_api.hosting_client.calls[0][2])
 
     def test_translatePath_private_repository(self):
         requester = self.factory.makePerson()
@@ -427,17 +426,32 @@
         self.assertCreates(
             requester, u"/~%s/%s/+git/random" % (requester.name, project.name))
 
-    def test_translatePath_create_project_with_default_target(self):
-        # translatePath creates a project repository cloned from the
-        # target default if it exists.
+    def test_translatePath_create_project_clone_from_target_default(self):
+        # translatePath creates a project repository cloned from the target
+        # default if it exists.
         target = self.factory.makeProduct()
         repository = self.factory.makeGitRepository(
             owner=target.owner, target=target)
         with person_logged_in(target.owner):
             self.repository_set.setDefaultRepository(target, repository)
             self.assertCreatesFromClone(
-                target.owner, u"/~%s/%s/+git/random" % (target.owner.name,
-                                                        target.name))
+                target.owner,
+                u"/~%s/%s/+git/random" % (target.owner.name, target.name),
+                repository)
+
+    def test_translatePath_create_project_clone_from_owner_default(self):
+        # translatePath creates a project repository cloned from the owner
+        # default if it exists and the target default does not.
+        target = self.factory.makeProduct()
+        repository = self.factory.makeGitRepository(
+            owner=target.owner, target=target)
+        with person_logged_in(target.owner):
+            self.repository_set.setDefaultRepositoryForOwner(
+                target.owner, target, repository)
+            self.assertCreatesFromClone(
+                target.owner,
+                u"/~%s/%s/+git/random" % (target.owner.name, target.name),
+                repository)
 
     def test_translatePath_create_package(self):
         # translatePath creates a package repository that doesn't exist, if