Repository: repobee/repobee

Issue:
Refactor all apimeta.APISpec configuration arguments to take enums
This is an important step to make supporting both GitLab and GitHub more smooth. Standout things that need refactoring are:

- [x] `ensure_teams_and_members`
    - `permission: str` -> `permission: TeamPermission`
- [x] `get_issues`
    - `state: str` -> `state: IssueState`

There may be a few others.


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

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

```diff
@@            Coverage Diff             @@
##           master     #254      +/-   ##
==========================================
- Coverage   89.45%   89.39%   -0.06%     
==========================================
  Files          19       19              
  Lines        1365     1377      +12     
  Branches      251      251              
==========================================
+ Hits         1221     1231      +10     
- Misses        123      125       +2     
  Partials       21       21
```


| [Impacted Files](https://codecov.io/gh/repobee/repobee/pull/254?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [repobee/gitlab\_api.py](https://codecov.io/gh/repobee/repobee/pull/254/diff?src=pr&el=tree#diff-cmVwb2JlZS9naXRsYWJfYXBpLnB5) | `64.86% <ø> (ø)` | :arrow_up: |
| [repobee/github\_api.py](https://codecov.io/gh/repobee/repobee/pull/254/diff?src=pr&el=tree#diff-cmVwb2JlZS9naXRodWJfYXBpLnB5) | `92.65% <100%> (+0.12%)` | :arrow_up: |
| [repobee/cli.py](https://codecov.io/gh/repobee/repobee/pull/254/diff?src=pr&el=tree#diff-cmVwb2JlZS9jbGkucHk=) | `94.5% <100%> (-0.79%)` | :arrow_down: |
| [repobee/apimeta.py](https://codecov.io/gh/repobee/repobee/pull/254/diff?src=pr&el=tree#diff-cmVwb2JlZS9hcGltZXRhLnB5) | `93.47% <100%> (+0.62%)` | :arrow_up: |

------

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


Patch:
diff --git a/repobee/apimeta.py b/repobee/apimeta.py
index 5b92c54..2c2ac49 100644
--- a/repobee/apimeta.py
+++ b/repobee/apimeta.py
@@ -19,6 +19,7 @@ NotImplementedError) for any unimplemented API methods.
 .. moduleauthor:: Simon Larsén
 """
 import inspect
+import enum
 import collections
 from typing import List, Iterable, Optional, Generator, Tuple, Mapping
 
@@ -55,6 +56,48 @@ class APIObject:
         return attr
 
 
+class _EnumAuto(collections.namedtuple("_EnumAuto", "value")):
+    """Simplistic emulation of the enum.auto() feature, as it is not available
+    in Python 3.5. Never create an instance of this class manually, always use
+    the :py:func:`build_auto` function.
+    """
+
+
+def _auto_factory_func():
+    """Return a factory function that builds _EnumAuto instances with
+    increasing values.
+    """
+    cur_value = 0
+
+    def _auto():
+        nonlocal cur_value
+        inst = _EnumAuto(value=cur_value)
+        cur_value += 1
+        return inst
+
+    return _auto
+
+
+build_auto = _auto_factory_func()
+
+
+class TeamPermission(enum.Enum):
+    """Enum specifying team permissions on creating teams. On GitHub, for
+    example, this can be e.g. `push` or `pull`.
+    """
+
+    PUSH = build_auto()
+    PULL = build_auto()
+
+
+class IssueState(enum.Enum):
+    """Enum specifying a possible issue state."""
+
+    OPEN = build_auto()
+    CLOSED = build_auto()
+    ALL = build_auto()
+
+
 def _check_name_length(name):
     """Check that a Team/Repository name does not exceed the maximum GitHub
     allows (100 characters)
@@ -147,7 +190,9 @@ class APISpec:
         _not_implemented()
 
     def ensure_teams_and_members(
-        self, teams: Iterable[Team], permission: str
+        self,
+        teams: Iterable[Team],
+        permission: TeamPermission = TeamPermission.PUSH,
     ) -> List[Team]:
         """Ensure that the teams exist, and that their members are added to the
         teams.
@@ -159,9 +204,7 @@ class APISpec:
         Args:
             teams: A list of teams specifying student groups.
             permission: The permission these teams (or members of them) should
-                be given in regards to repositories. For example, on GitHub,
-                the permission should be "push", and on GitLab, it should be
-                developer (30).
+                be given in regards to associated repositories.
         Returns:
             A list of Team API objects of the teams provided to the function,
             both those that were created and those that already existed.
@@ -219,7 +262,7 @@ class APISpec:
     def get_issues(
         self,
         repo_names: Iterable[str],
-        state: str = "open",
+        state: IssueState = IssueState.OPEN,
         title_regex: str = "",
     ) -> Generator[Tuple[str, Generator[Issue, None, None]], None, None]:
         """Get all issues for the repos in repo_names an return a generator
@@ -228,8 +271,7 @@ class APISpec:
 
         Args:
             repo_names: An iterable of repo names.
-            state: Specifying the state of the issue ('open', 'closed' or
-            'all'). Defaults to 'open'.
+            state: Specifies the state of the issue.
             title_regex: If specified, only issues matching this regex are
             returned. Defaults to the empty string (which matches anything).
         Returns:
diff --git a/repobee/cli.py b/repobee/cli.py
index 75a68a2..0d148bd 100644
--- a/repobee/cli.py
+++ b/repobee/cli.py
@@ -498,23 +498,23 @@ def _add_issue_parsers(base_parsers, subparsers):
         help="List open issues (default).",
         action="store_const",
         dest="state",
-        const="open",
+        const=apimeta.IssueState.OPEN,
     )
     state.add_argument(
         "--closed",
         help="List closed issues.",
         action="store_const",
         dest="state",
-        const="closed",
+        const=apimeta.IssueState.CLOSED,
     )
     state.add_argument(
         "--all",
         help="List all issues (open and closed).",
         action="store_const",
         dest="state",
-        const="all",
+        const=apimeta.IssueState.ALL,
     )
-    list_parser.set_defaults(state="open")
+    list_parser.set_defaults(state=apimeta.IssueState.OPEN)
 
 
 class _OrderedFormatter(argparse.HelpFormatter):
diff --git a/repobee/github_api.py b/repobee/github_api.py
index c96557a..c90868e 100644
--- a/repobee/github_api.py
+++ b/repobee/github_api.py
@@ -29,6 +29,17 @@ ISSUE_GENERATOR = Generator[apimeta.Issue, None, None]
 
 LOGGER = daiquiri.getLogger(__file__)
 
+_TEAM_PERMISSION_MAPPING = {
+    apimeta.TeamPermission.PUSH: "push",
+    apimeta.TeamPermission.PULL: "pull",
+}
+_ISSUE_STATE_MAPPING = {
+    apimeta.IssueState.OPEN: "open",
+    apimeta.IssueState.CLOSED: "closed",
+    apimeta.IssueState.ALL: "all",
+}
+
+
 # classes used internally in this module
 _Team = github.Team.Team
 _User = github.NamedUser.NamedUser
@@ -208,13 +219,16 @@ class GitHubAPI(apimeta.API):
         return existing_users
 
     def ensure_teams_and_members(
-        self, teams: Iterable[apimeta.Team], permission: str = "push"
+        self,
+        teams: Iterable[apimeta.Team],
+        permission: apimeta.TeamPermission = apimeta.TeamPermission.PUSH,
     ) -> List[apimeta.Team]:
         """See :py:func:`repobee.apimeta.APISpec.ensure_teams_and_members`."""
+        raw_permission = _TEAM_PERMISSION_MAPPING[permission]
         member_lists = {team.name: team.members for team in teams}
         raw_teams = self._ensure_teams_exist(
             [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]]:
@@ -379,11 +393,12 @@ class GitHubAPI(apimeta.API):
     def get_issues(
         self,
         repo_names: Iterable[str],
-        state: str = "open",
+        state: apimeta.IssueState = apimeta.IssueState.OPEN,
         title_regex: str = "",
     ) -> Generator[Tuple[str, ISSUE_GENERATOR], None, None]:
         """See :py:func:`repobee.apimeta.APISpec.get_issues`."""
         repos = self._get_repos_by_name(repo_names)
+        raw_state = _ISSUE_STATE_MAPPING[state]
 
         with _try_api_request():
             name_issues_pairs = (
@@ -398,7 +413,7 @@ class GitHubAPI(apimeta.API):
                             author=issue.user.login,
                             implementation=issue,
                         )
-                        for issue in repo.get_issues(state=state)
+                        for issue in repo.get_issues(state=raw_state)
                         if re.match(title_regex or "", issue.title)
                     ),
                 )
