From 3cf17cc67f2928cb8223f5baeca74d11ccaba024 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Thu, 3 Oct 2013 17:53:12 -0700 Subject: [PATCH] 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 --- .../herald/adapter/HeraldAdapter.php | 56 ++++++++++++------- .../herald/adapter/HeraldCommitAdapter.php | 53 +++++++++++------- .../HeraldDifferentialRevisionAdapter.php | 28 +++++----- .../adapter/HeraldManiphestTaskAdapter.php | 16 +++--- .../adapter/HeraldPholioMockAdapter.php | 14 +++-- .../herald/storage/HeraldRule.php | 2 +- .../js/application/herald/HeraldRuleEditor.js | 10 ++++ 7 files changed, 113 insertions(+), 66 deletions(-) diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index ff1cd4eea0..ad857424c4 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/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_NOT_CONTAINS = '!contains'; - const CONDITION_IS = 'is'; - const CONDITION_IS_NOT = '!is'; - const CONDITION_IS_ANY = 'isany'; - const CONDITION_IS_NOT_ANY = '!isany'; - const CONDITION_INCLUDE_ALL = 'all'; - const CONDITION_INCLUDE_ANY = 'any'; - const CONDITION_INCLUDE_NONE = 'none'; - const CONDITION_IS_ME = 'me'; - const CONDITION_IS_NOT_ME = '!me'; - const CONDITION_REGEXP = 'regexp'; - const CONDITION_RULE = 'conditions'; - const CONDITION_NOT_RULE = '!conditions'; - const CONDITION_EXISTS = 'exists'; - const CONDITION_NOT_EXISTS = '!exists'; - const CONDITION_REGEXP_PAIR = 'regexp-pair'; + const CONDITION_CONTAINS = 'contains'; + const CONDITION_NOT_CONTAINS = '!contains'; + const CONDITION_IS = 'is'; + const CONDITION_IS_NOT = '!is'; + const CONDITION_IS_ANY = 'isany'; + const CONDITION_IS_NOT_ANY = '!isany'; + const CONDITION_INCLUDE_ALL = 'all'; + const CONDITION_INCLUDE_ANY = 'any'; + const CONDITION_INCLUDE_NONE = 'none'; + const CONDITION_IS_ME = 'me'; + const CONDITION_IS_NOT_ME = '!me'; + const CONDITION_REGEXP = 'regexp'; + const CONDITION_RULE = 'conditions'; + const CONDITION_NOT_RULE = '!conditions'; + const CONDITION_EXISTS = 'exists'; + const CONDITION_NOT_EXISTS = '!exists'; + 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: diff --git a/src/applications/herald/adapter/HeraldCommitAdapter.php b/src/applications/herald/adapter/HeraldCommitAdapter.php index 8e07b0163c..94f6ae7fb8 100644 --- a/src/applications/herald/adapter/HeraldCommitAdapter.php +++ b/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_DIFFERENTIAL_REVISION = 'differential-revision'; - const FIELD_DIFFERENTIAL_REVIEWERS = 'differential-reviewers'; - const FIELD_DIFFERENTIAL_CCS = 'differential-ccs'; + const FIELD_NEED_AUDIT_FOR_PACKAGE = 'need-audit-for-package'; + const FIELD_DIFFERENTIAL_REVISION = 'differential-revision'; + const FIELD_DIFFERENTIAL_REVIEWERS = 'differential-reviewers'; + 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( - self::FIELD_BODY, - self::FIELD_AUTHOR, - self::FIELD_COMMITTER, - self::FIELD_REVIEWER, - self::FIELD_REPOSITORY, - self::FIELD_DIFF_FILE, - self::FIELD_DIFF_CONTENT, - self::FIELD_RULE, - self::FIELD_AFFECTED_PACKAGE, - self::FIELD_AFFECTED_PACKAGE_OWNER, - self::FIELD_NEED_AUDIT_FOR_PACKAGE, - self::FIELD_DIFFERENTIAL_REVISION, - self::FIELD_DIFFERENTIAL_REVIEWERS, - self::FIELD_DIFFERENTIAL_CCS, - ); + return array_merge( + array( + self::FIELD_BODY, + self::FIELD_AUTHOR, + self::FIELD_COMMITTER, + self::FIELD_REVIEWER, + self::FIELD_REPOSITORY, + self::FIELD_DIFF_FILE, + self::FIELD_DIFF_CONTENT, + self::FIELD_RULE, + self::FIELD_AFFECTED_PACKAGE, + self::FIELD_AFFECTED_PACKAGE_OWNER, + self::FIELD_NEED_AUDIT_FOR_PACKAGE, + self::FIELD_DIFFERENTIAL_REVISION, + 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); diff --git a/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php b/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php index bcfea24884..76bcf26791 100644 --- a/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php +++ b/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php @@ -34,19 +34,21 @@ final class HeraldDifferentialRevisionAdapter extends HeraldAdapter { } public function getFields() { - return array( - self::FIELD_TITLE, - self::FIELD_BODY, - self::FIELD_AUTHOR, - self::FIELD_REVIEWERS, - self::FIELD_CC, - self::FIELD_REPOSITORY, - self::FIELD_DIFF_FILE, - self::FIELD_DIFF_CONTENT, - self::FIELD_RULE, - self::FIELD_AFFECTED_PACKAGE, - self::FIELD_AFFECTED_PACKAGE_OWNER, - ); + return array_merge( + array( + self::FIELD_TITLE, + self::FIELD_BODY, + self::FIELD_AUTHOR, + self::FIELD_REVIEWERS, + self::FIELD_CC, + self::FIELD_REPOSITORY, + self::FIELD_DIFF_FILE, + self::FIELD_DIFF_CONTENT, + self::FIELD_RULE, + self::FIELD_AFFECTED_PACKAGE, + self::FIELD_AFFECTED_PACKAGE_OWNER, + ), + parent::getFields()); } public function getRepetitionOptions() { diff --git a/src/applications/herald/adapter/HeraldManiphestTaskAdapter.php b/src/applications/herald/adapter/HeraldManiphestTaskAdapter.php index b939a9ad72..5c461f8e89 100644 --- a/src/applications/herald/adapter/HeraldManiphestTaskAdapter.php +++ b/src/applications/herald/adapter/HeraldManiphestTaskAdapter.php @@ -47,13 +47,15 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter { } public function getFields() { - return array( - self::FIELD_TITLE, - self::FIELD_BODY, - self::FIELD_AUTHOR, - self::FIELD_CC, - self::FIELD_CONTENT_SOURCE, - ); + return array_merge( + array( + self::FIELD_TITLE, + self::FIELD_BODY, + self::FIELD_AUTHOR, + self::FIELD_CC, + self::FIELD_CONTENT_SOURCE, + ), + parent::getFields()); } public function getActions($rule_type) { diff --git a/src/applications/herald/adapter/HeraldPholioMockAdapter.php b/src/applications/herald/adapter/HeraldPholioMockAdapter.php index d3cfac9d5a..8c5fb1c1de 100644 --- a/src/applications/herald/adapter/HeraldPholioMockAdapter.php +++ b/src/applications/herald/adapter/HeraldPholioMockAdapter.php @@ -29,12 +29,14 @@ final class HeraldPholioMockAdapter extends HeraldAdapter { } public function getFields() { - return array( - self::FIELD_TITLE, - self::FIELD_BODY, - self::FIELD_AUTHOR, - self::FIELD_CC, - ); + return array_merge( + array( + self::FIELD_TITLE, + self::FIELD_BODY, + self::FIELD_AUTHOR, + self::FIELD_CC, + ), + parent::getFields()); } public function getActions($rule_type) { diff --git a/src/applications/herald/storage/HeraldRule.php b/src/applications/herald/storage/HeraldRule.php index 733de01ccf..78f3908604 100644 --- a/src/applications/herald/storage/HeraldRule.php +++ b/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; diff --git a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js index e3c9da86cb..55b50e3a29 100644 --- a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js +++ b/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');