Implement modular transactions for application policy changes

Summary: Still needs some cleanup, but ready for review in broad outline form.

Test Plan:
Made lots of policy changes to the Badges application and confirmed expected rows in `application_xactions`, confirmed expected changes to `phabricator.application-settings`.

See example output (not quite working for custom policy objects) here:

{F4922240}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, chad, epriestley

Maniphest Tasks: T11476

Differential Revision: https://secure.phabricator.com/D17757

src/__phutil_library_map__.php

@@ -1849,9 +1849,12 @@ phutil_register_library_map(array(
     'PhabricatorApplicationDatasource' => 'applications/meta/typeahead/PhabricatorApplicationDatasource.php',
     'PhabricatorApplicationDetailViewController' => 'applications/meta/controller/PhabricatorApplicationDetailViewController.php',
     'PhabricatorApplicationEditController' => 'applications/meta/controller/PhabricatorApplicationEditController.php',
+    'PhabricatorApplicationEditEngine' => 'applications/meta/editor/PhabricatorApplicationEditEngine.php',
     'PhabricatorApplicationEditHTTPParameterHelpView' => 'applications/transactions/view/PhabricatorApplicationEditHTTPParameterHelpView.php',
+    'PhabricatorApplicationEditor' => 'applications/meta/editor/PhabricatorApplicationEditor.php',
     'PhabricatorApplicationEmailCommandsController' => 'applications/meta/controller/PhabricatorApplicationEmailCommandsController.php',
     'PhabricatorApplicationPanelController' => 'applications/meta/controller/PhabricatorApplicationPanelController.php',
+    'PhabricatorApplicationPolicyChangeTransaction' => 'applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php',
     'PhabricatorApplicationProfileMenuItem' => 'applications/search/menuitem/PhabricatorApplicationProfileMenuItem.php',
     'PhabricatorApplicationQuery' => 'applications/meta/query/PhabricatorApplicationQuery.php',
     'PhabricatorApplicationSchemaSpec' => 'applications/meta/storage/PhabricatorApplicationSchemaSpec.php',
@@ -6889,9 +6892,12 @@ phutil_register_library_map(array(
     'PhabricatorApplicationDatasource' => 'PhabricatorTypeaheadDatasource',
     'PhabricatorApplicationDetailViewController' => 'PhabricatorApplicationsController',
     'PhabricatorApplicationEditController' => 'PhabricatorApplicationsController',
+    'PhabricatorApplicationEditEngine' => 'PhabricatorEditEngine',
     'PhabricatorApplicationEditHTTPParameterHelpView' => 'AphrontView',
+    'PhabricatorApplicationEditor' => 'PhabricatorApplicationTransactionEditor',
     'PhabricatorApplicationEmailCommandsController' => 'PhabricatorApplicationsController',
     'PhabricatorApplicationPanelController' => 'PhabricatorApplicationsController',
+    'PhabricatorApplicationPolicyChangeTransaction' => 'PhabricatorApplicationTransactionType',
     'PhabricatorApplicationProfileMenuItem' => 'PhabricatorProfileMenuItem',
     'PhabricatorApplicationQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorApplicationSchemaSpec' => 'PhabricatorConfigSchemaSpec',

src/applications/base/PhabricatorApplication.php

@@ -284,7 +284,6 @@ abstract class PhabricatorApplication
     throw new PhutilMethodNotImplementedException();
   }
 
-
 /* -(  Fact Integration  )--------------------------------------------------- */
 
 

src/applications/meta/controller/PhabricatorApplicationDetailViewController.php

@@ -38,6 +38,11 @@ final class PhabricatorApplicationDetailViewController
       $header->setStatus('fa-ban', 'dark', pht('Uninstalled'));
     }
 
+    $timeline = $this->buildTransactionTimeline(
+      $selected,
+      new PhabricatorApplicationApplicationTransactionQuery());
+    $timeline->setShouldTerminate(true);
+
     $curtain = $this->buildCurtain($selected);
     $details = $this->buildPropertySectionView($selected);
     $policies = $this->buildPolicyView($selected);
@@ -61,6 +66,7 @@ final class PhabricatorApplicationDetailViewController
       ->setMainColumn(array(
           $policies,
           $panels,
+          $timeline,
         ))
       ->addPropertySection(pht('Details'), $details);
 

src/applications/meta/controller/PhabricatorApplicationEditController.php

@@ -30,8 +30,15 @@ final class PhabricatorApplicationEditController
       ->execute();
 
     if ($request->isFormPost()) {
+      $xactions = array();
+
       $result = array();
+      $template = $application->getApplicationTransactionTemplate();
       foreach ($application->getCapabilities() as $capability) {
+        if (!$application->isCapabilityEditable($capability)) {
+          continue;
+        }
+
         $old = $application->getPolicy($capability);
         $new = $request->getStr('policy:'.$capability);
 
@@ -40,67 +47,36 @@ final class PhabricatorApplicationEditController
           continue;
         }
 
-        if (empty($policies[$new])) {
-          // Not a standard policy, check for a custom policy.
-          $policy = id(new PhabricatorPolicyQuery())
-            ->setViewer($user)
-            ->withPHIDs(array($new))
-            ->executeOne();
-          if (!$policy) {
-            // Not a custom policy either. Can't set the policy to something
-            // invalid, so skip this.
-            continue;
-          }
-        }
-
-        if ($new == PhabricatorPolicies::POLICY_PUBLIC) {
-          $capobj = PhabricatorPolicyCapability::getCapabilityByKey(
-            $capability);
-          if (!$capobj || !$capobj->shouldAllowPublicPolicySetting()) {
-            // Can't set non-public policies to public.
-            continue;
-          }
-        }
-
         $result[$capability] = $new;
+
+        $xactions[] = id(clone $template)
+          ->setTransactionType(
+              PhabricatorApplicationPolicyChangeTransaction::TRANSACTIONTYPE)
+          ->setMetadataValue(
+            PhabricatorApplicationPolicyChangeTransaction::METADATA_ATTRIBUTE,
+            $capability)
+          ->setNewValue($new);
       }
 
       if ($result) {
-        $key = 'phabricator.application-settings';
-        $config_entry = PhabricatorConfigEntry::loadConfigEntry($key);
-        $value = $config_entry->getValue();
-
-        $phid = $application->getPHID();
-        if (empty($value[$phid])) {
-          $value[$application->getPHID()] = array();
-        }
-        if (empty($value[$phid]['policy'])) {
-          $value[$phid]['policy'] = array();
-        }
-
-        $value[$phid]['policy'] = $result + $value[$phid]['policy'];
-
-        // Don't allow users to make policy edits which would lock them out of
-        // applications, since they would be unable to undo those actions.
-        PhabricatorEnv::overrideConfig($key, $value);
-        PhabricatorPolicyFilter::mustRetainCapability(
-          $user,
-          $application,
-          PhabricatorPolicyCapability::CAN_VIEW);
-
-        PhabricatorPolicyFilter::mustRetainCapability(
-          $user,
-          $application,
-          PhabricatorPolicyCapability::CAN_EDIT);
-
-        PhabricatorConfigEditor::storeNewValue(
-          $user,
-          $config_entry,
-          $value,
-          PhabricatorContentSource::newFromRequest($request));
-      }
+        $editor = id(new PhabricatorApplicationEditor())
+          ->setActor($user)
+          ->setContentSourceFromRequest($request)
+          ->setContinueOnNoEffect(true)
+          ->setContinueOnMissingFields(true);
 
+        try {
+          $editor->applyTransactions($application, $xactions);
           return id(new AphrontRedirectResponse())->setURI($view_uri);
+        } catch (PhabricatorApplicationTransactionValidationException $ex) {
+          $validation_exception = $ex;
+        }
+
+        return $this->newDialog()
+          ->setTitle('Validation Failed')
+          ->setValidationException($validation_exception)
+          ->addCancelButton($view_uri);
+      }
     }
 
     $descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions(

src/applications/meta/editor/PhabricatorApplicationEditEngine.php

@@ -0,0 +1,64 @@
+<?php
+
+final class PhabricatorApplicationEditEngine
+  extends PhabricatorEditEngine {
+
+  const ENGINECONST = 'application.application';
+
+  public function getEngineApplicationClass() {
+    return 'PhabricatorApplicationsApplication';
+  }
+
+  public function getEngineName() {
+    return pht('Applications');
+  }
+
+  public function getSummaryHeader() {
+    return pht('Configure Application Forms');
+  }
+
+  public function getSummaryText() {
+    return pht('Configure creation and editing forms in Applications.');
+  }
+
+  public function isEngineConfigurable() {
+    return false;
+  }
+
+  protected function newEditableObject() {
+    throw new PhutilMethodNotImplementedException();
+  }
+
+  protected function newObjectQuery() {
+    return new PhabricatorApplicationQuery();
+  }
+
+  protected function getObjectCreateTitleText($object) {
+    return pht('Create New Application');
+  }
+
+  protected function getObjectEditTitleText($object) {
+    return pht('Edit Application: %s', $object->getName());
+  }
+
+  protected function getObjectEditShortText($object) {
+    return $object->getName();
+  }
+
+  protected function getObjectCreateShortText() {
+    return pht('Create Application');
+  }
+
+  protected function getObjectName() {
+    return pht('Application');
+  }
+
+  protected function getObjectViewURI($object) {
+    return $object->getViewURI();
+  }
+
+  protected function buildCustomEditFields($object) {
+    return array();
+  }
+
+}

src/applications/meta/editor/PhabricatorApplicationEditor.php

@@ -0,0 +1,46 @@
+<?php
+
+final class PhabricatorApplicationEditor
+  extends PhabricatorApplicationTransactionEditor {
+
+  public function getEditorApplicationClass() {
+    return 'PhabricatorApplicationsApplication';
+  }
+
+  public function getEditorObjectsDescription() {
+    return pht('Application');
+  }
+
+  protected function supportsSearch() {
+    return true;
+  }
+
+  public function getTransactionTypes() {
+    $types = parent::getTransactionTypes();
+    $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
+    $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
+
+    return $types;
+  }
+
+  protected function shouldSendMail(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+    return false;
+  }
+
+  protected function shouldPublishFeedStory(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+    return true;
+  }
+
+  protected function getMailTo(PhabricatorLiskDAO $object) {
+    return array();
+  }
+
+  protected function getMailCC(PhabricatorLiskDAO $object) {
+    return array();
+  }
+
+}

src/applications/meta/storage/PhabricatorApplicationApplicationTransaction.php

@@ -11,10 +11,6 @@ final class PhabricatorApplicationApplicationTransaction
     return PhabricatorApplicationApplicationPHIDType::TYPECONST;
   }
 
