Improve policy description strings in ApplicationTransactions

Summary: Ref T3184. See discussion in D6042.

Test Plan: {F44265}

Reviewers: garoevans, btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T3184

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

src/applications/policy/filter/PhabricatorPolicy.php

@@ -7,6 +7,48 @@ final class PhabricatorPolicy {
   private $type;
   private $href;
 
+  public static function newFromPolicyAndHandle(
+    $policy_identifier,
+    PhabricatorObjectHandle $handle = null) {
+
+    $is_global = PhabricatorPolicyQuery::isGlobalPolicy($policy_identifier);
+    if ($is_global) {
+      return PhabricatorPolicyQuery::getGlobalPolicy($policy_identifier);
+    }
+
+    if (!$handle) {
+      throw new Exception(
+        "Policy identifier is an object PHID ('{$phid_identifier}'), but no ".
+        "object handle was provided. A handle must be provided for object ".
+        "policies.");
+    }
+
+    $handle_phid = $handle->getPHID();
+    if ($policy_identifier != $handle_phid) {
+      throw new Exception(
+        "Policy identifier is an object PHID ('{$phid_identifier}'), but ".
+        "the provided handle has a different PHID ('{$handle_phid}'). The ".
+        "handle must correspond to the policy identifier.");
+    }
+
+    $policy = id(new PhabricatorPolicy())
+      ->setPHID($policy_identifier)
+      ->setHref($handle->getURI());
+
+    $phid_type = phid_get_type($policy_identifier);
+    switch ($phid_type) {
+      case PhabricatorPHIDConstants::PHID_TYPE_PROJ:
+        $policy->setType(PhabricatorPolicyType::TYPE_PROJECT);
+        $policy->setName($handle->getName());
+        break;
+      default:
+        $policy->setType(PhabricatorPolicyType::TYPE_MASKED);
+        $policy->setName($handle->getFullName());
+        break;
+    }
+
+    return $policy;
+  }
 
   public function setType($type) {
     $this->type = $type;

src/applications/policy/query/PhabricatorPolicyQuery.php

@@ -104,12 +104,9 @@ final class PhabricatorPolicyQuery extends PhabricatorQuery {
         ->setViewer($this->viewer)
         ->loadHandles();
       foreach ($other_policies as $phid) {
-        $handle = $handles[$phid];
-        $results[$phid] = id(new PhabricatorPolicy())
-          ->setType(PhabricatorPolicyType::TYPE_MASKED)
-          ->setPHID($handle->getPHID())
-          ->setHref($handle->getLink())
-          ->setName($handle->getFullName());
+        $results[$phid] = PhabricatorPolicy::newFromPolicyAndHandle(
+          $phid,
+          $handles[$phid]);
       }
     }
 
@@ -128,6 +125,13 @@ final class PhabricatorPolicyQuery extends PhabricatorQuery {
     return false;
   }
 
+  public static function getGlobalPolicy($policy) {
+    if (!self::isGlobalPolicy($policy)) {
+      throw new Exception("Policy '{$policy}' is not a global policy!");
+    }
+    return idx(self::getGlobalPolicies(), $policy);
+  }
+
   private static function getGlobalPolicies() {
     static $constants = array(
       PhabricatorPolicies::POLICY_PUBLIC,

src/applications/transactions/storage/PhabricatorApplicationTransaction.php

@@ -150,6 +150,10 @@ abstract class PhabricatorApplicationTransaction
     return $this->handles[$phid];
   }
 
+  public function getHandleIfExists($phid) {
+    return idx($this->handles, $phid);
+  }
+
   public function getHandles() {
     if ($this->handles === null) {
       throw new Exception(
@@ -246,25 +250,27 @@ abstract class PhabricatorApplicationTransaction
           '%s added a comment.',
           $this->renderHandleLink($author_phid));
       case PhabricatorTransactions::TYPE_VIEW_POLICY:
-        // TODO: Render human-readable.
         return pht(
           '%s changed the visibility of this %s from "%s" to "%s".',
           $this->renderHandleLink($author_phid),
           $this->getApplicationObjectTypeName(),
-          PhabricatorPolicyQuery::isGlobalPolicy($old) ?
-            $old : $this->renderHandleLink($old),
-          PhabricatorPolicyQuery::isGlobalPolicy($new) ?
-            $new : $this->renderHandleLink($new));
+          PhabricatorPolicy::newFromPolicyAndHandle(
+            $old,
+            $this->getHandleIfExists($old))->renderDescription(),
+          PhabricatorPolicy::newFromPolicyAndHandle(
+            $new,
+            $this->getHandleIfExists($new))->renderDescription());
       case PhabricatorTransactions::TYPE_EDIT_POLICY:
-        // TODO: Render human-readable.
         return pht(
           '%s changed the edit policy of this %s from "%s" to "%s".',
           $this->renderHandleLink($author_phid),
           $this->getApplicationObjectTypeName(),
-          PhabricatorPolicyQuery::isGlobalPolicy($old) ?
-            $old : $this->renderHandleLink($old),
-          PhabricatorPolicyQuery::isGlobalPolicy($new) ?
-            $new : $this->renderHandleLink($new));
+          PhabricatorPolicy::newFromPolicyAndHandle(
+            $old,
+            $this->getHandleIfExists($old))->renderDescription(),
+          PhabricatorPolicy::newFromPolicyAndHandle(
+            $new,
+            $this->getHandleIfExists($new))->renderDescription());
       case PhabricatorTransactions::TYPE_SUBSCRIBERS:
         $add = array_diff($new, $old);
         $rem = array_diff($old, $new);