phabricator: allow specifying reviewers on phabsend
authorJun Wu <quark@fb.com>
Tue, 11 Jul 2017 08:52:55 -0700
changeset 33498 b7a75b9a3386
parent 33497 80e1331a7fe9
child 33499 0407a51b9d8c
phabricator: allow specifying reviewers on phabsend Sometimes people want to specify reviewer explicitly for a stack. The webpage only allows changing reviewer for one revision at a time. This patch adds a `--reviewer` flag to make it easier to specify reviewers. Test Plan: On a test Phabricator instance, enable `differential.allow-self-accept`, assign myself as a reviewer and make sure it works. Also try an invalid username and make sure it raises. Differential Revision: https://phab.mercurial-scm.org/D38
contrib/phabricator.py
--- a/contrib/phabricator.py	Tue Jul 11 10:46:55 2017 -0700
+++ b/contrib/phabricator.py	Tue Jul 11 08:52:55 2017 -0700
@@ -236,7 +236,8 @@
     }
     callconduit(ctx.repo(), 'differential.setdiffproperty', params)
 
-def createdifferentialrevision(ctx, revid=None, parentrevid=None, oldnode=None):
+def createdifferentialrevision(ctx, revid=None, parentrevid=None, oldnode=None,
+                               actions=None):
     """create or update a Differential Revision
 
     If revid is None, create a new Differential Revision, otherwise update
@@ -244,6 +245,8 @@
 
     If oldnode is not None, check if the patch content (without commit message
     and metadata) has changed before creating another diff.
+
+    If actions is not None, they will be appended to the transaction.
     """
     repo = ctx.repo()
     if oldnode:
@@ -268,6 +271,9 @@
         transactions += [{'type': 'summary', 'value': summary},
                          {'type': 'summary', 'value': ' '}]
 
+    if actions:
+        transactions += actions
+
     # Parse commit message and update related fields.
     desc = ctx.description()
     info = callconduit(repo, 'differential.parsecommitmessage',
@@ -287,8 +293,23 @@
 
     return revision
 
+def userphids(repo, names):
+    """convert user names to PHIDs"""
+    query = {'constraints': {'usernames': names}}
+    result = callconduit(repo, 'user.search', query)
+    # username not found is not an error of the API. So check if we have missed
+    # some names here.
+    data = result[r'data']
+    resolved = set(entry[r'fields'][r'username'] for entry in data)
+    unresolved = set(names) - resolved
+    if unresolved:
+        raise error.Abort(_('unknown username: %s')
+                          % ' '.join(sorted(unresolved)))
+    return [entry[r'phid'] for entry in data]
+
 @command('phabsend',
-         [('r', 'rev', [], _('revisions to send'), _('REV'))],
+         [('r', 'rev', [], _('revisions to send'), _('REV')),
+          ('', 'reviewer', [], _('specify reviewers'))],
          _('REV [OPTIONS]'))
 def phabsend(ui, repo, *revs, **opts):
     """upload changesets to Phabricator
@@ -308,6 +329,12 @@
     if not revs:
         raise error.Abort(_('phabsend requires at least one changeset'))
 
+    actions = []
+    reviewers = opts.get('reviewer', [])
+    if reviewers:
+        phids = userphids(repo, reviewers)
+        actions.append({'type': 'reviewers.add', 'value': phids})
+
     oldnodedrev = getoldnodedrevmap(repo, [repo[r].node() for r in revs])
 
     # Send patches one by one so we know their Differential Revision IDs and
@@ -322,7 +349,7 @@
         if oldnode != ctx.node():
             # Create or update Differential Revision
             revision = createdifferentialrevision(ctx, revid, lastrevid,
-                                                  oldnode)
+                                                  oldnode, actions)
             newrevid = int(revision[r'object'][r'id'])
             if revid:
                 action = _('updated')