bug: fix private repo listing for a user

ghcloneall.py

@@ -425,27 +425,38 @@ def list_gists(self, user, pattern=None):
     def list_repos(self, user=None, organization=None, pattern=None,
                    include_archived=False, include_forks=False,
                    include_private=True, include_disabled=True):
+
+        # User repositories default to sort=full_name, org repositories default
+        # to sort=created.  In theory we don't care because we will sort the
+        # list ourselves, but in the future I may want to start cloning in
+        # parallel with the paginated fetching.  This requires the sorting to
+        # happen before pagination, i.e. on the server side, as I want to
+        # process the repositories alphabetically (both for aesthetic reasons,
+        # and in order for --start-from to be useful).
+
         if organization and not user:
             owner = organization
-            list_url = 'https://api.github.com/orgs/{}/repos'.format(owner)
+            list_url = ('https://api.github.com/orgs/{}/repos'
+                        '?sort=full_name').format(
+                            owner)
         elif user and not organization:
             owner = user
-            list_url = 'https://api.github.com/users/{}/repos'.format(owner)
+            if include_private:
+                # users/$name/repos does not include private repos, so
+                # we have to query for the repos owned by the current
+                # user instead.  This only works if the current token
+                # is associated with that user.
+                list_url = ('https://api.github.com/user/repos'
+                            '?affiliation=owner&sort=full_name')
+            else:
+                list_url = ('https://api.github.com/users/{}/repos'
+                            '?sort=full_name').format(owner)
         else:
             raise ValueError('specify either user or organization, not both')
 
         message = "Fetching list of {}'s repositories from GitHub...".format(
             owner)
 
-        # User repositories default to sort=full_name, org repositories default
-        # to sort=created.  In theory we don't care because we will sort the
-        # list ourselves, but in the future I may want to start cloning in
-        # parallel with the paginated fetching.  This requires the sorting to
-        # happen before pagination, i.e. on the server side, as I want to
-        # process the repositories alphabetically (both for aesthetic reasons,
-        # and in order for --start-from to be useful).
-        list_url += '?sort=full_name'
-
         repos = self.get_github_list(list_url, message)
         if not include_archived:
             repos = (r for r in repos if not r['archived'])

tests.py

@@ -101,10 +101,15 @@ def mock_config_filename(monkeypatch):
 
 
 def make_page_url(url, page, extra):
+    # Some of the incoming test URLs already have query args. If so,
+    # append the page arguments using the appropriate separator.
+    sep = '?'
+    if '?' in url:
+        sep = '&'
     if page == 1:
-        return '%s?%sper_page=100' % (url, extra)
+        return '%s%s%sper_page=100' % (url, sep, extra)
     else:
-        return '%s?%spage=%d&per_page=100' % (url, extra, page)
+        return '%s%s%spage=%d&per_page=100' % (url, sep, extra, page)
 
 
 def mock_multi_page_api_responses(url, pages, extra='sort=full_name&'):