diff --git a/repobee/gitlab_api.py b/repobee/gitlab_api.py
index 6c9ac1e..7cd827d 100644
--- a/repobee/gitlab_api.py
+++ b/repobee/gitlab_api.py
@@ -109,7 +109,9 @@ class GitLabAPI(apimeta.API):
             )
 
     def ensure_teams_and_members(
-        self, teams: Iterable[apimeta.Team], permission: str = "push"
+        self,
+        teams: Iterable[apimeta.Team],
+        permission: apimeta.TeamPermission = apimeta.TeamPermission.PUSH,
     ) -> List[apimeta.Team]:
         """See :py:func:`repobee.apimeta.APISpec.ensure_teams_and_members`."""
         member_lists = {team.name: team.members for team in teams}


Test patch:
diff --git a/tests/unit_tests/test_cli.py b/tests/unit_tests/test_cli.py
index 0fe4ec9..8083086 100644
--- a/tests/unit_tests/test_cli.py
+++ b/tests/unit_tests/test_cli.py
@@ -42,7 +42,7 @@ VALID_PARSED_ARGS = dict(
     issue=ISSUE,
     title_regex="some regex",
     traceback=False,
-    state="open",
+    state=apimeta.IssueState.OPEN,
     show_body=True,
     author=None,
     token=TOKEN,
diff --git a/tests/unit_tests/test_command.py b/tests/unit_tests/test_command.py
index 6334359..71ad428 100644
--- a/tests/unit_tests/test_command.py
+++ b/tests/unit_tests/test_command.py
@@ -5,6 +5,7 @@ from unittest.mock import patch, MagicMock, call, ANY, PropertyMock
 import pytest
 
 import repobee
+import repobee.github_api
 from repobee import command
 from repobee import git
 from repobee import util
@@ -109,18 +110,18 @@ def git_mock(request, mocker):
     return git_mock
 
 
-def _get_issues(repo_names, state="open", title_regex=""):
+def _get_issues(repo_names, state=apimeta.IssueState.OPEN, title_regex=""):
     """Bogus version of GitHubAPI.get_issues"""
     for repo_name in repo_names:
         if repo_name == STUDENT_REPO_NAMES[-2]:
             # repo without issues
             yield repo_name, iter([])
         elif repo_name in STUDENT_REPO_NAMES:
-            if state == "open":
+            if state == apimeta.IssueState.OPEN:
                 issues = iter(OPEN_ISSUES)
-            elif state == "closed":
+            elif state == apimeta.IssueState.CLOSED:
                 issues = iter(CLOSED_ISSUES)
-            elif state == "all":
+            elif state == apimeta.IssueState.ALL:
                 issues = iter(OPEN_ISSUES + CLOSED_ISSUES)
             else:
                 raise ValueError("Unexpected value for 'state': ", state)
@@ -609,7 +610,14 @@ class TestListIssues:
     it is only tested for stability.
     """
 
-    @pytest.mark.parametrize("state", ("open", "closed", "all"))
+    @pytest.mark.parametrize(
+        "state",
+        (
+            apimeta.IssueState.OPEN,
+            apimeta.IssueState.CLOSED,
+            apimeta.IssueState.ALL,
+        ),
+    )
     @pytest.mark.parametrize("regex", ("", r"^.*$"))
     @pytest.mark.parametrize("show_body", (True, False))
     @pytest.mark.parametrize("author", (None, "slarse"))
diff --git a/tests/unit_tests/test_github_api.py b/tests/unit_tests/test_github_api.py
index 5806fe0..ca68248 100644
--- a/tests/unit_tests/test_github_api.py
+++ b/tests/unit_tests/test_github_api.py
@@ -637,7 +637,9 @@ class TestGetIssues:
     def test_get_all_open_issues(self, repos, issues, api):
         repo_names = [repo.name for repo in repos]
 
-        name_issues_pairs = api.get_issues(repo_names, state="open")
+        name_issues_pairs = api.get_issues(
+            repo_names, state=apimeta.IssueState.OPEN
+        )
 
         found_repos = []
         for repo_name, issue_gen in name_issues_pairs:
@@ -652,9 +654,9 @@ class TestGetIssues:
     def test_get_all_closed_issues(self, repos, issues, api):
         repo_names = [repo.name for repo in repos]
 
-        print(api.get_issues(repo_names, state="closed"))
-
-        name_issues_pairs = api.get_issues(repo_names, state="closed")
+        name_issues_pairs = api.get_issues(
+            repo_names, state=apimeta.IssueState.CLOSED
+        )
 
         found_repos = []
         for repo_name, issue_gen in name_issues_pairs:
@@ -673,7 +675,9 @@ class TestGetIssues:
         repo_names = [repo.name for repo in repos] + [non_existing]
         random.shuffle(repo_names)
 
-        name_issues_pairs = api.get_issues(repo_names, state="open")
+        name_issues_pairs = api.get_issues(
+            repo_names, state=apimeta.IssueState.OPEN
+        )
 
         found_repos = []
         for repo_name, issue_gen in name_issues_pairs:
@@ -693,7 +697,7 @@ class TestGetIssues:
         regex = "^{}$".format(sought_issue.title)
 
         name_issues_pairs = api.get_issues(
-            repo_names, state="open", title_regex=regex
+            repo_names, state=apimeta.IssueState.OPEN, title_regex=regex
         )
 
         found_repos = []