Page MenuHomePhabricator

D20288.id48490.diff
No OneTemporary

D20288.id48490.diff

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
@@ -4171,16 +4171,22 @@
'PhabricatorProjectTransactionType' => 'applications/project/xaction/PhabricatorProjectTransactionType.php',
'PhabricatorProjectTrigger' => 'applications/project/storage/PhabricatorProjectTrigger.php',
'PhabricatorProjectTriggerController' => 'applications/project/controller/trigger/PhabricatorProjectTriggerController.php',
+ 'PhabricatorProjectTriggerCorruptionException' => 'applications/project/exception/PhabricatorProjectTriggerCorruptionException.php',
'PhabricatorProjectTriggerEditController' => 'applications/project/controller/trigger/PhabricatorProjectTriggerEditController.php',
'PhabricatorProjectTriggerEditor' => 'applications/project/editor/PhabricatorProjectTriggerEditor.php',
+ 'PhabricatorProjectTriggerInvalidRule' => 'applications/project/trigger/PhabricatorProjectTriggerInvalidRule.php',
'PhabricatorProjectTriggerListController' => 'applications/project/controller/trigger/PhabricatorProjectTriggerListController.php',
+ 'PhabricatorProjectTriggerManiphestStatusRule' => 'applications/project/trigger/PhabricatorProjectTriggerManiphestStatusRule.php',
'PhabricatorProjectTriggerNameTransaction' => 'applications/project/xaction/trigger/PhabricatorProjectTriggerNameTransaction.php',
'PhabricatorProjectTriggerPHIDType' => 'applications/project/phid/PhabricatorProjectTriggerPHIDType.php',
'PhabricatorProjectTriggerQuery' => 'applications/project/query/PhabricatorProjectTriggerQuery.php',
+ 'PhabricatorProjectTriggerRule' => 'applications/project/trigger/PhabricatorProjectTriggerRule.php',
+ 'PhabricatorProjectTriggerRuleRecord' => 'applications/project/trigger/PhabricatorProjectTriggerRuleRecord.php',
'PhabricatorProjectTriggerSearchEngine' => 'applications/project/query/PhabricatorProjectTriggerSearchEngine.php',
'PhabricatorProjectTriggerTransaction' => 'applications/project/storage/PhabricatorProjectTriggerTransaction.php',
'PhabricatorProjectTriggerTransactionQuery' => 'applications/project/query/PhabricatorProjectTriggerTransactionQuery.php',
'PhabricatorProjectTriggerTransactionType' => 'applications/project/xaction/trigger/PhabricatorProjectTriggerTransactionType.php',
+ 'PhabricatorProjectTriggerUnknownRule' => 'applications/project/trigger/PhabricatorProjectTriggerUnknownRule.php',
'PhabricatorProjectTriggerViewController' => 'applications/project/controller/trigger/PhabricatorProjectTriggerViewController.php',
'PhabricatorProjectTypeTransaction' => 'applications/project/xaction/PhabricatorProjectTypeTransaction.php',
'PhabricatorProjectUIEventListener' => 'applications/project/events/PhabricatorProjectUIEventListener.php',
@@ -10293,16 +10299,22 @@
'PhabricatorDestructibleInterface',
),
'PhabricatorProjectTriggerController' => 'PhabricatorProjectController',
+ 'PhabricatorProjectTriggerCorruptionException' => 'Exception',
'PhabricatorProjectTriggerEditController' => 'PhabricatorProjectTriggerController',
'PhabricatorProjectTriggerEditor' => 'PhabricatorApplicationTransactionEditor',
+ 'PhabricatorProjectTriggerInvalidRule' => 'PhabricatorProjectTriggerRule',
'PhabricatorProjectTriggerListController' => 'PhabricatorProjectTriggerController',
+ 'PhabricatorProjectTriggerManiphestStatusRule' => 'PhabricatorProjectTriggerRule',
'PhabricatorProjectTriggerNameTransaction' => 'PhabricatorProjectTriggerTransactionType',
'PhabricatorProjectTriggerPHIDType' => 'PhabricatorPHIDType',
'PhabricatorProjectTriggerQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+ 'PhabricatorProjectTriggerRule' => 'Phobject',
+ 'PhabricatorProjectTriggerRuleRecord' => 'Phobject',
'PhabricatorProjectTriggerSearchEngine' => 'PhabricatorApplicationSearchEngine',
'PhabricatorProjectTriggerTransaction' => 'PhabricatorModularTransaction',
'PhabricatorProjectTriggerTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
'PhabricatorProjectTriggerTransactionType' => 'PhabricatorModularTransactionType',
+ 'PhabricatorProjectTriggerUnknownRule' => 'PhabricatorProjectTriggerRule',
'PhabricatorProjectTriggerViewController' => 'PhabricatorProjectTriggerController',
'PhabricatorProjectTypeTransaction' => 'PhabricatorProjectTransactionType',
'PhabricatorProjectUIEventListener' => 'PhabricatorEventListener',
diff --git a/src/applications/project/controller/PhabricatorProjectBoardViewController.php b/src/applications/project/controller/PhabricatorProjectBoardViewController.php
--- a/src/applications/project/controller/PhabricatorProjectBoardViewController.php
+++ b/src/applications/project/controller/PhabricatorProjectBoardViewController.php
@@ -1269,6 +1269,7 @@
array(
'items' => hsprintf('%s', $trigger_menu),
'tip' => $trigger_tip,
+ 'size' => 300,
));
return $trigger_button;
diff --git a/src/applications/project/controller/PhabricatorProjectMoveController.php b/src/applications/project/controller/PhabricatorProjectMoveController.php
--- a/src/applications/project/controller/PhabricatorProjectMoveController.php
+++ b/src/applications/project/controller/PhabricatorProjectMoveController.php
@@ -70,6 +70,7 @@
$columns = id(new PhabricatorProjectColumnQuery())
->setViewer($viewer)
->withProjectPHIDs(array($project->getPHID()))
+ ->needTriggers(true)
->execute();
$columns = mpull($columns, null, 'getPHID');
@@ -110,6 +111,19 @@
$xactions[] = $header_xaction;
}
+ if ($column->canHaveTrigger()) {
+ $trigger = $column->getTrigger();
+ if ($trigger) {
+ $trigger_xactions = $trigger->newDropTransactions(
+ $viewer,
+ $column,
+ $object);
+ foreach ($trigger_xactions as $trigger_xaction) {
+ $xactions[] = $trigger_xaction;
+ }
+ }
+ }
+
$editor = id(new ManiphestTransactionEditor())
->setActor($viewer)
->setContinueOnMissingFields(true)
diff --git a/src/applications/project/exception/PhabricatorProjectTriggerCorruptionException.php b/src/applications/project/exception/PhabricatorProjectTriggerCorruptionException.php
new file mode 100644
--- /dev/null
+++ b/src/applications/project/exception/PhabricatorProjectTriggerCorruptionException.php
@@ -0,0 +1,4 @@
+<?php
+
+final class PhabricatorProjectTriggerCorruptionException
+ extends Exception {}
diff --git a/src/applications/project/storage/PhabricatorProjectTrigger.php b/src/applications/project/storage/PhabricatorProjectTrigger.php
--- a/src/applications/project/storage/PhabricatorProjectTrigger.php
+++ b/src/applications/project/storage/PhabricatorProjectTrigger.php
@@ -11,6 +11,8 @@
protected $ruleset = array();
protected $editPolicy;
+ private $triggerRules;
+
public static function initializeNewTrigger() {
$default_edit = PhabricatorPolicies::POLICY_USER;
@@ -60,11 +62,182 @@
return pht('Trigger %d', $this->getID());
}
+ public function getTriggerRules() {
+ if ($this->triggerRules === null) {
+
+ // TODO: Temporary hard-coded rule specification.
+ $rule_specifications = array(
+ array(
+ 'type' => 'status',
+ 'value' => ManiphestTaskStatus::getDefaultClosedStatus(),
+ ),
+ // This is an intentionally unknown rule.
+ array(
+ 'type' => 'quack',
+ 'value' => 'aaa',
+ ),
+ // This is an intentionally invalid rule.
+ array(
+ 'type' => 'status',
+ 'value' => 'quack',
+ ),
+ );
+
+ // NOTE: We're trying to preserve the database state in the rule
+ // structure, even if it includes rule types we don't have implementations
+ // for, or rules with invalid rule values.
+
+ // If an administrator adds or removes extensions which add rules, or
+ // an upgrade affects rule validity, existing rules may become invalid.
+ // When they do, we still want the UI to reflect the ruleset state
+ // accurately and "Edit" + "Save" shouldn't destroy data unless the
+ // user explicitly modifies the ruleset.
+
+ // When we run into rules which are structured correctly but which have
+ // types we don't know about, we replace them with "Unknown Rules". If
+ // we know about the type of a rule but the value doesn't validate, we
+ // replace it with "Invalid Rules". These two rule types don't take any
+ // actions when a card is dropped into the column, but they show the user
+ // what's wrong with the ruleset and can be saved without causing any
+ // collateral damage.
+
+ $rule_map = PhabricatorProjectTriggerRule::getAllTriggerRules();
+
+ // If the stored rule data isn't a list of rules (or we encounter other
+ // fundamental structural problems, below), there isn't much we can do
+ // to try to represent the state.
+ if (!is_array($rule_specifications)) {
+ throw new PhabricatorProjectTriggerCorruptionException(
+ pht(
+ 'Trigger ("%s") has a corrupt ruleset: expected a list of '.
+ 'rule specifications, found "%s".',
+ $this->getPHID(),
+ phutil_describe_type($rule_specifications)));
+ }
+
+ $trigger_rules = array();
+ foreach ($rule_specifications as $key => $rule) {
+ if (!is_array($rule)) {
+ throw new PhabricatorProjectTriggerCorruptionException(
+ pht(
+ 'Trigger ("%s") has a corrupt ruleset: rule (with key "%s") '.
+ 'should be a rule specification, but is actually "%s".',
+ $this->getPHID(),
+ $key,
+ phutil_describe_type($rule)));
+ }
+
+ try {
+ PhutilTypeSpec::checkMap(
+ $rule,
+ array(
+ 'type' => 'string',
+ 'value' => 'wild',
+ ));
+ } catch (PhutilTypeCheckException $ex) {
+ throw new PhabricatorProjectTriggerCorruptionException(
+ pht(
+ 'Trigger ("%s") has a corrupt ruleset: rule (with key "%s") '.
+ 'is not a valid rule specification: %s',
+ $this->getPHID(),
+ $key,
+ $ex->getMessage()));
+ }
+
+ $record = id(new PhabricatorProjectTriggerRuleRecord())
+ ->setType(idx($rule, 'type'))
+ ->setValue(idx($rule, 'value'));
+
+ if (!isset($rule_map[$record->getType()])) {
+ $rule = new PhabricatorProjectTriggerUnknownRule();
+ } else {
+ $rule = clone $rule_map[$record->getType()];
+ }
+
+ try {
+ $rule->setRecord($record);
+ } catch (Exception $ex) {
+ $rule = id(new PhabricatorProjectTriggerInvalidRule())
+ ->setRecord($record);
+ }
+
+ $trigger_rules[] = $rule;
+ }
+
+ $this->triggerRules = $trigger_rules;
+ }
+
+ return $this->triggerRules;
+ }
+
public function getRulesDescription() {
- // TODO: Summarize the trigger rules in human-readable text.
- return pht('Does things.');
+ $rules = $this->getTriggerRules();
+ if (!$rules) {
+ return pht('Does nothing.');
+ }
+
+ $things = array();
+
+ $count = count($rules);
+ $limit = 3;
+
+ if ($count > $limit) {
+ $show_rules = array_slice($rules, 0, ($limit - 1));
+ } else {
+ $show_rules = $rules;
+ }
+
+ foreach ($show_rules as $rule) {
+ $things[] = $rule->getDescription();
+ }
+
+ if ($count > $limit) {
+ $things[] = pht(
+ '(Applies %s more actions.)',
+ new PhutilNumber($count - $limit));
+ }
+
+ return implode("\n", $things);
}
+ public function newDropTransactions(
+ PhabricatorUser $viewer,
+ PhabricatorProjectColumn $column,
+ $object) {
+
+ $trigger_xactions = array();
+ foreach ($this->getTriggerRules() as $rule) {
+ $rule
+ ->setViewer($viewer)
+ ->setTrigger($this)
+ ->setColumn($column)
+ ->setObject($object);
+
+ $xactions = $rule->getDropTransactions(
+ $object,
+ $rule->getRecord()->getValue());
+
+ if (!is_array($xactions)) {
+ throw new Exception(
+ pht(
+ 'Expected trigger rule (of class "%s") to return a list of '.
+ 'transactions from "newDropTransactions()", but got "%s".',
+ get_class($rule),
+ phutil_describe_type($xactions)));
+ }
+
+ $expect_type = get_class($object->getApplicationTransactionTemplate());
+ assert_instances_of($xactions, $expect_type);
+
+ foreach ($xactions as $xaction) {
+ $trigger_xactions[] = $xaction;
+ }
+ }
+
+ return $trigger_xactions;
+ }
+
+
/* -( PhabricatorApplicationTransactionInterface )------------------------- */
diff --git a/src/applications/project/trigger/PhabricatorProjectTriggerInvalidRule.php b/src/applications/project/trigger/PhabricatorProjectTriggerInvalidRule.php
new file mode 100644
--- /dev/null
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerInvalidRule.php
@@ -0,0 +1,22 @@
+<?php
+
+final class PhabricatorProjectTriggerInvalidRule
+ extends PhabricatorProjectTriggerRule {
+
+ const TRIGGERTYPE = 'invalid';
+
+ public function getDescription() {
+ return pht(
+ 'Invalid rule (of type "%s").',
+ $this->getRecord()->getType());
+ }
+
+ protected function assertValidRuleValue($value) {
+ return;
+ }
+
+ protected function newDropTransactions($object, $value) {
+ return array();
+ }
+
+}
diff --git a/src/applications/project/trigger/PhabricatorProjectTriggerManiphestStatusRule.php b/src/applications/project/trigger/PhabricatorProjectTriggerManiphestStatusRule.php
new file mode 100644
--- /dev/null
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerManiphestStatusRule.php
@@ -0,0 +1,41 @@
+<?php
+
+final class PhabricatorProjectTriggerManiphestStatusRule
+ extends PhabricatorProjectTriggerRule {
+
+ const TRIGGERTYPE = 'task.status';
+
+ public function getDescription() {
+ $value = $this->getValue();
+
+ return pht(
+ 'Changes status to "%s".',
+ ManiphestTaskStatus::getTaskStatusName($value));
+ }
+
+ protected function assertValidRuleValue($value) {
+ if (!is_string($value)) {
+ throw new Exception(
+ pht(
+ 'Status rule value should be a string, but is not (value is "%s").',
+ phutil_describe_type($value)));
+ }
+
+ $map = ManiphestTaskStatus::getTaskStatusMap();
+ if (!isset($map[$value])) {
+ throw new Exception(
+ pht(
+ 'Rule value ("%s") is not a valid task status.',
+ $value));
+ }
+ }
+
+ protected function newDropTransactions($object, $value) {
+ return array(
+ $this->newTransaction()
+ ->setTransactionType(ManiphestTaskStatusTransaction::TRANSACTIONTYPE)
+ ->setNewValue($value),
+ );
+ }
+
+}
diff --git a/src/applications/project/trigger/PhabricatorProjectTriggerRule.php b/src/applications/project/trigger/PhabricatorProjectTriggerRule.php
new file mode 100644
--- /dev/null
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerRule.php
@@ -0,0 +1,89 @@
+<?php
+
+abstract class PhabricatorProjectTriggerRule
+ extends Phobject {
+
+ private $record;
+ private $viewer;
+ private $column;
+ private $trigger;
+ private $object;
+
+ final public function getTriggerType() {
+ return $this->getPhobjectClassConstant('TRIGGERTYPE', 64);
+ }
+
+ final public static function getAllTriggerRules() {
+ return id(new PhutilClassMapQuery())
+ ->setAncestorClass(__CLASS__)
+ ->setUniqueMethod('getTriggerType')
+ ->execute();
+ }
+
+ final public function setRecord(PhabricatorProjectTriggerRuleRecord $record) {
+ $value = $record->getValue();
+
+ $this->assertValidRuleValue($value);
+
+ $this->record = $record;
+ return $this;
+ }
+
+ final public function getRecord() {
+ return $this->record;
+ }
+
+ final protected function getValue() {
+ return $this->getRecord()->getValue();
+ }
+
+ abstract public function getDescription();
+ abstract protected function assertValidRuleValue($value);
+ abstract protected function newDropTransactions($object, $value);
+
+ final public function getDropTransactions($object, $value) {
+ return $this->newDropTransactions($object, $value);
+ }
+
+ final public function setViewer(PhabricatorUser $viewer) {
+ $this->viewer = $viewer;
+ return $this;
+ }
+
+ final public function getViewer() {
+ return $this->viewer;
+ }
+
+ final public function setColumn(PhabricatorProjectColumn $column) {
+ $this->column = $column;
+ return $this;
+ }
+
+ final public function getColumn() {
+ return $this->column;
+ }
+
+ final public function setTrigger(PhabricatorProjectTrigger $trigger) {
+ $this->trigger = $trigger;
+ return $this;
+ }
+
+ final public function getTrigger() {
+ return $this->trigger;
+ }
+
+ final public function setObject(
+ PhabricatorApplicationTransactionInterface $object) {
+ $this->object = $object;
+ return $this;
+ }
+
+ final public function getObject() {
+ return $this->object;
+ }
+
+ final protected function newTransaction() {
+ return $this->getObject()->getApplicationTransactionTemplate();
+ }
+
+}
diff --git a/src/applications/project/trigger/PhabricatorProjectTriggerRuleRecord.php b/src/applications/project/trigger/PhabricatorProjectTriggerRuleRecord.php
new file mode 100644
--- /dev/null
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerRuleRecord.php
@@ -0,0 +1,27 @@
+<?php
+
+final class PhabricatorProjectTriggerRuleRecord
+ extends Phobject {
+
+ private $type;
+ private $value;
+
+ public function setType($type) {
+ $this->type = $type;
+ return $this;
+ }
+
+ public function getType() {
+ return $this->type;
+ }
+
+ public function setValue($value) {
+ $this->value = $value;
+ return $this;
+ }
+
+ public function getValue() {
+ return $this->value;
+ }
+
+}
diff --git a/src/applications/project/trigger/PhabricatorProjectTriggerUnknownRule.php b/src/applications/project/trigger/PhabricatorProjectTriggerUnknownRule.php
new file mode 100644
--- /dev/null
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerUnknownRule.php
@@ -0,0 +1,22 @@
+<?php
+
+final class PhabricatorProjectTriggerUnknownRule
+ extends PhabricatorProjectTriggerRule {
+
+ const TRIGGERTYPE = 'unknown';
+
+ public function getDescription() {
+ return pht(
+ 'Unknown rule (of type "%s").',
+ $this->getRecord()->getType());
+ }
+
+ protected function assertValidRuleValue($value) {
+ return;
+ }
+
+ protected function newDropTransactions($object, $value) {
+ return array();
+ }
+
+}

File Metadata

Mime Type
text/plain
Expires
Sat, Dec 28, 5:54 AM (7 h, 23 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6934619
Default Alt Text
D20288.id48490.diff (18 KB)

Event Timeline