Bug 1575178 - Fail when requesting a review from disabled user. r=glob

Reviewers: glob

Reviewed By: glob

Subscribers: smaug

Bug #: 1575178

Differential Revision: https://phabricator.services.mozilla.com/D43040

moz-phab

@@ -1543,7 +1543,9 @@ class Repository(object):
                 commit_errors.append("contains arc fields")
 
             for reviewer in commit_invalid_reviewers[commit["node"]]:
-                if "until" in reviewer:
+                if "disabled" in reviewer:
+                    commit_errors.append("User %s is disabled" % reviewer["name"])
+                elif "until" in reviewer:
                     unavailable_reviewers_warning = True
                     msg = "%s is not available until %s" % (
                         reviewer["name"],
@@ -3312,10 +3314,10 @@ def check_for_invalid_reviewers(reviewers):
 
     all_reviewers.extend(all_groups)
     found_names.extend(found_groups)
-    invalid_reviewers = list(set(all_reviewers) - set(found_names))
+    invalid = list(set(all_reviewers) - set(found_names))
 
     # Find users availability:
-    unavailable_reviewers = [
+    unavailable = [
         dict(
             name=r["userName"],
             until=datetime.datetime.fromtimestamp(r["currentStatusUntil"]).strftime(
@@ -3326,7 +3328,13 @@ def check_for_invalid_reviewers(reviewers):
         if r.get("currentStatus") == "away"
     ]
 
-    return unavailable_reviewers + [dict(name=r) for r in invalid_reviewers]
+    # Find disabled users:
+    disabled = [
+        dict(name=r["userName"], disabled=True)
+        for r in users
+        if "disabled" in r.get("roles", [])
+    ]
+    return disabled + unavailable + [dict(name=r) for r in invalid]
 
 
 #

tests/test_helpers.py

@@ -241,6 +241,22 @@ def test_valid_reviewers_in_phabricator_returns_no_errors(call_conduit):
     assert [] == mozphab.check_for_invalid_reviewers(reviewers)
 
 
+@mock.patch("mozphab.ConduitAPI.call")
+def test_disabled_reviewers(call_conduit):
+    reviewers = dict(granted=[], request=["alice", "goober"])
+    call_conduit.side_effect = (
+        # user.query
+        [
+            dict(userName="alice", phid="PHID-USER-1"),
+            dict(userName="goober", phid="PHID-USER-2", roles=[u"disabled"]),
+        ],
+        # project.search
+        {"data": [], "maps": {"slugMap": {}}},
+    )
+    expected_errors = [dict(name="goober", disabled=True)]
+    assert expected_errors == mozphab.check_for_invalid_reviewers(reviewers)
+
+
 @mock.patch("mozphab.ConduitAPI.call")
 def test_non_existent_reviewers_or_groups_generates_error_list(call_conduit):
     ts = 1543622400
@@ -283,7 +299,7 @@ def test_non_existent_reviewers_or_groups_generates_error_list(call_conduit):
 
 
 @mock.patch("mozphab.ConduitAPI.call")
-def test_reviwer_case_sensitivity(call_conduit):
+def test_reviewer_case_sensitivity(call_conduit):
     reviewers = dict(granted=[], request=["Alice", "#uSeR-gRoUp"])
     call_conduit.side_effect = (
         # See https://phabricator.services.mozilla.com/conduit/method/user.query/