From 959504a4881cc238d81208244e6801b9afb087ca Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 18 Nov 2019 22:21:15 -0800
Subject: [PATCH] When predicting object policies for project milestones,
 adjust objects so they behave like milestones

Summary:
Ref T13462. Currently, when testing milestone edit policies during creation, the project object does not behave like a milestone:

  - it doesn't have a milestone number yet, so it doesn't try to access the parent project; and
  - the parent project isn't attached yet.

Instead: attach the parent project sooner (which "should" be harmless, although it's possible this has weird side effects); and give the adjusted policy object a dummy milestone number if it doesn't have one yet. This forces it to act like a milestone when emitting policies.

Test Plan:
  - Set "Projects" application default edit policy to "No One".
  - Created a milestone I had permission to create.
    - Before: failed with a policy error, because the project behaved like a non-milestone and returned "No One" as the effective edit policy.
    - After: worked properly, correctly evaluting the parent project edit policy as the effective edit policy.
  - Tried to create a milestone I did not have permission to create (no edit permission on parent project).
    - Got an appropriate edit policy error.

Maniphest Tasks: T13462

Differential Revision: https://secure.phabricator.com/D20919
---
 .../project/__tests__/PhabricatorProjectCoreTestCase.php | 3 +--
 .../editor/PhabricatorProjectTransactionEditor.php       | 9 +++++++++
 src/applications/project/storage/PhabricatorProject.php  | 2 +-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
index 186ac7dea4..a31bf8853c 100644
--- a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
+++ b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
@@ -1537,8 +1537,7 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
     PhabricatorProject $parent = null,
     $is_milestone = false) {
 
-    $project = PhabricatorProject::initializeNewProject($user);
-
+    $project = PhabricatorProject::initializeNewProject($user, $parent);
 
     $name = pht('Test Project %d', mt_rand());
 
diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
index b714f66830..729674e120 100644
--- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
+++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
@@ -336,6 +336,15 @@ final class PhabricatorProjectTransactionEditor
     $type_edge = PhabricatorTransactions::TYPE_EDGE;
     $edgetype_member = PhabricatorProjectProjectHasMemberEdgeType::EDGECONST;
 
+    // See T13462. If we're creating a milestone, set a dummy milestone
+    // number so the project behaves like a milestone and uses milestone
+    // policy rules. Otherwise, we'll end up checking the default policies
+    // (which are not relevant to milestones) instead of the parent project
+    // policies (which are the correct policies).
+    if ($this->getIsMilestone() && !$copy->isMilestone()) {
+      $copy->setMilestoneNumber(1);
+    }
+
     $member_xaction = null;
     foreach ($xactions as $xaction) {
       if ($xaction->getTransactionType() !== $type_edge) {
diff --git a/src/applications/project/storage/PhabricatorProject.php b/src/applications/project/storage/PhabricatorProject.php
index 765c8a6b43..860d6e1749 100644
--- a/src/applications/project/storage/PhabricatorProject.php
+++ b/src/applications/project/storage/PhabricatorProject.php
@@ -107,7 +107,7 @@ final class PhabricatorProject extends PhabricatorProjectDAO
       ->setHasMilestones(0)
       ->setHasSubprojects(0)
       ->setSubtype(PhabricatorEditEngineSubtype::SUBTYPE_DEFAULT)
-      ->attachParentProject(null);
+      ->attachParentProject($parent);
   }
 
   public function getCapabilities() {