From fcec4c368c94d3bac112a4e7a5fb7bc66f444889 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 30 Mar 2012 13:51:54 -0700 Subject: [PATCH] Allow users to add flags via Herald rules Summary: Add "Mark with flag" rules to Herald. Test Plan: Created / edited a "Mark with flag" rule. Parsed revisions / commits, got flags added. Reviewers: btrahan, jungejason Reviewed By: btrahan CC: aran, epriestley, vrana Maniphest Tasks: T1041 Differential Revision: https://secure.phabricator.com/D2060 --- src/__celerity_resource_map__.php | 74 +++++++++---------- .../conduit/method/base/ConduitAPIMethod.php | 12 ++- .../edit/PhabricatorFlagEditController.php | 4 +- .../adapter/base/HeraldObjectAdapter.php | 39 +++++++++- .../herald/adapter/base/__init__.php | 9 +++ .../adapter/commit/HeraldCommitAdapter.php | 9 +++ .../HeraldDifferentialRevisionAdapter.php | 7 ++ .../config/action/HeraldActionConfig.php | 51 ++++++++----- .../herald/config/action/__init__.php | 1 + .../valuetype/HeraldValueTypeConfig.php | 44 +++++++---- .../controller/rule/HeraldRuleController.php | 18 ++++- .../herald/controller/rule/__init__.php | 1 + .../transcript/HeraldTranscriptController.php | 33 +++++---- .../herald/controller/transcript/__init__.php | 2 +- .../js/application/herald/HeraldRuleEditor.js | 7 +- 15 files changed, 212 insertions(+), 99 deletions(-) diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 7272b26706..98a450821b 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -223,7 +223,7 @@ celerity_register_resource_map(array( ), 'differential-revision-comment-css' => array( - 'uri' => '/res/cac35316/rsrc/css/application/differential/revision-comment.css', + 'uri' => '/res/726471a9/rsrc/css/application/differential/revision-comment.css', 'type' => 'css', 'requires' => array( @@ -313,7 +313,7 @@ celerity_register_resource_map(array( ), 'herald-rule-editor' => array( - 'uri' => '/res/209ed8c4/rsrc/js/application/herald/HeraldRuleEditor.js', + 'uri' => '/res/3a2979de/rsrc/js/application/herald/HeraldRuleEditor.js', 'type' => 'js', 'requires' => array( @@ -575,7 +575,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-differential-feedback-preview' => array( - 'uri' => '/res/768b60c9/rsrc/js/application/differential/behavior-comment-preview.js', + 'uri' => '/res/f27f3f49/rsrc/js/application/differential/behavior-comment-preview.js', 'type' => 'js', 'requires' => array( @@ -590,7 +590,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-differential-keyboard-navigation' => array( - 'uri' => '/res/d9755787/rsrc/js/application/differential/behavior-keyboard-nav.js', + 'uri' => '/res/a3f05d5b/rsrc/js/application/differential/behavior-keyboard-nav.js', 'type' => 'js', 'requires' => array( @@ -2039,7 +2039,7 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/21d01ed8/core.pkg.js', 'type' => 'js', ), - '90c90e95' => + '28d8e85a' => array( 'name' => 'differential.pkg.css', 'symbols' => @@ -2057,10 +2057,10 @@ celerity_register_resource_map(array( 10 => 'phabricator-content-source-view-css', 11 => 'differential-local-commits-view-css', ), - 'uri' => '/res/pkg/90c90e95/differential.pkg.css', + 'uri' => '/res/pkg/28d8e85a/differential.pkg.css', 'type' => 'css', ), - '9b256876' => + '06ebcd69' => array( 'name' => 'differential.pkg.js', 'symbols' => @@ -2083,7 +2083,7 @@ celerity_register_resource_map(array( 15 => 'javelin-behavior-differential-dropdown-menus', 16 => 'javelin-behavior-buoyant', ), - 'uri' => '/res/pkg/9b256876/differential.pkg.js', + 'uri' => '/res/pkg/06ebcd69/differential.pkg.js', 'type' => 'js', ), '61f9d480' => @@ -2164,7 +2164,7 @@ celerity_register_resource_map(array( 'aphront-crumbs-view-css' => '82263727', 'aphront-dialog-view-css' => '82263727', 'aphront-form-view-css' => '82263727', - 'aphront-headsup-action-list-view-css' => '90c90e95', + 'aphront-headsup-action-list-view-css' => '28d8e85a', 'aphront-list-filter-view-css' => '82263727', 'aphront-pager-view-css' => '82263727', 'aphront-panel-view-css' => '82263727', @@ -2172,40 +2172,40 @@ celerity_register_resource_map(array( 'aphront-table-view-css' => '82263727', 'aphront-tokenizer-control-css' => '82263727', 'aphront-typeahead-control-css' => '82263727', - 'differential-changeset-view-css' => '90c90e95', - 'differential-core-view-css' => '90c90e95', - 'differential-inline-comment-editor' => '9b256876', - 'differential-local-commits-view-css' => '90c90e95', - 'differential-revision-add-comment-css' => '90c90e95', - 'differential-revision-comment-css' => '90c90e95', - 'differential-revision-comment-list-css' => '90c90e95', - 'differential-revision-detail-css' => '90c90e95', - 'differential-revision-history-css' => '90c90e95', - 'differential-table-of-contents-css' => '90c90e95', + 'differential-changeset-view-css' => '28d8e85a', + 'differential-core-view-css' => '28d8e85a', + 'differential-inline-comment-editor' => '06ebcd69', + 'differential-local-commits-view-css' => '28d8e85a', + 'differential-revision-add-comment-css' => '28d8e85a', + 'differential-revision-comment-css' => '28d8e85a', + 'differential-revision-comment-list-css' => '28d8e85a', + 'differential-revision-detail-css' => '28d8e85a', + 'differential-revision-history-css' => '28d8e85a', + 'differential-table-of-contents-css' => '28d8e85a', 'diffusion-commit-view-css' => '61f9d480', 'javelin-behavior' => '4fbae2af', 'javelin-behavior-aphront-basic-tokenizer' => '2af849fb', - 'javelin-behavior-aphront-drag-and-drop' => '9b256876', - 'javelin-behavior-aphront-drag-and-drop-textarea' => '9b256876', + 'javelin-behavior-aphront-drag-and-drop' => '06ebcd69', + 'javelin-behavior-aphront-drag-and-drop-textarea' => '06ebcd69', 'javelin-behavior-aphront-form-disable-on-submit' => '21d01ed8', - 'javelin-behavior-buoyant' => '9b256876', - 'javelin-behavior-differential-accept-with-errors' => '9b256876', - 'javelin-behavior-differential-add-reviewers-and-ccs' => '9b256876', - 'javelin-behavior-differential-comment-jump' => '9b256876', - 'javelin-behavior-differential-diff-radios' => '9b256876', - 'javelin-behavior-differential-dropdown-menus' => '9b256876', - 'javelin-behavior-differential-edit-inline-comments' => '9b256876', - 'javelin-behavior-differential-feedback-preview' => '9b256876', - 'javelin-behavior-differential-keyboard-navigation' => '9b256876', - 'javelin-behavior-differential-populate' => '9b256876', - 'javelin-behavior-differential-show-more' => '9b256876', + 'javelin-behavior-buoyant' => '06ebcd69', + 'javelin-behavior-differential-accept-with-errors' => '06ebcd69', + 'javelin-behavior-differential-add-reviewers-and-ccs' => '06ebcd69', + 'javelin-behavior-differential-comment-jump' => '06ebcd69', + 'javelin-behavior-differential-diff-radios' => '06ebcd69', + 'javelin-behavior-differential-dropdown-menus' => '06ebcd69', + 'javelin-behavior-differential-edit-inline-comments' => '06ebcd69', + 'javelin-behavior-differential-feedback-preview' => '06ebcd69', + 'javelin-behavior-differential-keyboard-navigation' => '06ebcd69', + 'javelin-behavior-differential-populate' => '06ebcd69', + 'javelin-behavior-differential-show-more' => '06ebcd69', 'javelin-behavior-maniphest-batch-selector' => '86fc0b0c', 'javelin-behavior-maniphest-transaction-controls' => '86fc0b0c', 'javelin-behavior-maniphest-transaction-expand' => '86fc0b0c', 'javelin-behavior-maniphest-transaction-preview' => '86fc0b0c', 'javelin-behavior-phabricator-autofocus' => '21d01ed8', 'javelin-behavior-phabricator-keyboard-shortcuts' => '21d01ed8', - 'javelin-behavior-phabricator-object-selector' => '9b256876', + 'javelin-behavior-phabricator-object-selector' => '06ebcd69', 'javelin-behavior-phabricator-watch-anchor' => '21d01ed8', 'javelin-behavior-refresh-csrf' => '21d01ed8', 'javelin-behavior-workflow' => '21d01ed8', @@ -2230,20 +2230,20 @@ celerity_register_resource_map(array( 'maniphest-task-summary-css' => '31583232', 'maniphest-transaction-detail-css' => '31583232', 'phabricator-app-buttons-css' => '82263727', - 'phabricator-content-source-view-css' => '90c90e95', + 'phabricator-content-source-view-css' => '28d8e85a', 'phabricator-core-buttons-css' => '82263727', 'phabricator-core-css' => '82263727', 'phabricator-directory-css' => '82263727', - 'phabricator-drag-and-drop-file-upload' => '9b256876', + 'phabricator-drag-and-drop-file-upload' => '06ebcd69', 'phabricator-dropdown-menu' => '21d01ed8', 'phabricator-jump-nav' => '82263727', 'phabricator-keyboard-shortcut' => '21d01ed8', 'phabricator-keyboard-shortcut-manager' => '21d01ed8', 'phabricator-menu-item' => '21d01ed8', - 'phabricator-object-selector-css' => '90c90e95', + 'phabricator-object-selector-css' => '28d8e85a', 'phabricator-paste-file-upload' => '21d01ed8', 'phabricator-remarkup-css' => '82263727', - 'phabricator-shaped-request' => '9b256876', + 'phabricator-shaped-request' => '06ebcd69', 'phabricator-standard-page-view' => '82263727', 'phabricator-transaction-view-css' => '82263727', 'syntax-highlighting-css' => '82263727', diff --git a/src/applications/conduit/method/base/ConduitAPIMethod.php b/src/applications/conduit/method/base/ConduitAPIMethod.php index a46c144d16..04aef555d1 100644 --- a/src/applications/conduit/method/base/ConduitAPIMethod.php +++ b/src/applications/conduit/method/base/ConduitAPIMethod.php @@ -86,11 +86,15 @@ abstract class ConduitAPIMethod { return; } - $host = new PhutilURI($host); - $host->setPath('/'); - $host = (string)$host; + // NOTE: Compare domains only so we aren't sensitive to port specification + // or omission. + + $host = new PhutilURI($host); + $host = $host->getDomain(); + + $self = new PhutilURI(PhabricatorEnv::getURI('/')); + $self = $self->getDomain(); - $self = PhabricatorEnv::getURI('/'); if ($self !== $host) { throw new Exception( "Your client is connecting to this install as '{$host}', but it is ". diff --git a/src/applications/flag/controller/edit/PhabricatorFlagEditController.php b/src/applications/flag/controller/edit/PhabricatorFlagEditController.php index 02c40be2ad..aadf5c1fae 100644 --- a/src/applications/flag/controller/edit/PhabricatorFlagEditController.php +++ b/src/applications/flag/controller/edit/PhabricatorFlagEditController.php @@ -29,9 +29,7 @@ final class PhabricatorFlagEditController extends PhabricatorFlagController { $user = $request->getUser(); $phid = $this->phid; - $handles = id(new PhabricatorObjectHandleData(array($phid))) - ->loadHandles(); - $handle = $handles[$phid]; + $handle = PhabricatorObjectHandleData::loadOneHandle($phid); if (!$handle->isComplete()) { return new Aphront404Response(); diff --git a/src/applications/herald/adapter/base/HeraldObjectAdapter.php b/src/applications/herald/adapter/base/HeraldObjectAdapter.php index 6acab0f27f..e3d97c21e1 100644 --- a/src/applications/herald/adapter/base/HeraldObjectAdapter.php +++ b/src/applications/herald/adapter/base/HeraldObjectAdapter.php @@ -1,7 +1,7 @@ getTarget(); + + // TODO: Silly that we need to load this again here. + $rule = id(new HeraldRule())->load($effect->getRuleID()); + $user = id(new PhabricatorUser())->loadOneWhere( + 'phid = %s', + $rule->getAuthorPHID()); + + $flag = PhabricatorFlagQuery::loadUserFlag($user, $phid); + if ($flag) { + return new HeraldApplyTranscript( + $effect, + false, + 'Object already flagged.'); + } + + $handle = PhabricatorObjectHandleData::loadOneHandle($phid); + + $flag = new PhabricatorFlag(); + $flag->setOwnerPHID($user->getPHID()); + $flag->setType($handle->getType()); + $flag->setObjectPHID($handle->getPHID()); + + // TOOD: Should really be transcript PHID, but it doesn't exist yet. + $flag->setReasonPHID($user->getPHID()); + + $flag->setColor($color); + $flag->setNote('Flagged by Herald Rule "'.$rule->getName().'".'); + $flag->save(); + + return new HeraldApplyTranscript( + $effect, + true, + 'Added flag.'); + } + } diff --git a/src/applications/herald/adapter/base/__init__.php b/src/applications/herald/adapter/base/__init__.php index c23545edf1..ed5c3a4570 100644 --- a/src/applications/herald/adapter/base/__init__.php +++ b/src/applications/herald/adapter/base/__init__.php @@ -6,5 +6,14 @@ +phutil_require_module('phabricator', 'applications/flag/query/flag'); +phutil_require_module('phabricator', 'applications/flag/storage/flag'); +phutil_require_module('phabricator', 'applications/herald/storage/rule'); +phutil_require_module('phabricator', 'applications/herald/storage/transcript/apply'); +phutil_require_module('phabricator', 'applications/people/storage/user'); +phutil_require_module('phabricator', 'applications/phid/handle/data'); + +phutil_require_module('phutil', 'utils'); + phutil_require_source('HeraldObjectAdapter.php'); diff --git a/src/applications/herald/adapter/commit/HeraldCommitAdapter.php b/src/applications/herald/adapter/commit/HeraldCommitAdapter.php index 4ee4ec2bd3..2ad6ce5dce 100644 --- a/src/applications/herald/adapter/commit/HeraldCommitAdapter.php +++ b/src/applications/herald/adapter/commit/HeraldCommitAdapter.php @@ -217,6 +217,15 @@ final class HeraldCommitAdapter extends HeraldObjectAdapter { } $this->auditMap[$phid][] = $effect->getRuleID(); } + $result[] = new HeraldApplyTranscript( + $effect, + true, + 'Triggered an audit.'); + break; + case HeraldActionConfig::ACTION_FLAG: + $result[] = parent::applyFlagEffect( + $effect, + $this->commit->getPHID()); break; default: throw new Exception("No rules to handle action '{$action}'."); diff --git a/src/applications/herald/adapter/differential/HeraldDifferentialRevisionAdapter.php b/src/applications/herald/adapter/differential/HeraldDifferentialRevisionAdapter.php index a4fb6135e7..f06aca63bd 100644 --- a/src/applications/herald/adapter/differential/HeraldDifferentialRevisionAdapter.php +++ b/src/applications/herald/adapter/differential/HeraldDifferentialRevisionAdapter.php @@ -225,6 +225,8 @@ final class HeraldDifferentialRevisionAdapter extends HeraldObjectAdapter { } public function applyHeraldEffects(array $effects) { + assert_instances_of($effects, 'HeraldEffect'); + $result = array(); if ($this->explicitCCs) { $effect = new HeraldEffect(); @@ -252,6 +254,11 @@ final class HeraldDifferentialRevisionAdapter extends HeraldObjectAdapter { true, 'OK, did nothing.'); break; + case HeraldActionConfig::ACTION_FLAG: + $result[] = parent::applyFlagEffect( + $effect, + $this->revision->getPHID()); + break; case HeraldActionConfig::ACTION_EMAIL: case HeraldActionConfig::ACTION_ADD_CC: $op = ($action == HeraldActionConfig::ACTION_EMAIL) ? 'email' : 'CC'; diff --git a/src/applications/herald/config/action/HeraldActionConfig.php b/src/applications/herald/config/action/HeraldActionConfig.php index a2ac6aa7a6..c5a359d80a 100644 --- a/src/applications/herald/config/action/HeraldActionConfig.php +++ b/src/applications/herald/config/action/HeraldActionConfig.php @@ -23,36 +23,40 @@ final class HeraldActionConfig { const ACTION_EMAIL = 'email'; const ACTION_NOTHING = 'nothing'; const ACTION_AUDIT = 'audit'; + const ACTION_FLAG = 'flag'; public static function getActionMessageMapForRuleType($rule_type) { - $generic_mappings = - array( - self::ACTION_NOTHING => 'Do nothing', - ); + $generic_mappings = array( + self::ACTION_NOTHING => 'Do nothing', + self::ACTION_ADD_CC => 'Add emails to CC', + self::ACTION_REMOVE_CC => 'Remove emails from CC', + self::ACTION_EMAIL => 'Send an email to', + self::ACTION_AUDIT => 'Trigger an Audit', + self::ACTION_FLAG => 'Mark with flag', + ); 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', - self::ACTION_AUDIT => 'Trigger an Audit for project', - ); + $specific_mappings = array( + self::ACTION_AUDIT => 'Trigger an Audit for project', + ); 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', - self::ACTION_AUDIT => 'Trigger an Audit by me', - ); + $specific_mappings = array( + self::ACTION_ADD_CC => 'CC me', + self::ACTION_REMOVE_CC => 'Remove me from CC', + self::ACTION_EMAIL => 'Email me', + self::ACTION_AUDIT => 'Trigger an Audit by me', + ); + break; + case null: + $specific_mappings = array(); + // Use generic mappings, used on transcript. break; default: throw new Exception("Unknown rule type '${rule_type}'"); } - return $generic_mappings + $specific_mappings; + return $specific_mappings + $generic_mappings; } public static function getActionMessageMap($content_type, @@ -66,6 +70,7 @@ final class HeraldActionConfig { self::ACTION_ADD_CC, self::ACTION_REMOVE_CC, self::ACTION_EMAIL, + self::ACTION_FLAG, self::ACTION_NOTHING, )); case HeraldContentTypeConfig::CONTENT_TYPE_COMMIT: @@ -74,6 +79,7 @@ final class HeraldActionConfig { array( self::ACTION_EMAIL, self::ACTION_AUDIT, + self::ACTION_FLAG, self::ACTION_NOTHING, )); case HeraldContentTypeConfig::CONTENT_TYPE_MERGE: @@ -119,6 +125,13 @@ final class HeraldActionConfig { case HeraldActionConfig::ACTION_AUDIT: $data[1] = array($author_phid => $author_phid); break; + case HeraldActionConfig::ACTION_FLAG: + // Make sure flag color is valid; set to blue if not. + $color_map = PhabricatorFlagColor::getColorNameMap(); + if (empty($color_map[$data[1]])) { + $data[1] = PhabricatorFlagColor::COLOR_BLUE; + } + break; case HeraldActionConfig::ACTION_NOTHING: break; default: diff --git a/src/applications/herald/config/action/__init__.php b/src/applications/herald/config/action/__init__.php index 09df5cee40..e93a0aa2e9 100644 --- a/src/applications/herald/config/action/__init__.php +++ b/src/applications/herald/config/action/__init__.php @@ -6,6 +6,7 @@ +phutil_require_module('phabricator', 'applications/flag/constants/color'); phutil_require_module('phabricator', 'applications/herald/config/contenttype'); phutil_require_module('phabricator', 'applications/herald/config/ruletype'); phutil_require_module('phabricator', 'applications/herald/storage/action'); diff --git a/src/applications/herald/config/valuetype/HeraldValueTypeConfig.php b/src/applications/herald/config/valuetype/HeraldValueTypeConfig.php index 86fbcfc9b6..59edf8cc3f 100644 --- a/src/applications/herald/config/valuetype/HeraldValueTypeConfig.php +++ b/src/applications/herald/config/valuetype/HeraldValueTypeConfig.php @@ -27,6 +27,7 @@ final class HeraldValueTypeConfig { const VALUE_REPOSITORY = 'repository'; const VALUE_OWNERS_PACKAGE = 'package'; const VALUE_PROJECT = 'project'; + const VALUE_FLAG_COLOR = 'flagcolor'; public static function getValueTypeForFieldAndCondition($field, $condition) { switch ($condition) { @@ -78,22 +79,35 @@ final class HeraldValueTypeConfig { } public static function getValueTypeForAction($action, $rule_type) { - // users can't change targets for personal rule actions - if ($rule_type == HeraldRuleTypeConfig::RULE_TYPE_PERSONAL) { - return self::VALUE_NONE; - } + $is_personal = ($rule_type == HeraldRuleTypeConfig::RULE_TYPE_PERSONAL); - switch ($action) { - case HeraldActionConfig::ACTION_ADD_CC: - case HeraldActionConfig::ACTION_REMOVE_CC: - case HeraldActionConfig::ACTION_EMAIL: - return self::VALUE_EMAIL; - case HeraldActionConfig::ACTION_NOTHING: - return self::VALUE_NONE; - case HeraldActionConfig::ACTION_AUDIT: - return self::VALUE_PROJECT; - default: - throw new Exception("Unknown action '{$action}'."); + if ($is_personal) { + switch ($action) { + case HeraldActionConfig::ACTION_ADD_CC: + case HeraldActionConfig::ACTION_REMOVE_CC: + case HeraldActionConfig::ACTION_EMAIL: + case HeraldActionConfig::ACTION_NOTHING: + case HeraldActionConfig::ACTION_AUDIT: + return self::VALUE_NONE; + case HeraldActionConfig::ACTION_FLAG: + return self::VALUE_FLAG_COLOR; + default: + throw new Exception("Unknown or invalid action '{$action}'."); + } + } else { + switch ($action) { + case HeraldActionConfig::ACTION_ADD_CC: + case HeraldActionConfig::ACTION_REMOVE_CC: + case HeraldActionConfig::ACTION_EMAIL: + return self::VALUE_EMAIL; + case HeraldActionConfig::ACTION_NOTHING: + return self::VALUE_NONE; + case HeraldActionConfig::ACTION_AUDIT: + return self::VALUE_PROJECT; + case HeraldActionConfig::ACTION_FLAG: + default: + throw new Exception("Unknown or invalid action '{$action}'."); + } } } } diff --git a/src/applications/herald/controller/rule/HeraldRuleController.php b/src/applications/herald/controller/rule/HeraldRuleController.php index b93d0f9adf..7ab272fd12 100644 --- a/src/applications/herald/controller/rule/HeraldRuleController.php +++ b/src/applications/herald/controller/rule/HeraldRuleController.php @@ -378,14 +378,22 @@ final class HeraldRuleController extends HeraldController { $serial_actions = array(); foreach ($rule->getActions() as $action) { - $target_map = array(); - foreach ((array)$action->getTarget() as $fbid) { - $target_map[$fbid] = $handles[$fbid]->getName(); + switch ($action->getAction()) { + case HeraldActionConfig::ACTION_FLAG: + $current_value = $action->getTarget(); + break; + default: + $target_map = array(); + foreach ((array)$action->getTarget() as $fbid) { + $target_map[$fbid] = $handles[$fbid]->getName(); + } + $current_value = $target_map; + break; } $serial_actions[] = array( $action->getAction(), - $target_map, + $current_value, ); } } @@ -431,6 +439,8 @@ final class HeraldRuleController extends HeraldController { 'actions' => (object)$serial_actions, 'template' => $this->buildTokenizerTemplates() + array( 'rules' => $all_rules, + 'colors' => PhabricatorFlagColor::getColorNameMap(), + 'defaultColor' => PhabricatorFlagColor::COLOR_BLUE, ), 'author' => array($rule->getAuthorPHID() => $handles[$rule->getAuthorPHID()]->getName()), diff --git a/src/applications/herald/controller/rule/__init__.php b/src/applications/herald/controller/rule/__init__.php index f353801131..1469d8bd88 100644 --- a/src/applications/herald/controller/rule/__init__.php +++ b/src/applications/herald/controller/rule/__init__.php @@ -8,6 +8,7 @@ phutil_require_module('phabricator', 'aphront/response/404'); phutil_require_module('phabricator', 'aphront/response/redirect'); +phutil_require_module('phabricator', 'applications/flag/constants/color'); phutil_require_module('phabricator', 'applications/herald/config/action'); phutil_require_module('phabricator', 'applications/herald/config/condition'); phutil_require_module('phabricator', 'applications/herald/config/contenttype'); diff --git a/src/applications/herald/controller/transcript/HeraldTranscriptController.php b/src/applications/herald/controller/transcript/HeraldTranscriptController.php index 16875c5942..b48f481e8c 100644 --- a/src/applications/herald/controller/transcript/HeraldTranscriptController.php +++ b/src/applications/herald/controller/transcript/HeraldTranscriptController.php @@ -289,24 +289,29 @@ final class HeraldTranscriptController extends HeraldController { private function buildApplyTranscriptPanel($xscript) { $handles = $this->handles; - $action_names = HeraldActionConfig::getActionMessageMapForRuleType( - HeraldRuleTypeConfig::RULE_TYPE_GLOBAL); + $action_names = HeraldActionConfig::getActionMessageMapForRuleType(null); $rows = array(); foreach ($xscript->getApplyTranscripts() as $apply_xscript) { - // TODO: Hacks, this is an approximate guess at the target type. - $target = (array)$apply_xscript->getTarget(); - if (!$target) { - if ($apply_xscript->getAction() == HeraldActionConfig::ACTION_NOTHING) { + + $target = $apply_xscript->getTarget(); + switch ($apply_xscript->getAction()) { + case HeraldActionConfig::ACTION_NOTHING: $target = ''; - } else { - $target = ''; - } - } else { - foreach ($target as $k => $phid) { - $target[$k] = $handles[$phid]->getName(); - } - $target = implode("\n", $target); + break; + case HeraldActionConfig::ACTION_FLAG: + $target = PhabricatorFlagColor::getColorName($target); + break; + default: + if ($target) { + foreach ($target as $k => $phid) { + $target[$k] = $handles[$phid]->getName(); + } + $target = implode("\n", $target); + } else { + $target = ''; + } + break; } $target = phutil_escape_html($target); diff --git a/src/applications/herald/controller/transcript/__init__.php b/src/applications/herald/controller/transcript/__init__.php index 9c9a56c3d5..f124cc41a1 100644 --- a/src/applications/herald/controller/transcript/__init__.php +++ b/src/applications/herald/controller/transcript/__init__.php @@ -6,10 +6,10 @@ +phutil_require_module('phabricator', 'applications/flag/constants/color'); phutil_require_module('phabricator', 'applications/herald/config/action'); phutil_require_module('phabricator', 'applications/herald/config/condition'); phutil_require_module('phabricator', 'applications/herald/config/field'); -phutil_require_module('phabricator', 'applications/herald/config/ruletype'); phutil_require_module('phabricator', 'applications/herald/controller/base'); phutil_require_module('phabricator', 'applications/herald/storage/transcript/base'); phutil_require_module('phabricator', 'applications/phid/handle/data'); diff --git a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js index 2f4098a845..b226195d7c 100644 --- a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js +++ b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js @@ -188,7 +188,6 @@ JX.install('HeraldRuleEditor', { JX.Stratcom.addSigil(node, 'action-target'); } - var old_type = this._actionTypes[row_id]; if (old_type == type || !old_type) { set_fn(this._getActionTarget(row_id)); @@ -226,6 +225,12 @@ JX.install('HeraldRuleEditor', { get_fn = JX.bag; set_fn = JX.bag; break; + case 'flagcolor': + input = this._renderSelect(this._config.template.colors); + get_fn = function() { return input.value; }; + set_fn = function(v) { input.value = v; }; + set_fn(this._config.template.defaultColor); + break; default: input = JX.$N('input'); get_fn = function() { return input.value; };