Skip to content

Commit

Permalink
feat(request_reviews): allow to pick random user/teams
Browse files Browse the repository at this point in the history
This adds a new option `random_count` to the request_reviews action which
allows to randomly select users and teams to request a review.

Weight can be assigned to users and teams.

The selected set of teams and users is always the same for a particular pull
request.

Related #1388
  • Loading branch information
jd authored and mergify[bot] committed Sep 30, 2020
1 parent d1a2809 commit 948c1e8
Show file tree
Hide file tree
Showing 4 changed files with 265 additions and 18 deletions.
12 changes: 10 additions & 2 deletions doc/source/actions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -494,13 +494,21 @@ request.
- Default
- Value Description
* - ``users``
- list of string
- list of string or dictionary of login and weight
-
- The username to request reviews from.
* - ``teams``
- list of string
- list of string or dictionary of login and weight
-
- The team name to request reviews from.
* - ``random_count``
- integer between 1 and 15
-
- Pick random users and teams from the provided lists. When
``random_count`` is specified, ``users`` and ``teams`` can be a
dictionary where the key is the login and the value is the weight to use.

|essential plan tag|

.. note::

Expand Down
112 changes: 96 additions & 16 deletions mergify_engine/actions/request_reviews.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
# -*- encoding: utf-8 -*-
#
# Copyright © 2019–2020 Mergify SAS
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
Expand All @@ -18,6 +16,8 @@
import voluptuous

from mergify_engine import actions
from mergify_engine import subscription
from mergify_engine import utils
from mergify_engine.clients import http
from mergify_engine.rules import types

Expand All @@ -29,22 +29,104 @@ class RequestReviewsAction(actions.Action):
# Any review passed that number is ignored by GitHub API.
GITHUB_MAXIMUM_REVIEW_REQUEST = 15

_random_weight = voluptuous.Required(
voluptuous.All(int, voluptuous.Range(min=1, max=65535)), default=1
)

