diff --git a/resources/sql/patches/097.heraldruletypes.sql b/resources/sql/patches/097.heraldruletypes.sql new file mode 100644 index 0000000000..cfc63e9d4a --- /dev/null +++ b/resources/sql/patches/097.heraldruletypes.sql @@ -0,0 +1,2 @@ +ALTER TABLE phabricator_herald.herald_rule ADD ruleType varchar(255) not null DEFAULT 'global'; +CREATE INDEX IDX_RULE_TYPE on phabricator_herald.herald_rule (ruleType); \ No newline at end of file diff --git a/resources/sql/patches/098.heraldruletypemigration.php b/resources/sql/patches/098.heraldruletypemigration.php new file mode 100644 index 0000000000..1ce2bc1cb4 --- /dev/null +++ b/resources/sql/patches/098.heraldruletypemigration.php @@ -0,0 +1,62 @@ +loadAll(); + +foreach ($rules as $rule) { + if ($rule->getRuleType() !== HeraldRuleTypeConfig::RULE_TYPE_PERSONAL) { + $actions = $rule->loadActions(); + $can_be_personal = true; + foreach ($actions as $action) { + $target = $action->getTarget(); + if (is_array($target)) { + if (count($target) > 1) { + $can_be_personal = false; + break; + } else { + $targetPHID = head($target); + if ($targetPHID !== $rule->getAuthorPHID()) { + $can_be_personal = false; + break; + } + } + } else if ($target) { + if ($target !== $rule->getAuthorPHID()) { + $can_be_personal = false; + break; + } + } + } + + if ($can_be_personal) { + $rule->setRuleType(HeraldRuleTypeConfig::RULE_TYPE_PERSONAL); + queryfx( + $rule->establishConnection('w'), + 'UPDATE %T SET ruleType = %s WHERE id = %d', + $rule->getTableName(), + $rule->getRuleType(), + $rule->getID()); + + echo "Setting rule '" . $rule->getName() . "' to personal. "; + } + } +} + +echo "Done. "; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 65cfbd8ae0..fbbd7e1a01 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -335,6 +335,7 @@ phutil_register_library_map(array( 'HeraldRuleController' => 'applications/herald/controller/rule', 'HeraldRuleListView' => 'applications/herald/view/rulelist', 'HeraldRuleTranscript' => 'applications/herald/storage/transcript/rule', + 'HeraldRuleTypeConfig' => 'applications/herald/config/ruletype', 'HeraldTestConsoleController' => 'applications/herald/controller/test', 'HeraldTranscript' => 'applications/herald/storage/transcript/base', 'HeraldTranscriptController' => 'applications/herald/controller/transcript', diff --git a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php index 89c648ddd3..f420b3bfa7 100644 --- a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php @@ -270,7 +270,10 @@ class AphrontDefaultApplicationConfiguration '/herald/' => array( '$' => 'HeraldHomeController', - 'view/(?P[^/]+)/$' => 'HeraldHomeController', + 'view/(?P[^/]+)/' => array( + '$' => 'HeraldHomeController', + '(?Pglobal)/$' => 'HeraldHomeController' + ), 'new/(?:(?P[^/]+)/)?$' => 'HeraldNewController', 'rule/(?:(?P\d+)/)?$' => 'HeraldRuleController', 'delete/(?P\d+)/$' => 'HeraldDeleteController', diff --git a/src/applications/herald/config/action/HeraldActionConfig.php b/src/applications/herald/config/action/HeraldActionConfig.php index 3974c6f59c..a1ceb6c840 100644 --- a/src/applications/herald/config/action/HeraldActionConfig.php +++ b/src/applications/herald/config/action/HeraldActionConfig.php @@ -1,7 +1,7 @@ 'Add emails to CC', - self::ACTION_REMOVE_CC => 'Remove emails from CC', - self::ACTION_EMAIL => 'Send an email to', - self::ACTION_NOTHING => 'Do nothing', - ); + public static function getActionMessageMapForRuleType($rule_type) { + $generic_mappings = + array( + self::ACTION_NOTHING => 'Do nothing', + ); + + switch ($rule_type) { + case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: + $specific_mappings = + array( + self::ACTION_ADD_CC => 'Add emails to CC', + self::ACTION_REMOVE_CC => 'Remove emails from CC', + self::ACTION_EMAIL => 'Send an email to', + ); + break; + case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: + $specific_mappings = + array( + self::ACTION_ADD_CC => 'CC me', + self::ACTION_REMOVE_CC => 'Remove me from CC', + self::ACTION_EMAIL => 'Email me', + ); + break; + default: + throw new Exception("Unknown rule type '${rule_type}'"); + } + return $generic_mappings + $specific_mappings; } - public static function getActionMapForContentType($type) { - $map = self::getActionMap(); - switch ($type) { + public static function getActionMessageMap($content_type, + $rule_type) { + $map = self::getActionMessageMapForRuleType($rule_type); + switch ($content_type) { case HeraldContentTypeConfig::CONTENT_TYPE_DIFFERENTIAL: return array_select_keys( $map, @@ -70,4 +91,44 @@ class HeraldActionConfig { } } + /** + * Create a HeraldAction to save from data. + * + * $data is of the form: + * array( + * 0 => + * 1 => array() + * ) + */ + public static function willSaveAction($rule_type, + $author_phid, + $data) { + $obj = new HeraldAction(); + $obj->setAction($data[0]); + + // for personal rule types, set the target to be the owner of the rule + if ($rule_type == HeraldRuleTypeConfig::RULE_TYPE_PERSONAL) { + switch ($obj->getAction()) { + case HeraldActionConfig::ACTION_EMAIL: + case HeraldActionConfig::ACTION_ADD_CC: + case HeraldActionConfig::ACTION_REMOVE_CC: + $data[1] = array($author_phid => $author_phid); + break; + case HeraldActionConfig::ACTION_NOTHING: + break; + default: + throw new Exception('Unrecognized action type: ' . + $obj->getAction()); + } + } + + if (is_array($data[1])) { + $obj->setTarget(array_keys($data[1])); + } else { + $obj->setTarget($data[1]); + } + + return $obj; + } + } diff --git a/src/applications/herald/config/action/__init__.php b/src/applications/herald/config/action/__init__.php index 67737f7071..09df5cee40 100644 --- a/src/applications/herald/config/action/__init__.php +++ b/src/applications/herald/config/action/__init__.php @@ -7,6 +7,8 @@ phutil_require_module('phabricator', 'applications/herald/config/contenttype'); +phutil_require_module('phabricator', 'applications/herald/config/ruletype'); +phutil_require_module('phabricator', 'applications/herald/storage/action'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/herald/config/ruletype/HeraldRuleTypeConfig.php b/src/applications/herald/config/ruletype/HeraldRuleTypeConfig.php new file mode 100644 index 0000000000..143f29c031 --- /dev/null +++ b/src/applications/herald/config/ruletype/HeraldRuleTypeConfig.php @@ -0,0 +1,31 @@ + 'Global', + self::RULE_TYPE_PERSONAL => 'Personal', + ); + return $map; + } +} diff --git a/src/applications/herald/config/ruletype/__init__.php b/src/applications/herald/config/ruletype/__init__.php new file mode 100644 index 0000000000..5c1e01b5b5 --- /dev/null +++ b/src/applications/herald/config/ruletype/__init__.php @@ -0,0 +1,10 @@ +setBaseURI(new PhutilURI('/herald/')) - ->addLabel('Rules') + ->addLabel('My Rules') ->addFilter('new', 'Create Rule'); $rules_map = HeraldContentTypeConfig::getContentTypeMap(); $first_filter = null; @@ -41,6 +41,12 @@ abstract class HeraldController extends PhabricatorController { $first_filter = 'view/'.$key; } } + $nav + ->addSpacer() + ->addLabel('Global Rules'); + foreach ($rules_map as $key => $value) { + $nav->addFilter("view/${key}/global", $value); + } $nav ->addSpacer() diff --git a/src/applications/herald/controller/home/HeraldHomeController.php b/src/applications/herald/controller/home/HeraldHomeController.php index 9771c632a9..cb258b7ca8 100644 --- a/src/applications/herald/controller/home/HeraldHomeController.php +++ b/src/applications/herald/controller/home/HeraldHomeController.php @@ -1,7 +1,7 @@ view = idx($data, 'view'); - $this->setFilter($this->view); + $this->global = idx($data, 'global'); + if ($this->global) { + $this->setFilter($this->view . '/global'); + } else { + $this->setFilter($this->view); + } } public function getFilter() { @@ -45,10 +51,17 @@ class HeraldHomeController extends HeraldController { $this->view = key($map); } - $rules = id(new HeraldRule())->loadAllWhere( - 'contentType = %s AND authorPHID = %s', - $this->view, - $user->getPHID()); + if ($this->global) { + $rules = id(new HeraldRule())->loadAllWhere( + 'contentType = %s AND ruleType = %s', + $this->view, + 'global'); + } else { + $rules = id(new HeraldRule())->loadAllWhere( + 'contentType = %s AND authorPHID = %s', + $this->view, + $user->getPHID()); + } $need_phids = mpull($rules, 'getAuthorPHID'); $handles = id(new PhabricatorObjectHandleData($need_phids)) diff --git a/src/applications/herald/controller/new/HeraldNewController.php b/src/applications/herald/controller/new/HeraldNewController.php index 71e34da1cc..b8df026849 100644 --- a/src/applications/herald/controller/new/HeraldNewController.php +++ b/src/applications/herald/controller/new/HeraldNewController.php @@ -1,7 +1,7 @@ type = idx($data, 'type'); + $this->contentType = idx($data, 'type'); } public function processRequest() { @@ -33,25 +33,32 @@ class HeraldNewController extends HeraldController { $request = $this->getRequest(); $user = $request->getUser(); - $map = HeraldContentTypeConfig::getContentTypeMap(); - if (empty($map[$this->type])) { - reset($map); - $this->type = key($map); + $content_type_map = HeraldContentTypeConfig::getContentTypeMap(); + if (empty($content_type_map[$this->contentType])) { + reset($content_type_map); + $this->contentType = key($content_type_map); } + $rule_type_map = HeraldRuleTypeConfig::getRuleTypeMap(); + $form = id(new AphrontFormView()) ->setUser($user) ->setAction('/herald/rule/') ->appendChild( id(new AphrontFormSelectControl()) ->setLabel('New rule for') - ->setName('type') - ->setValue($this->type) - ->setOptions($map)) + ->setName('content_type') + ->setValue($this->contentType) + ->setOptions($content_type_map)) + ->appendChild( + id(new AphrontFormSelectControl()) + ->setLabel('Type') + ->setName('rule_type') + ->setOptions($rule_type_map)) ->appendChild( id(new AphrontFormSubmitControl()) ->setValue('Create Rule') - ->addCancelButton('/herald/view/'.$this->type.'/')); + ->addCancelButton('/herald/view/'.$this->contentType.'/')); $panel = new AphrontPanelView(); $panel->setHeader('Create New Herald Rule'); diff --git a/src/applications/herald/controller/new/__init__.php b/src/applications/herald/controller/new/__init__.php index a90d20cf3e..8724556b3d 100644 --- a/src/applications/herald/controller/new/__init__.php +++ b/src/applications/herald/controller/new/__init__.php @@ -7,6 +7,7 @@ phutil_require_module('phabricator', 'applications/herald/config/contenttype'); +phutil_require_module('phabricator', 'applications/herald/config/ruletype'); phutil_require_module('phabricator', 'applications/herald/controller/base'); phutil_require_module('phabricator', 'view/form/base'); phutil_require_module('phabricator', 'view/form/control/select'); diff --git a/src/applications/herald/controller/rule/HeraldRuleController.php b/src/applications/herald/controller/rule/HeraldRuleController.php index 63cd1fed42..2368c369c9 100644 --- a/src/applications/herald/controller/rule/HeraldRuleController.php +++ b/src/applications/herald/controller/rule/HeraldRuleController.php @@ -38,13 +38,14 @@ class HeraldRuleController extends HeraldController { $user = $request->getUser(); $content_type_map = HeraldContentTypeConfig::getContentTypeMap(); + $rule_type_map = HeraldRuleTypeConfig::getRuleTypeMap(); if ($this->id) { $rule = id(new HeraldRule())->load($this->id); if (!$rule) { return new Aphront404Response(); } - if ($rule->getAuthorPHID() != $user->getPHID() && !$user->getIsAdmin()) { + if (!$this->canEditRule($rule, $user)) { throw new Exception("You don't own this rule and can't edit it."); } } else { @@ -52,11 +53,17 @@ class HeraldRuleController extends HeraldController { $rule->setAuthorPHID($user->getPHID()); $rule->setMustMatchAll(true); - $type = $request->getStr('type'); - if (!isset($content_type_map[$type])) { - $type = HeraldContentTypeConfig::CONTENT_TYPE_DIFFERENTIAL; + $content_type = $request->getStr('content_type'); + if (!isset($content_type_map[$content_type])) { + $content_type = HeraldContentTypeConfig::CONTENT_TYPE_DIFFERENTIAL; } - $rule->setContentType($type); + $rule->setContentType($content_type); + + $rule_type = $request->getStr('rule_type'); + if (!isset($rule_type_map[$rule_type])) { + $rule_type = HeraldRuleTypeConfig::RULE_TYPE_GLOBAL; + } + $rule->setRuleType($rule_type); } $this->setFilter($rule->getContentType()); @@ -104,12 +111,14 @@ class HeraldRuleController extends HeraldController { require_celerity_resource('herald-css'); - $type_name = $content_type_map[$rule->getContentType()]; + $content_type_name = $content_type_map[$rule->getContentType()]; + $rule_type_name = $rule_type_map[$rule->getRuleType()]; $form = id(new AphrontFormView()) ->setUser($user) ->setID('herald-rule-edit-form') - ->addHiddenInput('type', $rule->getContentType()) + ->addHiddenInput('content_type', $rule->getContentType()) + ->addHiddenInput('rule_type', $rule->getRuleType()) ->addHiddenInput('save', 1) ->appendChild( // Build this explicitly so we can add a sigil to it. @@ -142,7 +151,8 @@ class HeraldRuleController extends HeraldController { ->appendChild( id(new AphrontFormMarkupControl()) ->setValue( - "This rule triggers for {$type_name}.")) + "This ${rule_type_name} rule triggers for " . + "${content_type_name}.")) ->appendChild( '

Conditions

'. '
'. @@ -215,6 +225,13 @@ class HeraldRuleController extends HeraldController { )); } + private function canEditRule($rule, $user) { + return + $user->getIsAdmin() || + $rule->getRuleType() == HeraldRuleTypeConfig::RULE_TYPE_GLOBAL || + $rule->getAuthorPHID() == $user->getPHID(); + } + private function saveRule($rule, $request) { $rule->setName($request->getStr('name')); $rule->setMustMatchAll(($request->getStr('must_match') == 'all')); @@ -301,6 +318,11 @@ class HeraldRuleController extends HeraldController { $conditions[] = $obj; } + $author = $request->getStr('author'); + if ($author) { + $rule->setAuthorPHID($author); + } + $actions = array(); foreach ($data['actions'] as $action) { if ($action === null) { @@ -308,27 +330,15 @@ class HeraldRuleController extends HeraldController { continue; } - $obj = new HeraldAction(); - $obj->setAction($action[0]); - if (!isset($action[1])) { // Legitimate for any action which doesn't need a target, like // "Do nothing". $action[1] = null; } - if (is_array($action[1])) { - $obj->setTarget(array_keys($action[1])); - } else { - $obj->setTarget($action[1]); - } - - $actions[] = $obj; - } - - $author = $request->getStr('author'); - if ($author) { - $rule->setAuthorPHID($author); + $actions[] = HeraldActionConfig::willSaveAction($rule->getRuleType(), + $rule->getAuthorPHID(), + $action); } $rule->attachConditions($conditions); @@ -425,11 +435,15 @@ class HeraldRuleController extends HeraldController { } $config_info['actions'] = - HeraldActionConfig::getActionMapForContentType($rule->getContentType()); + HeraldActionConfig::getActionMessageMap($rule->getContentType(), + $rule->getRuleType()); + + $config_info['rule_type'] = $rule->getRuleType(); foreach ($config_info['actions'] as $action => $name) { $config_info['targets'][$action] = - HeraldValueTypeConfig::getValueTypeForAction($action); + HeraldValueTypeConfig::getValueTypeForAction($action, + $rule->getRuleType()); } Javelin::initBehavior( diff --git a/src/applications/herald/controller/rule/__init__.php b/src/applications/herald/controller/rule/__init__.php index 9202bab04c..cc92ef1ec7 100644 --- a/src/applications/herald/controller/rule/__init__.php +++ b/src/applications/herald/controller/rule/__init__.php @@ -13,9 +13,9 @@ phutil_require_module('phabricator', 'applications/herald/config/condition'); phutil_require_module('phabricator', 'applications/herald/config/contenttype'); phutil_require_module('phabricator', 'applications/herald/config/field'); phutil_require_module('phabricator', 'applications/herald/config/repetitionpolicy'); +phutil_require_module('phabricator', 'applications/herald/config/ruletype'); phutil_require_module('phabricator', 'applications/herald/config/valuetype'); phutil_require_module('phabricator', 'applications/herald/controller/base'); -phutil_require_module('phabricator', 'applications/herald/storage/action'); phutil_require_module('phabricator', 'applications/herald/storage/condition'); phutil_require_module('phabricator', 'applications/herald/storage/rule'); phutil_require_module('phabricator', 'applications/phid/handle/data'); diff --git a/src/applications/herald/storage/rule/HeraldRule.php b/src/applications/herald/storage/rule/HeraldRule.php index 79acda53be..bc1702a15f 100644 --- a/src/applications/herald/storage/rule/HeraldRule.php +++ b/src/applications/herald/storage/rule/HeraldRule.php @@ -1,7 +1,7 @@