Repository: repobee/repobee

Issue:
GitLabAPI.ensure_teams_and_members does not decode permission enum
It just passes the enum along. Not sure what effect that has.

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

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

```diff
@@            Coverage Diff            @@
##           master     #265     +/-   ##
=========================================
- Coverage   89.38%   89.18%   -0.2%     
=========================================
  Files          19       19             
  Lines        1394     1396      +2     
  Branches      253      253             
=========================================
- Hits         1246     1245      -1     
- Misses        126      129      +3     
  Partials       22       22
```


| [Impacted Files](https://codecov.io/gh/repobee/repobee/pull/265?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [repobee/gitlab\_api.py](https://codecov.io/gh/repobee/repobee/pull/265/diff?src=pr&el=tree#diff-cmVwb2JlZS9naXRsYWJfYXBpLnB5) | `63.69% <100%> (-0.18%)` | :arrow_down: |
| [repobee/cli.py](https://codecov.io/gh/repobee/repobee/pull/265/diff?src=pr&el=tree#diff-cmVwb2JlZS9jbGkucHk=) | `94.5% <0%> (-0.79%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/repobee/repobee/pull/265?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/265?src=pr&el=footer). Last update [ada98ee...865502c](https://codecov.io/gh/repobee/repobee/pull/265?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 0c71ad6..2be0e2f 100644
--- a/repobee/gitlab_api.py
+++ b/repobee/gitlab_api.py
@@ -36,6 +36,11 @@ _ISSUE_STATE_MAPPING = {
     apimeta.IssueState.CLOSED: "closed",
     apimeta.IssueState.ALL: "all",
 }
+# see https://docs.gitlab.com/ee/user/permissions.html for permission details
+_TEAM_PERMISSION_MAPPING = {
+    apimeta.TeamPermission.PULL: gitlab.GUEST_ACCESS,
+    apimeta.TeamPermission.PUSH: gitlab.DEVELOPER_ACCESS,
+}
 
 
 @contextlib.contextmanager
@@ -126,13 +131,16 @@ class GitLabAPI(apimeta.API):
     ) -> List[apimeta.Team]:
         """See :py:func:`repobee.apimeta.APISpec.ensure_teams_and_members`."""
         member_lists = {team.name: team.members for team in teams}
+        raw_permission = _TEAM_PERMISSION_MAPPING[permission]
         raw_teams = self._ensure_teams_exist(
             [str(team_name) for team_name in member_lists.keys()],
-            permission=permission,
+            permission=raw_permission,
         )
 
         for team in [team for team in raw_teams if member_lists[team.name]]:
-            self._ensure_members_in_team(team, member_lists[team.name])
+            self._ensure_members_in_team(
+                team, member_lists[team.name], raw_permission
+            )
 
         return [
             apimeta.Team(
@@ -144,7 +152,9 @@ class GitLabAPI(apimeta.API):
             for t in raw_teams
         ]
 
-    def _ensure_members_in_team(self, team, members: Iterable[str]):
+    def _ensure_members_in_team(
+        self, team, members: Iterable[str], permission: int
+    ):
         """Add all of the users in ``members`` to a team. Skips any users that
         don't exist, or are already in the team.
         """
@@ -167,7 +177,7 @@ class GitLabAPI(apimeta.API):
                     ", ".join(existing_members), team.name
                 )
             )
-        self._add_to_team(missing_members, team)
+        self._add_to_team(missing_members, team, permission)
 
     def _get_organization(self, org_name):
         return [
@@ -191,7 +201,7 @@ class GitLabAPI(apimeta.API):
             for t in self._gitlab.groups.list(id=self._group.id)
         ]
 