validator = {
voluptuous.Required("users", default=[]): voluptuous.All(
[types.GitHubLogin],
voluptuous.Length(max=GITHUB_MAXIMUM_REVIEW_REQUEST),
voluptuous.Required("users", default=[]): voluptuous.Any(
voluptuous.All(
[types.GitHubLogin],
voluptuous.Length(max=GITHUB_MAXIMUM_REVIEW_REQUEST),
),
{
types.GitHubLogin: _random_weight,
},
),
voluptuous.Required("teams", default=[]): voluptuous.Any(
voluptuous.All(
[types.GitHubTeam],
voluptuous.Length(max=GITHUB_MAXIMUM_REVIEW_REQUEST),
),
{
types.GitHubTeam: _random_weight,
},
),
voluptuous.Required("teams", default=[]): voluptuous.All(
[types.GitHubTeam],
voluptuous.Length(max=GITHUB_MAXIMUM_REVIEW_REQUEST),
"random_count": voluptuous.All(
int,
voluptuous.Range(1, GITHUB_MAXIMUM_REVIEW_REQUEST),
),
}

silent_report = True

always_run = True

def _get_random_reviewers(self, random_number: int, pr_author: str) -> set:
if isinstance(self.config["users"], dict):
user_weights = self.config["users"]
else:
user_weights = {user: 1 for user in self.config["users"]}

if isinstance(self.config["teams"], dict):
team_weights = self.config["teams"]
else:
team_weights = {team: 1 for team in self.config["teams"]}

choices = {
**{user.lower(): weight for user, weight in user_weights.items()},
**{f"@{team}": weight for team, weight in team_weights.items()},
}

try:
del choices[pr_author.lower()]
except KeyError:
pass

count = min(self.config["random_count"], len(choices))

return utils.get_random_choices(
random_number,
choices,
count,
)

def _get_reviewers(
self, pr_id: int, existing_reviews: set, pr_author: str
) -> (set, set):
if "random_count" in self.config:
team_reviews_to_request = set()
user_reviews_to_request = set()

for reviewer in self._get_random_reviewers(pr_id, pr_author):
if reviewer.startswith("@"):
team_reviews_to_request.add(reviewer[1:])
else:
user_reviews_to_request.add(reviewer)
else:
user_reviews_to_request = set(self.config["users"])
team_reviews_to_request = set(self.config["teams"])

user_reviews_to_request -= existing_reviews
user_reviews_to_request -= {pr_author}

# Team starts with @
team_reviews_to_request -= {
e[1:] for e in existing_reviews if e.startswith("@")
}

return user_reviews_to_request, team_reviews_to_request

def run(self, ctxt, rule, missing_conditions):
if "random_count" in self.config and not ctxt.subscription.has_feature(
subscription.Features.RANDOM_REQUEST_REVIEWS
):
return (
"action_required",
"Random request reviews are disabled",
f"⚠ The [subscription](https://dashboard.mergify.io/installation/{ctxt.client.auth.installation['id']}/subscription) needed to be updated to enable them.",
)

# Using consolidated data to avoid already done API lookup
reviews_keys = (
"approved-reviews-by",
Expand All @@ -56,15 +138,13 @@ def run(self, ctxt, rule, missing_conditions):
existing_reviews = set(
itertools.chain(*[getattr(ctxt.pull_request, key) for key in reviews_keys])
)
user_reviews_to_request = (
set(self.config["users"])
- existing_reviews
- set((ctxt.pull["user"]["login"],))
)
team_reviews_to_request = set(self.config["teams"]).difference(
# Team starts with @
{e[1:] for e in existing_reviews if e.startswith("@")}

user_reviews_to_request, team_reviews_to_request = self._get_reviewers(
ctxt.pull["id"],
existing_reviews,
ctxt.pull["user"]["login"],
)

if user_reviews_to_request or team_reviews_to_request:
requested_reviews_nb = len(ctxt.pull_request.review_requested)

Expand Down
1 change: 1 addition & 0 deletions mergify_engine/subscription.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class Features(enum.Enum):
LARGE_REPOSITORY = "large_repository"
PRIORITY_QUEUES = "priority_queues"
CUSTOM_CHECKS = "custom_checks"
RANDOM_REQUEST_REVIEWS = "random_request_reviews"


@dataclasses.dataclass
Expand Down
158 changes: 158 additions & 0 deletions mergify_engine/tests/unit/actions/test_request_reviews.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,13 @@
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from unittest import mock

import pytest
import voluptuous

from mergify_engine import context
from mergify_engine import subscription
from mergify_engine.actions import request_reviews


Expand Down Expand Up @@ -62,3 +66,157 @@ def test_config_not_ok(config, error):
with pytest.raises(voluptuous.MultipleInvalid) as p:
request_reviews.RequestReviewsAction.get_schema()(config)
assert str(p.value) == error


def test_random_reviewers():
action = request_reviews.RequestReviewsAction.get_schema()(
{
"random_count": 2,
"teams": {
"foobar": 2,
"foobaz": 1,
},
"users": {
"jd": 2,
"sileht": 1,
},
},
)
reviewers = action._get_random_reviewers(123, "jd")
assert reviewers == {"@foobar", "sileht"}
reviewers = action._get_random_reviewers(124, "sileht")
assert reviewers == {"jd", "@foobar"}
reviewers = action._get_random_reviewers(124, "jd")
assert reviewers == {"@foobaz", "@foobar"}


def test_random_reviewers_no_weight():
action = request_reviews.RequestReviewsAction.get_schema()(
{
"random_count": 2,
"teams": {
"foobar": 2,
"foobaz": 1,
},
"users": ["jd", "sileht"],
},
)
reviewers = action._get_random_reviewers(123, "another-jd")
assert reviewers == {"sileht", "jd"}
reviewers = action._get_random_reviewers(124, "another-jd")
assert reviewers == {"sileht", "@foobar"}
reviewers = action._get_random_reviewers(124, "sileht")
assert reviewers == {"@foobaz", "@foobar"}


def test_random_reviewers_count_bigger():
action = request_reviews.RequestReviewsAction.get_schema()(
{
"random_count": 15,
"teams": {
"foobar": 2,
"foobaz": 1,
},
"users": {
"jd": 2,
"sileht": 45,
},
}
)
reviewers = action._get_random_reviewers(123, "foobar")
assert reviewers == {"@foobar", "@foobaz", "jd", "sileht"}
reviewers = action._get_random_reviewers(124, "another-jd")
assert reviewers == {"@foobar", "@foobaz", "jd", "sileht"}
reviewers = action._get_random_reviewers(124, "jd")
assert reviewers == {"@foobar", "@foobaz", "sileht"}


def test_random_config_too_much_count():
with pytest.raises(voluptuous.MultipleInvalid) as p:
request_reviews.RequestReviewsAction.get_schema()(
{
"random_count": 20,
"teams": {
"foobar": 2,
"foobaz": 1,
},
"users": {
"foobar": 2,
"foobaz": 1,
},
},
)
assert (
str(p.value)
== "value must be at most 15 for dictionary value @ data['random_count']"
)


def test_get_reviewers():
action = request_reviews.RequestReviewsAction.get_schema()(
{
"random_count": 2,
"teams": {
"foobar": 2,
"foobaz": 1,
},
"users": {
"jd": 2,
"sileht": 1,
},
},
)
reviewers = action._get_reviewers(843, set(), "another-jd")
assert reviewers == ({"jd", "sileht"}, set())
reviewers = action._get_reviewers(844, set(), "another-jd")
assert reviewers == ({"jd"}, {"foobar"})
reviewers = action._get_reviewers(845, set(), "another-jd")
assert reviewers == ({"sileht"}, {"foobar"})
reviewers = action._get_reviewers(845, {"sileht"}, "another-jd")
assert reviewers == (set(), {"foobar"})
reviewers = action._get_reviewers(845, {"jd"}, "another-jd")
assert reviewers == ({"sileht"}, {"foobar"})
reviewers = action._get_reviewers(845, set(), "SILEHT")
assert reviewers == ({"jd"}, {"foobar"})


def test_disabled():
action = request_reviews.RequestReviewsAction.get_schema()(
{
"random_count": 2,
"teams": {
"foobar": 2,
"foobaz": 1,
},
"users": {
"jd": 2,
"sileht": 1,
},
},
)
client = mock.MagicMock()
client.auth.installation.__getitem__.return_value = 123
sub = subscription.Subscription(
123,
False,
"No sub",
{},
frozenset({}),
)
ctxt = context.Context(
client,
{
"number": 123,
"state": None,
"mergeable_state": "ok",
"merged_by": None,
"merged": None,
"merged_at": None,
},
sub,
)
assert action.run(ctxt, None, None) == (
"action_required",
"Random request reviews are disabled",
"⚠ The [subscription](https://dashboard.mergify.io/installation/123/subscription) needed to be updated to enable them.",
)

0 comments on commit 948c1e8

Please sign in to comment.