Add a "branches" rule for commits

Summary:
Fixes T1353. Also some minor unrelated cleanup:

  - `openTransaction()` / `saveTransaction()` exist now, fix TODOs.
  - Fix some instructions.
  - Make `diffusion.branchquery` return empty for SVN rather than fataling.

Test Plan:
  - Added a branches rule.
  - Ran a dry run against commits in different VCSes.

{F105574}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, Nopik

Maniphest Tasks: T1353

Differential Revision: https://secure.phabricator.com/D8086

src/applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php

@@ -100,6 +100,12 @@ final class ConduitAPI_diffusion_branchquery_Method
     return $this->processBranchRefs($request, $refs);
   }
 
+  protected function getSVNResult() {
+    // Since SVN doesn't have meaningful branches, just return nothing for all
+    // queries.
+    return array();
+  }
+
   private function processBranchRefs(ConduitAPIRequest $request, array $refs) {
     $drequest = $this->getDiffusionRequest();
     $repository = $drequest->getRepository();
@@ -125,4 +131,5 @@ final class ConduitAPI_diffusion_branchquery_Method
 
     return mpull($refs, 'toDictionary');
   }
+
 }

src/applications/herald/adapter/HeraldCommitAdapter.php

@@ -88,7 +88,8 @@ final class HeraldCommitAdapter extends HeraldAdapter {
     return array(
       self::FIELD_NEED_AUDIT_FOR_PACKAGE =>
         pht('Affected packages that need audit'),
-      self::FIELD_REPOSITORY_AUTOCLOSE_BRANCH => pht('On autoclose branch'),
+      self::FIELD_REPOSITORY_AUTOCLOSE_BRANCH
+        => pht('Commit is on closing branch'),
     ) + parent::getFieldNameMap();
   }
 
@@ -114,6 +115,7 @@ final class HeraldCommitAdapter extends HeraldAdapter {
         self::FIELD_DIFFERENTIAL_ACCEPTED,
         self::FIELD_DIFFERENTIAL_REVIEWERS,
         self::FIELD_DIFFERENTIAL_CCS,
+        self::FIELD_BRANCHES,
         self::FIELD_REPOSITORY_AUTOCLOSE_BRANCH,
       ),
       parent::getFields());
@@ -461,6 +463,18 @@ final class HeraldCommitAdapter extends HeraldAdapter {
           return array();
         }
         return $revision->getCCPHIDs();
+      case self::FIELD_BRANCHES:
+        $params = array(
+          'callsign' => $this->repository->getCallsign(),
+          'contains' => $this->commit->getCommitIdentifier(),
+        );
+
+        $result = id(new ConduitCall('diffusion.branchquery', $params))
+          ->setUser(PhabricatorUser::getOmnipotentUser())
+          ->execute();
+
+        $refs = DiffusionRepositoryRef::loadAllFromDictionaries($result);
+        return mpull($refs, 'getShortName');
       case self::FIELD_REPOSITORY_AUTOCLOSE_BRANCH:
         return $this->repository->shouldAutocloseCommit(
           $this->commit,

src/applications/herald/controller/HeraldTestConsoleController.php

@@ -74,15 +74,13 @@ final class HeraldTestConsoleController extends HeraldController {
       }
     }
 
-    $text = pht(
-      'Enter an object to test rules for, like a Diffusion commit (e.g., '.
-      'rX123) or a Differential revision (e.g., D123). You will be shown '.
-      'the results of a dry run on the object.');
-
     $form = id(new AphrontFormView())
       ->setUser($user)
-      ->appendChild(
-        phutil_tag('p', array('class' => 'aphront-form-instructions'), $text))
+      ->appendRemarkupInstructions(
+        pht(
+        'Enter an object to test rules for, like a Diffusion commit (e.g., '.
+        '`rX123`) or a Differential revision (e.g., `D123`). You will be '.
+        'shown the results of a dry run on the object.'))
       ->appendChild(
         id(new AphrontFormTextControl())
           ->setLabel(pht('Object Name'))

src/applications/herald/storage/HeraldRule.php

@@ -17,7 +17,7 @@ final class HeraldRule extends HeraldDAO
   protected $isDisabled = 0;
   protected $triggerObjectPHID;
 
-  protected $configVersion = 27;
+  protected $configVersion = 28;
 
   // phids for which this rule has been applied
   private $ruleApplied = self::ATTACHABLE;
@@ -134,8 +134,7 @@ final class HeraldRule extends HeraldDAO
       $child->setRuleID($this->getID());
     }
 
-// TODO:
-//    $this->openTransaction();
+    $this->openTransaction();
       queryfx(
         $this->establishConnection('w'),
         'DELETE FROM %T WHERE ruleID = %d',
@@ -144,7 +143,7 @@ final class HeraldRule extends HeraldDAO
       foreach ($children as $child) {
         $child->save();
       }
-//    $this->saveTransaction();
+    $this->saveTransaction();
   }
 
   public function delete() {