Page MenuHomePhabricator
Files F88381

D7314.diff
All Users
Actions

D7314.diff
View Options

diff --git a/src/applications/policy/controller/PhabricatorPolicyEditController.php b/src/applications/policy/controller/PhabricatorPolicyEditController.php
--- a/src/applications/policy/controller/PhabricatorPolicyEditController.php
+++ b/src/applications/policy/controller/PhabricatorPolicyEditController.php
@@ -49,6 +49,7 @@
$default_action = $policy->getDefaultAction();
$rule_data = $policy->getRules();
+ $errors = array();
if ($request->isFormPost()) {
$data = $request->getStr('rules');
$data = @json_decode($data, true);
@@ -83,26 +84,42 @@
);
}
+ // Filter out nonsense rules, like a "users" rule without any users
+ // actually specified.
+ $valid_rules = array();
+ foreach ($rule_data as $rule) {
+ $rule_class = $rule['rule'];
+ if ($rules[$rule_class]->ruleHasEffect($rule['value'])) {
+ $valid_rules[] = $rule;
+ }
+ }
+
+ if (!$valid_rules) {
+ $errors[] = pht('None of these policy rules have any effect.');
+ }
+
// NOTE: Policies are immutable once created, and we always create a new
// policy here. If we didn't, we would need to lock this endpoint down,
// as users could otherwise just go edit the policies of objects with
// custom policies.
- $new_policy = new PhabricatorPolicy();
- $new_policy->setRules($rule_data);
- $new_policy->setDefaultAction($request->getStr('default'));
- $new_policy->save();
-
- $data = array(
- 'phid' => $new_policy->getPHID(),
- 'info' => array(
- 'name' => $new_policy->getName(),
- 'full' => $new_policy->getName(),
- 'icon' => $new_policy->getIcon(),
- ),
- );
-
- return id(new AphrontAjaxResponse())->setContent($data);
+ if (!$errors) {
+ $new_policy = new PhabricatorPolicy();
+ $new_policy->setRules($valid_rules);
+ $new_policy->setDefaultAction($request->getStr('default'));
+ $new_policy->save();
+
+ $data = array(
+ 'phid' => $new_policy->getPHID(),
+ 'info' => array(
+ 'name' => $new_policy->getName(),
+ 'full' => $new_policy->getName(),
+ 'icon' => $new_policy->getIcon(),
+ ),
+ );
+
+ return id(new AphrontAjaxResponse())->setContent($data);
+ }
}
// Convert rule values to display format (for example, expanding PHIDs
@@ -120,7 +137,13 @@
'name' => 'default',
));
+ if ($errors) {
+ $errors = id(new AphrontErrorView())
+ ->setErrors($errors);
+ }
+
$form = id(new PHUIFormLayoutView())
+ ->appendChild($errors)
->appendChild(
javelin_tag(
'input',
diff --git a/src/applications/policy/rule/PhabricatorPolicyRule.php b/src/applications/policy/rule/PhabricatorPolicyRule.php
--- a/src/applications/policy/rule/PhabricatorPolicyRule.php
+++ b/src/applications/policy/rule/PhabricatorPolicyRule.php
@@ -34,4 +34,15 @@
return $value;
}
+ /**
+ * Return true if the given value creates a rule with a meaningful effect.
+ * An example of a rule with no meaningful effect is a "users" rule with no
+ * users specified.
+ *
+ * @return bool True if the value creates a meaningful rule.
+ */
+ public function ruleHasEffect($value) {
+ return true;
+ }
+
}
diff --git a/src/applications/policy/rule/PhabricatorPolicyRuleProjects.php b/src/applications/policy/rule/PhabricatorPolicyRuleProjects.php
--- a/src/applications/policy/rule/PhabricatorPolicyRuleProjects.php
+++ b/src/applications/policy/rule/PhabricatorPolicyRuleProjects.php
@@ -64,4 +64,8 @@
return mpull($handles, 'getFullName', 'getPHID');
}
+ public function ruleHasEffect($value) {
+ return (bool)$value;
+ }
+
}
diff --git a/src/applications/policy/rule/PhabricatorPolicyRuleUsers.php b/src/applications/policy/rule/PhabricatorPolicyRuleUsers.php
--- a/src/applications/policy/rule/PhabricatorPolicyRuleUsers.php
+++ b/src/applications/policy/rule/PhabricatorPolicyRuleUsers.php
@@ -50,4 +50,8 @@
return mpull($handles, 'getFullName', 'getPHID');
}
+ public function ruleHasEffect($value) {
+ return (bool)$value;
+ }
+
}

File Metadata

Mime Type
text/x-diff
Storage Engine
amazon-s3
Storage Format
Raw Data
Storage Handle
phabricator/ej/o3/bveitjmh3fucae4e
Default Alt Text
D7314.diff (4 KB)

Event Timeline

Phabricator · Built by Phacility