-  public function getApplicationTransactionCommentObject() {
-  return new PhabricatorApplicationTransactionComment();
-  }
-
   public function getBaseTransactionClass() {
     return 'PhabricatorApplicationTransactionType';
   }

src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php

@@ -0,0 +1,197 @@
+<?php
+
+final class PhabricatorApplicationPolicyChangeTransaction
+  extends PhabricatorApplicationTransactionType {
+
+  const TRANSACTIONTYPE = 'application.policy';
+  const METADATA_ATTRIBUTE = 'capability.name';
+
+  private $policies;
+
+  public function generateOldValue($object) {
+    $application = $object;
+    $capability = $this->getCapabilityName();
+    return $application->getPolicy($capability);
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $application = $object;
+    $user = $this->getActor();
+
+    $key = 'phabricator.application-settings';
+    $config_entry = PhabricatorConfigEntry::loadConfigEntry($key);
+    $current_value = $config_entry->getValue();
+
+    $phid = $application->getPHID();
+    if (empty($current_value[$phid])) {
+      $current_value[$application->getPHID()] = array();
+    }
+    if (empty($current_value[$phid]['policy'])) {
+      $current_value[$phid]['policy'] = array();
+    }
+
+    $new = array($this->getCapabilityName() => $value);
+    $current_value[$phid]['policy'] = $new + $current_value[$phid]['policy'];
+
+    $editor = $this->getEditor();
+    $content_source = $editor->getContentSource();
+    PhabricatorConfigEditor::storeNewValue(
+      $user,
+      $config_entry,
+      $current_value,
+      $content_source);
+  }
+
+  public function getTitle() {
+    $old = $this->renderPolicy($this->getOldValue());
+    $new = $this->renderPolicy($this->getNewValue());
+
+    return pht(
+      '%s changed the "%s" policy from "%s" to "%s".',
+      $this->renderAuthor(),
+      $this->renderCapability(),
+      $old,
+      $new);
+  }
+
+  public function getTitleForFeed() {
+    $old = $this->renderPolicy($this->getOldValue());
+    $new = $this->renderPolicy($this->getNewValue());
+
+    return pht(
+      '%s changed the "%s" policy for application %s from "%s" to "%s".',
+      $this->renderAuthor(),
+      $this->renderCapability(),
+      $this->renderObject(),
+      $old,
+      $new);
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $user = $this->getActor();
+    $application = $object;
+    $policies = id(new PhabricatorPolicyQuery())
+      ->setViewer($user)
+      ->setObject($application)
+      ->execute();
+
+    $errors = array();
+    foreach ($xactions as $xaction) {
+      $new = $xaction->getNewValue();
+      $capability = $xaction->getMetadataValue(self::METADATA_ATTRIBUTE);
+
+      if (empty($policies[$new])) {
+        // Not a standard policy, check for a custom policy.
+        $policy = id(new PhabricatorPolicyQuery())
+          ->setViewer($user)
+          ->withPHIDs(array($new))
+          ->executeOne();
+        if (!$policy) {
+          $errors[] = $this->newInvalidError(
+            pht('Policy does not exist.'));
+          continue;
+        }
+      } else {
+        $policy = idx($policies, $new);
+      }
+
+      if (!$policy->isValidPolicyForEdit()) {
+        $errors[] = $this->newInvalidError(
+            pht('Can\'t set the policy to a policy you can\'t view!'));
+          continue;
+      }
+
+      if ($new == PhabricatorPolicies::POLICY_PUBLIC) {
+        $capobj = PhabricatorPolicyCapability::getCapabilityByKey(
+          $capability);
+        if (!$capobj || !$capobj->shouldAllowPublicPolicySetting()) {
+          $errors[] = $this->newInvalidError(
+            pht('Can\'t set non-public policies to public.'));
+          continue;
+        }
+      }
+
+      if (!$application->isCapabilityEditable($capability)) {
+        $errors[] = $this->newInvalidError(
+          pht('Capability "%s" is not editable for this application.',
+            $capability));
+        continue;
+      }
+    }
+
+    // If we're changing these policies, the viewer needs to still be able to
+    // view or edit the application under the new policy.
+    $validate_map = array(
+      PhabricatorPolicyCapability::CAN_VIEW,
+      PhabricatorPolicyCapability::CAN_EDIT,
+    );
+    $validate_map = array_fill_keys($validate_map, array());
+
+    foreach ($xactions as $xaction) {
+      $capability = $xaction->getMetadataValue(self::METADATA_ATTRIBUTE);
+      if (!isset($validate_map[$capability])) {
+        continue;
+      }
+
+      $validate_map[$capability][] = $xaction;
+    }
+
+    foreach ($validate_map as $capability => $cap_xactions) {
+      if (!$cap_xactions) {
+        continue;
+      }
+
+      $editor = $this->getEditor();
+      $policy_errors = $editor->validatePolicyTransaction(
+        $object,
+        $cap_xactions,
+        self::TRANSACTIONTYPE,
+        $capability);
+
+      foreach ($policy_errors as $error) {
+        $errors[] = $error;
+      }
+    }
+
+    return $errors;
+  }
+
+  private function renderPolicy($name) {
+    $policies = $this->getAllPolicies();
+    if (empty($policies[$name])) {
+      // Not a standard policy, check for a custom policy.
+      $policy = id(new PhabricatorPolicyQuery())
+        ->setViewer($this->getViewer())
+        ->withPHIDs(array($name))
+        ->executeOne();
+      $policies[$name] = $policy;
+    }
+
+    $policy = idx($policies, $name);
+    return $this->renderValue($policy->getFullName());
+  }
+
+  private function getAllPolicies() {
+    if (!$this->policies) {
+      $viewer = $this->getViewer();
+      $application = $this->getObject();
+      $this->policies = id(new PhabricatorPolicyQuery())
+        ->setViewer($viewer)
+        ->setObject($application)
+        ->execute();
+    }
+
+    return $this->policies;
+  }
+
+  private function renderCapability() {
+    $application = $this->getObject();
+    $capability = $this->getCapabilityName();
+    return $application->getCapabilityLabel($capability);
+  }
+
+  private function getCapabilityName() {
+    return $this->getMetadataValue(self::METADATA_ATTRIBUTE);
+  }
+
+}

