Bug 1569785 - Do not request the review if revision is in `needs-revi…

…ew` status. Reviewers: glob Reviewed By: glob Subscribers: MattN Bug #: 1569785 Differential Revision: https://phabricator.services.mozilla.com/D40036
mozilla-conduit · Aug 6, 2019 · 63dcb6b · 63dcb6b
1 parent 9909549
commit 63dcb6b
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 13 deletions.
diff --git a/moz-phab b/moz-phab
@@ -3646,11 +3646,12 @@ def submit(repo, args):
     for commit in commits:
         # Only revisions being updated have an ID.  Newly created ones don't.
         is_update = bool(commit["rev-id"])
+        revision_to_update = (
+            revisions_to_update[commit["rev-id"]] if is_update else None
+        )
         existing_reviewers = (
-            revisions_to_update[commit["rev-id"]]["attachments"]["reviewers"][
-                "reviewers"
-            ]
-            if commit["rev-id"]
+            revision_to_update["attachments"]["reviewers"]["reviewers"]
+            if revision_to_update
             else None
         )
         has_commit_reviewers = bool(
@@ -3764,6 +3765,7 @@ def submit(repo, args):
                 raise Error("Failed to find 'Revision URL' in arc output")
 
             if is_update:
+                current_status = revision_to_update["fields"]["status"]["value"]
                 with wait_message("Updating D%s.." % commit["rev-id"]):
                     transactions = []
                     revision = conduit.get_revisions(ids=[int(commit["rev-id"])])[0]
@@ -3774,7 +3776,8 @@ def submit(repo, args):
                     # Add reviewers only if revision lacks them
                     if not args.wip and has_commit_reviewers and not existing_reviewers:
                         update_revision_reviewers(transactions, commit)
-                        transactions.append(dict(type="request-review"))
+                        if current_status != "needs-review":
+                            transactions.append(dict(type="request-review"))
 
                     if transactions:
                         arc_call_conduit(

diff --git a/tests/test_integration-git.py b/tests/test_integration-git.py
@@ -161,7 +161,10 @@ def test_submit_update(in_process, git_repo_path, init_sha):
         dict(
             data=[
                 {
-                    "fields": {"bugzilla.bug-id": "1"},
+                    "fields": {
+                        "bugzilla.bug-id": "1",
+                        "status": {"value": "needs-review"},
+                    },
                     "phid": "PHID-DREV-y7x5hvdpe2gyerctdqqz",
                     "id": 123,
                     "attachments": {"reviewers": {"reviewers": []}},
@@ -274,7 +277,10 @@ def test_submit_update_arc(in_process, git_repo_path, init_sha):
         {  # differential.revision.search
             "data": [
                 {
-                    "fields": {"bugzilla.bug-id": "1"},
+                    "fields": {
+                        "bugzilla.bug-id": "1",
+                        "status": {"value": "needs-review"},
+                    },
                     "phid": "PHID-DREV-y7x5hvdpe2gyerctdqqz",
                     "id": 123,
                     "attachments": {"reviewers": {"reviewers": []}},
@@ -324,7 +330,10 @@ def test_submit_update_bug_id(in_process, git_repo_path, init_sha):
                 {
                     "id": 123,
                     "phid": "PHID-REV-1",
-                    "fields": {"bugzilla.bug-id": "1"},
+                    "fields": {
+                        "bugzilla.bug-id": "1",
+                        "status": {"value": "needs-review"},
+                    },
                     "attachments": {"reviewers": {"reviewers": []}},
                 }
             ]

diff --git a/tests/test_integration-hg.py b/tests/test_integration-hg.py
@@ -76,7 +76,10 @@ def test_submit_update(in_process, hg_repo_path):
                 {
                     "id": 123,
                     "phid": "PHID-REV-1",
-                    "fields": {"bugzilla.bug-id": "1"},
+                    "fields": {
+                        "bugzilla.bug-id": "1",
+                        "status": {"value": "needs-review"},
+                    },
                     "attachments": {"reviewers": {"reviewers": []}},
                 }
             ]
@@ -86,7 +89,10 @@ def test_submit_update(in_process, hg_repo_path):
                 {
                     "id": "123",
                     "phid": "PHID-REV-1",
-                    "fields": {"bugzilla.bug-id": "1"},
+                    "fields": {
+                        "bugzilla.bug-id": "1",
+                        "status": {"value": "needs-review"},
+                    },
                     "attachments": {
                         "reviewers": {"reviewers": [{"reviewerPHID": "PHID-USER-1"}]}
                     },
@@ -134,7 +140,10 @@ def test_submit_update_reviewers_not_updated(in_process, hg_repo_path):
                 {
                     "id": 123,
                     "phid": "PHID-REV-1",
-                    "fields": {"bugzilla.bug-id": "1"},
+                    "fields": {
+                        "bugzilla.bug-id": "1",
+                        "status": {"value": "needs-review"},
+                    },
                     "attachments": {
                         "reviewers": {"reviewers": [{"reviewerPHID": "PHID-USER-1"}]}
                     },
@@ -176,7 +185,10 @@ def test_submit_update_no_new_reviewers(in_process, hg_repo_path):
                 {
                     "id": 123,
                     "phid": "PHID-REV-1",
-                    "fields": {"bugzilla.bug-id": "1"},
+                    "fields": {
+                        "bugzilla.bug-id": "1",
+                        "status": {"value": "changes-planned"},
+                    },
                     "attachments": {"reviewers": {"reviewers": []}},
                 }
             ]
@@ -226,7 +238,10 @@ def test_submit_update_bug_id(in_process, hg_repo_path):
                 {
                     "id": 123,
                     "phid": "PHID-REV-1",
-                    "fields": {"bugzilla.bug-id": "1"},
+                    "fields": {
+                        "bugzilla.bug-id": "1",
+                        "status": {"value": "needs-review"},
+                    },
                     "attachments": {
                         "reviewers": {"reviewers": [{"reviewerPHID": "PHID-USER-1"}]}
                     },