Page MenuHomePhabricator
Files F14351738

D15167.id36618.diff
No OneTemporary
Actions

D15167.id36618.diff
View Options

diff --git a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
--- a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
+++ b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
@@ -808,6 +808,114 @@
pht('Engineering + Scan'));
}
+ public function testTagAncestryConflicts() {
+ $user = $this->createUser();
+ $user->save();
+
+ $stonework = $this->createProject($user);
+ $stonework_masonry = $this->createProject($user, $stonework);
+ $stonework_sculpting = $this->createProject($user, $stonework);
+
+ $task = $this->newTask($user, array());
+ $this->assertEqual(array(), $this->getTaskProjects($task));
+
+ $this->addProjectTags($user, $task, array($stonework->getPHID()));
+ $this->assertEqual(
+ array(
+ $stonework->getPHID(),
+ ),
+ $this->getTaskProjects($task));
+
+ // Adding a descendant should remove the parent.
+ $this->addProjectTags($user, $task, array($stonework_masonry->getPHID()));
+ $this->assertEqual(
+ array(
+ $stonework_masonry->getPHID(),
+ ),
+ $this->getTaskProjects($task));
+
+ // Adding an ancestor should remove the descendant.
+ $this->addProjectTags($user, $task, array($stonework->getPHID()));
+ $this->assertEqual(
+ array(
+ $stonework->getPHID(),
+ ),
+ $this->getTaskProjects($task));
+
+ // Adding two tags in the same hierarchy which are not mutual ancestors
+ // should remove the ancestor but otherwise work fine.
+ $this->addProjectTags(
+ $user,
+ $task,
+ array(
+ $stonework_masonry->getPHID(),
+ $stonework_sculpting->getPHID(),
+ ));
+
+ $expect = array(
+ $stonework_masonry->getPHID(),
+ $stonework_sculpting->getPHID(),
+ );
+ sort($expect);
+
+ $this->assertEqual($expect, $this->getTaskProjects($task));
+ }
+
+ public function testTagMilestoneConflicts() {
+ $user = $this->createUser();
+ $user->save();
+
+ $stonework = $this->createProject($user);
+ $stonework_1 = $this->createProject($user, $stonework, true);
+ $stonework_2 = $this->createProject($user, $stonework, true);
+
+ $task = $this->newTask($user, array());
+ $this->assertEqual(array(), $this->getTaskProjects($task));
+
+ $this->addProjectTags($user, $task, array($stonework->getPHID()));
+ $this->assertEqual(
+ array(
+ $stonework->getPHID(),
+ ),
+ $this->getTaskProjects($task));
+
+ // Adding a milesone should remove the parent.
+ $this->addProjectTags($user, $task, array($stonework_1->getPHID()));
+ $this->assertEqual(
+ array(
+ $stonework_1->getPHID(),
+ ),
+ $this->getTaskProjects($task));
+
+ // Adding the parent should remove the milestone.
+ $this->addProjectTags($user, $task, array($stonework->getPHID()));
+ $this->assertEqual(
+ array(
+ $stonework->getPHID(),
+ ),
+ $this->getTaskProjects($task));
+
+ // First, add one milestone.
+ $this->addProjectTags($user, $task, array($stonework_1->getPHID()));
+ // Now, adding a second milestone should remove the first milestone.
+ $this->addProjectTags($user, $task, array($stonework_2->getPHID()));
+ $this->assertEqual(
+ array(
+ $stonework_2->getPHID(),
+ ),
+ $this->getTaskProjects($task));
+ }
+
+ private function getTaskProjects(ManiphestTask $task) {
+ $project_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
+ $task->getPHID(),
+ PhabricatorProjectObjectHasProjectEdgeType::EDGECONST);
+
+ sort($project_phids);
+
+ return $project_phids;
+ }
+
private function attemptProjectEdit(
PhabricatorProject $proj,
PhabricatorUser $user,
@@ -827,6 +935,30 @@
}
+ private function addProjectTags(
+ PhabricatorUser $viewer,
+ ManiphestTask $task,
+ array $phids) {
+
+ $xactions = array();
+
+ $xactions[] = id(new ManiphestTransaction())
+ ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
+ ->setMetadataValue(
+ 'edge:type',
+ PhabricatorProjectObjectHasProjectEdgeType::EDGECONST)
+ ->setNewValue(
+ array(
+ '+' => array_fuse($phids),
+ ));
+
+ $editor = id(new ManiphestTransactionEditor())
+ ->setActor($viewer)
+ ->setContentSource(PhabricatorContentSource::newConsoleSource())
+ ->setContinueOnNoEffect(true)
+ ->applyTransactions($task, $xactions);
+ }
+
private function newTask(
PhabricatorUser $viewer,
array $projects,
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -400,7 +400,15 @@
return $space_phid;
}
case PhabricatorTransactions::TYPE_EDGE:
- return $this->getEdgeTransactionNewValue($xaction);
+ $new_value = $this->getEdgeTransactionNewValue($xaction);
+
+ $edge_type = $xaction->getMetadataValue('edge:type');
+ $type_project = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST;
+ if ($edge_type == $type_project) {
+ $new_value = $this->applyProjectConflictRules($new_value);
+ }
+
+ return $new_value;
case PhabricatorTransactions::TYPE_CUSTOMFIELD:
$field = $this->getCustomFieldForTransaction($object, $xaction);
return $field->getNewValueFromApplicationTransactions($xaction);
@@ -3346,4 +3354,127 @@
return $state;
}
+
+ /**
+ * Remove conflicts from a list of projects.
+ *
+ * Objects aren't allowed to be tagged with multiple milestones in the same
+ * group, nor projects such that one tag is the ancestor of any other tag.
+ * If the list of PHIDs include mutually exclusive projects, remove the
+ * conflicting projects.
+ *
+ * @param list<phid> List of project PHIDs.
+ * @return list<phid> List with conflicts removed.
+ */
+ private function applyProjectConflictRules(array $phids) {
+ if (!$phids) {
+ return array();
+ }
+
+ // Overall, the last project in the list wins in cases of conflict (so when
+ // you add something, the thing you just added sticks and removes older
+ // values).
+
+ // Beyond that, there are two basic cases:
+
+ // Milestones: An object can't be in "A > Sprint 3" and "A > Sprint 4".
+ // If multiple projects are milestones of the same parent, we only keep the
+ // last one.
+
+ // Ancestor: You can't be in "A" and "A > B". If "A > B" comes later
+ // in the list, we remove "A" and keep "A > B". If "A" comes later, we
+ // remove "A > B" and keep "A".
+
+ // Note that it's OK to be in "A > B" and "A > C". There's only a conflict
+ // if one project is an ancestor of another. It's OK to have something
+ // tagged with multiple projects which share a common ancestor, so long as
+ // they are not mutual ancestors.
+
+ $viewer = PhabricatorUser::getOmnipotentUser();
+
+ $projects = id(new PhabricatorProjectQuery())
+ ->setViewer($viewer)
+ ->withPHIDs(array_keys($phids))
+ ->execute();
+ $projects = mpull($projects, null, 'getPHID');
+
+ // We're going to build a map from each project with milestones to the last
+ // milestone in the list. This last milestone is the milestone we'll keep.
+ $milestone_map = array();
+
+ // We're going to build a set of the projects which have no descendants
+ // later in the list. This allows us to apply both ancestor rules.
+ $ancestor_map = array();
+
+ foreach ($phids as $phid => $ignored) {
+ $project = idx($projects, $phid);
+ if (!$project) {
+ continue;
+ }
+
+ // This is the last milestone we've seen, so set it as the selection for
+ // the project's parent. This might be setting a new value or overwriting
+ // an earlier value.
+ if ($project->isMilestone()) {
+ $parent_phid = $project->getParentProjectPHID();
+ $milestone_map[$parent_phid] = $phid;
+ }
+
+ // Since this is the last item in the list we've examined so far, add it
+ // to the set of projects with no later descendants.
+ $ancestor_map[$phid] = $phid;
+
+ // Remove any ancestors from the set, since this is a later descendant.
+ foreach ($project->getAncestorProjects() as $ancestor) {
+ $ancestor_phid = $ancestor->getPHID();
+ unset($ancestor_map[$ancestor_phid]);
+ }
+ }
+
+ // Now that we've built the maps, we can throw away all the projects which
+ // have conflicts.
+ foreach ($phids as $phid => $ignored) {
+ $project = idx($projects, $phid);
+
+ if (!$project) {
+ // If a PHID is invalid, we just leave it as-is. We could clean it up,
+ // but leaving it untouched is less likely to cause collateral damage.
+ continue;
+ }
+
+ // If this was a milestone, check if it was the last milestone from its
+ // group in the list. If not, remove it from the list.
+ if ($project->isMilestone()) {
+ $parent_phid = $project->getParentProjectPHID();
+ if ($milestone_map[$parent_phid] !== $phid) {
+ unset($phids[$phid]);
+ continue;
+ }
+ }
+
+ // If a later project in the list is a subproject of this one, it will
+ // have removed ancestors from the map. If this project does not point
+ // at itself in the ancestor map, it should be discarded in favor of a
+ // subproject that comes later.
+ if (idx($ancestor_map, $phid) !== $phid) {
+ unset($phids[$phid]);
+ continue;
+ }
+
+ // If a later project in the list is an ancestor of this one, it will
+ // have added itself to the map. If any ancestor of this project points
+ // at itself in the map, this project should be dicarded in favor of
+ // that later ancestor.
+ foreach ($project->getAncestorProjects() as $ancestor) {
+ $ancestor_phid = $ancestor->getPHID();
+ if (isset($ancestor_map[$ancestor_phid])) {
+ unset($phids[$phid]);
+ continue 2;
+ }
+ }
+ }
+
+ return $phids;
+ }
+
}
diff --git a/src/docs/user/userguide/projects.diviner b/src/docs/user/userguide/projects.diviner
--- a/src/docs/user/userguide/projects.diviner
+++ b/src/docs/user/userguide/projects.diviner
@@ -184,6 +184,27 @@
Subprojects can have normal workboards.
+The maximum subproject depth is 16. This limit is intended to grossly exceed
+the depth necessary in normal usage.
+
+Objects may not be tagged with projects that are ancestors or descendants of
+one another. For example, a task may not be tagged with both {nav Stonework}
+and {nav Stonework > Masonry}, because the ancestor tag is redundant and makes
+tag management more complex.
+
+When a project tag is added that is the ancestor or descendant of one or more
+existing tags, the old tags are replaced. For example, adding
+{nav Stonework > Masonry} to a task tagged with {nav Stonework} will replace
+{nav Stonework} with the newer, more specific tag.
+
+This restriction does not apply to projects which share some common ancestor
+but are not themselves mutual ancestors. For example, a task may be tagged
+with both {nav Stonework > Masonry} and {nav Stonework > Sculpting}.
+
+This restriction //does// apply when the descendant is a milestone. For
+example, a task may not be tagged with both {nav Stonework} and
+{nav Stonework > Iteration II}.
+
Milestones
==========
@@ -204,6 +225,19 @@
Milestones can have normal workboards.
+Objects may not be tagged with two different milestones of the same parent
+project. For example, a task may not be tagged with both {nav Stonework >
+Iteration III} and {nav Stonework > Iteration V}.
+
+When a milestone tag is added to an object which already has a tag from the
+same series of milestones, the old tag is removed. For example, adding the
+{nav Stonework > Iteration V} tag to a task which already has the
+{nav Stonework > Iteration III} tag will remove the {nav Iteration III} tag.
+
+This restriction does not apply to milestones which are not part of the same
+series. For example, a task may be tagged with both
+{nav Stonework > Iteration V} and {nav Heraldry > Iteration IX}.
+
Parent Projects
===============

File Metadata

Mime Type
text/plain
Expires
Fri, Dec 20, 9:56 AM (25 m, 27 s)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6910120
Default Alt Text
D15167.id36618.diff (12 KB)

Event Timeline

Phabricator · Built by Phacility