From a35d7c3c218a4f053ad35849a972eee513161dc0 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 11 Sep 2019 21:50:25 -0700
Subject: [PATCH] Update rendering of policy edit transactions in Applications

Summary:
Ref T13411. Since circa D19829, transactions have rendered policy changes in a modern way, notably making "Custom Policy" clickable to show the policy rules.

Edit transactions in Applications still use a separate, older approach to render policies. This produces policy renderings which don't use modern quoting rules and don't link in a modern way.

Make Applications use the same rendering code that other transactions (like normal edit/view edits) use.

Test Plan: Edited policies in Applications, saw more useful transactions in the log. Clicked "Custom Policy" in the transaction log and got a useful explanation of the policy.

Maniphest Tasks: T13411

Differential Revision: https://secure.phabricator.com/D20801
---
 ...atorApplicationPolicyChangeTransaction.php | 46 ++++---------------
 ...rApplicationTransactionValueController.php |  1 +
 2 files changed, 9 insertions(+), 38 deletions(-)

diff --git a/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php b/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php
index 5364a3d4fa..3c58b42ab5 100644
--- a/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php
+++ b/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php
@@ -52,15 +52,12 @@ final class PhabricatorApplicationPolicyChangeTransaction
   }
 
   public function getTitle() {
-    $old = $this->renderApplicationPolicy($this->getOldValue());
-    $new = $this->renderApplicationPolicy($this->getNewValue());
-
     return pht(
-      '%s changed the "%s" policy from "%s" to "%s".',
+      '%s changed the %s policy from %s to %s.',
       $this->renderAuthor(),
       $this->renderCapability(),
-      $old,
-      $new);
+      $this->renderOldPolicy(),
+      $this->renderNewPolicy());
   }
 
   public function getTitleForFeed() {
@@ -68,12 +65,12 @@ final class PhabricatorApplicationPolicyChangeTransaction
     $new = $this->renderApplicationPolicy($this->getNewValue());
 
     return pht(
-      '%s changed the "%s" policy for application %s from "%s" to "%s".',
+      '%s changed the %s policy for application %s from %s to %s.',
       $this->renderAuthor(),
       $this->renderCapability(),
       $this->renderObject(),
-      $old,
-      $new);
+      $this->renderOldPolicy(),
+      $this->renderNewPolicy());
   }
 
   public function validateTransactions($object, array $xactions) {
@@ -165,38 +162,11 @@ final class PhabricatorApplicationPolicyChangeTransaction
     return $errors;
   }
 
-  private function renderApplicationPolicy($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);
+    $label = $application->getCapabilityLabel($capability);
+    return $this->renderValue($label);
   }
 
   private function getCapabilityName() {
diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php
index bef6fef5a8..9c82ff99c8 100644
--- a/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php
+++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php
@@ -33,6 +33,7 @@ final class PhabricatorApplicationTransactionValueController
       case PhabricatorTransactions::TYPE_EDIT_POLICY:
       case PhabricatorTransactions::TYPE_JOIN_POLICY:
       case PhabricatorRepositoryPushPolicyTransaction::TRANSACTIONTYPE:
+      case PhabricatorApplicationPolicyChangeTransaction::TRANSACTIONTYPE:
         break;
       default:
         return new Aphront404Response();