diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -73,7 +73,7 @@ 'rsrc/css/application/files/global-drag-and-drop.css' => '697324ad', 'rsrc/css/application/flag/flag.css' => '5337623f', 'rsrc/css/application/harbormaster/harbormaster.css' => '49d64eb4', - 'rsrc/css/application/herald/herald-test.css' => '778b008e', + 'rsrc/css/application/herald/herald-test.css' => 'a52e323e', 'rsrc/css/application/herald/herald.css' => '826075fa', 'rsrc/css/application/maniphest/batch-editor.css' => 'b0f0b6d5', 'rsrc/css/application/maniphest/report.css' => 'f6931fdf', @@ -539,7 +539,7 @@ 'harbormaster-css' => '49d64eb4', 'herald-css' => '826075fa', 'herald-rule-editor' => '91a6031b', - 'herald-test-css' => '778b008e', + 'herald-test-css' => 'a52e323e', 'inline-comment-summary-css' => '51efda3a', 'javelin-aphlict' => '5359e785', 'javelin-behavior' => '61cbc29a', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2110,6 +2110,7 @@ 'PhabricatorFilesManagementWorkflow' => 'applications/files/management/PhabricatorFilesManagementWorkflow.php', 'PhabricatorFilesOutboundRequestAction' => 'applications/files/action/PhabricatorFilesOutboundRequestAction.php', 'PhabricatorFlag' => 'applications/flag/storage/PhabricatorFlag.php', + 'PhabricatorFlagAddFlagHeraldAction' => 'applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php', 'PhabricatorFlagColor' => 'applications/flag/constants/PhabricatorFlagColor.php', 'PhabricatorFlagConstants' => 'applications/flag/constants/PhabricatorFlagConstants.php', 'PhabricatorFlagController' => 'applications/flag/controller/PhabricatorFlagController.php', @@ -5988,6 +5989,7 @@ 'PhabricatorFlagDAO', 'PhabricatorPolicyInterface', ), + 'PhabricatorFlagAddFlagHeraldAction' => 'HeraldAction', 'PhabricatorFlagColor' => 'PhabricatorFlagConstants', 'PhabricatorFlagConstants' => 'Phobject', 'PhabricatorFlagController' => 'PhabricatorController', diff --git a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php --- a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php +++ b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php @@ -177,7 +177,6 @@ self::ACTION_ADD_CC, self::ACTION_REMOVE_CC, self::ACTION_EMAIL, - self::ACTION_FLAG, self::ACTION_ADD_REVIEWERS, self::ACTION_ADD_BLOCKING_REVIEWERS, ), diff --git a/src/applications/diffusion/herald/HeraldCommitAdapter.php b/src/applications/diffusion/herald/HeraldCommitAdapter.php --- a/src/applications/diffusion/herald/HeraldCommitAdapter.php +++ b/src/applications/diffusion/herald/HeraldCommitAdapter.php @@ -102,7 +102,6 @@ self::ACTION_ADD_CC, self::ACTION_REMOVE_CC, self::ACTION_EMAIL, - self::ACTION_FLAG, self::ACTION_AUDIT, ), parent::getActions($rule_type)); diff --git a/src/applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php b/src/applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php new file mode 100644 --- /dev/null +++ b/src/applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php @@ -0,0 +1,88 @@ +getAdapter()->getPHID(); + $rule = $effect->getRule(); + $author = $rule->getAuthor(); + + $flag = PhabricatorFlagQuery::loadUserFlag($author, $phid); + if ($flag) { + $this->logEffect(self::DO_IGNORE, $flag->getColor()); + return; + } + + $flag = id(new PhabricatorFlag()) + ->setOwnerPHID($author->getPHID()) + ->setType(phid_get_type($phid)) + ->setObjectPHID($phid) + ->setReasonPHID($rule->getPHID()) + ->setColor($effect->getTarget()) + ->setNote('') + ->save(); + + $this->logEffect(self::DO_FLAG, $flag->getColor()); + } + + public function getHeraldActionValueType() { + return id(new HeraldSelectFieldValue()) + ->setKey('flag.color') + ->setOptions(PhabricatorFlagColor::getColorNameMap()) + ->setDefault(PhabricatorFlagColor::COLOR_BLUE); + } + + protected function getActionEffectMap() { + return array( + self::DO_IGNORE => array( + 'icon' => 'fa-times', + 'color' => 'grey', + 'name' => pht('Already Marked'), + ), + self::DO_FLAG => array( + 'icon' => 'fa-flag', + 'name' => pht('Flagged'), + ), + ); + } + + public function renderActionDescription($value) { + $color = PhabricatorFlagColor::getColorName($value); + return pht('Mark with %s flag.', $color); + } + + public function renderActionEffectDescription($type, $data) { + switch ($type) { + case self::DO_IGNORE: + return pht( + 'Already marked with %s flag.', + PhabricatorFlagColor::getColorName($data)); + case self::DO_FLAG: + return pht( + 'Marked with "%s" flag.', + PhabricatorFlagColor::getColorName($data)); + } + } + +} diff --git a/src/applications/herald/action/HeraldAction.php b/src/applications/herald/action/HeraldAction.php --- a/src/applications/herald/action/HeraldAction.php +++ b/src/applications/herald/action/HeraldAction.php @@ -3,6 +3,7 @@ abstract class HeraldAction extends Phobject { private $adapter; + private $viewer; private $applyLog = array(); const STANDARD_NONE = 'standard.none'; @@ -12,6 +13,7 @@ abstract public function supportsObject($object); abstract public function supportsRuleType($rule_type); abstract public function applyEffect($object, HeraldEffect $effect); + abstract public function renderActionEffectDescription($type, $data); public function getActionGroupKey() { return null; @@ -66,6 +68,15 @@ return $this->adapter; } + final public function setViewer(PhabricatorUser $viewer) { + $this->viewer = $viewer; + return $this; + } + + final public function getViewer() { + return $this->viewer; + } + final public function getActionConstant() { $class = new ReflectionClass($this); @@ -106,13 +117,49 @@ } protected function logEffect($type, $data = null) { - return; + if (!is_string($type)) { + throw new Exception( + pht( + 'Effect type passed to "%s" must be a scalar string.', + 'logEffect()')); + } + + $this->applyLog[] = array( + 'type' => $type, + 'data' => $data, + ); + + return $this; } final public function getApplyTranscript(HeraldEffect $effect) { - $context = 'v2/'.phutil_json_encode($this->applyLog); + $context = $this->applyLog; $this->applyLog = array(); return new HeraldApplyTranscript($effect, true, $context); } + protected function getActionEffectMap() { + throw new PhutilMethodNotImplementedException(); + } + + private function getActionEffectSpec($type) { + $map = $this->getActionEffectMap(); + return idx($map, $type, array()); + } + + public function renderActionEffectIcon($type, $data) { + $map = $this->getActionEffectSpec($type); + return idx($map, 'icon'); + } + + public function renderActionEffectColor($type, $data) { + $map = $this->getActionEffectSpec($type); + return idx($map, 'color'); + } + + public function renderActionEffectName($type, $data) { + $map = $this->getActionEffectSpec($type); + return idx($map, 'name'); + } + } diff --git a/src/applications/herald/action/HeraldDoNothingAction.php b/src/applications/herald/action/HeraldDoNothingAction.php --- a/src/applications/herald/action/HeraldDoNothingAction.php +++ b/src/applications/herald/action/HeraldDoNothingAction.php @@ -22,11 +22,29 @@ } public function applyEffect($object, HeraldEffect $effect) { - $this->logEffect($effect, self::DO_NOTHING); + $this->logEffect(self::DO_NOTHING); } public function getHeraldActionStandardType() { return self::STANDARD_NONE; } + protected function getActionEffectMap() { + return array( + self::DO_NOTHING => array( + 'icon' => 'fa-check', + 'color' => 'grey', + 'name' => pht('Did Nothing'), + ), + ); + } + + public function renderActionDescription($value) { + return pht('Do nothing.'); + } + + public function renderActionEffectDescription($type, $data) { + return pht('Did nothing.'); + } + } diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -30,7 +30,6 @@ const ACTION_REMOVE_CC = 'remcc'; const ACTION_EMAIL = 'email'; const ACTION_AUDIT = 'audit'; - const ACTION_FLAG = 'flag'; const ACTION_ASSIGN_TASK = 'assigntask'; const ACTION_ADD_PROJECTS = 'addprojects'; const ACTION_REMOVE_PROJECTS = 'removeprojects'; @@ -670,7 +669,7 @@ return $actions; } - private function getActionImplementation($key) { + public function getActionImplementation($key) { return idx($this->getActionImplementationMap(), $key); } @@ -728,7 +727,6 @@ self::ACTION_REMOVE_CC => pht('Remove Subscribers'), self::ACTION_EMAIL => pht('Send an email to'), self::ACTION_AUDIT => pht('Trigger an Audit by'), - self::ACTION_FLAG => pht('Mark with flag'), self::ACTION_ASSIGN_TASK => pht('Assign task to'), self::ACTION_ADD_PROJECTS => pht('Add projects'), self::ACTION_REMOVE_PROJECTS => pht('Remove projects'), @@ -745,7 +743,6 @@ self::ACTION_REMOVE_CC => pht('Remove me as a subscriber'), self::ACTION_EMAIL => pht('Send me an email'), self::ACTION_AUDIT => pht('Trigger an Audit by me'), - self::ACTION_FLAG => pht('Mark with flag'), self::ACTION_ASSIGN_TASK => pht('Assign task to me'), self::ACTION_ADD_REVIEWERS => pht('Add me as a reviewer'), self::ACTION_ADD_BLOCKING_REVIEWERS => @@ -798,13 +795,6 @@ // For personal rules, force these actions to target the rule owner. $target = array($author_phid); break; - case self::ACTION_FLAG: - // Make sure flag color is valid; set to blue if not. - $color_map = PhabricatorFlagColor::getColorNameMap(); - if (empty($color_map[$target])) { - $target = PhabricatorFlagColor::COLOR_BLUE; - } - break; case self::ACTION_BLOCK: break; default: @@ -846,8 +836,6 @@ case self::ACTION_ADD_REVIEWERS: case self::ACTION_ADD_BLOCKING_REVIEWERS: return new HeraldEmptyFieldValue(); - case self::ACTION_FLAG: - return $this->buildFlagColorFieldValue(); case self::ACTION_ADD_PROJECTS: case self::ACTION_REMOVE_PROJECTS: return $this->buildTokenizerFieldValue( @@ -864,8 +852,6 @@ case self::ACTION_REMOVE_PROJECTS: return $this->buildTokenizerFieldValue( new PhabricatorProjectDatasource()); - case self::ACTION_FLAG: - return $this->buildFlagColorFieldValue(); case self::ACTION_ASSIGN_TASK: return $this->buildTokenizerFieldValue( new PhabricatorPeopleDatasource()); @@ -893,13 +879,6 @@ throw new Exception(pht("Unknown or invalid action '%s'.", $action)); } - private function buildFlagColorFieldValue() { - return id(new HeraldSelectFieldValue()) - ->setKey('flag.color') - ->setOptions(PhabricatorFlagColor::getColorNameMap()) - ->setDefault(PhabricatorFlagColor::COLOR_BLUE); - } - private function buildTokenizerFieldValue( PhabricatorTypeaheadDatasource $datasource) { @@ -1051,7 +1030,7 @@ ), array( $icon, - $this->renderActionAsText($action, $handles), + $this->renderActionAsText($viewer, $action, $handles), )); } @@ -1083,8 +1062,16 @@ } private function renderActionAsText( + PhabricatorUser $viewer, HeraldActionRecord $action, PhabricatorHandleList $handles) { + + $impl = $this->getActionImplementation($action->getAction()); + if ($impl) { + $value = $action->getTarget(); + return $impl->renderActionDescription($viewer, $value); + } + $rule_global = HeraldRuleTypeConfig::RULE_TYPE_GLOBAL; $action_type = $action->getAction(); @@ -1118,15 +1105,14 @@ HeraldActionRecord $action, PhabricatorHandleList $handles) { + // TODO: This should be driven through HeraldAction. + $target = $action->getTarget(); if (!is_array($target)) { $target = array($target); } foreach ($target as $index => $val) { switch ($action->getAction()) { - case self::ACTION_FLAG: - $target[$index] = PhabricatorFlagColor::getColorName($val); - break; default: $handle = $handles->getHandleIfExists($val); if ($handle) { @@ -1222,8 +1208,6 @@ case self::ACTION_ADD_CC: case self::ACTION_REMOVE_CC: return $this->applySubscribersEffect($effect); - case self::ACTION_FLAG: - return $this->applyFlagEffect($effect); case self::ACTION_EMAIL: return $this->applyEmailEffect($effect); default: @@ -1364,50 +1348,6 @@ return new HeraldApplyTranscript($effect, true, $message); } - - /** - * @task apply - */ - private function applyFlagEffect(HeraldEffect $effect) { - $phid = $this->getPHID(); - $color = $effect->getTarget(); - - $rule = $effect->getRule(); - $user = $rule->getAuthor(); - - $flag = PhabricatorFlagQuery::loadUserFlag($user, $phid); - if ($flag) { - return new HeraldApplyTranscript( - $effect, - false, - pht('Object already flagged.')); - } - - $handle = id(new PhabricatorHandleQuery()) - ->setViewer($user) - ->withPHIDs(array($phid)) - ->executeOne(); - - $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( - pht('Flagged by Herald Rule "%s".', $rule->getName())); - $flag->save(); - - return new HeraldApplyTranscript( - $effect, - true, - pht('Added flag.')); - } - - /** * @task apply */ diff --git a/src/applications/herald/application/PhabricatorHeraldApplication.php b/src/applications/herald/application/PhabricatorHeraldApplication.php --- a/src/applications/herald/application/PhabricatorHeraldApplication.php +++ b/src/applications/herald/application/PhabricatorHeraldApplication.php @@ -58,7 +58,7 @@ 'transcript/' => array( '' => 'HeraldTranscriptListController', '(?:query/(?P[^/]+)/)?' => 'HeraldTranscriptListController', - '(?P[1-9]\d*)/(?:(?P\w+)/)?' + '(?P[1-9]\d*)/' => 'HeraldTranscriptController', ), ), diff --git a/src/applications/herald/controller/HeraldRuleController.php b/src/applications/herald/controller/HeraldRuleController.php --- a/src/applications/herald/controller/HeraldRuleController.php +++ b/src/applications/herald/controller/HeraldRuleController.php @@ -367,7 +367,6 @@ $serial_actions = array(); foreach ($rule->getActions() as $action) { switch ($action->getAction()) { - case HeraldAdapter::ACTION_FLAG: case HeraldAdapter::ACTION_BLOCK: $current_value = $action->getTarget(); break; diff --git a/src/applications/herald/controller/HeraldTranscriptController.php b/src/applications/herald/controller/HeraldTranscriptController.php --- a/src/applications/herald/controller/HeraldTranscriptController.php +++ b/src/applications/herald/controller/HeraldTranscriptController.php @@ -2,43 +2,26 @@ final class HeraldTranscriptController extends HeraldController { - const FILTER_AFFECTED = 'affected'; - const FILTER_OWNED = 'owned'; - const FILTER_ALL = 'all'; - - private $id; - private $filter; private $handles; private $adapter; - public function willProcessRequest(array $data) { - $this->id = $data['id']; - $map = $this->getFilterMap(); - $this->filter = idx($data, 'filter'); - if (empty($map[$this->filter])) { - $this->filter = self::FILTER_ALL; - } - } - private function getAdapter() { return $this->adapter; } - public function processRequest() { - $request = $this->getRequest(); - $viewer = $request->getUser(); + public function handleRequest(AphrontRequest $request) { + $viewer = $this->getViewer(); $xscript = id(new HeraldTranscriptQuery()) ->setViewer($viewer) - ->withIDs(array($this->id)) + ->withIDs(array($request->getURIData('id'))) ->executeOne(); if (!$xscript) { return new Aphront404Response(); } require_celerity_resource('herald-test-css'); - - $nav = $this->buildSideNav(); + $content = array(); $object_xscript = $xscript->getObjectTranscript(); if (!$object_xscript) { @@ -49,7 +32,7 @@ 'p', array(), pht('Details of this transcript have been garbage collected.'))); - $nav->appendChild($notice); + $content[] = $notice; } else { $map = HeraldAdapter::getEnabledAdapterMap($viewer); $object_type = $object_xscript->getType(); @@ -65,9 +48,7 @@ $this->adapter = HeraldAdapter::getAdapterForContentType($object_type); - $filter = $this->getFilterPHIDs(); - $this->filterTranscript($xscript, $filter); - $phids = array_merge($filter, $this->getTranscriptPHIDs($xscript)); + $phids = $this->getTranscriptPHIDs($xscript); $phids = array_unique($phids); $phids = array_filter($phids); @@ -82,23 +63,16 @@ pht( 'This was a dry run to test Herald rules, '. 'no actions were executed.')); - $nav->appendChild($notice); + $content[] = $notice; } $warning_panel = $this->buildWarningPanel($xscript); - $nav->appendChild($warning_panel); - - $apply_xscript_panel = $this->buildApplyTranscriptPanel( - $xscript); - $nav->appendChild($apply_xscript_panel); + $content[] = $warning_panel; - $action_xscript_panel = $this->buildActionTranscriptPanel( - $xscript); - $nav->appendChild($action_xscript_panel); - - $object_xscript_panel = $this->buildObjectTranscriptPanel( - $xscript); - $nav->appendChild($object_xscript_panel); + $content[] = array( + $this->buildActionTranscriptPanel($xscript), + $this->buildObjectTranscriptPanel($xscript), + ); } $crumbs = id($this->buildApplicationCrumbs()) @@ -106,10 +80,12 @@ pht('Transcripts'), $this->getApplicationURI('/transcript/')) ->addTextCrumb($xscript->getID()); - $nav->setCrumbs($crumbs); return $this->buildApplicationPage( - $nav, + array( + $crumbs, + $content, + ), array( 'title' => pht('Transcript'), )); @@ -146,33 +122,6 @@ return phutil_tag('span', array('class' => 'condition-test-value'), $value); } - private function buildSideNav() { - $nav = new AphrontSideNavFilterView(); - $nav->setBaseURI(new PhutilURI('/herald/transcript/'.$this->id.'/')); - - $items = array(); - $filters = $this->getFilterMap(); - foreach ($filters as $key => $name) { - $nav->addFilter($key, $name); - } - $nav->selectFilter($this->filter, null); - - return $nav; - } - - protected function getFilterMap() { - return array( - self::FILTER_ALL => pht('All Rules'), - self::FILTER_OWNED => pht('Rules I Own'), - self::FILTER_AFFECTED => pht('Rules that Affected Me'), - ); - } - - - protected function getFilterPHIDs() { - return array($this->getRequest()->getUser()->getPHID()); - } - protected function getTranscriptPHIDs($xscript) { $phids = array(); @@ -228,71 +177,6 @@ return $phids; } - protected function filterTranscript($xscript, $filter_phids) { - $filter_owned = ($this->filter == self::FILTER_OWNED); - $filter_affected = ($this->filter == self::FILTER_AFFECTED); - - if (!$filter_owned && !$filter_affected) { - // No filtering to be done. - return; - } - - if (!$xscript->getObjectTranscript()) { - return; - } - - $user_phid = $this->getRequest()->getUser()->getPHID(); - - $keep_apply_xscripts = array(); - $keep_rule_xscripts = array(); - - $filter_phids = array_fill_keys($filter_phids, true); - - $rule_xscripts = $xscript->getRuleTranscripts(); - foreach ($xscript->getApplyTranscripts() as $id => $apply_xscript) { - $rule_id = $apply_xscript->getRuleID(); - if ($filter_owned) { - if (empty($rule_xscripts[$rule_id])) { - // No associated rule so you can't own this effect. - continue; - } - if ($rule_xscripts[$rule_id]->getRuleOwner() != $user_phid) { - continue; - } - } else if ($filter_affected) { - $targets = (array)$apply_xscript->getTarget(); - if (!array_select_keys($filter_phids, $targets)) { - continue; - } - } - $keep_apply_xscripts[$id] = true; - if ($rule_id) { - $keep_rule_xscripts[$rule_id] = true; - } - } - - foreach ($rule_xscripts as $rule_id => $rule_xscript) { - if ($filter_owned && $rule_xscript->getRuleOwner() == $user_phid) { - $keep_rule_xscripts[$rule_id] = true; - } - } - - $xscript->setRuleTranscripts( - array_intersect_key( - $xscript->getRuleTranscripts(), - $keep_rule_xscripts)); - - $xscript->setApplyTranscripts( - array_intersect_key( - $xscript->getApplyTranscripts(), - $keep_apply_xscripts)); - - $xscript->setConditionTranscripts( - array_intersect_key( - $xscript->getConditionTranscripts(), - $keep_rule_xscripts)); - } - private function buildWarningPanel(HeraldTranscript $xscript) { $request = $this->getRequest(); $panel = null; @@ -333,165 +217,185 @@ return $panel; } - private function buildApplyTranscriptPanel(HeraldTranscript $xscript) { - $handles = $this->handles; + private function buildActionTranscriptPanel(HeraldTranscript $xscript) { + $action_xscript = mgroup($xscript->getApplyTranscripts(), 'getRuleID'); + $adapter = $this->getAdapter(); - $rule_type_global = HeraldRuleTypeConfig::RULE_TYPE_GLOBAL; - $action_names = $adapter->getActionNameMap($rule_type_global); + $field_names = $adapter->getFieldNameMap(); + $condition_names = $adapter->getConditionNameMap(); - $list = new PHUIObjectItemListView(); - $list->setStates(true); - $list->setNoDataString(pht('No actions were taken.')); - foreach ($xscript->getApplyTranscripts() as $apply_xscript) { + $handles = $this->handles; - $target = $apply_xscript->getTarget(); - switch ($apply_xscript->getAction()) { - case HeraldAdapter::ACTION_FLAG: - $target = PhabricatorFlagColor::getColorName($target); - break; - case HeraldAdapter::ACTION_BLOCK: - // Target is a text string. - $target = $target; - break; - default: - // TODO: This should be driven by HeraldActions. + $action_map = $xscript->getApplyTranscripts(); + $action_map = mgroup($action_map, 'getRuleID'); - if (is_array($target) && $target) { - foreach ($target as $k => $phid) { - if (isset($handles[$phid])) { - $target[$k] = $handles[$phid]->getName(); - } - } - $target = implode(', ', $target); - } else if (is_string($target)) { - $target = $target; - } else { - $target = ''; - } - break; - } + $rule_list = id(new PHUIObjectItemListView()) + ->setNoDataString(pht('No Herald rules applied to this object.')); - $item = new PHUIObjectItemView(); + foreach ($xscript->getRuleTranscripts() as $rule_xscript) { + $rule_id = $rule_xscript->getRuleID(); - if ($apply_xscript->getApplied()) { - $item->setState(PHUIObjectItemView::STATE_SUCCESS); - } else { - $item->setState(PHUIObjectItemView::STATE_FAIL); + $rule_item = id(new PHUIObjectItemView()) + ->setObjectName(pht('H%d', $rule_id)) + ->setHeader($rule_xscript->getRuleName()); + + if (!$rule_xscript->getResult()) { + $rule_item->setDisabled(true); } - $rule = idx( - $action_names, - $apply_xscript->getAction(), - pht('Unknown Action "%s"', $apply_xscript->getAction())); + $rule_list->addItem($rule_item); - $item->setHeader(pht('%s: %s', $rule, $target)); - $item->addAttribute($apply_xscript->getReason()); + // Build the field/condition transcript. - // TODO: This is a bit of a mess while actions convert. + $cond_xscripts = $xscript->getConditionTranscriptsForRule($rule_id); - $item->addAttribute( - pht('Outcome: %s', $apply_xscript->getAppliedReason())); + $cond_list = id(new PHUIStatusListView()); + $cond_list->addItem( + id(new PHUIStatusItemView()) + ->setTarget(phutil_tag('strong', array(), pht('Conditions')))); - $list->addItem($item); - } + foreach ($cond_xscripts as $cond_xscript) { + if ($cond_xscript->getResult()) { + $icon = 'fa-check'; + $color = 'green'; + $result = pht('Passed'); + } else { + $icon = 'fa-times'; + $color = 'red'; + $result = pht('Failed'); + } - $box = new PHUIObjectBoxView(); - $box->setHeaderText(pht('Actions Taken')); - $box->appendChild($list); + if ($cond_xscript->getNote()) { + $note = phutil_tag( + 'div', + array( + 'class' => 'herald-condition-note', + ), + $cond_xscript->getNote()); + } else { + $note = null; + } - return $box; - } + // TODO: This is not really translatable and should be driven through + // HeraldField. + $explanation = pht( + '%s %s %s', + idx($field_names, $cond_xscript->getFieldName(), pht('Unknown')), + idx($condition_names, $cond_xscript->getCondition(), pht('Unknown')), + $this->renderConditionTestValue($cond_xscript, $handles)); - private function buildActionTranscriptPanel(HeraldTranscript $xscript) { - $action_xscript = mgroup($xscript->getApplyTranscripts(), 'getRuleID'); + $cond_item = id(new PHUIStatusItemView()) + ->setIcon($icon, $color) + ->setTarget($result) + ->setNote(array($explanation, $note)); - $adapter = $this->getAdapter(); + $cond_list->addItem($cond_item); + } + if ($rule_xscript->getResult()) { + $last_icon = 'fa-check-circle'; + $last_color = 'green'; + $last_result = pht('Passed'); + $last_note = pht('Rule passed.'); + } else { + $last_icon = 'fa-times-circle'; + $last_color = 'red'; + $last_result = pht('Failed'); + $last_note = pht('Rule failed.'); + } - $field_names = $adapter->getFieldNameMap(); - $condition_names = $adapter->getConditionNameMap(); + $cond_last = id(new PHUIStatusItemView()) + ->setIcon($last_icon, $last_color) + ->setTarget(phutil_tag('strong', array(), $last_result)) + ->setNote($last_note); + $cond_list->addItem($cond_last); - $handles = $this->handles; + $cond_box = id(new PHUIBoxView()) + ->appendChild($cond_list) + ->addMargin(PHUI::MARGIN_LARGE_LEFT); + + $rule_item->appendChild($cond_box); + + if (!$rule_xscript->getResult()) { + // If the rule didn't pass, don't generate an action transcript since + // actions didn't apply. + continue; + } + + $cond_box->addMargin(PHUI::MARGIN_MEDIUM_BOTTOM); + + $action_xscripts = idx($action_map, $rule_id, array()); + foreach ($action_xscripts as $action_xscript) { + $action_key = $action_xscript->getAction(); + $action = $adapter->getActionImplementation($action_key); - $rule_markup = array(); - foreach ($xscript->getRuleTranscripts() as $rule_id => $rule) { - $cond_markup = array(); - foreach ($xscript->getConditionTranscriptsForRule($rule_id) as $cond) { - if ($cond->getNote()) { - $note = phutil_tag_div('herald-condition-note', $cond->getNote()); + if ($action) { + $name = $action->getHeraldActionName(); + $action->setViewer($this->getViewer()); } else { - $note = null; + $name = pht('Unknown Action ("%s")', $action_key); } - if ($cond->getResult()) { - $result = phutil_tag( - 'span', - array('class' => 'herald-outcome condition-pass'), - "\xE2\x9C\x93"); - } else { - $result = phutil_tag( - 'span', - array('class' => 'herald-outcome condition-fail'), - "\xE2\x9C\x98"); + $name = pht('Action: %s', $name); + + $action_list = id(new PHUIStatusListView()); + $action_list->addItem( + id(new PHUIStatusItemView()) + ->setTarget(phutil_tag('strong', array(), $name))); + + $action_box = id(new PHUIBoxView()) + ->appendChild($action_list) + ->addMargin(PHUI::MARGIN_LARGE_LEFT); + + $rule_item->appendChild($action_box); + + $log = $action_xscript->getAppliedReason(); + + // Handle older transcripts which used a static string to record + // action results. + if (!is_array($log)) { + $action_list->addItem( + id(new PHUIStatusItemView()) + ->setIcon('fa-clock-o', 'grey') + ->setTarget(pht('Old Transcript')) + ->setNote( + pht( + 'This is an old transcript which uses an obsolete log '. + 'format. Detailed action information is not available.'))); + continue; } - $cond_markup[] = phutil_tag( - 'li', - array(), - pht( - '%s Condition: %s %s %s%s', - $result, - idx($field_names, $cond->getFieldName(), pht('Unknown')), - idx($condition_names, $cond->getCondition(), pht('Unknown')), - $this->renderConditionTestValue($cond, $handles), - $note)); - } + foreach ($log as $entry) { + $type = idx($entry, 'type'); + $data = idx($entry, 'data'); - if ($rule->getResult()) { - $result = phutil_tag( - 'span', - array('class' => 'herald-outcome rule-pass'), - pht('PASS')); - $class = 'herald-rule-pass'; - } else { - $result = phutil_tag( - 'span', - array('class' => 'herald-outcome rule-fail'), - pht('FAIL')); - $class = 'herald-rule-fail'; - } + if ($action) { + $icon = $action->renderActionEffectIcon($type, $data); + $color = $action->renderActionEffectColor($type, $data); + $name = $action->renderActionEffectName($type, $data); + $note = $action->renderActionEffectDescription($type, $data); + } else { + $icon = 'fa-question-circle'; + $color = 'indigo'; + $name = pht('Unknown Effect ("%s")', $type); + $note = null; + } - $cond_markup[] = phutil_tag( - 'li', - array(), - array($result, $rule->getReason())); - $user_phid = $this->getRequest()->getUser()->getPHID(); - - $name = $rule->getRuleName(); - - $rule_markup[] = - phutil_tag( - 'li', - array( - 'class' => $class, - ), - phutil_tag_div('rule-name', array( - phutil_tag('strong', array(), $name), - ' ', - phutil_tag('ul', array(), $cond_markup), - ))); - } + $action_item = id(new PHUIStatusItemView()) + ->setIcon($icon, $color) + ->setTarget($name) + ->setNote($note); - $box = null; - if ($rule_markup) { - $box = new PHUIObjectBoxView(); - $box->setHeaderText(pht('Rule Details')); - $box->appendChild(phutil_tag( - 'ul', - array('class' => 'herald-explain-list'), - $rule_markup)); + $action_list->addItem($action_item); + } + } } + + $box = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Rule Transcript')) + ->appendChild($rule_list); + return $box; } diff --git a/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php b/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php --- a/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php +++ b/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php @@ -84,7 +84,6 @@ self::ACTION_ADD_CC, self::ACTION_REMOVE_CC, self::ACTION_EMAIL, - self::ACTION_FLAG, self::ACTION_ASSIGN_TASK, ), parent::getActions($rule_type)); diff --git a/src/applications/pholio/herald/HeraldPholioMockAdapter.php b/src/applications/pholio/herald/HeraldPholioMockAdapter.php --- a/src/applications/pholio/herald/HeraldPholioMockAdapter.php +++ b/src/applications/pholio/herald/HeraldPholioMockAdapter.php @@ -61,7 +61,6 @@ array( self::ACTION_ADD_CC, self::ACTION_REMOVE_CC, - self::ACTION_FLAG, ), parent::getActions($rule_type)); } diff --git a/src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php b/src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php --- a/src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php +++ b/src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php @@ -64,7 +64,6 @@ self::ACTION_ADD_CC, self::ACTION_REMOVE_CC, self::ACTION_EMAIL, - self::ACTION_FLAG, ), parent::getActions($rule_type)); } diff --git a/webroot/rsrc/css/application/herald/herald-test.css b/webroot/rsrc/css/application/herald/herald-test.css --- a/webroot/rsrc/css/application/herald/herald-test.css +++ b/webroot/rsrc/css/application/herald/herald-test.css @@ -2,79 +2,10 @@ * @provides herald-test-css */ -ul.herald-explain-list { - margin: 12px; -} - -div.herald-condition-note { - clear: both; - margin: .5em 0em .53em 86px; - padding: .5em 1em; - background: #FFFF00; - font-weight: bold; -} - -ul.herald-explain-list .herald-outcome { - margin-right: 6px; - width: 50px; - text-align: center; - position: relative; - float: left; - font-weight: bold; - padding: 1px 2px; -} - -ul.herald-explain-list .condition-fail, -ul.herald-explain-list .rule-fail { +.herald-condition-note { color: {$red}; } -ul.herald-explain-list .condition-pass, -ul.herald-explain-list .rule-pass { - color: {$green}; -} - -ul.herald-explain-list li.herald-rule-pass, -ul.herald-explain-list li.herald-rule-fail { - margin: 0 0 8px; -} - -ul.herald-explain-list div.rule-name { - padding: 4px 0 8px 0; -} - -ul.herald-explain-list li.herald-rule-fail, -ul.herald-explain-list li.herald-rule-pass { - background: #ffffff; -} - -ul.herald-explain-list ul { - margin: 4px; -} - -ul.herald-explain-list ul li { - padding: 2px 0; -} - -.outcome-failure, -.outcome-success { - font-weight: bold; -} - -.outcome-failure { - color: {$red}; -} - -.outcome-success { - color: {$green}; -} - -.action-header { - font-weight: bold; - padding-top: 12px; - border-bottom: 1px solid {$thinblueborder}; -} - textarea.herald-field-value-transcript { width: 100%; height: 10em;