Repository: repobee/repobee

Issue:
GitLabAPI.get_repo_urls ignores org_name argument
It always uses the target group's name, but supplying org_name should override it. This becomes problematic when trying to clone master repos from a master group that is separate from the target group (as is recommended).

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

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

```diff
@@            Coverage Diff             @@
##           master     #207      +/-   ##
==========================================
+ Coverage   91.48%   91.56%   +0.08%     
==========================================
  Files          19       19              
  Lines        1303     1304       +1     
  Branches      231      231              
==========================================
+ Hits         1192     1194       +2     
  Misses         90       90              
+ Partials       21       20       -1
```


| [Impacted Files](https://codecov.io/gh/repobee/repobee/pull/207?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [repobee/gitlab\_api.py](https://codecov.io/gh/repobee/repobee/pull/207/diff?src=pr&el=tree#diff-cmVwb2JlZS9naXRsYWJfYXBpLnB5) | `81.57% <100%> (+1.04%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/repobee/repobee/pull/207?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/207?src=pr&el=footer). Last update [56406c9...0aaf811](https://codecov.io/gh/repobee/repobee/pull/207?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/repobee/gitlab_api.py b/repobee/gitlab_api.py
index 66e43e4..6038109 100644
--- a/repobee/gitlab_api.py
+++ b/repobee/gitlab_api.py
@@ -294,7 +294,8 @@ class GitLabAPI(apimeta.API):
         Returns:
             a list of urls corresponding to the repo names.
         """
-        group_url = "{}/{}".format(self._base_url, self._group_name)
+        group_name = org_name if org_name else self._group_name
+        group_url = "{}/{}".format(self._base_url, group_name)
         repo_urls = (
             [
                 "{}/{}.git".format(group_url, repo_name)


Test patch:
diff --git a/tests/test_gitlab_api.py b/tests/test_gitlab_api.py
index 545c735..de395b1 100644
--- a/tests/test_gitlab_api.py
+++ b/tests/test_gitlab_api.py
@@ -332,15 +332,93 @@ class TestEnsureTeamsAndMembers:
                 assert not group.members.list()
 
 
+@pytest.fixture
+def master_repo_names():
+    return ["task-1", "task-2", "task-3"]
+
+
+class TestGetRepoUrls:
+    def test_get_master_repo_urls(self, master_repo_names):
+        """When supplied with only master_repo_names, get_repo_urls should
+        return urls for those master repos, expecting them to be in the target
+        group.
+        """
+        # arrange
+        api = repobee.gitlab_api.GitLabAPI(BASE_URL, TOKEN, TARGET_GROUP)
+        expected_urls = [
+            api._insert_auth("{}/{}/{}.git".format(BASE_URL, TARGET_GROUP, mn))
+            for mn in master_repo_names
+        ]
+        assert (
+            expected_urls
+        ), "there must be at least some urls for this test to make sense"
+
+        # act
+        actual_urls = api.get_repo_urls(master_repo_names)
+
+        # assert
+        assert sorted(actual_urls) == sorted(expected_urls)
+
+    def test_get_master_repo_urls_in_master_group(self, master_repo_names):
+        """When supplied with master_repo_names and org_name, the urls
+        generated should go to the group named org_name instead of the default
+        target group.
+        """
+        # arrange
+        master_group = "master-" + TARGET_GROUP  # guaranteed != TARGET_GROUP
+        api = repobee.gitlab_api.GitLabAPI(BASE_URL, TOKEN, TARGET_GROUP)
+        expected_urls = [
+            api._insert_auth("{}/{}/{}.git".format(BASE_URL, master_group, mn))
+            for mn in master_repo_names
+        ]
+        assert (
+            expected_urls
+        ), "there must be at least some urls for this test to make sense"
+
+        # act
+        actual_urls = api.get_repo_urls(
+            master_repo_names, org_name=master_group
+        )
+
+        # assert
+        assert sorted(actual_urls) == sorted(expected_urls)
+
+    def test_get_student_repo_urls(self, master_repo_names):
+        """When supplied with the students argument, the generated urls should
+        go to the student repos related to the supplied master repos.
+        """
+        # arrange
+        api = repobee.gitlab_api.GitLabAPI(BASE_URL, TOKEN, TARGET_GROUP)
+        expected_urls = [
+            api._insert_auth(
+                "{}/{}/{}/{}.git".format(
+                    BASE_URL,
+                    TARGET_GROUP,
+                    str(student_group),
+                    repobee.util.generate_repo_name(str(student_group), mn),
+                )
+            )
+            for student_group in constants.STUDENTS
+            for mn in master_repo_names
+        ]
+        assert (
+            expected_urls
+        ), "there must be at least some urls for this test to make sense"
+
+        # act
+        actual_urls = api.get_repo_urls(
+            master_repo_names, students=constants.STUDENTS
+        )
+
+        # assert
+        assert sorted(actual_urls) == sorted(expected_urls)
+
+
 class TestCreateRepos:
     @pytest.fixture
     def api(self, api_mock):
         yield repobee.gitlab_api.GitLabAPI(BASE_URL, TOKEN, TARGET_GROUP)
 
-    @pytest.fixture
-    def master_repo_names(self):
-        return ["task-1", "task-2", "task-3"]
-
     @pytest.fixture
     def repos(self, api, master_repo_names):
         """Setup repo tuples along with groups for the repos to be created