Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13176820
D20417.id48711.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
14 KB
Referenced Files
None
Subscribers
None
D20417.id48711.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
@@ -100,7 +100,7 @@
'rsrc/css/application/policy/policy.css' => 'ceb56a08',
'rsrc/css/application/ponder/ponder-view.css' => '05a09d0a',
'rsrc/css/application/project/project-card-view.css' => '4e7371cd',
- 'rsrc/css/application/project/project-triggers.css' => 'cb866c2d',
+ 'rsrc/css/application/project/project-triggers.css' => 'cd9c8bb9',
'rsrc/css/application/project/project-view.css' => '567858b3',
'rsrc/css/application/releeph/releeph-core.css' => 'f81ff2db',
'rsrc/css/application/releeph/releeph-preview-branch.css' => '22db5c07',
@@ -882,7 +882,7 @@
'policy-transaction-detail-css' => 'c02b8384',
'ponder-view-css' => '05a09d0a',
'project-card-view-css' => '4e7371cd',
- 'project-triggers-css' => 'cb866c2d',
+ 'project-triggers-css' => 'cd9c8bb9',
'project-view-css' => '567858b3',
'releeph-core' => 'f81ff2db',
'releeph-preview-branch' => '22db5c07',
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
@@ -104,9 +104,9 @@
$allow_invalid,
PhabricatorUser $viewer) {
- // NOTE: With "$allow_invalid" set, we're trying to preserve the database
+ // NOTE: With "$allow_invalid" set, we're trying to preserve the database
// state in the rule structure, even if it includes rule types we don't
- // ha ve implementations for, or rules with invalid rule values.
+ // 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.
diff --git a/src/applications/project/trigger/PhabricatorProjectTriggerAddProjectsRule.php b/src/applications/project/trigger/PhabricatorProjectTriggerAddProjectsRule.php
--- a/src/applications/project/trigger/PhabricatorProjectTriggerAddProjectsRule.php
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerAddProjectsRule.php
@@ -5,15 +5,15 @@
const TRIGGERTYPE = 'task.projects.add';
- public function getSelectControLname() {
- return pht('Add projects');
+ public function getSelectControlName() {
+ return pht('Add project tags');
}
protected function getValueForEditorField() {
return $this->getDatasource()->getWireTokens($this->getValue());
}
- protected function assertValidRuleValue($value) {
+ protected function assertValidRuleRecordFormat($value) {
if (!is_array($value)) {
throw new Exception(
pht(
@@ -23,6 +23,14 @@
}
}
+ protected function assertValidRuleRecordValue($value) {
+ if (!$value) {
+ throw new Exception(
+ pht(
+ 'You must select at least one project tag to add.'));
+ }
+ }
+
protected function newDropTransactions($object, $value) {
$project_edge_type = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST;
@@ -78,12 +86,12 @@
}
public function getRuleViewLabel() {
- return pht('Add Projects');
+ return pht('Add Project Tags');
}
public function getRuleViewDescription($value) {
return pht(
- 'Add projects: %s.',
+ 'Add project tags: %s.',
phutil_tag(
'strong',
array(),
diff --git a/src/applications/project/trigger/PhabricatorProjectTriggerInvalidRule.php b/src/applications/project/trigger/PhabricatorProjectTriggerInvalidRule.php
--- a/src/applications/project/trigger/PhabricatorProjectTriggerInvalidRule.php
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerInvalidRule.php
@@ -24,7 +24,7 @@
return false;
}
- protected function assertValidRuleValue($value) {
+ protected function assertValidRuleRecordFormat($value) {
return;
}
diff --git a/src/applications/project/trigger/PhabricatorProjectTriggerManiphestOwnerRule.php b/src/applications/project/trigger/PhabricatorProjectTriggerManiphestOwnerRule.php
--- a/src/applications/project/trigger/PhabricatorProjectTriggerManiphestOwnerRule.php
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerManiphestOwnerRule.php
@@ -21,21 +21,43 @@
return $value;
}
- protected function assertValidRuleValue($value) {
+ protected function assertValidRuleRecordFormat($value) {
if (!is_array($value)) {
throw new Exception(
pht(
'Owner rule value should be a list, but is not (value is "%s").',
phutil_describe_type($value)));
}
+ }
+
+ protected function assertValidRuleRecordValue($value) {
+ if (!$value) {
+ throw new Exception(
+ pht(
+ 'Owner rule value is required. Specify a user to assign tasks '.
+ 'to, or the token "none()" to unassign tasks.'));
+ }
- if (count($value) != 1) {
+ if (count($value) > 1) {
throw new Exception(
pht(
- 'Owner rule value should be a list of exactly one user PHID, or the '.
- 'token "none()" (value is "%s").',
+ 'Owner rule value must have only one elmement (value is "%s").',
implode(', ', $value)));
}
+
+ $owner_phid = $this->convertTokenizerValueToOwner($value);
+ if ($owner_phid !== null) {
+ $user = id(new PhabricatorPeopleQuery())
+ ->setViewer($this->getViewer())
+ ->withPHIDs(array($owner_phid))
+ ->executeOne();
+ if (!$user) {
+ throw new Exception(
+ pht(
+ 'User PHID ("%s") is not a valid user.',
+ $owner_phid));
+ }
+ }
}
protected function newDropTransactions($object, $value) {
diff --git a/src/applications/project/trigger/PhabricatorProjectTriggerManiphestPriorityRule.php b/src/applications/project/trigger/PhabricatorProjectTriggerManiphestPriorityRule.php
--- a/src/applications/project/trigger/PhabricatorProjectTriggerManiphestPriorityRule.php
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerManiphestPriorityRule.php
@@ -9,20 +9,24 @@
return pht('Change priority to');
}
- protected function assertValidRuleValue($value) {
+ protected function assertValidRuleRecordFormat($value) {
if (!is_string($value)) {
throw new Exception(
pht(
'Priority rule value should be a string, but is not (value is "%s").',
phutil_describe_type($value)));
}
+ }
+ protected function assertValidRuleRecordValue($value) {
$map = ManiphestTaskPriority::getTaskPriorityMap();
if (!isset($map[$value])) {
throw new Exception(
pht(
- 'Rule value ("%s") is not a valid task priority.',
- $value));
+ 'Task priority value ("%s") is not a valid task priority. '.
+ 'Valid priorities are: %s.',
+ $value,
+ implode(', ', array_keys($map))));
}
}
diff --git a/src/applications/project/trigger/PhabricatorProjectTriggerManiphestStatusRule.php b/src/applications/project/trigger/PhabricatorProjectTriggerManiphestStatusRule.php
--- a/src/applications/project/trigger/PhabricatorProjectTriggerManiphestStatusRule.php
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerManiphestStatusRule.php
@@ -9,20 +9,24 @@
return pht('Change status to');
}
- protected function assertValidRuleValue($value) {
+ protected function assertValidRuleRecordFormat($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)));
}
+ }
+ protected function assertValidRuleRecordValue($value) {
$map = ManiphestTaskStatus::getTaskStatusMap();
if (!isset($map[$value])) {
throw new Exception(
pht(
- 'Rule value ("%s") is not a valid task status.',
- $value));
+ 'Task status value ("%s") is not a valid task status. '.
+ 'Valid statues are: %s.',
+ $value,
+ implode(', ', array_keys($map))));
}
}
diff --git a/src/applications/project/trigger/PhabricatorProjectTriggerPlaySoundRule.php b/src/applications/project/trigger/PhabricatorProjectTriggerPlaySoundRule.php
--- a/src/applications/project/trigger/PhabricatorProjectTriggerPlaySoundRule.php
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerPlaySoundRule.php
@@ -9,20 +9,21 @@
return pht('Play sound');
}
- protected function assertValidRuleValue($value) {
+ protected function assertValidRuleRecordFormat($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)));
}
+ }
+ protected function assertValidRuleRecordValue($value) {
$map = self::getSoundMap();
-
if (!isset($map[$value])) {
throw new Exception(
pht(
- 'Rule value ("%s") is not a valid sound.',
+ 'Sound ("%s") is not a valid sound.',
$value));
}
}
diff --git a/src/applications/project/trigger/PhabricatorProjectTriggerRemoveProjectsRule.php b/src/applications/project/trigger/PhabricatorProjectTriggerRemoveProjectsRule.php
--- a/src/applications/project/trigger/PhabricatorProjectTriggerRemoveProjectsRule.php
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerRemoveProjectsRule.php
@@ -5,15 +5,15 @@
const TRIGGERTYPE = 'task.projects.remove';
- public function getSelectControLname() {
- return pht('Remove projects');
+ public function getSelectControlname() {
+ return pht('Remove project tags');
}
protected function getValueForEditorField() {
return $this->getDatasource()->getWireTokens($this->getValue());
}
- protected function assertValidRuleValue($value) {
+ protected function assertValidRuleRecordFormat($value) {
if (!is_array($value)) {
throw new Exception(
pht(
@@ -23,6 +23,14 @@
}
}
+ protected function assertValidRuleRecordValue($value) {
+ if (!$value) {
+ throw new Exception(
+ pht(
+ 'You must select at least one project tag to remove.'));
+ }
+ }
+
protected function newDropTransactions($object, $value) {
$project_edge_type = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST;
@@ -78,12 +86,12 @@
}
public function getRuleViewLabel() {
- return pht('Remove Projects');
+ return pht('Remove Project Tags');
}
public function getRuleViewDescription($value) {
return pht(
- 'Remove projects: %s.',
+ 'Remove project tags: %s.',
phutil_tag(
'strong',
array(),
diff --git a/src/applications/project/trigger/PhabricatorProjectTriggerRule.php b/src/applications/project/trigger/PhabricatorProjectTriggerRule.php
--- a/src/applications/project/trigger/PhabricatorProjectTriggerRule.php
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerRule.php
@@ -23,7 +23,7 @@
final public function setRecord(PhabricatorProjectTriggerRuleRecord $record) {
$value = $record->getValue();
- $this->assertValidRuleValue($value);
+ $this->assertValidRuleRecordFormat($value);
$this->record = $record;
return $this;
@@ -45,7 +45,22 @@
abstract public function getRuleViewLabel();
abstract public function getRuleViewDescription($value);
abstract public function getRuleViewIcon($value);
- abstract protected function assertValidRuleValue($value);
+ abstract protected function assertValidRuleRecordFormat($value);
+
+ final public function getRuleRecordValueValidationException() {
+ try {
+ $this->assertValidRuleRecordValue($this->getRecord()->getValue());
+ } catch (Exception $ex) {
+ return $ex;
+ }
+
+ return null;
+ }
+
+ protected function assertValidRuleRecordValue($value) {
+ return;
+ }
+
abstract protected function newDropTransactions($object, $value);
abstract protected function newDropEffects($value);
abstract protected function getDefaultValue();
diff --git a/src/applications/project/trigger/PhabricatorProjectTriggerUnknownRule.php b/src/applications/project/trigger/PhabricatorProjectTriggerUnknownRule.php
--- a/src/applications/project/trigger/PhabricatorProjectTriggerUnknownRule.php
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerUnknownRule.php
@@ -13,7 +13,7 @@
return false;
}
- protected function assertValidRuleValue($value) {
+ protected function assertValidRuleRecordFormat($value) {
return;
}
diff --git a/src/applications/project/xaction/trigger/PhabricatorProjectTriggerRulesetTransaction.php b/src/applications/project/xaction/trigger/PhabricatorProjectTriggerRulesetTransaction.php
--- a/src/applications/project/xaction/trigger/PhabricatorProjectTriggerRulesetTransaction.php
+++ b/src/applications/project/xaction/trigger/PhabricatorProjectTriggerRulesetTransaction.php
@@ -20,16 +20,18 @@
}
public function validateTransactions($object, array $xactions) {
+ $actor = $this->getActor();
$errors = array();
foreach ($xactions as $xaction) {
$ruleset = $xaction->getNewValue();
try {
- PhabricatorProjectTrigger::newTriggerRulesFromRuleSpecifications(
- $ruleset,
- $allow_invalid = false,
- $xaction->getViewer());
+ $rules =
+ PhabricatorProjectTrigger::newTriggerRulesFromRuleSpecifications(
+ $ruleset,
+ $allow_invalid = false,
+ $actor);
} catch (PhabricatorProjectTriggerCorruptionException $ex) {
$errors[] = $this->newInvalidError(
pht(
@@ -38,6 +40,19 @@
$xaction);
continue;
}
+
+ foreach ($rules as $rule) {
+ $exception = $rule->getRuleRecordValueValidationException();
+ if ($exception) {
+ $errors[] = $this->newInvalidError(
+ pht(
+ 'Value for "%s" rule is invalid: %s',
+ $rule->getSelectControlName(),
+ $exception->getMessage()),
+ $xaction);
+ continue;
+ }
+ }
}
return $errors;
diff --git a/webroot/rsrc/css/application/project/project-triggers.css b/webroot/rsrc/css/application/project/project-triggers.css
--- a/webroot/rsrc/css/application/project/project-triggers.css
+++ b/webroot/rsrc/css/application/project/project-triggers.css
@@ -27,6 +27,7 @@
.trigger-rules-table td.invalid-cell {
padding-left: 12px;
+ width: 100%;
}
.trigger-rules-table td.invalid-cell .phui-icon-view {
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Thu, May 9, 5:28 PM (1 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6275269
Default Alt Text
D20417.id48711.diff (14 KB)
Attached To
Mode
D20417: Distinguish between "bad record format" and "bad record value" when validating Trigger rules
Attached
Detach File
Event Timeline
Log In to Comment