Page MenuHomePhabricator
Files F15309765

D7853.diff
No OneTemporary
Actions

D7853.diff
View Options

Index: src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
===================================================================
--- src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
+++ src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
@@ -44,14 +44,33 @@
public function supportsRuleType($rule_type) {
switch ($rule_type) {
case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL:
+ case HeraldRuleTypeConfig::RULE_TYPE_OBJECT:
return true;
case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
- case HeraldRuleTypeConfig::RULE_TYPE_OBJECT:
default:
return false;
}
}
+ public function canTriggerOnObject($object) {
+ if ($object instanceof PhabricatorRepository) {
+ return true;
+ }
+ return false;
+ }
+
+ public function explainValidTriggerObjects() {
+ return pht(
+ 'This rule can trigger for **repositories**.');
+ }
+
+ public function getTriggerObjectPHIDs() {
+ return array(
+ $this->hookEngine->getRepository()->getPHID(),
+ $this->getPHID(),
+ );
+ }
+
public function getFieldNameMap() {
return array(
) + parent::getFieldNameMap();
@@ -92,6 +111,7 @@
public function getActions($rule_type) {
switch ($rule_type) {
case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL:
+ case HeraldRuleTypeConfig::RULE_TYPE_OBJECT:
return array(
self::ACTION_BLOCK,
self::ACTION_NOTHING
Index: src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php
===================================================================
--- src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php
+++ src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php
@@ -47,14 +47,33 @@
public function supportsRuleType($rule_type) {
switch ($rule_type) {
case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL:
+ case HeraldRuleTypeConfig::RULE_TYPE_OBJECT:
return true;
case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
- case HeraldRuleTypeConfig::RULE_TYPE_OBJECT:
default:
return false;
}
}
+ public function canTriggerOnObject($object) {
+ if ($object instanceof PhabricatorRepository) {
+ return true;
+ }
+ return false;
+ }
+
+ public function explainValidTriggerObjects() {
+ return pht(
+ 'This rule can trigger for **repositories**.');
+ }
+
+ public function getTriggerObjectPHIDs() {
+ return array(
+ $this->hookEngine->getRepository()->getPHID(),
+ $this->getPHID(),
+ );
+ }
+
public function getFieldNameMap() {
return array(
self::FIELD_REF_TYPE => pht('Ref type'),
@@ -103,6 +122,7 @@
public function getActions($rule_type) {
switch ($rule_type) {
case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL:
+ case HeraldRuleTypeConfig::RULE_TYPE_OBJECT:
return array(
self::ACTION_BLOCK,
self::ACTION_NOTHING
Index: src/applications/herald/adapter/HeraldAdapter.php
===================================================================
--- src/applications/herald/adapter/HeraldAdapter.php
+++ src/applications/herald/adapter/HeraldAdapter.php
@@ -144,6 +144,18 @@
return false;
}
+ public function canTriggerOnObject($object) {
+ return false;
+ }
+
+ public function explainValidTriggerObjects() {
+ return pht('This adapter can not trigger on objects.');
+ }
+
+ public function getTriggerObjectPHIDs() {
+ return array($this->getPHID());
+ }
+
public function getAdapterSortKey() {
return sprintf(
'%08d%s',
@@ -577,6 +589,7 @@
public function getActionNameMap($rule_type) {
switch ($rule_type) {
case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL:
+ case HeraldRuleTypeConfig::RULE_TYPE_OBJECT:
return array(
self::ACTION_NOTHING => pht('Do nothing'),
self::ACTION_ADD_CC => pht('Add emails to CC'),
@@ -1002,6 +1015,11 @@
}
}
}
+
+ if ($rule->isObjectRule()) {
+ $phids[] = $rule->getTriggerObjectPHID();
+ }
+
return $phids;
}
Index: src/applications/herald/adapter/HeraldCommitAdapter.php
===================================================================
--- src/applications/herald/adapter/HeraldCommitAdapter.php
+++ src/applications/herald/adapter/HeraldCommitAdapter.php
@@ -53,13 +53,32 @@
switch ($rule_type) {
case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL:
case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
- return true;
case HeraldRuleTypeConfig::RULE_TYPE_OBJECT:
+ return true;
default:
return false;
}
}
+ public function canTriggerOnObject($object) {
+ if ($object instanceof PhabricatorRepository) {
+ return true;
+ }
+ return false;
+ }
+
+ public function getTriggerObjectPHIDs() {
+ return array(
+ $this->repository->getPHID(),
+ $this->getPHID(),
+ );
+ }
+
+ public function explainValidTriggerObjects() {
+ return pht(
+ 'This rule can trigger for **repositories**.');
+ }
+
public function getFieldNameMap() {
return array(
self::FIELD_NEED_AUDIT_FOR_PACKAGE =>
@@ -111,6 +130,7 @@
public function getActions($rule_type) {
switch ($rule_type) {
case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL:
+ case HeraldRuleTypeConfig::RULE_TYPE_OBJECT:
return array(
self::ACTION_ADD_CC,
self::ACTION_EMAIL,
Index: src/applications/herald/controller/HeraldNewController.php
===================================================================
--- src/applications/herald/controller/HeraldNewController.php
+++ src/applications/herald/controller/HeraldNewController.php
@@ -4,19 +4,19 @@
public function processRequest() {
$request = $this->getRequest();
- $user = $request->getUser();
+ $viewer = $request->getUser();
- $content_type_map = HeraldAdapter::getEnabledAdapterMap($user);
+ $content_type_map = HeraldAdapter::getEnabledAdapterMap($viewer);
$rule_type_map = HeraldRuleTypeConfig::getRuleTypeMap();
$errors = array();
$e_type = null;
$e_rule = null;
+ $e_object = null;
- $step = 0;
+ $step = $request->getInt('step');
if ($request->isFormPost()) {
- $step = $request->getInt('step');
$content_type = $request->getStr('content_type');
if (empty($content_type_map[$content_type])) {
$errors[] = pht('You must choose a content type for this rule.');
@@ -33,24 +33,77 @@
}
}
- if (!$errors && $step == 2) {
- $uri = id(new PhutilURI('edit/'))
- ->setQueryParams(
- array(
- 'content_type' => $content_type,
- 'rule_type' => $rule_type,
- ));
- $uri = $this->getApplicationURI($uri);
- return id(new AphrontRedirectResponse())->setURI($uri);
+ if (!$errors && $step >= 2) {
+ $target_phid = null;
+ $object_name = $request->getStr('objectName');
+ $done = false;
+ if ($rule_type != HeraldRuleTypeConfig::RULE_TYPE_OBJECT) {
+ $done = true;
+ } else if (strlen($object_name)) {
+ $target_object = id(new PhabricatorObjectQuery())
+ ->setViewer($viewer)
+ ->withNames(array($object_name))
+ ->executeOne();
+ if ($target_object) {
+ $can_edit = PhabricatorPolicyFilter::hasCapability(
+ $viewer,
+ $target_object,
+ PhabricatorPolicyCapability::CAN_EDIT);
+ if (!$can_edit) {
+ $errors[] = pht(
+ 'You can not create a rule for that object, because you do '.
+ 'not have permission to edit it. You can only create rules '.
+ 'for objects you can edit.');
+ $e_object = pht('Not Editable');
+ $step = 2;
+ } else {
+ $adapter = HeraldAdapter::getAdapterForContentType($content_type);
+ if (!$adapter->canTriggerOnObject($target_object)) {
+ $errors[] = pht(
+ 'This object is not of an allowed type for the rule. '.
+ 'Rules can only trigger on certain objects.');
+ $e_object = pht('Invalid');
+ $step = 2;
+ } else {
+ $target_phid = $target_object->getPHID();
+ $done = true;
+ }
+ }
+ } else {
+ $errors[] = pht('No object exists by that name.');
+ $e_object = pht('Invalid');
+ $step = 2;
+ }
+ } else if ($step > 2) {
+ $errors[] = pht(
+ 'You must choose an object to associate this rule with.');
+ $e_object = pht('Required');
+ $step = 2;
+ }
+
+ if (!$errors && $done) {
+ $uri = id(new PhutilURI('edit/'))
+ ->setQueryParams(
+ array(
+ 'content_type' => $content_type,
+ 'rule_type' => $rule_type,
+ 'targetPHID' => $target_phid,
+ ));
+ $uri = $this->getApplicationURI($uri);
+ return id(new AphrontRedirectResponse())->setURI($uri);
+ }
}
}
+ $content_type = $request->getStr('content_type');
+ $rule_type = $request->getStr('rule_type');
+
if ($errors) {
$errors = id(new AphrontErrorView())->setErrors($errors);
}
$form = id(new AphrontFormView())
- ->setUser($user)
+ ->setUser($viewer)
->setAction($this->getApplicationURI('new/'));
switch ($step) {
@@ -73,7 +126,7 @@
$e_rule);
$form
- ->addHiddenInput('content_type', $request->getStr('content_type'))
+ ->addHiddenInput('content_type', $content_type)
->addHiddenInput('step', 2)
->appendChild(
id(new AphrontFormStaticControl())
@@ -89,7 +142,52 @@
$cancel_uri = id(new PhutilURI('new/'))
->setQueryParams(
array(
- 'content_type' => $request->getStr('content_type'),
+ 'content_type' => $content_type,
+ 'step' => 0,
+ ));
+ $cancel_uri = $this->getApplicationURI($cancel_uri);
+ break;
+ case 2:
+ $adapter = HeraldAdapter::getAdapterForContentType($content_type);
+ $form
+ ->addHiddenInput('content_type', $content_type)
+ ->addHiddenInput('rule_type', $rule_type)
+ ->addHiddenInput('step', 3)
+ ->appendChild(
+ id(new AphrontFormStaticControl())
+ ->setLabel(pht('Rule for'))
+ ->setValue(
+ phutil_tag(
+ 'strong',
+ array(),
+ idx($content_type_map, $content_type))))
+ ->appendChild(
+ id(new AphrontFormStaticControl())
+ ->setLabel(pht('Rule Type'))
+ ->setValue(
+ phutil_tag(
+ 'strong',
+ array(),
+ idx($rule_type_map, $rule_type))))
+ ->appendRemarkupInstructions(
+ pht(
+ 'Choose the object this rule will act on (for example, enter '.
+ '`rX` to act on the `rX` repository).'))
+ ->appendRemarkupInstructions(
+ $adapter->explainValidTriggerObjects())
+ ->appendChild(
+ id(new AphrontFormTextControl())
+ ->setName('objectName')
+ ->setError($e_object)
+ ->setValue($request->getStr('objectName'))
+ ->setLabel(pht('Object')));
+
+ $cancel_text = pht('Back');
+ $cancel_uri = id(new PhutilURI('new/'))
+ ->setQueryParams(
+ array(
+ 'content_type' => $content_type,
+ 'rule_type' => $rule_type,
'step' => 1,
));
$cancel_uri = $this->getApplicationURI($cancel_uri);
@@ -156,9 +254,6 @@
HeraldRuleTypeConfig::RULE_TYPE_GLOBAL,
)) + $rule_type_map;
- // TODO: Enable this.
- unset($rule_type_map[HeraldRuleTypeConfig::RULE_TYPE_OBJECT]);
-
list($can_global, $global_link) = $this->explainApplicationCapability(
HeraldCapabilityManageGlobalRules::CAPABILITY,
pht('You have permission to create and manage global rules.'),
@@ -170,6 +265,12 @@
'Personal rules notify you about events. You own them, but they can '.
'only affect you. Personal rules only trigger for objects you have '.
'permission to see.'),
+ HeraldRuleTypeConfig::RULE_TYPE_OBJECT =>
+ pht(
+ 'Object rules notify anyone about events. They are bound to an '.
+ 'object (like a repository) and can only act on that object. You '.
+ 'must be able to edit an object to create object rules for it. '.
+ 'Other users who an edit the object can edit its rules.'),
HeraldRuleTypeConfig::RULE_TYPE_GLOBAL =>
array(
pht(
@@ -180,7 +281,7 @@
);
$radio = id(new AphrontFormRadioButtonControl())
- ->setLabel(pht('Type'))
+ ->setLabel(pht('Rule Type'))
->setName('rule_type')
->setValue($request->getStr('rule_type'))
->setError($e_rule);
Index: src/applications/herald/controller/HeraldRuleController.php
===================================================================
--- src/applications/herald/controller/HeraldRuleController.php
+++ src/applications/herald/controller/HeraldRuleController.php
@@ -46,6 +46,38 @@
}
$rule->setRuleType($rule_type);
+ $adapter = HeraldAdapter::getAdapterForContentType(
+ $rule->getContentType());
+
+ if (!$adapter->supportsRuleType($rule->getRuleType())) {
+ throw new Exception(
+ pht(
+ "This rule's content type does not support the selected rule ".
+ "type."));
+ }
+
+ if ($rule->isObjectRule()) {
+ $rule->setTriggerObjectPHID($request->getStr('targetPHID'));
+ $object = id(new PhabricatorObjectQuery())
+ ->setViewer($user)
+ ->withPHIDs(array($rule->getTriggerObjectPHID()))
+ ->requireCapabilities(
+ array(
+ PhabricatorPolicyCapability::CAN_VIEW,
+ PhabricatorPolicyCapability::CAN_EDIT,
+ ))
+ ->executeOne();
+ if (!$object) {
+ throw new Exception(
+ pht('No valid object provided for object rule!'));
+ }
+
+ if (!$adapter->canTriggerOnObject($object)) {
+ throw new Exception(
+ pht('Object is of wrong type for adapter!'));
+ }
+ }
+
$cancel_uri = $this->getApplicationURI();
}
@@ -65,12 +97,6 @@
"deployment."));
}
- if (!$adapter->supportsRuleType($rule->getRuleType())) {
- throw new Exception(
- pht(
- "This rule's content type does not support the selected rule type."));
- }
-
// Upgrade rule version to our version, since we might add newly-defined
// conditions, etc.
$rule->setConfigVersion($local_version);
@@ -133,6 +159,16 @@
->setError($e_name)
->setValue($rule->getName()));
+ $trigger_object_control = false;
+ if ($rule->isObjectRule()) {
+ $trigger_object_control = id(new AphrontFormStaticControl())
+ ->setValue(
+ pht(
+ 'This rule triggers for %s.',
+ $handles[$rule->getTriggerObjectPHID()]->renderLink()));
+ }
+
+
$form
->appendChild(
id(new AphrontFormMarkupControl())
@@ -140,6 +176,7 @@
"This %s rule triggers for %s.",
phutil_tag('strong', array(), $rule_type_name),
phutil_tag('strong', array(), $content_type_name))))
+ ->appendChild($trigger_object_control)
->appendChild(
id(new AphrontFormInsetView())
->setTitle(pht('Conditions'))
@@ -489,6 +526,10 @@
$phids[] = $rule->getAuthorPHID();
+ if ($rule->isObjectRule()) {
+ $phids[] = $rule->getTriggerObjectPHID();
+ }
+
return $this->loadViewerHandles($phids);
}
Index: src/applications/herald/controller/HeraldRuleViewController.php
===================================================================
--- src/applications/herald/controller/HeraldRuleViewController.php
+++ src/applications/herald/controller/HeraldRuleViewController.php
@@ -131,6 +131,7 @@
$this->getHandle($rule->getAuthorPHID())->renderLink());
}
+
$adapter = HeraldAdapter::getAdapterForContentType($rule->getContentType());
if ($adapter) {
$view->addProperty(
@@ -139,6 +140,12 @@
HeraldAdapter::getEnabledAdapterMap($viewer),
$rule->getContentType()));
+ if ($rule->isObjectRule()) {
+ $view->addProperty(
+ pht('Trigger Object'),
+ $this->getHandle($rule->getTriggerObjectPHID())->renderLink());
+ }
+
$view->invokeWillRenderEvent();
$view->addSectionHeader(pht('Rule Description'));
Index: src/applications/herald/controller/HeraldTranscriptController.php
===================================================================
--- src/applications/herald/controller/HeraldTranscriptController.php
+++ src/applications/herald/controller/HeraldTranscriptController.php
@@ -16,7 +16,7 @@
$map = $this->getFilterMap();
$this->filter = idx($data, 'filter');
if (empty($map[$this->filter])) {
- $this->filter = self::FILTER_AFFECTED;
+ $this->filter = self::FILTER_ALL;
}
}
@@ -149,9 +149,9 @@
protected function getFilterMap() {
return array(
- self::FILTER_AFFECTED => pht('Rules that Affected Me'),
- self::FILTER_OWNED => pht('Rules I Own'),
self::FILTER_ALL => pht('All Rules'),
+ self::FILTER_OWNED => pht('Rules I Own'),
+ self::FILTER_AFFECTED => pht('Rules that Affected Me'),
);
}
Index: src/applications/herald/engine/HeraldEngine.php
===================================================================
--- src/applications/herald/engine/HeraldEngine.php
+++ src/applications/herald/engine/HeraldEngine.php
@@ -420,14 +420,14 @@
}
$trigger_phid = $rule->getTriggerObjectPHID();
- $object_phid = $adapter->getPHID();
+ $object_phids = $adapter->getTriggerObjectPHIDs();
- if ($trigger_phid == $object_phid) {
- return true;
+ if ($object_phids) {
+ if (in_array($trigger_phid, $object_phids)) {
+ return true;
+ }
}
- // TODO: We should also handle projects.
-
return false;
}
Index: src/applications/herald/storage/HeraldRule.php
===================================================================
--- src/applications/herald/storage/HeraldRule.php
+++ src/applications/herald/storage/HeraldRule.php
@@ -17,7 +17,7 @@
protected $isDisabled = 0;
protected $triggerObjectPHID;
- protected $configVersion = 22;
+ protected $configVersion = 23;
// phids for which this rule has been applied
private $ruleApplied = self::ATTACHABLE;
Index: src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
===================================================================
--- src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
+++ src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
@@ -36,8 +36,10 @@
$commit->getID());
if (!$data) {
- // TODO: Permanent failure.
- return;
+ throw new PhabricatorWorkerPermanentFailureException(
+ pht(
+ 'Unable to load commit data. The data for this task is invalid '.
+ 'or no longer exists.'));
}
$adapter = HeraldCommitAdapter::newLegacyAdapter(

File Metadata

Mime Type
text/plain
Expires
Fri, Mar 7, 7:41 AM (2 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7327182
Default Alt Text
D7853.diff (19 KB)

Event Timeline

Phabricator · Built by Phacility