@@ -759,7 +764,7 @@ def Repo(name, **kwargs):
 
 def test_RepoWrangler_list_repos_for_user(mock_requests_get):
     mock_requests_get.update(mock_multi_page_api_responses(
-        url='https://api.github.com/users/test_user/repos',
+        url='https://api.github.com/user/repos?affiliation=owner',
         pages=[
             [
                 repo('xyzzy'),
@@ -793,7 +798,7 @@ def test_RepoWrangler_list_repos_for_org(mock_requests_get):
 
 def test_RepoWrangler_list_repos_filter_by_name(mock_requests_get):
     mock_requests_get.update(mock_multi_page_api_responses(
-        url='https://api.github.com/users/test_user/repos',
+        url='https://api.github.com/user/repos?affiliation=owner',
         pages=[
             [
                 repo('xyzzy'),
@@ -810,7 +815,7 @@ def test_RepoWrangler_list_repos_filter_by_name(mock_requests_get):
 
 def test_RepoWrangler_list_repos_filter_by_status(mock_requests_get):
     mock_requests_get.update(mock_multi_page_api_responses(
-        url='https://api.github.com/users/test_user/repos',
+        url='https://api.github.com/user/repos?affiliation=owner',
         pages=[
             [
                 repo('a', archived=True),
@@ -847,15 +852,56 @@ def test_RepoWrangler_list_repos_filter_by_status(mock_requests_get):
         Repo('c'),
         Repo('p', private=True),
     ]
+
+
+def test_RepoWrangler_list_repos_no_private(mock_requests_get):
+    mock_requests_get.update(mock_multi_page_api_responses(
+        url='https://api.github.com/users/test_user/repos',
+        pages=[
+            [
+                repo('a', archived=True),
+                repo('f', fork=True),
+                repo('p', private=True),
+                repo('d', disabled=True),
+                repo('c'),
+            ],
+        ],
+    ))
+    wrangler = ghcloneall.RepoWrangler()
+    result = wrangler.list_repos(user='test_user', include_private=False)
+    assert result == [
+        Repo('c'),
+        Repo('d', disabled=True),
+    ]
+    result = wrangler.list_repos(user='test_user', include_private=False,
+                                 include_archived=True)
+    assert result == [
+        Repo('a', archived=True),
+        Repo('c'),
+        Repo('d', disabled=True),
+    ]
+    result = wrangler.list_repos(user='test_user', include_private=False,
+                                 include_forks=True)
+    assert result == [
+        Repo('c'),
+        Repo('d', disabled=True),
+        Repo('f', fork=True),
+    ]
+    result = wrangler.list_repos(user='test_user', include_private=False,
+                                 include_disabled=False)
+    assert result == [
+        Repo('c'),
+    ]
     result = wrangler.list_repos(user='test_user', include_private=False)
     assert result == [
         Repo('c'),
+        Repo('d', disabled=True),
     ]
 
 
 def test_RepoWrangler_list_repos_progress_bar(mock_requests_get):
     mock_requests_get.update(mock_multi_page_api_responses(
-        url='https://api.github.com/users/test_user/repos',
+        url='https://api.github.com/user/repos?affiliation=owner',
         pages=[
             [
                 repo('xyzzy'),
@@ -1332,6 +1378,19 @@ def test_main_run_error_handling(monkeypatch, capsys):
     monkeypatch.setattr(sys, 'argv', [
         'ghcloneall', '--user', 'mgedmin',
     ])
+    with pytest.raises(SystemExit) as ctx:
+        ghcloneall.main()
+    assert str(ctx.value) == (
+        'Failed to fetch https://api.github.com/user/repos?affiliation=owner'
+        '&sort=full_name&per_page=100:\n'
+        'not found'
+    )
+
+
+def test_main_run_error_handling_no_private(monkeypatch, capsys):
+    monkeypatch.setattr(sys, 'argv', [
+        'ghcloneall', '--user', 'mgedmin', '--exclude-private',
+    ])
     with pytest.raises(SystemExit) as ctx:
         ghcloneall.main()
     assert str(ctx.value) == (
@@ -1346,7 +1405,7 @@ def test_main_run(monkeypatch, mock_requests_get, capsys):
         'ghcloneall', '--user', 'mgedmin', '--concurrency=1',
     ])
     mock_requests_get.update(mock_multi_page_api_responses(
-        url='https://api.github.com/users/mgedmin/repos',
+        url='https://api.github.com/user/repos?affiliation=owner',
         pages=[
             [
                 repo('ghcloneall'),
@@ -1369,7 +1428,7 @@ def test_main_run_start_from(monkeypatch, mock_requests_get, capsys):
         'ghcloneall', '--user', 'mgedmin', '--start-from', 'x',
     ])
     mock_requests_get.update(mock_multi_page_api_responses(
-        url='https://api.github.com/users/mgedmin/repos',
+        url='https://api.github.com/user/repos?affiliation=owner',
         pages=[
             [
                 repo('ghcloneall'),