From 587faa6f677c8d7e914051e5307ade00e2bf43b0 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 6 Nov 2017 10:53:14 -0800
Subject: [PATCH] Remove some defunct old-style transaction policy checks

Summary:
Ref PHI193. This method of enforcing policy checks is now (mostly) obsolete, and they're generally checked at the Controller/API level instead.

Notably, this method does not call `adjustObjectForPolicyChecks(...)` properly, so it can not handle special cases like "creating a project and taking its newly created members into account" for object policies like "Project Members".

Just remove these checks, which are redundant with checks elsewhere.

Test Plan:
  - Set Project application default edit policy to "Administrators and Project Members".
  - Tried to create a project as a non-administrator, adding myself.
  - Before patch: policy fatal on a VOID object (the project with no PHID generated yet).
  - After patch: object created properly. Got a sensible policy error if I didn't include myself as a member.
  - Also verified that other edit rules are still enforced/respected (I can't edit stuff I shouldn't be able to edit).
  - There's at least a bit of unit test coverage of this, too, which I updated to work via API (which hits the new broad capability checks) instead of via low-level transactions (which enforce only a subset of policy operations now).

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18763
---
 .../PhabricatorProjectCoreTestCase.php        | 20 ++++++++++++-------
 .../PhabricatorProjectTransactionEditor.php   | 10 ----------
 2 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
index d9356357b3..f34f224521 100644
--- a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
+++ b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
@@ -146,8 +146,7 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
     $user = $this->createUser();
     $user->save();
 
-    $user2 = $this->createUser();
-    $user2->save();
+    $user->setAllowInlineCacheGeneration(true);
 
     $proj = $this->createProject($user);
 
@@ -1289,12 +1288,19 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
 
     $new_name = $proj->getName().' '.mt_rand();
 
-    $xaction = new PhabricatorProjectTransaction();
-    $xaction->setTransactionType(
-      PhabricatorProjectNameTransaction::TRANSACTIONTYPE);
-    $xaction->setNewValue($new_name);
+    $params = array(
+      'objectIdentifier' => $proj->getID(),
+      'transactions' => array(
+        array(
+          'type' => 'name',
+          'value' => $new_name,
+        ),
+      ),
+    );
 
-    $this->applyTransactions($proj, $user, array($xaction));
+    id(new ConduitCall('project.edit', $params))
+      ->setUser($user)
+      ->execute();
 
     return true;
   }
diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
index adf4a3138b..2764ce6322 100644
--- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
+++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
@@ -120,16 +120,6 @@ final class PhabricatorProjectTransactionEditor
     PhabricatorApplicationTransaction $xaction) {
 
     switch ($xaction->getTransactionType()) {
-      case PhabricatorProjectNameTransaction::TRANSACTIONTYPE:
-      case PhabricatorProjectStatusTransaction::TRANSACTIONTYPE:
-      case PhabricatorProjectImageTransaction::TRANSACTIONTYPE:
-      case PhabricatorProjectIconTransaction::TRANSACTIONTYPE:
-      case PhabricatorProjectColorTransaction::TRANSACTIONTYPE:
-        PhabricatorPolicyFilter::requireCapability(
-          $this->requireActor(),
-          $object,
-          PhabricatorPolicyCapability::CAN_EDIT);
-        return;
       case PhabricatorProjectLockTransaction::TRANSACTIONTYPE:
         PhabricatorPolicyFilter::requireCapability(
           $this->requireActor(),