D13649.id32996.diff
No OneTemporary
Actions

Size

36 KB

Referenced Files

None

Subscribers

None

D13649.id32996.diff
View Options

	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
	@@ -2071,6 +2071,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',
	@@ -5871,6 +5872,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 @@
	+<?php
	+
	+final class PhabricatorFlagAddFlagHeraldAction extends HeraldAction {
	+
	+ const ACTIONCONST = 'flag';
	+
	+ const DO_FLAG = 'do.flag';
	+ const DO_IGNORE = 'do.flagged';
	+
	+ public function getHeraldActionName() {
	+ return pht('Mark with flag');
	+ }
	+
	+ public function getActionGroupKey() {
	+ return HeraldSupportActionGroup::ACTIONGROUPKEY;
	+ }
	+
	+ public function supportsObject($object) {
	+ return ($object instanceof PhabricatorFlaggableInterface);
	+ }
	+
	+ public function supportsRuleType($rule_type) {
	+ return ($rule_type == HeraldRuleTypeConfig::RULE_TYPE_PERSONAL);
	+ }
	+
	+ public function applyEffect($object, HeraldEffect $effect) {
	+ $phid = $this->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<queryKey>[^/]+)/)?' => 'HeraldTranscriptListController',
	- '(?P<id>[1-9]\d*)/(?:(?P<filter>\w+)/)?'
	+ '(?P<id>[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
	@@ -375,7 +375,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 = '<empty>';
	- }
	- 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;

File Metadata

Mime Type: text/plain
Expires: Sun, Mar 16, 4:27 AM (1 w, 16 h ago)
Storage Engine: amazon-s3
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: phabricator/secure/kx/4c/gxjdljract4lqjxk
Default Alt Text: D13649.id32996.diff (36 KB)

D13649.id32996.diffNo OneTemporaryActions

D13649.id32996.diffView Options

File Metadata

Event Timeline

D13649.id32996.diff
No OneTemporary
Actions

D13649.id32996.diff
View Options