Repository: repobee/repobee

Issue:
Support peer review for group assignments
Related to #157 

Peer review commands only support single students at the moment. Supporting groups requires a change in the plugin hook for review allocations (should take Group tuples instead of str).

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

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

```diff
@@            Coverage Diff            @@
##           master    #316      +/-   ##
=========================================
+ Coverage   89.81%   89.9%   +0.09%     
=========================================
  Files          20      20              
  Lines        1433    1436       +3     
  Branches      262     260       -2     
=========================================
+ Hits         1287    1291       +4     
+ Misses        123     121       -2     
- Partials       23      24       +1
```


| [Impacted Files](https://codecov.io/gh/repobee/repobee/pull/316?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/\_repobee/cli.py](https://codecov.io/gh/repobee/repobee/pull/316/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2NsaS5weQ==) | `99.36% <100%> (+1.25%)` | :arrow_up: |
| [src/\_repobee/command.py](https://codecov.io/gh/repobee/repobee/pull/316/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2NvbW1hbmQucHk=) | `92.85% <100%> (-0.66%)` | :arrow_down: |
| [src/\_repobee/formatters.py](https://codecov.io/gh/repobee/repobee/pull/316/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2Zvcm1hdHRlcnMucHk=) | `91.66% <100%> (+0.17%)` | :arrow_up: |
| [src/\_repobee/ext/github.py](https://codecov.io/gh/repobee/repobee/pull/316/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2V4dC9naXRodWIucHk=) | `91.28% <73.33%> (-0.51%)` | :arrow_down: |

------

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


Patch:
diff --git a/docs/code.rst b/docs/code.rst
index 340a594..9b41e87 100644
--- a/docs/code.rst
+++ b/docs/code.rst
@@ -44,15 +44,6 @@ util
     :members:
 
 
-API-related modules
-===================
-
-apimeta
--------
-
-.. automodule:: _repobee.apimeta
-   :members:
-
 Core plugins
 ============
 
@@ -97,3 +88,9 @@ pylint
 
 .. automodule:: _repobee.ext.pylint
     :members:
+
+config-wizard
+-------------
+
+.. automodule:: _repobee.ext.configwizard
+   :members:
diff --git a/src/_repobee/cli.py b/src/_repobee/cli.py
index e3d234f..29a7f16 100644
--- a/src/_repobee/cli.py
+++ b/src/_repobee/cli.py
@@ -179,16 +179,6 @@ def parse_args(
     master_urls = _repo_names_to_urls(master_names, master_org_name, api)
     assert master_urls and master_names
 
-    groups = _extract_groups(args)
-    if subparser in [
-        ASSIGN_REVIEWS_PARSER,
-        CHECK_REVIEW_PROGRESS_PARSER,
-        ASSIGN_REVIEWS_PARSER,
-    ] and any([len(g.members) > 1 for g in groups]):
-        raise exception.ParseError(
-            "review commands do not currently support groups of students"
-        )
-
     args_dict = vars(args)
     args_dict.setdefault("master_org_name", None)
     args_dict.setdefault("title_regex", None)
@@ -196,11 +186,10 @@ def parse_args(
     args_dict.setdefault("show_body", None)
     args_dict.setdefault("author", None)
     args_dict.setdefault("num_reviews", None)
-
+    args_dict["students"] = _extract_groups(args)
     args_dict["issue"] = (
         util.read_issue(args.issue) if "issue" in args and args.issue else None
     )
-    args_dict["students"] = _extract_groups(args)
     args_dict["master_repo_urls"] = master_urls
     args_dict["master_repo_names"] = master_names
     args_dict["token"] = token
diff --git a/src/_repobee/command.py b/src/_repobee/command.py
index 2ac4e7e..32851f7 100644
--- a/src/_repobee/command.py
+++ b/src/_repobee/command.py
@@ -483,10 +483,6 @@ def assign_peer_reviews(
         api: An implementation of :py:class:`apimeta.API` used to interface
             with the platform (e.g. GitHub or GitLab) instance.
     """
-    # currently only supports single student teams
-    # TODO support groups of students
-    assert all(map(lambda g: len(g.members) == 1, teams))
-
     for master_name in master_repo_names:
         allocations = plug.manager.hook.generate_review_allocations(
             teams=teams, num_reviews=num_reviews
@@ -511,7 +507,6 @@ def assign_peer_reviews(
         api.ensure_teams_and_members(
             review_teams, permission=plug.TeamPermission.PULL
         )
-        # TODO finish up here, this is entirely broken!
         api.add_repos_to_review_teams(
             {
                 review_team.name: [
@@ -535,7 +530,7 @@ def purge_review_teams(
 
     Args:
         master_repo_names: Names of master repos.
-        students: An iterable of student GitHub usernames.
+        students: An iterble of student teams.
         api: An implementation of :py:class:`apimeta.API` used to interface
             with the platform (e.g. GitHub or GitLab) instance.
     """
@@ -549,39 +544,33 @@ def purge_review_teams(
 
 def check_peer_review_progress(
     master_repo_names: Iterable[str],
-    students: Iterable[plug.Team],
+    teams: Iterable[plug.Team],
     title_regex: str,
     num_reviews: int,
     api: plug.API,
 ) -> None:
-    """Check which students have opened peer review issues in their allotted
+    """Check which teams have opened peer review issues in their allotted
     review repos
 
     Args:
         master_repo_names: Names of master repos.
-        students: An iterable of student GitHub usernames.
+        teams: An iterable of student teams.
         title_regex: A regex to match against issue titles.
         num_reviews: Amount of reviews each student is expected to have made.
         api: An implementation of :py:class:`apimeta.API` used to interface
             with the platform (e.g. GitHub or GitLab) instance.
 
     """
-    # TODO support groups
-    assert all(map(lambda g: len(g.members) == 1, students))
-    single_students = [g.members[0] for g in students]
-
     review_team_names = [
-        util.generate_review_team_name(student, master_name)
-        for student in single_students
+        util.generate_review_team_name(team, master_name)
+        for team in teams
         for master_name in master_repo_names
     ]
-    reviews = api.get_review_progress(
-        review_team_names, single_students, title_regex
-    )
+    reviews = api.get_review_progress(review_team_names, teams, title_regex)
 
     LOGGER.info(
         formatters.format_peer_review_progress_output(
-            reviews, single_students, num_reviews
+            reviews, teams, num_reviews
         )
     )
 
diff --git a/src/_repobee/ext/github.py b/src/_repobee/ext/github.py
index 0565aca..bd06984 100644
--- a/src/_repobee/ext/github.py
+++ b/src/_repobee/ext/github.py
@@ -491,16 +491,19 @@ class GitHubAPI(apimeta.API):
     ) -> Mapping[str, List[tuples.Review]]:
         """See :py:func:`_repobee.apimeta.APISpec.get_review_progress`."""
         reviews = collections.defaultdict(list)
-        review_teams = self._get_teams_in(review_team_names)
-        for review_team in review_teams:
+        review_team_impls = self._get_teams_in(review_team_names)
+        for review_team_impl in review_team_impls:
             with _try_api_request():
-                LOGGER.info("Processing {}".format(review_team.name))
-                reviewers = set(m.login for m in review_team.get_members())
-                repos = list(review_team.get_repos())
+                LOGGER.info("Processing {}".format(review_team_impl.name))
+                reviewers = set(
+                    m.login for m in review_team_impl.get_members()
+                )
+                review_teams = self._extract_review_teams(teams, reviewers)
+                repos = list(review_team_impl.get_repos())
                 if len(repos) != 1:
                     LOGGER.warning(
                         "Expected {} to have 1 associated repo, found {}."
-                        "Skipping...".format(review_team.name, len(repos))
+                        "Skipping...".format(review_team_impl.name, len(repos))
                     )
                     continue
 
@@ -511,16 +514,29 @@ class GitHubAPI(apimeta.API):
                     if re.match(title_regex, issue.title)
                 }
 
-                for reviewer in reviewers:
-                    reviews[reviewer].append(
+                for team in review_teams:
+                    reviews[str(team)].append(
                         tuples.Review(
                             repo=repo.name,
-                            done=reviewer in review_issue_authors,
+                            done=any(
+                                map(
+                                    review_issue_authors.__contains__,
+                                    team.members,
+                                )
+                            ),
                         )
                     )
 
         return reviews
 
+    def _extract_review_teams(self, teams, reviewers):
+        review_teams = []
+        for team in teams:
+            if any(map(team.members.__contains__, reviewers)):
+                review_teams.append(team)
+        print(teams, reviewers, review_teams)
+        return review_teams
+
     def _add_repos_to_teams(
         self, team_to_repos: Mapping[str, Iterable[str]]
     ) -> Generator[
diff --git a/src/_repobee/formatters.py b/src/_repobee/formatters.py
index 9752a0a..f14bd51 100644
--- a/src/_repobee/formatters.py
+++ b/src/_repobee/formatters.py
@@ -21,7 +21,7 @@ LOGGER = daiquiri.getLogger(__name__)
 
 def format_peer_review_progress_output(
     reviews: Mapping[str, List[tuples.Review]],
-    students: List[str],
+    teams: List[str],
     num_reviews: int,
 ):
     # can't use tabs for spacing as they are not background colored in output
@@ -34,13 +34,12 @@ def format_peer_review_progress_output(
         ),
     ]
     even = False
-    for reviewer in students:
+    print(reviews)
+    for review_team in teams:
         even = not even
-        review_list = reviews[reviewer]
+        review_list = reviews[str(review_team)]
         output.append(
-            _format_reviewer(
-                reviewer, review_list, students, num_reviews, even
-            )
+            _format_reviewer(review_team, review_list, num_reviews, even)
         )
     return os.linesep.join(output)
 
@@ -53,7 +52,6 @@ def _format_row(items: List[str]) -> str:
 def _format_reviewer(
     reviewer: str,
     review_list: List[tuples.Review],
-    students: List[str],
     num_reviews: bool,
     even: bool,
 ):


Test patch:
diff --git a/tests/unit_tests/plugin_tests/test_github.py b/tests/unit_tests/plugin_tests/test_github.py
index d6273d2..d0315aa 100644
--- a/tests/unit_tests/plugin_tests/test_github.py
+++ b/tests/unit_tests/plugin_tests/test_github.py
@@ -4,12 +4,14 @@ import pytest
 from unittest.mock import MagicMock, PropertyMock, call
 
 import github
+
+import repobee_plug as plug
+
 import _repobee
 import _repobee.ext
 import _repobee.ext.github
 from _repobee import util
 from _repobee import exception
-from repobee_plug import apimeta
 from _repobee.ext.github import REQUIRED_OAUTH_SCOPES
 
 import constants
@@ -45,23 +47,23 @@ from_magic_mock_issue = functions.from_magic_mock_issue
 
 User = constants.User
 
-CLOSE_ISSUE = apimeta.Issue(
+CLOSE_ISSUE = plug.Issue(
     "close this issue", "This is a body", 3, random_date(), "slarse"
 )
-DONT_CLOSE_ISSUE = apimeta.Issue(
+DONT_CLOSE_ISSUE = plug.Issue(
     "Don't close this issue", "Another body", 4, random_date(), "glassey"
 )
 OPEN_ISSUES = [CLOSE_ISSUE, DONT_CLOSE_ISSUE]
 
 CLOSED_ISSUES = [
-    apimeta.Issue(
+    plug.Issue(
         "This is a closed issue",
         "With an uninteresting body",
         1,
         random_date(),
         "tmore",
     ),
-    apimeta.Issue(
+    plug.Issue(
         "Yet another closed issue",
         "Even less interesting body",
         2,
@@ -84,17 +86,24 @@ def raise_401(*args, **kwargs):
 
 
 @pytest.fixture
-def review_students():
-    return ["ham", "spam", "bacon", "eggs"]
+def review_student_teams():
+    return [
+        plug.Team(members=[student])
+        for student in ("ham", "spam", "bacon", "eggs")
+    ]
 
 
 @pytest.fixture
-def review_teams(review_students):
+def review_teams(review_student_teams):
     master_repo = "week-1"
     review_teams = {}
-    for i, student in enumerate(review_students):
-        review_teams[util.generate_review_team_name(student, master_repo)] = (
-            review_students[:i] + review_students[i + 1 :]
+    for i, student_team in enumerate(review_student_teams):
+        review_teams[
+            util.generate_review_team_name(student_team, master_repo)
+        ] = itertools.chain.from_iterable(
+            team.members
+            for team in review_student_teams[:i]
+            + review_student_teams[i + 1 :]
         )
     return review_teams
 
@@ -236,7 +245,7 @@ def repo_infos(teams_and_members, teams):
     descriptions = ["A nice repo for {}".format(team.name) for team in teams]
     repo_names = ["{}-week-2".format(team.name) for team in teams]
     return [
-        apimeta.Repo(name, description, True, team.id)
+        plug.Repo(name, description, True, team.id)
         for name, description, team in zip(repo_names, descriptions, teams)
     ]
 
@@ -326,12 +335,12 @@ class TestEnsureTeamsAndMembers:
 
     @pytest.fixture
     def team_wrappers(self, teams_and_members):
-        """Wrap the teams_and_members dictionaries into apimeta.Team classes.
+        """Wrap the teams_and_members dictionaries into plug.Team classes.
 
         TODO: Remove this when this test suite is rewritten.
         """
         return [
-            apimeta.Team(name=team_name, members=members)
+            plug.Team(name=team_name, members=members)
             for team_name, members in teams_and_members.items()
         ]
 
@@ -462,7 +471,7 @@ class TestCreateRepos:
         github.Organization.create_repo must be called without the team_id
         argument (because if it is called with team_id=None, there is a crash).
         """
-        repo = apimeta.Repo(
+        repo = plug.Repo(
             name="repo",
             description="Some description",
             private=True,
@@ -610,7 +619,7 @@ class TestGetIssues:
         assert len(actual_issues) == len(expected_issues)
         for act, exp in zip(sorted(actual_issues), sorted(expected_issues)):
             assert act.implementation
-            for field_name in apimeta.Issue._fields:
+            for field_name in plug.Issue._fields:
                 if field_name == "implementation":
                     continue
                 assert getattr(act, field_name) == getattr(exp, field_name)
@@ -619,7 +628,7 @@ class TestGetIssues:
         repo_names = [repo.name for repo in repos]
 
         name_issues_pairs = api.get_issues(
-            repo_names, state=apimeta.IssueState.OPEN
+            repo_names, state=plug.IssueState.OPEN
         )
 
         found_repos = []
@@ -636,7 +645,7 @@ class TestGetIssues:
         repo_names = [repo.name for repo in repos]
 
         name_issues_pairs = api.get_issues(
-            repo_names, state=apimeta.IssueState.CLOSED
+            repo_names, state=plug.IssueState.CLOSED
         )
 
         found_repos = []
@@ -657,7 +666,7 @@ class TestGetIssues:
         random.shuffle(repo_names)
 
         name_issues_pairs = api.get_issues(
-            repo_names, state=apimeta.IssueState.OPEN
+            repo_names, state=plug.IssueState.OPEN
         )
 
         found_repos = []
@@ -678,7 +687,7 @@ class TestGetIssues:
         regex = "^{}$".format(sought_issue.title)
 
         name_issues_pairs = api.get_issues(
-            repo_names, state=apimeta.IssueState.OPEN, title_regex=regex
+            repo_names, state=plug.IssueState.OPEN, title_regex=regex
         )
 
         found_repos = []
@@ -880,15 +889,19 @@ class TestGetPeerReviewProgress:
     TODO: These tests need to be expanded. A lot.
     """
 
-    def test_nothing_returns(self, review_students, review_teams, api):
+    def test_nothing_returns(self, review_student_teams, review_teams, api):
         """Test calling the function when none of the functions return
         iterables.
         """
         review_team_names = list(review_teams.keys())
-        api.get_review_progress(review_team_names, review_students, "peer")
+        api.get_review_progress(
+            review_team_names, review_student_teams, "peer"
+        )
 
     def test_with_review_teams_but_no_repos(
-        self, review_students, review_teams, teams, api
+        self, review_student_teams, review_teams, teams, api
     ):
         review_team_names = list(review_teams.keys())
-        api.get_review_progress(review_team_names, review_students, "peer")
+        api.get_review_progress(
+            review_team_names, review_student_teams, "peer"
+        )
diff --git a/tests/unit_tests/test_command.py b/tests/unit_tests/test_command.py
index 32a30b5..2353d97 100644
--- a/tests/unit_tests/test_command.py
+++ b/tests/unit_tests/test_command.py
@@ -756,13 +756,11 @@ class TestCheckPeerReviewProgress:
             for student in students
             for master_name in master_names
         ]
-        # TODO change this when groups are supported
-        single_students = [g.members[0] for g in students]
 
         command.check_peer_review_progress(
             master_names, students, title_regex, 2, api_mock
         )
 
         api_mock.get_review_progress.assert_called_once_with(
-            review_team_names, single_students, title_regex
+            review_team_names, students, title_regex
         )