src/applications/policy/storage/PhabricatorPolicy.php

@@ -264,9 +264,11 @@ final class PhabricatorPolicy
   public function getFullName() {
     switch ($this->getType()) {
       case PhabricatorPolicyType::TYPE_PROJECT:
-        return pht('Project: %s', $this->getName());
+        return pht('Members of Project: %s', $this->getName());
       case PhabricatorPolicyType::TYPE_MASKED:
         return pht('Other: %s', $this->getName());
+      case PhabricatorPolicyType::TYPE_USER:
+        return pht('Only User: %s', $this->getName());
       default:
         return $this->getName();
     }
@@ -422,6 +424,10 @@ final class PhabricatorPolicy
     return ($this_strength > $other_strength);
   }
 
+  public function isValidPolicyForEdit() {
+    return $this->getType() !== PhabricatorPolicyType::TYPE_MASKED;
+  }
+
   public static function getSpecialRules(
     PhabricatorPolicyInterface $object,
     PhabricatorUser $viewer,

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php

@@ -334,6 +334,8 @@ abstract class PhabricatorApplicationTransactionEditor
 
     $xtype = $this->getModularTransactionType($type);
     if ($xtype) {
+      $xtype = clone $xtype;
+      $xtype->setStorage($xaction);
       return $xtype->generateOldValue($object);
     }
 
@@ -414,6 +416,8 @@ abstract class PhabricatorApplicationTransactionEditor
 
     $xtype = $this->getModularTransactionType($type);
     if ($xtype) {
+      $xtype = clone $xtype;
+      $xtype->setStorage($xaction);
       return $xtype->generateNewValue($object, $xaction->getNewValue());
     }
 
@@ -553,6 +557,8 @@ abstract class PhabricatorApplicationTransactionEditor
 
     $xtype = $this->getModularTransactionType($type);
     if ($xtype) {
+      $xtype = clone $xtype;
+      $xtype->setStorage($xaction);
       return $xtype->applyInternalEffects($object, $xaction->getNewValue());
     }
 
@@ -2163,7 +2169,7 @@ abstract class PhabricatorApplicationTransactionEditor
     return array_mergev($errors);
   }
 
-  private function validatePolicyTransaction(
+  public function validatePolicyTransaction(
     PhabricatorLiskDAO $object,
     array $xactions,
     $transaction_type,
@@ -2772,7 +2778,11 @@ abstract class PhabricatorApplicationTransactionEditor
     }
 
     if (!$has_support) {
-      throw new Exception(pht('Capability not supported.'));
+      throw new Exception(
+        pht('The object being edited does not implement any standard '.
+          'interfaces (like PhabricatorSubscribableInterface) which allow '.
+          'CCs to be generated automatically. Override the "getMailCC()" '.
+          'method and generate CCs explicitly.'));
     }
 
     return array_mergev($phids);

src/applications/transactions/storage/PhabricatorModularTransactionType.php

@@ -315,4 +315,8 @@ abstract class PhabricatorModularTransactionType
     return $editor->getPHIDList($old, $new);
   }
 
+  public function getMetadataValue($key, $default = null) {
+    return $this->getStorage()->getMetadataValue($key, $default);
+  }
+
 }