-    def _add_to_team(self, members: Iterable[str], team):
+    def _add_to_team(self, members: Iterable[str], team, permission):
         """Add members to a team.
 
         Args:
@@ -202,10 +212,7 @@ class GitLabAPI(apimeta.API):
             users = self._get_users(members)
             for user in users:
                 team.members.create(
-                    {
-                        "user_id": user.id,
-                        "access_level": gitlab.DEVELOPER_ACCESS,
-                    }
+                    {"user_id": user.id, "access_level": permission}
                 )
 
     def _get_users(self, usernames):
@@ -218,7 +225,7 @@ class GitLabAPI(apimeta.API):
         return users
 
     def _ensure_teams_exist(
-        self, team_names: Iterable[str], permission: str = "push"
+        self, team_names: Iterable[str], permission
     ) -> List[apimeta.Team]:
         """Create any teams that do not yet exist.
 


Test patch:
diff --git a/tests/unit_tests/test_gitlab_api.py b/tests/unit_tests/test_gitlab_api.py
index 90fadc9..1f37ebe 100644
--- a/tests/unit_tests/test_gitlab_api.py
+++ b/tests/unit_tests/test_gitlab_api.py
@@ -24,6 +24,14 @@ class Group:
         )
         self._member_list = []
         self._users_read_only = users_read_only
+        # the group owner is always added to the group with max access level
+        owner_id = [
+            uid
+            for uid, user in users_read_only.items()
+            if user.username == constants.USER
+        ][0]
+        owner_data = {"user_id": owner_id, "access_level": gitlab.OWNER_ACCESS}
+        self._create_member(owner_data)
 
     def _create_member(self, data):
         user_id = data["user_id"]
@@ -79,14 +87,14 @@ class GitLabMock:
     _Projects = namedtuple("_Projects", "create get".split())
 
     def __init__(self, url, private_token, ssl_verify):
+        self._users = {
+            id: self._User(id=id, username=str(grp))
+            for id, grp in enumerate(constants.STUDENTS + (constants.USER,))
+        }
         self._base_url = url
         self._private_token = private_token
         self._groups = {}
         self._projects = {}
-        self._users = {
-            id: self._User(id=id, username=str(grp))
-            for id, grp in enumerate(constants.STUDENTS)
-        }
         self._id = len(self._users)
         self._create_group({"name": TARGET_GROUP, "path": TARGET_GROUP})
 
@@ -267,7 +275,7 @@ class TestEnsureTeamsAndMembers:
         )
         for team in actual_teams:
             if team.name != TARGET_GROUP:
-                assert team.members == [team.name]
+                assert team.members == [constants.USER, team.name]
             else:
                 assert not team.members
 
@@ -282,7 +290,14 @@ class TestEnsureTeamsAndMembers:
                 constants.STUDENTS[num_students // 2 :],
             )
         ]
-        expected_teams = list(teams)
+        expected_teams = [
+            repobee.apimeta.Team(
+                members=[constants.USER] + t.members,
+                # the owner should not be included in the generated name
+                name=repobee.apimeta.Team(members=t.members).name,
+            )
+            for t in teams
+        ]
 
         # act
         api.ensure_teams_and_members(teams)
@@ -320,10 +335,32 @@ class TestEnsureTeamsAndMembers:
         )
         for team in actual_teams:
             if team.name != TARGET_GROUP:
-                assert team.members == [team.name]
+                assert team.members == [constants.USER, team.name]
             else:
                 assert not team.members
 
+    def test_respects_permission(self):
+        """Test that the permission is correctly decoded into a GitLab-specific
+        access level.
+        """
+        api = repobee.gitlab_api.GitLabAPI(BASE_URL, TOKEN, TARGET_GROUP)
+
+        api.ensure_teams_and_members(
+            constants.STUDENTS, permission=repobee.apimeta.TeamPermission.PULL
+        )
+
+        actual_teams = api.get_teams()
+        member_access_levels = [
+            member.access_level
+            for team in actual_teams
+            for member in team.implementation.members.list()
+            if member.username != constants.USER
+        ]
+
+        assert member_access_levels
+        for access_level in member_access_levels:
+            assert access_level == gitlab.GUEST_ACCESS
+
 
 @pytest.fixture
 def master_repo_names():