From d58eddcf0ad4bec4762632a351f6b604f0a6afb4 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 18 Nov 2019 22:49:40 -0800
Subject: [PATCH] When predicting project membership during edits, predict
 milestones will have parent membership

Summary:
Depends on D20919. Ref T13462. When editing milestones, we currently predict they will have no members for policy evaluation purposes. This isn't the right rule.

Instead, predict that their membership will be the same as the parent project's membership, and pass this hint to the policy layer.

See T13462 for additional context and discussion.

Test Plan:
  - Set project A's edit policy to "Project Members".
  - Joined project A.
  - Tried to create a milestone of project A.
    - Before: policy exception that the edit policy excludes me.
    - After: clean milestone creation.
  - As a non-member, tried to create a milestone. Received appropriate policy error.

Maniphest Tasks: T13462

Differential Revision: https://secure.phabricator.com/D20920
---
 .../PhabricatorProjectTransactionEditor.php   | 75 ++++++++++++-------
 1 file changed, 47 insertions(+), 28 deletions(-)

diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
index 729674e120..eb57c39b2c 100644
--- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
+++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
@@ -345,40 +345,59 @@ final class PhabricatorProjectTransactionEditor
       $copy->setMilestoneNumber(1);
     }
 
-    $member_xaction = null;
-    foreach ($xactions as $xaction) {
-      if ($xaction->getTransactionType() !== $type_edge) {
-        continue;
+    $hint = null;
+    if ($this->getIsMilestone()) {
+      // See T13462. If we're creating a milestone, predict that the members
+      // of the newly created milestone will be the same as the members of the
+      // parent project, since this is the governing rule.
+
+      $parent = $copy->getParentProject();
+
+      $parent = id(new PhabricatorProjectQuery())
+        ->setViewer($this->getActor())
+        ->withPHIDs(array($parent->getPHID()))
+        ->needMembers(true)
+        ->executeOne();
+      $members = $parent->getMemberPHIDs();
+
+      $hint = array_fuse($members);
+    } else {
+      $member_xaction = null;
+      foreach ($xactions as $xaction) {
+        if ($xaction->getTransactionType() !== $type_edge) {
+          continue;
+        }
+
+        $edgetype = $xaction->getMetadataValue('edge:type');
+        if ($edgetype !== $edgetype_member) {
+          continue;
+        }
+
+        $member_xaction = $xaction;
       }
 
-      $edgetype = $xaction->getMetadataValue('edge:type');
-      if ($edgetype !== $edgetype_member) {
-        continue;
-      }
+      if ($member_xaction) {
+        $object_phid = $object->getPHID();
 
-      $member_xaction = $xaction;
+        if ($object_phid) {
+          $project = id(new PhabricatorProjectQuery())
+            ->setViewer($this->getActor())
+            ->withPHIDs(array($object_phid))
+            ->needMembers(true)
+            ->executeOne();
+          $members = $project->getMemberPHIDs();
+        } else {
+          $members = array();
+        }
+
+        $clone_xaction = clone $member_xaction;
+        $hint = $this->getPHIDTransactionNewValue($clone_xaction, $members);
+        $hint = array_fuse($hint);
+      }
     }
 
-    if ($member_xaction) {
-      $object_phid = $object->getPHID();
-
-      if ($object_phid) {
-        $project = id(new PhabricatorProjectQuery())
-          ->setViewer($this->getActor())
-          ->withPHIDs(array($object_phid))
-          ->needMembers(true)
-          ->executeOne();
-        $members = $project->getMemberPHIDs();
-      } else {
-        $members = array();
-      }
-
-      $clone_xaction = clone $member_xaction;
-      $hint = $this->getPHIDTransactionNewValue($clone_xaction, $members);
+    if ($hint !== null) {
       $rule = new PhabricatorProjectMembersPolicyRule();
-
-      $hint = array_fuse($hint);
-
       PhabricatorPolicyRule::passTransactionHintToRule(
         $copy,
         $rule,