Herald - add field + condition for Diffusion Commits for "On autoclose branch"

Summary:
Fixes T1461.

Adds

- FIELD_ALWAYS - now you could add this to a content type to always get notified
- FIELD_REPOSITORY_AUTOCLOSE_BRANCH - solves T1461
- CONDITION_UNCONDITIONALLY - used by these two fields to not show any value for the user to select

Test Plan: made a herald rule where diffs on autoclose branches would get flagged blue. made a diff on an autoclose branch and committed it. commit was flagged!

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T1461

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

src/applications/herald/adapter/HeraldAdapter.php

@@ -20,24 +20,26 @@ abstract class HeraldAdapter {
   const FIELD_AFFECTED_PACKAGE       = 'affected-package';
   const FIELD_AFFECTED_PACKAGE_OWNER = 'affected-package-owner';
   const FIELD_CONTENT_SOURCE         = 'contentsource';
+  const FIELD_ALWAYS                 = 'always';
 
-  const CONDITION_CONTAINS      = 'contains';
+  const CONDITION_CONTAINS        = 'contains';
-  const CONDITION_NOT_CONTAINS  = '!contains';
+  const CONDITION_NOT_CONTAINS    = '!contains';
-  const CONDITION_IS            = 'is';
+  const CONDITION_IS              = 'is';
-  const CONDITION_IS_NOT        = '!is';
+  const CONDITION_IS_NOT          = '!is';
-  const CONDITION_IS_ANY        = 'isany';
+  const CONDITION_IS_ANY          = 'isany';
-  const CONDITION_IS_NOT_ANY    = '!isany';
+  const CONDITION_IS_NOT_ANY      = '!isany';
-  const CONDITION_INCLUDE_ALL   = 'all';
+  const CONDITION_INCLUDE_ALL     = 'all';
-  const CONDITION_INCLUDE_ANY   = 'any';
+  const CONDITION_INCLUDE_ANY     = 'any';
-  const CONDITION_INCLUDE_NONE  = 'none';
+  const CONDITION_INCLUDE_NONE    = 'none';
-  const CONDITION_IS_ME         = 'me';
+  const CONDITION_IS_ME           = 'me';
-  const CONDITION_IS_NOT_ME     = '!me';
+  const CONDITION_IS_NOT_ME       = '!me';
-  const CONDITION_REGEXP        = 'regexp';
+  const CONDITION_REGEXP          = 'regexp';
-  const CONDITION_RULE          = 'conditions';
+  const CONDITION_RULE            = 'conditions';
-  const CONDITION_NOT_RULE      = '!conditions';
+  const CONDITION_NOT_RULE        = '!conditions';
-  const CONDITION_EXISTS        = 'exists';
+  const CONDITION_EXISTS          = 'exists';
-  const CONDITION_NOT_EXISTS    = '!exists';
+  const CONDITION_NOT_EXISTS      = '!exists';
-  const CONDITION_REGEXP_PAIR   = 'regexp-pair';
+  const CONDITION_UNCONDITIONALLY = 'unconditionally';
+  const CONDITION_REGEXP_PAIR     = 'regexp-pair';
 
   const ACTION_ADD_CC       = 'addcc';
   const ACTION_REMOVE_CC    = 'remcc';
@@ -79,6 +81,8 @@ abstract class HeraldAdapter {
         return null;
       case self::FIELD_CONTENT_SOURCE:
         return $this->getContentSource()->getSource();
+      case self::FIELD_ALWAYS:
+        return true;
       default:
         throw new Exception(
           "Unknown field '{$field_name}'!");
@@ -105,7 +109,11 @@ abstract class HeraldAdapter {
 /* -(  Fields  )------------------------------------------------------------- */
 
 
-  abstract public function getFields();
+  public function getFields() {
+    return array(
+      self::FIELD_ALWAYS,
+    );
+  }
 
   public function getFieldNameMap() {
     return array(
@@ -124,7 +132,8 @@ abstract class HeraldAdapter {
       self::FIELD_AFFECTED_PACKAGE => pht('Any affected package'),
       self::FIELD_AFFECTED_PACKAGE_OWNER =>
         pht("Any affected package's owner"),
-      self:: FIELD_CONTENT_SOURCE => pht('Content Source')
+      self::FIELD_CONTENT_SOURCE => pht('Content Source'),
+      self::FIELD_ALWAYS => pht('Always'),
     );
   }
 
@@ -150,6 +159,7 @@ abstract class HeraldAdapter {
       self::CONDITION_NOT_RULE        => pht('does not match:'),
       self::CONDITION_EXISTS          => pht('exists'),
       self::CONDITION_NOT_EXISTS      => pht('does not exist'),
+      self::CONDITION_UNCONDITIONALLY => '',  // don't show anything!
       self::CONDITION_REGEXP_PAIR     => pht('matches regexp pair'),
     );
   }
@@ -208,6 +218,10 @@ abstract class HeraldAdapter {
           self::CONDITION_IS,
           self::CONDITION_IS_NOT,
         );
+      case self::FIELD_ALWAYS:
+        return array(
+          self::CONDITION_UNCONDITIONALLY,
+        );
       default:
         throw new Exception(
           "This adapter does not define conditions for field '{$field}'!");
@@ -281,6 +295,8 @@ abstract class HeraldAdapter {
         return (bool)$field_value;
       case self::CONDITION_NOT_EXISTS:
         return !$field_value;
+      case self::CONDITION_UNCONDITIONALLY:
+        return (bool)$field_value;
       case self::CONDITION_REGEXP:
         foreach ((array)$field_value as $value) {
           // We add the 'S' flag because we use the regexp multiple times.
@@ -422,6 +438,7 @@ abstract class HeraldAdapter {
       case self::CONDITION_NOT_RULE:
       case self::CONDITION_EXISTS:
       case self::CONDITION_NOT_EXISTS:
+      case self::CONDITION_UNCONDITIONALLY:
         // No explicit validation for these types, although there probably
         // should be in some cases.
         break;
@@ -559,6 +576,7 @@ abstract class HeraldAdapter {
       case self::CONDITION_IS_NOT_ME:
       case self::CONDITION_EXISTS:
       case self::CONDITION_NOT_EXISTS:
+      case self::CONDITION_UNCONDITIONALLY:
         return self::VALUE_NONE;
       case self::CONDITION_RULE:
       case self::CONDITION_NOT_RULE:

src/applications/herald/adapter/HeraldCommitAdapter.php

@@ -5,10 +5,11 @@
  */
 final class HeraldCommitAdapter extends HeraldAdapter {
 
-  const FIELD_NEED_AUDIT_FOR_PACKAGE = 'need-audit-for-package';
+  const FIELD_NEED_AUDIT_FOR_PACKAGE      = 'need-audit-for-package';
-  const FIELD_DIFFERENTIAL_REVISION  = 'differential-revision';
+  const FIELD_DIFFERENTIAL_REVISION       = 'differential-revision';
-  const FIELD_DIFFERENTIAL_REVIEWERS = 'differential-reviewers';
+  const FIELD_DIFFERENTIAL_REVIEWERS      = 'differential-reviewers';
-  const FIELD_DIFFERENTIAL_CCS       = 'differential-ccs';
+  const FIELD_DIFFERENTIAL_CCS            = 'differential-ccs';
+  const FIELD_REPOSITORY_AUTOCLOSE_BRANCH = 'repository-autoclose-branch';
 
   protected $diff;
   protected $revision;
@@ -46,26 +47,30 @@ final class HeraldCommitAdapter extends HeraldAdapter {
       self::FIELD_DIFFERENTIAL_REVISION => pht('Differential revision'),
       self::FIELD_DIFFERENTIAL_REVIEWERS => pht('Differential reviewers'),
       self::FIELD_DIFFERENTIAL_CCS => pht('Differential CCs'),
+      self::FIELD_REPOSITORY_AUTOCLOSE_BRANCH => pht('On autoclose branch'),
     ) + parent::getFieldNameMap();
   }
 
   public function getFields() {
-    return array(
+    return array_merge(
-      self::FIELD_BODY,
+      array(
-      self::FIELD_AUTHOR,
+        self::FIELD_BODY,
-      self::FIELD_COMMITTER,
+        self::FIELD_AUTHOR,
-      self::FIELD_REVIEWER,
+        self::FIELD_COMMITTER,
-      self::FIELD_REPOSITORY,
+        self::FIELD_REVIEWER,
-      self::FIELD_DIFF_FILE,
+        self::FIELD_REPOSITORY,
-      self::FIELD_DIFF_CONTENT,
+        self::FIELD_DIFF_FILE,
-      self::FIELD_RULE,
+        self::FIELD_DIFF_CONTENT,
-      self::FIELD_AFFECTED_PACKAGE,
+        self::FIELD_RULE,
-      self::FIELD_AFFECTED_PACKAGE_OWNER,
+        self::FIELD_AFFECTED_PACKAGE,
-      self::FIELD_NEED_AUDIT_FOR_PACKAGE,
+        self::FIELD_AFFECTED_PACKAGE_OWNER,
-      self::FIELD_DIFFERENTIAL_REVISION,
+        self::FIELD_NEED_AUDIT_FOR_PACKAGE,
-      self::FIELD_DIFFERENTIAL_REVIEWERS,
+        self::FIELD_DIFFERENTIAL_REVISION,
-      self::FIELD_DIFFERENTIAL_CCS,
+        self::FIELD_DIFFERENTIAL_REVIEWERS,
-    );
+        self::FIELD_DIFFERENTIAL_CCS,
+        self::FIELD_REPOSITORY_AUTOCLOSE_BRANCH,
+      ),
+      parent::getFields());
   }
 
   public function getConditionsForField($field) {
@@ -94,6 +99,10 @@ final class HeraldCommitAdapter extends HeraldAdapter {
           self::CONDITION_INCLUDE_ANY,
           self::CONDITION_INCLUDE_NONE,
         );
+      case self::FIELD_REPOSITORY_AUTOCLOSE_BRANCH:
+        return array(
+          self::CONDITION_UNCONDITIONALLY,
+        );
     }
     return parent::getConditionsForField($field);
   }
@@ -307,6 +316,10 @@ final class HeraldCommitAdapter extends HeraldAdapter {
           return array();
         }
         return $revision->getCCPHIDs();
+      case self::FIELD_REPOSITORY_AUTOCLOSE_BRANCH:
+        return $this->repository->shouldAutocloseCommit(
+          $this->commit,
+          $this->commitData);
     }
 
     return parent::getHeraldField($field);

src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php

@@ -34,19 +34,21 @@ final class HeraldDifferentialRevisionAdapter extends HeraldAdapter {
   }
 
   public function getFields() {
-    return array(
+    return array_merge(
-      self::FIELD_TITLE,
+      array(
-      self::FIELD_BODY,
+        self::FIELD_TITLE,
-      self::FIELD_AUTHOR,
+        self::FIELD_BODY,
-      self::FIELD_REVIEWERS,
+        self::FIELD_AUTHOR,
-      self::FIELD_CC,
+        self::FIELD_REVIEWERS,
-      self::FIELD_REPOSITORY,
+        self::FIELD_CC,
-      self::FIELD_DIFF_FILE,
+        self::FIELD_REPOSITORY,
-      self::FIELD_DIFF_CONTENT,
+        self::FIELD_DIFF_FILE,
-      self::FIELD_RULE,
+        self::FIELD_DIFF_CONTENT,
-      self::FIELD_AFFECTED_PACKAGE,
+        self::FIELD_RULE,
-      self::FIELD_AFFECTED_PACKAGE_OWNER,
+        self::FIELD_AFFECTED_PACKAGE,
-    );
+        self::FIELD_AFFECTED_PACKAGE_OWNER,
+      ),
+      parent::getFields());
   }
 
   public function getRepetitionOptions() {

src/applications/herald/adapter/HeraldManiphestTaskAdapter.php

@@ -47,13 +47,15 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter {
   }
 
   public function getFields() {
-    return array(
+    return array_merge(
-      self::FIELD_TITLE,
+      array(
-      self::FIELD_BODY,
+        self::FIELD_TITLE,
-      self::FIELD_AUTHOR,
+        self::FIELD_BODY,
-      self::FIELD_CC,
+        self::FIELD_AUTHOR,
-      self::FIELD_CONTENT_SOURCE,
+        self::FIELD_CC,
-    );
+        self::FIELD_CONTENT_SOURCE,
+      ),
+      parent::getFields());
   }
 
   public function getActions($rule_type) {

src/applications/herald/adapter/HeraldPholioMockAdapter.php

@@ -29,12 +29,14 @@ final class HeraldPholioMockAdapter extends HeraldAdapter {
   }
 
   public function getFields() {
-    return array(
+    return array_merge(
-      self::FIELD_TITLE,
+      array(
-      self::FIELD_BODY,
+        self::FIELD_TITLE,
-      self::FIELD_AUTHOR,
+        self::FIELD_BODY,
-      self::FIELD_CC,
+        self::FIELD_AUTHOR,
-    );
+        self::FIELD_CC,
+      ),
+      parent::getFields());
   }
 
   public function getActions($rule_type) {

src/applications/herald/storage/HeraldRule.php

@@ -13,7 +13,7 @@ final class HeraldRule extends HeraldDAO
   protected $repetitionPolicy;
   protected $ruleType;
 
-  protected $configVersion = 10;
+  protected $configVersion = 11;
 
   private $ruleApplied = self::ATTACHABLE; // phids for which this rule has been applied
   private $validAuthor = self::ATTACHABLE;

webroot/rsrc/js/application/herald/HeraldRuleEditor.js

@@ -163,6 +163,16 @@ JX.install('HeraldRuleEditor', {
       JX.DOM.setContent(condition_cell, condition_select);
 
       this._onconditionchange(r);
+
+      var condition_name = this._config.conditions[row_id][1];
+      switch (condition_name) {
+        case 'unconditionally':
+          JX.DOM.hide(condition_cell);
+          break;
+        default:
+          JX.DOM.show(condition_cell);
+          break;
+      }
     },
     _onconditionchange : function(r) {
       var target = JX.DOM.find(r, 'select', 'condition-select');