Repository: repobee/repobee

Issue:
Pass student usernames + master repo names to get_repo_urls
This is required for GitLab support to work, as the url to a GitLab project in a subgroup is `https://HOST/GROUP/SUBGROUP/REPO_NAME`

`GROUP` corresponds to the target organization, and SUBGROUP to the student's username.

The GitHub API can simply discard the excess information. Cant' really think of a smoother way to do it at the moment.

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/repobee/repobee/pull/186?src=pr&el=h1) Report
> Merging [#186](https://codecov.io/gh/repobee/repobee/pull/186?src=pr&el=desc) into [master](https://codecov.io/gh/repobee/repobee/commit/aa55df0f0e245e67b07938e9aac1f6299064da06?src=pr&el=desc) will **decrease** coverage by `0.17%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/repobee/repobee/pull/186/graphs/tree.svg?width=650&token=5vOKgkphZe&height=150&src=pr)](https://codecov.io/gh/repobee/repobee/pull/186?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #186      +/-   ##
==========================================
- Coverage   92.94%   92.77%   -0.17%     
==========================================
  Files          17       17              
  Lines        1134     1135       +1     
  Branches      198      198              
==========================================
- Hits         1054     1053       -1     
- Misses         65       67       +2     
  Partials       15       15
```


| [Impacted Files](https://codecov.io/gh/repobee/repobee/pull/186?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [repobee/github\_api.py](https://codecov.io/gh/repobee/repobee/pull/186/diff?src=pr&el=tree#diff-cmVwb2JlZS9naXRodWJfYXBpLnB5) | `92.27% <100%> (+0.06%)` | :arrow_up: |
| [repobee/command.py](https://codecov.io/gh/repobee/repobee/pull/186/diff?src=pr&el=tree#diff-cmVwb2JlZS9jb21tYW5kLnB5) | `95.34% <100%> (-0.03%)` | :arrow_down: |
| [repobee/cli.py](https://codecov.io/gh/repobee/repobee/pull/186/diff?src=pr&el=tree#diff-cmVwb2JlZS9jbGkucHk=) | `97.46% <0%> (-0.85%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/repobee/repobee/pull/186?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/repobee/repobee/pull/186?src=pr&el=footer). Last update [aa55df0...81ac240](https://codecov.io/gh/repobee/repobee/pull/186?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/repobee/command.py b/repobee/command.py
index 7a11982..86887b6 100644
--- a/repobee/command.py
+++ b/repobee/command.py
@@ -166,9 +166,8 @@ def update_student_repos(
         raise ValueError("master_repo_urls contains duplicates")
 
     master_repo_names = [util.repo_name(url) for url in urls]
-    student_repo_names = util.generate_repo_names(students, master_repo_names)
 
-    repo_urls = api.get_repo_urls(student_repo_names)
+    repo_urls = api.get_repo_urls(master_repo_names, students=students)
 
     with tempfile.TemporaryDirectory() as tmpdir:
         LOGGER.info("cloning into master repos ...")
@@ -356,8 +355,7 @@ def clone_repos(
         students: Student usernames.
         api: A GitHubAPI instance.
     """
-    repo_names = util.generate_repo_names(students, master_repo_names)
-    repo_urls = api.get_repo_urls(repo_names)
+    repo_urls = api.get_repo_urls(master_repo_names, students=students)
 
     LOGGER.info("cloning into student repos ...")
     git.clone(repo_urls)
@@ -365,6 +363,7 @@ def clone_repos(
     if (
         len(plug.manager.get_plugins()) > 1
     ):  # something else than the default loaded
+        repo_names = util.generate_repo_names(students, master_repo_names)
         _execute_post_clone_hooks(repo_names, api)
 
 
diff --git a/repobee/github_api.py b/repobee/github_api.py
index d16e6e1..01e8353 100644
--- a/repobee/github_api.py
+++ b/repobee/github_api.py
@@ -20,6 +20,7 @@ import github
 
 from repobee import exception
 from repobee import tuples
+from repobee import util
 
 REQUIRED_OAUTH_SCOPES = {"admin:org", "repo"}
 
@@ -327,16 +328,23 @@ class GitHubAPI:
         return [self._insert_auth(url) for url in repo_urls]
 
     def get_repo_urls(
-        self, repo_names: Iterable[str], org_name: Optional[str] = None
+        self,
+        master_repo_names: Iterable[str],
+        org_name: Optional[str] = None,
+        students: Optional[List[tuples.Group]] = None,
     ) -> List[str]:
-        """Get repo urls for all specified repo names in organization.  Assumes
+        """Get repo urls for all specified repo names in organization. Assumes
         that the repos exist, there is no guarantee that they actually do as
         checking this with the REST API takes too much time.
 
+        If the `students` argument is supplied, student repo urls are
+        computed instead of master repo urls.
+
         Args:
-            repo_names: A list of repository names.
+            master_repo_names: A list of master repository names.
             org_name: Organization in which repos are expected. Defaults to the
                 target organization of the API instance.
+            students: A list of student groups.
 
         Returns:
             a list of urls corresponding to the repo names.
@@ -347,6 +355,11 @@ class GitHubAPI:
                 if not org_name
                 else self._github.get_organization(org_name)
             )
+        repo_names = (
+            master_repo_names
+            if not students
+            else util.generate_repo_names(students, master_repo_names)
+        )
         return [
             self._insert_auth(url)
             for url in (


Test patch:
diff --git a/tests/test_command.py b/tests/test_command.py
index 9fa6929..9d09848 100644
--- a/tests/test_command.py
+++ b/tests/test_command.py
@@ -78,6 +78,17 @@ def generate_repo_url(name):
     return generate_team_repo_url(name, "d")[:-2]
 
 
+def get_repo_urls_fake(master_repo_names, org_name=None, students=None):
+    return list(
+        map(
+            generate_repo_url,
+            master_repo_names
+            if not students
+            else util.generate_repo_names(students, master_repo_names),
+        )
+    )
+
+
 MASTER_NAMES = ("week-1", "week-2", "week-3")
 MASTER_URLS = tuple(generate_repo_url(name) for name in MASTER_NAMES)
 
@@ -125,16 +136,15 @@ def _get_issues(repo_names, state="open", title_regex=""):
 def api_mock(request, mocker):
     if "noapimock" in request.keywords:
         return
-    mock = MagicMock(spec=repobee.command.GitHubAPI)
-    api_class = mocker.patch("repobee.command.GitHubAPI", autospec=True)
-    api_class.return_value = mock
 
     def url_from_repo_info(repo_info):
         return generate_repo_url(repo_info.name)
 
-    mock.get_repo_urls.side_effect = lambda repo_names: list(
-        map(generate_repo_url, repo_names)
-    )
+    mock = MagicMock(spec=repobee.command.GitHubAPI)
+    api_class = mocker.patch("repobee.command.GitHubAPI", autospec=True)
+    api_class.return_value = mock
+
+    mock.get_repo_urls.side_effect = get_repo_urls_fake
     mock.create_repos.side_effect = lambda repo_infos: list(
         map(url_from_repo_info, repo_infos)
     )
@@ -328,10 +338,6 @@ class TestUpdateStudentRepos:
             call(url, cwd=str(tmpdir)) for url in master_urls
         ]
 
-        api_mock.get_repo_urls.side_effect = lambda repo_names: list(
-            map(generate_repo_url, repo_names)
-        )
-
         command.update_student_repos(master_urls, students, api_mock)
 
         git_mock.clone_single.assert_has_calls(expected_clone_calls)
@@ -358,14 +364,13 @@ class TestUpdateStudentRepos:
             for name in [master_name, "week-3"]
         ]
 
-        fail_repo_names = [
-            util.generate_repo_name(stud, master_name) for stud in ["a", "c"]
-        ]
-        fail_repo_urls = [self.generate_url(name) for name in fail_repo_names]
-
-        api_mock.get_repo_urls.side_effect = lambda repo_names: [
-            self.generate_url(name) for name in repo_names
-        ]
+        fail_students = ["a", "c"]
+        fail_repo_names = util.generate_repo_names(
+            fail_students, [master_name]
+        )
+        fail_repo_urls = get_repo_urls_fake(
+            [master_name], students=fail_students
+        )
 
         async def raise_specific(pt):
             if pt.repo_url in fail_repo_urls:
@@ -411,10 +416,6 @@ class TestUpdateStudentRepos:
         ]
         fail_repo_urls = [self.generate_url(name) for name in fail_repo_names]
 
-        api_mock.get_repo_urls.side_effect = lambda repo_names: [
-            self.generate_url(name) for name in repo_names
-        ]
-
         async def raise_specific(pt):
             if pt.repo_url in fail_repo_urls:
                 raise exception.PushFailedError(
diff --git a/tests/test_github_api.py b/tests/test_github_api.py
index c087bff..5985226 100644
--- a/tests/test_github_api.py
+++ b/tests/test_github_api.py
@@ -471,6 +471,24 @@ class TestGetRepoUrls:
         for url in urls:
             assert "{}:{}".format(USER, TOKEN) in url
 
+    def test_with_students(self, repos, api):
+        """Test that supplying students causes student repo names to be
+        generated as the Cartesian product of the supplied repo names and the
+        students.
+        """
+        students = list(constants.STUDENTS)
+        master_repo_names = [repo.name for repo in repos]
+        expected_repo_names = util.generate_repo_names(
+            students, master_repo_names
+        )
+        # assume works correctly when called with just repo names
+        expected_urls = api.get_repo_urls(expected_repo_names)
+
+        actual_urls = api.get_repo_urls(master_repo_names, students=students)
+
+        assert len(actual_urls) == len(students) * len(master_repo_names)
+        assert sorted(expected_urls) == sorted(actual_urls)
+
 
 class TestOpenIssue:
     """Tests for open_issue."""