Page MenuHomePhabricator

Fix PolicyControl defaulting nonsense in every application
Closed, ResolvedPublic

Description

We currently do this thing in a bunch of places:

$some_value = $request->getStr('viewPolicy');
try {
  $editor->apply(...);
} catch (ValidationException $ex) {
  $object->setViewPolicy($some_value);
}

id(new AphrontFormPolicyControl())
  ->setObject($object);

This is unintuitive and silly. It should look like this instead:

$some_value = $request->getStr('viewPolicy');
...
id(new AphrontFormPolicyControl())
  ->setObject($object)
  ->setValue($some_value);

That is, behave like other types of controls do. The logic should be:

  • If an explicit value is set, use that.
  • Otherwise, use the value on the object.

I think this actually already works, based on reading AphrontFormPolicyControl. We should convert Calendar to work like this, then convert all other applications which use the bad version to work like this.