diff --git a/resources/sql/patches/migrate-maniphest-revisions.php b/resources/sql/patches/migrate-maniphest-revisions.php new file mode 100644 index 0000000000..4b0fb049c1 --- /dev/null +++ b/resources/sql/patches/migrate-maniphest-revisions.php @@ -0,0 +1,42 @@ +getID(); + echo "Task {$id}: "; + + $revs = $task->getAttachedPHIDs(PhabricatorPHIDConstants::PHID_TYPE_DREV); + if (!$revs) { + echo "-\n"; + continue; + } + + $editor = new PhabricatorEdgeEditor(); + $editor->setSuppressEvents(true); + foreach ($revs as $rev) { + $editor->addEdge( + $task->getPHID(), + PhabricatorEdgeConfig::TYPE_TASK_HAS_RELATED_DREV, + $rev); + } + $editor->save(); + echo "OKAY\n"; +} + +echo "Done.\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 32c72cd619..f9f2b9758c 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -814,7 +814,6 @@ phutil_register_library_map(array( 'PhabricatorOAuthServerTestController' => 'applications/oauthserver/controller/PhabricatorOAuthServerTestController.php', 'PhabricatorOAuthServerTokenController' => 'applications/oauthserver/controller/PhabricatorOAuthServerTokenController.php', 'PhabricatorOAuthUnlinkController' => 'applications/auth/controller/PhabricatorOAuthUnlinkController.php', - 'PhabricatorObjectAttachmentEditor' => 'applications/search/editor/PhabricatorObjectAttachmentEditor.php', 'PhabricatorObjectHandle' => 'applications/phid/PhabricatorObjectHandle.php', 'PhabricatorObjectHandleConstants' => 'applications/phid/handle/const/PhabricatorObjectHandleConstants.php', 'PhabricatorObjectHandleData' => 'applications/phid/handle/PhabricatorObjectHandleData.php', diff --git a/src/applications/differential/field/specification/DifferentialManiphestTasksFieldSpecification.php b/src/applications/differential/field/specification/DifferentialManiphestTasksFieldSpecification.php index e21536efc1..136beceadc 100644 --- a/src/applications/differential/field/specification/DifferentialManiphestTasksFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialManiphestTasksFieldSpecification.php @@ -49,8 +49,12 @@ final class DifferentialManiphestTasksFieldSpecification private function getManiphestTaskPHIDs() { $revision = $this->getRevision(); - return $revision->getAttachedPHIDs( - PhabricatorPHIDConstants::PHID_TYPE_TASK); + if (!$revision->getPHID()) { + return array(); + } + return PhabricatorEdgeQuery::loadDestinationPHIDs( + $revision->getPHID(), + PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK); } /** @@ -59,14 +63,28 @@ final class DifferentialManiphestTasksFieldSpecification * @return void */ public function didWriteRevision(DifferentialRevisionEditor $editor) { - $aeditor = new PhabricatorObjectAttachmentEditor( - PhabricatorPHIDConstants::PHID_TYPE_DREV, - $editor->getRevision()); - $aeditor->setUser($this->getUser()); - $aeditor->attachObjects( - PhabricatorPHIDConstants::PHID_TYPE_TASK, - $this->maniphestTasks, - $two_way = true); + $revision = $editor->getRevision(); + $revision_phid = $revision->getPHID(); + $edge_type = PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK; + + $old_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( + $revision_phid, + $edge_type); + $add_phids = $this->maniphestTasks; + $rem_phids = array_diff($old_phids, $add_phids); + + $edge_editor = id(new PhabricatorEdgeEditor()) + ->setUser($this->getUser()); + + foreach ($add_phids as $phid) { + $edge_editor->addEdge($revision_phid, $edge_type, $phid); + } + + foreach ($rem_phids as $phid) { + $edge_editor->removeEdge($revision_phid, $edge_type, $phid); + } + + $edge_editor->save(); } protected function didSetRevision() { diff --git a/src/applications/maniphest/constants/ManiphestAction.php b/src/applications/maniphest/constants/ManiphestAction.php index 9b76394109..a449e489a0 100644 --- a/src/applications/maniphest/constants/ManiphestAction.php +++ b/src/applications/maniphest/constants/ManiphestAction.php @@ -38,6 +38,8 @@ final class ManiphestAction extends ManiphestConstants { const ACTION_DESCRIPTION = ManiphestTransactionType::TYPE_DESCRIPTION; const ACTION_REASSIGN = ManiphestTransactionType::TYPE_OWNER; const ACTION_ATTACH = ManiphestTransactionType::TYPE_ATTACH; + const ACTION_EDGE = ManiphestTransactionType::TYPE_EDGE; + const ACTION_AUXILIARY = ManiphestTransactionType::TYPE_AUXILIARY; public static function getActionPastTenseVerb($action) { static $map = array( @@ -45,7 +47,7 @@ final class ManiphestAction extends ManiphestConstants { self::ACTION_CLOSE => 'closed', self::ACTION_UPDATE => 'updated', self::ACTION_ASSIGN => 'assigned', - self::ACTION_REASSIGN => 'reassigned', + self::ACTION_REASSIGN => 'reassigned', self::ACTION_COMMENT => 'commented on', self::ACTION_CC => 'updated cc\'s of', self::ACTION_PRIORITY => 'changed the priority of', @@ -53,7 +55,9 @@ final class ManiphestAction extends ManiphestConstants { self::ACTION_TITLE => 'updated title of', self::ACTION_DESCRIPTION => 'updated description of', self::ACTION_ATTACH => 'attached something to', + self::ACTION_EDGE => 'changed related objects of', self::ACTION_REOPEN => 'reopened', + self::ACTION_AUXILIARY => 'updated an auxiliary field of', ); return idx($map, $action, "brazenly {$action}'d"); @@ -67,12 +71,14 @@ final class ManiphestAction extends ManiphestConstants { */ public static function selectStrongestAction(array $actions) { static $strengths = array( + self::ACTION_AUXILIARY => -1, self::ACTION_UPDATE => 0, self::ACTION_CC => 1, self::ACTION_PROJECT => 2, self::ACTION_DESCRIPTION => 3, self::ACTION_TITLE => 4, self::ACTION_ATTACH => 5, + self::ACTION_EDGE => 5, self::ACTION_COMMENT => 6, self::ACTION_PRIORITY => 7, self::ACTION_REASSIGN => 8, diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index f2e8199141..440af67794 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -54,6 +54,7 @@ final class ManiphestTaskDetailController extends ManiphestController { $e_commit = PhabricatorEdgeConfig::TYPE_TASK_HAS_COMMIT; $e_dep_on = PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK; $e_dep_by = PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK; + $e_rev = PhabricatorEdgeConfig::TYPE_TASK_HAS_RELATED_DREV; $phid = $task->getPHID(); @@ -64,12 +65,14 @@ final class ManiphestTaskDetailController extends ManiphestController { $e_commit, $e_dep_on, $e_dep_by, + $e_rev, )); $edges = $query->execute(); $commit_phids = array_keys($edges[$phid][$e_commit]); $dep_on_tasks = array_keys($edges[$phid][$e_dep_on]); $dep_by_tasks = array_keys($edges[$phid][$e_dep_by]); + $revs = array_keys($edges[$phid][$e_rev]); $phids = array_fill_keys($query->getDestinationPHIDs(), true); @@ -177,10 +180,9 @@ final class ManiphestTaskDetailController extends ManiphestController { array_select_keys($handles, $dep_on_tasks)); } - $revs = idx($attached, PhabricatorPHIDConstants::PHID_TYPE_DREV); if ($revs) { $dict['Revisions'] = $this->renderHandleList( - array_select_keys($handles, array_keys($revs))); + array_select_keys($handles, $revs)); } if ($commit_phids) { diff --git a/src/applications/maniphest/controller/ManiphestTaskEditController.php b/src/applications/maniphest/controller/ManiphestTaskEditController.php index c35d26bb02..abc832e912 100644 --- a/src/applications/maniphest/controller/ManiphestTaskEditController.php +++ b/src/applications/maniphest/controller/ManiphestTaskEditController.php @@ -248,22 +248,13 @@ final class ManiphestTaskEditController extends ManiphestController { if ($parent_task) { - $type_task = PhabricatorPHIDConstants::PHID_TYPE_TASK; - - // NOTE: It's safe to simply apply this transaction without doing - // cycle detection because we know the new task has no children. - $new_value = $parent_task->getAttached(); - $new_value[$type_task][$task->getPHID()] = array(); - - $parent_xaction = clone $template; - $attach_type = ManiphestTransactionType::TYPE_ATTACH; - $parent_xaction->setTransactionType($attach_type); - $parent_xaction->setNewValue($new_value); - - $editor = new ManiphestTransactionEditor(); - $editor->setAuxiliaryFields($aux_fields); - $editor->applyTransactions($parent_task, array($parent_xaction)); - + id(new PhabricatorEdgeEditor()) + ->setUser($user) + ->addEdge( + $parent_task->getPHID(), + PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK, + $task->getPHID()) + ->save(); $workflow = $parent_task->getID(); } diff --git a/src/applications/maniphest/view/ManiphestTransactionDetailView.php b/src/applications/maniphest/view/ManiphestTransactionDetailView.php index fb264a4dac..a8a906f6d5 100644 --- a/src/applications/maniphest/view/ManiphestTransactionDetailView.php +++ b/src/applications/maniphest/view/ManiphestTransactionDetailView.php @@ -623,7 +623,7 @@ final class ManiphestTransactionDetailView extends ManiphestView { $links = array(); foreach ($phids as $phid) { if ($this->forEmail) { - $links[] = $this->handles[$phid]->getName(); + $links[] = $this->handles[$phid]->getFullName(); } else { $links[] = $this->handles[$phid]->renderLink(); } @@ -664,6 +664,8 @@ final class ManiphestTransactionDetailView extends ManiphestView { private function getEdgeEmailTitle($type, array $list) { $count = count($list); switch ($type) { + case PhabricatorEdgeConfig::TYPE_TASK_HAS_RELATED_DREV: + return pht('DIFFERENTIAL %d REVISION(S)', $count); case PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK: return pht('DEPENDS ON %d TASK(S)', $count); case PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK: @@ -681,6 +683,8 @@ final class ManiphestTransactionDetailView extends ManiphestView { */ private function getEdgeAddVerb($type) { switch ($type) { + case PhabricatorEdgeConfig::TYPE_TASK_HAS_RELATED_DREV: + return pht('Added Revision'); case PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK: return pht('Added Dependency'); case PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK: @@ -698,6 +702,8 @@ final class ManiphestTransactionDetailView extends ManiphestView { */ private function getEdgeRemVerb($type) { switch ($type) { + case PhabricatorEdgeConfig::TYPE_TASK_HAS_RELATED_DREV: + return pht('Removed Revision'); case PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK: return pht('Removed Dependency'); case PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK: @@ -715,6 +721,8 @@ final class ManiphestTransactionDetailView extends ManiphestView { */ private function getEdgeEditVerb($type) { switch ($type) { + case PhabricatorEdgeConfig::TYPE_TASK_HAS_RELATED_DREV: + return pht('Changed Revisions'); case PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK: return pht('Changed Dependencies'); case PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK: @@ -735,6 +743,8 @@ final class ManiphestTransactionDetailView extends ManiphestView { $count = count($add); switch ($type) { + case PhabricatorEdgeConfig::TYPE_TASK_HAS_RELATED_DREV: + return pht('added %d revision(s): %s', $count, $list); case PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK: return pht('added %d dependencie(s): %s', $count, $list); case PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK: @@ -755,6 +765,8 @@ final class ManiphestTransactionDetailView extends ManiphestView { $count = count($rem); switch ($type) { + case PhabricatorEdgeConfig::TYPE_TASK_HAS_RELATED_DREV: + return pht('removed %d revision(s): %s', $count, $list); case PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK: return pht('removed %d dependencie(s): %s', $count, $list); case PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK: @@ -777,6 +789,14 @@ final class ManiphestTransactionDetailView extends ManiphestView { $rem_count = count($rem_list); switch ($type) { + case PhabricatorEdgeConfig::TYPE_TASK_HAS_RELATED_DREV: + return pht( + 'changed %d revision(s), added %d: %s; removed %d: %s', + $add_count + $rem_count, + $add_count, + $add_list, + $rem_count, + $rem_list); case PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK: return pht( 'changed %d dependencie(s), added %d: %s; removed %d: %s', diff --git a/src/applications/search/controller/PhabricatorSearchAttachController.php b/src/applications/search/controller/PhabricatorSearchAttachController.php index 4c04ff5f04..58a98e3549 100644 --- a/src/applications/search/controller/PhabricatorSearchAttachController.php +++ b/src/applications/search/controller/PhabricatorSearchAttachController.php @@ -60,6 +60,7 @@ final class PhabricatorSearchAttachController switch ($this->action) { case self::ACTION_EDGE: case self::ACTION_DEPENDENCIES: + case self::ACTION_ATTACH: $edge_type = $this->getEdgeType($object_type, $attach_type); break; } @@ -93,23 +94,7 @@ final class PhabricatorSearchAttachController return id(new AphrontReloadResponse())->setURI($handle->getURI()); } else { - switch ($this->action) { - case self::ACTION_MERGE: - return $this->performMerge($object, $handle, $phids); - case self::ACTION_ATTACH: - $editor = new PhabricatorObjectAttachmentEditor( - $object_type, - $object); - $editor->setUser($this->getRequest()->getUser()); - $editor->attachObjects( - $attach_type, - $phids, - $two_way = true); - - return id(new AphrontReloadResponse())->setURI($handle->getURI()); - default: - throw new Exception("Unsupported attach action."); - } + return $this->performMerge($object, $handle, $phids); } } else { if ($edge_type) { @@ -117,14 +102,8 @@ final class PhabricatorSearchAttachController $this->phid, $edge_type); } else { - switch ($this->action) { - case self::ACTION_ATTACH: - $phids = $object->getAttachedPHIDs($attach_type); - break; - default: - $phids = array(); - break; - } + // This is a merge. + $phids = array(); } } @@ -282,9 +261,11 @@ final class PhabricatorSearchAttachController $t_task => array( $t_cmit => PhabricatorEdgeConfig::TYPE_TASK_HAS_COMMIT, $t_task => PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK, + $t_drev => PhabricatorEdgeConfig::TYPE_TASK_HAS_RELATED_DREV, ), $t_drev => array( $t_drev => PhabricatorEdgeConfig::TYPE_DREV_DEPENDS_ON_DREV, + $t_task => PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK, ), ); diff --git a/src/applications/search/editor/PhabricatorObjectAttachmentEditor.php b/src/applications/search/editor/PhabricatorObjectAttachmentEditor.php deleted file mode 100644 index 3d4bf4d149..0000000000 --- a/src/applications/search/editor/PhabricatorObjectAttachmentEditor.php +++ /dev/null @@ -1,139 +0,0 @@ -objectType = $object_type; - $this->object = $object; - } - - public function setUser(PhabricatorUser $user) { - $this->user = $user; - return $this; - } - - public function attachObjects($attach_type, array $phids, $two_way) { - $object_type = $this->objectType; - $object = $this->object; - $object_phid = $object->getPHID(); - - // sort() so that removing [X, Y] and then adding [Y, X] is correctly - // detected as a no-op. - sort($phids); - $old_phids = $object->getAttachedPHIDs($attach_type); - sort($old_phids); - $phids = array_values($phids); - $old_phids = array_values($old_phids); - - if ($phids === $old_phids) { - return; - } - - $all_phids = array_merge($phids, $old_phids); - $attach_objs = id(new PhabricatorObjectHandleData($all_phids)) - ->loadObjects(); - - // Remove PHIDs which don't actually exist, to prevent silliness. - $phids = array_keys(array_select_keys($attach_objs, $phids)); - if ($phids) { - $phids = array_combine($phids, $phids); - } - - // Update the primary object. - $this->writeOutboundEdges($object_type, $object, $attach_type, $phids); - - if (!$two_way) { - return; - } - - // Loop through all of the attached/detached objects and update them. - foreach ($attach_objs as $phid => $attach_obj) { - $attached_phids = $attach_obj->getAttachedPHIDs($object_type); - // Figure out if we're attaching or detaching this object. - if (isset($phids[$phid])) { - if (in_array($object_phid, $attached_phids)) { - // Already attached. - continue; - } - $attached_phids[] = $object_phid; - } else { - $attached_phids = array_fill_keys($attached_phids, true); - unset($attached_phids[$object_phid]); - $attached_phids = array_keys($attached_phids); - } - $this->writeOutboundEdges( - $attach_type, - $attach_obj, - $object_type, - $attached_phids); - } - } - - private function writeOutboundEdges( - $object_type, - $object, - $attach_type, - array $attach_phids) { - switch ($object_type) { - case PhabricatorPHIDConstants::PHID_TYPE_DREV: - $object->setAttachedPHIDs($attach_type, $attach_phids); - $object->save(); - break; - case PhabricatorPHIDConstants::PHID_TYPE_TASK: - $this->applyTaskTransaction( - $object, - $attach_type, - $attach_phids); - break; - } - } - - private function applyTaskTransaction( - ManiphestTask $task, - $attach_type, - array $new_phids) { - - if (!$this->user) { - throw new Exception("Call setUser() before editing attachments!"); - } - $user = $this->user; - - $editor = new ManiphestTransactionEditor(); - $type = ManiphestTransactionType::TYPE_ATTACH; - - $transaction = new ManiphestTransaction(); - $transaction->setAuthorPHID($user->getPHID()); - $transaction->setTransactionType($type); - - $new = $task->getAttached(); - $new[$attach_type] = array_fill_keys($new_phids, array()); - - $transaction->setNewValue($new); - $editor->applyTransactions($task, array($transaction)); - } - -} diff --git a/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php b/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php index 7a08cddb20..aaf02a8e7f 100644 --- a/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php +++ b/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php @@ -35,6 +35,9 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { const TYPE_BLOG_HAS_BLOGGER = 9; const TYPE_BLOGGER_HAS_BLOG = 10; + const TYPE_TASK_HAS_RELATED_DREV = 11; + const TYPE_DREV_HAS_RELATED_TASK = 12; + const TYPE_TEST_NO_CYCLE = 9000; public static function getInverse($edge_type) { @@ -52,6 +55,9 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { self::TYPE_POST_HAS_BLOG => self::TYPE_BLOG_HAS_POST, self::TYPE_BLOG_HAS_BLOGGER => self::TYPE_BLOGGER_HAS_BLOG, self::TYPE_BLOGGER_HAS_BLOG => self::TYPE_BLOG_HAS_BLOGGER, + + self::TYPE_TASK_HAS_RELATED_DREV => self::TYPE_DREV_HAS_RELATED_TASK, + self::TYPE_DREV_HAS_RELATED_TASK => self::TYPE_TASK_HAS_RELATED_DREV, ); return idx($map, $edge_type); diff --git a/src/infrastructure/internationalization/PhabricatorBaseEnglishTranslation.php b/src/infrastructure/internationalization/PhabricatorBaseEnglishTranslation.php index eb92b62f26..0567a11a6a 100644 --- a/src/infrastructure/internationalization/PhabricatorBaseEnglishTranslation.php +++ b/src/infrastructure/internationalization/PhabricatorBaseEnglishTranslation.php @@ -103,6 +103,24 @@ abstract class PhabricatorBaseEnglishTranslation 'DEPENDS ON TASKS', ), + 'DIFFERENTIAL %d REVISION(S)' => array( + 'DIFFERENTIAL REVISION', + 'DIFFERENTIAL REVISIONS', + ), + + 'added %d revision(s): %s' => array( + 'added revision: %2$s', + 'added revisions: %2$s', + ), + + 'removed %d revision(s): %s' => array( + 'removed revision: %2$s', + 'removed revisions: %2$s', + ), + + 'changed %d revision(s), added %d: %s; removed %d: %s' => + 'changed revisions, added %3$s; removed %5$s', + ); } diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index ee04c3aa12..7729e5aef3 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -928,6 +928,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('phameblog.sql'), ), + 'migrate-maniphest-revisions.php' => array( + 'type' => 'php', + 'name' => $this->getPatchPath('migrate-maniphest-revisions.php'), + ), ); }