diff --git a/moz-phab b/moz-phab index b39fdcc..99fc220 100755 --- 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 index 4e8be43..bcdd099 100644 --- 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 index 343238d..48bbbde 100644 --- 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"}]} },