Index: src/__phutil_library_map__.php =================================================================== --- src/__phutil_library_map__.php +++ src/__phutil_library_map__.php @@ -3676,7 +3676,11 @@ 'PhabricatorPhrequentConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorPhrictionConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorPolicies' => 'PhabricatorPolicyConstants', - 'PhabricatorPolicy' => 'PhabricatorPolicyDAO', + 'PhabricatorPolicy' => + array( + 0 => 'PhabricatorPolicyDAO', + 1 => 'PhabricatorPolicyInterface', + ), 'PhabricatorPolicyAwareQuery' => 'PhabricatorOffsetPagedQuery', 'PhabricatorPolicyAwareTestQuery' => 'PhabricatorPolicyAwareQuery', 'PhabricatorPolicyCapability' => 'Phobject', @@ -3694,7 +3698,7 @@ 'PhabricatorPolicyManagementUnlockWorkflow' => 'PhabricatorPolicyManagementWorkflow', 'PhabricatorPolicyManagementWorkflow' => 'PhutilArgumentWorkflow', 'PhabricatorPolicyPHIDTypePolicy' => 'PhabricatorPHIDType', - 'PhabricatorPolicyQuery' => 'PhabricatorQuery', + 'PhabricatorPolicyQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorPolicyRuleAdministrators' => 'PhabricatorPolicyRule', 'PhabricatorPolicyRuleLunarPhase' => 'PhabricatorPolicyRule', 'PhabricatorPolicyRuleProjects' => 'PhabricatorPolicyRule', Index: src/applications/policy/application/PhabricatorApplicationPolicy.php =================================================================== --- src/applications/policy/application/PhabricatorApplicationPolicy.php +++ src/applications/policy/application/PhabricatorApplicationPolicy.php @@ -15,7 +15,7 @@ '/policy/' => array( 'explain/(?P[^/]+)/(?P[^/]+)/' => 'PhabricatorPolicyExplainController', - 'edit/' => 'PhabricatorPolicyEditController', + 'edit/(?:(?P[^/]+)/)?' => 'PhabricatorPolicyEditController', ), ); } Index: src/applications/policy/controller/PhabricatorPolicyEditController.php =================================================================== --- src/applications/policy/controller/PhabricatorPolicyEditController.php +++ src/applications/policy/controller/PhabricatorPolicyEditController.php @@ -3,32 +3,52 @@ final class PhabricatorPolicyEditController extends PhabricatorPolicyController { + private $phid; + + public function willProcessRequest(array $data) { + $this->phid = idx($data, 'phid'); + } + public function processRequest() { $request = $this->getRequest(); $viewer = $request->getUser(); - $policy = new PhabricatorPolicy(); - - $root_id = celerity_generate_unique_node_id(); - $action_options = array( - 'allow' => pht('Allow'), - 'deny' => pht('Deny'), + PhabricatorPolicy::ACTION_ALLOW => pht('Allow'), + PhabricatorPolicy::ACTION_DENY => pht('Deny'), ); $rules = id(new PhutilSymbolLoader()) ->setAncestorClass('PhabricatorPolicyRule') ->loadObjects(); - $rules = msort($rules, 'getRuleOrder'); - $default_value = 'deny'; $default_rule = array( 'action' => head_key($action_options), 'rule' => head_key($rules), 'value' => null, ); + if ($this->phid) { + $policies = id(new PhabricatorPolicyQuery()) + ->setViewer($viewer) + ->withPHIDs(array($this->phid)) + ->execute(); + if (!$policies) { + return new Aphront404Response(); + } + $policy = head($policies); + } else { + $policy = id(new PhabricatorPolicy()) + ->setRules(array($default_rule)) + ->setDefaultAction(PhabricatorPolicy::ACTION_DENY); + } + + $root_id = celerity_generate_unique_node_id(); + + $default_action = $policy->getDefaultAction(); + $rule_data = $policy->getRules(); + if ($request->isFormPost()) { $data = $request->getStr('rules'); $data = @json_decode($data, true); @@ -63,21 +83,38 @@ ); } - $policy->setRules($rule_data); - $policy->setDefaultAction($request->getStr('default')); - $policy->save(); - - // TODO: Integrate with policy editors. - $id = $policy->getID(); - throw new Exception("OK, saved policy {$id}!"); - } else { - $rule_data = array( - $default_rule, + // 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); + } + + // Convert rule values to display format (for example, expanding PHIDs + // into tokens). + foreach ($rule_data as $key => $rule) { + $rule_data[$key]['value'] = $rules[$rule['rule']]->getValueForDisplay( + $viewer, + $rule['value']); } $default_select = AphrontFormSelectControl::renderSelectTag( - $default_value, + $default_action, $action_options, array( 'name' => 'default', Index: src/applications/policy/rule/PhabricatorPolicyRuleUsers.php =================================================================== --- src/applications/policy/rule/PhabricatorPolicyRuleUsers.php +++ src/applications/policy/rule/PhabricatorPolicyRuleUsers.php @@ -38,6 +38,10 @@ } public function getValueForDisplay(PhabricatorUser $viewer, $value) { + if (!$value) { + return array(); + } + $handles = id(new PhabricatorHandleQuery()) ->setViewer($viewer) ->withPHIDs($value)