diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1422,8 +1422,10 @@ 'ManiphestTaskHasCommitRelationship' => 'applications/maniphest/relationship/ManiphestTaskHasCommitRelationship.php', 'ManiphestTaskHasMockEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasMockEdgeType.php', 'ManiphestTaskHasMockRelationship' => 'applications/maniphest/relationship/ManiphestTaskHasMockRelationship.php', + 'ManiphestTaskHasParentRelationship' => 'applications/maniphest/relationship/ManiphestTaskHasParentRelationship.php', 'ManiphestTaskHasRevisionEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasRevisionEdgeType.php', 'ManiphestTaskHasRevisionRelationship' => 'applications/maniphest/relationship/ManiphestTaskHasRevisionRelationship.php', + 'ManiphestTaskHasSubtaskRelationship' => 'applications/maniphest/relationship/ManiphestTaskHasSubtaskRelationship.php', 'ManiphestTaskHeraldField' => 'applications/maniphest/herald/ManiphestTaskHeraldField.php', 'ManiphestTaskHeraldFieldGroup' => 'applications/maniphest/herald/ManiphestTaskHeraldFieldGroup.php', 'ManiphestTaskListController' => 'applications/maniphest/controller/ManiphestTaskListController.php', @@ -5913,8 +5915,10 @@ 'ManiphestTaskHasCommitRelationship' => 'ManiphestTaskRelationship', 'ManiphestTaskHasMockEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskHasMockRelationship' => 'ManiphestTaskRelationship', + 'ManiphestTaskHasParentRelationship' => 'ManiphestTaskRelationship', 'ManiphestTaskHasRevisionEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskHasRevisionRelationship' => 'ManiphestTaskRelationship', + 'ManiphestTaskHasSubtaskRelationship' => 'ManiphestTaskRelationship', 'ManiphestTaskHeraldField' => 'HeraldField', 'ManiphestTaskHeraldFieldGroup' => 'HeraldFieldGroup', 'ManiphestTaskListController' => 'ManiphestController', diff --git a/src/applications/auth/controller/PhabricatorAuthStartController.php b/src/applications/auth/controller/PhabricatorAuthStartController.php --- a/src/applications/auth/controller/PhabricatorAuthStartController.php +++ b/src/applications/auth/controller/PhabricatorAuthStartController.php @@ -225,10 +225,10 @@ } // Often, users end up here by clicking a disabled action link in the UI - // (for example, they might click "Edit Blocking Tasks" on a Maniphest - // task page). After they log in we want to send them back to that main - // object page if we can, since it's confusing to end up on a standalone - // page with only a dialog (particularly if that dialog is another error, + // (for example, they might click "Edit Subtasks" on a Maniphest task + // page). After they log in we want to send them back to that main object + // page if we can, since it's confusing to end up on a standalone page with + // only a dialog (particularly if that dialog is another error, // like a policy exception). $via_header = AphrontRequest::getViaHeaderName(); diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -195,12 +195,18 @@ ->setDisabled(!$can_create) ->setWorkflow(!$can_create); - $task_submenu[] = id(new PhabricatorActionView()) - ->setName(pht('Edit Blocking Tasks')) - ->setHref("/search/attach/{$phid}/TASK/blocks/") - ->setIcon('fa-link') - ->setDisabled(!$can_edit) - ->setWorkflow(true); + $relationship_list = PhabricatorObjectRelationshipList::newForObject( + $viewer, + $task); + + $parent_key = ManiphestTaskHasParentRelationship::RELATIONSHIPKEY; + $subtask_key = ManiphestTaskHasSubtaskRelationship::RELATIONSHIPKEY; + + $task_submenu[] = $relationship_list->getRelationship($parent_key) + ->newAction($task); + + $task_submenu[] = $relationship_list->getRelationship($subtask_key) + ->newAction($task); $task_submenu[] = id(new PhabricatorActionView()) ->setName(pht('Merge Duplicates In')) @@ -215,10 +221,6 @@ ->setIcon('fa-anchor') ->setSubmenu($task_submenu)); - $relationship_list = PhabricatorObjectRelationshipList::newForObject( - $viewer, - $task); - $relationship_submenu = $relationship_list->newActionMenu(); if ($relationship_submenu) { $curtain->addAction($relationship_submenu); @@ -288,9 +290,9 @@ $edge_types = array( ManiphestTaskDependedOnByTaskEdgeType::EDGECONST - => pht('Blocks'), + => pht('Parent Tasks'), ManiphestTaskDependsOnTaskEdgeType::EDGECONST - => pht('Blocked By'), + => pht('Subtasks'), ManiphestTaskHasRevisionEdgeType::EDGECONST => pht('Differential Revisions'), ManiphestTaskHasMockEdgeType::EDGECONST diff --git a/src/applications/maniphest/edge/ManiphestTaskDependedOnByTaskEdgeType.php b/src/applications/maniphest/edge/ManiphestTaskDependedOnByTaskEdgeType.php --- a/src/applications/maniphest/edge/ManiphestTaskDependedOnByTaskEdgeType.php +++ b/src/applications/maniphest/edge/ManiphestTaskDependedOnByTaskEdgeType.php @@ -17,7 +17,7 @@ $add_edges) { return pht( - '%s added %s blocked task(s): %s.', + '%s added %s parent task(s): %s.', $actor, $add_count, $add_edges); @@ -29,7 +29,7 @@ $rem_edges) { return pht( - '%s removed %s blocked task(s): %s.', + '%s removed %s parent task(s): %s.', $actor, $rem_count, $rem_edges); @@ -44,7 +44,7 @@ $rem_edges) { return pht( - '%s edited blocked task(s), added %s: %s; removed %s: %s.', + '%s edited parent task(s), added %s: %s; removed %s: %s.', $actor, $add_count, $add_edges, @@ -59,7 +59,7 @@ $add_edges) { return pht( - '%s added %s blocked task(s) for %s: %s.', + '%s added %s parent task(s) for %s: %s.', $actor, $add_count, $object, @@ -73,7 +73,7 @@ $rem_edges) { return pht( - '%s removed %s blocked task(s) for %s: %s.', + '%s removed %s parent task(s) for %s: %s.', $actor, $rem_count, $object, @@ -90,7 +90,7 @@ $rem_edges) { return pht( - '%s edited blocked task(s) for %s, added %s: %s; removed %s: %s.', + '%s edited parent task(s) for %s, added %s: %s; removed %s: %s.', $actor, $object, $add_count, diff --git a/src/applications/maniphest/edge/ManiphestTaskDependsOnTaskEdgeType.php b/src/applications/maniphest/edge/ManiphestTaskDependsOnTaskEdgeType.php --- a/src/applications/maniphest/edge/ManiphestTaskDependsOnTaskEdgeType.php +++ b/src/applications/maniphest/edge/ManiphestTaskDependsOnTaskEdgeType.php @@ -22,7 +22,7 @@ $add_edges) { return pht( - '%s added %s blocking task(s): %s.', + '%s added %s subtask(s): %s.', $actor, $add_count, $add_edges); @@ -34,7 +34,7 @@ $rem_edges) { return pht( - '%s removed %s blocking task(s): %s.', + '%s removed %s subtask(s): %s.', $actor, $rem_count, $rem_edges); @@ -49,7 +49,7 @@ $rem_edges) { return pht( - '%s edited blocking task(s), added %s: %s; removed %s: %s.', + '%s edited subtask(s), added %s: %s; removed %s: %s.', $actor, $add_count, $add_edges, @@ -64,7 +64,7 @@ $add_edges) { return pht( - '%s added %s blocking task(s) for %s: %s.', + '%s added %s subtask(s) for %s: %s.', $actor, $add_count, $object, @@ -78,7 +78,7 @@ $rem_edges) { return pht( - '%s removed %s blocking task(s) for %s: %s.', + '%s removed %s subtask(s) for %s: %s.', $actor, $rem_count, $object, @@ -95,7 +95,7 @@ $rem_edges) { return pht( - '%s edited blocking task(s) for %s, added %s: %s; removed %s: %s.', + '%s edited subtask(s) for %s, added %s: %s; removed %s: %s.', $actor, $object, $add_count, diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -325,7 +325,7 @@ ManiphestTransaction::MAILTAG_PROJECTS => pht("A task's associated projects change."), ManiphestTransaction::MAILTAG_UNBLOCK => - pht('One of the tasks a task is blocked by changes status.'), + pht("One of a task's subtasks changes status."), ManiphestTransaction::MAILTAG_COLUMN => pht('A task is moved between columns on a workboard.'), ManiphestTransaction::MAILTAG_COMMENT => diff --git a/src/applications/maniphest/relationship/ManiphestTaskHasParentRelationship.php b/src/applications/maniphest/relationship/ManiphestTaskHasParentRelationship.php new file mode 100644 --- /dev/null +++ b/src/applications/maniphest/relationship/ManiphestTaskHasParentRelationship.php @@ -0,0 +1,40 @@ +getMetadataValue('blocker.new')) { return pht( - '%s created blocking task %s.', + '%s created subtask %s.', $this->renderHandleLink($author_phid), $this->renderHandleLink($blocker_phid)); } else if ($old_closed && !$new_closed) { return pht( - '%s reopened blocking task %s as "%s".', + '%s reopened subtask %s as "%s".', $this->renderHandleLink($author_phid), $this->renderHandleLink($blocker_phid), $new_name); } else if (!$old_closed && $new_closed) { return pht( - '%s closed blocking task %s as "%s".', + '%s closed subtask %s as "%s".', $this->renderHandleLink($author_phid), $this->renderHandleLink($blocker_phid), $new_name); } else { return pht( - '%s changed the status of blocking task %s from "%s" to "%s".', + '%s changed the status of subtask %s from "%s" to "%s".', $this->renderHandleLink($author_phid), $this->renderHandleLink($blocker_phid), $old_name, @@ -753,21 +753,21 @@ if ($old_closed && !$new_closed) { return pht( - '%s reopened %s, a task blocking %s, as "%s".', + '%s reopened %s, a subtask of %s, as "%s".', $this->renderHandleLink($author_phid), $this->renderHandleLink($blocker_phid), $this->renderHandleLink($object_phid), $new_name); } else if (!$old_closed && $new_closed) { return pht( - '%s closed %s, a task blocking %s, as "%s".', + '%s closed %s, a subtask of %s, as "%s".', $this->renderHandleLink($author_phid), $this->renderHandleLink($blocker_phid), $this->renderHandleLink($object_phid), $new_name); } else { return pht( - '%s changed the status of %s, a task blocking %s, '. + '%s changed the status of %s, a subtasktask of %s, '. 'from "%s" to "%s".', $this->renderHandleLink($author_phid), $this->renderHandleLink($blocker_phid), diff --git a/src/applications/search/controller/PhabricatorSearchRelationshipController.php b/src/applications/search/controller/PhabricatorSearchRelationshipController.php --- a/src/applications/search/controller/PhabricatorSearchRelationshipController.php +++ b/src/applications/search/controller/PhabricatorSearchRelationshipController.php @@ -140,6 +140,8 @@ ManiphestTaskHasCommitEdgeType::EDGECONST => 'CMIT', ManiphestTaskHasMockEdgeType::EDGECONST => 'MOCK', ManiphestTaskHasRevisionEdgeType::EDGECONST => 'DREV', + ManiphestTaskDependsOnTaskEdgeType::EDGECONST => 'TASK', + ManiphestTaskDependedOnByTaskEdgeType::EDGECONST => 'TASK', ); $edge_type = $relationship->getEdgeConstant(); @@ -180,12 +182,14 @@ $message = pht( 'You can not create that relationship because it would create a '. - 'circular dependency: %s.', - implode(" \xE2\x86\x92 ", $names)); + 'circular dependency:'); + + $list = implode(" \xE2\x86\x92 ", $names); return $this->newDialog() ->setTitle(pht('Circular Dependency')) ->appendParagraph($message) + ->appendParagraph($list) ->addCancelButton($done_uri); } 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 @@ -931,6 +931,7 @@ $object->openTransaction(); } + try { foreach ($xactions as $xaction) { $this->applyInternalEffects($object, $xaction); } @@ -940,8 +941,6 @@ try { $object->save(); } catch (AphrontDuplicateKeyQueryException $ex) { - $object->killTransaction(); - // This callback has an opportunity to throw a better exception, // so execution may end here. $this->didCatchDuplicateKeyException($object, $xactions, $ex); @@ -973,7 +972,11 @@ $read_locking = false; } - $object->saveTransaction(); + $object->saveTransaction(); + } catch (Exception $ex) { + $object->killTransaction(); + throw $ex; + } // Now that we've completely applied the core transaction set, try to apply // Herald rules. Herald rules are allowed to either take direct actions on diff --git a/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php b/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php --- a/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php +++ b/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php @@ -134,7 +134,6 @@ $caught = $ex; } - if ($caught) { $this->killTransactions(); } diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -206,73 +206,73 @@ ), ), - '%s added %s blocking task(s): %s.' => array( + '%s added %s subtask(s): %s.' => array( array( - '%s added a blocking task: %3$s.', - '%s added blocking tasks: %3$s.', + '%s added a subtask: %3$s.', + '%s added subtasks: %3$s.', ), ), - '%s added %s blocked task(s): %s.' => array( + '%s added %s parent task(s): %s.' => array( array( - '%s added a blocked task: %3$s.', - '%s added blocked tasks: %3$s.', + '%s added a parent task: %3$s.', + '%s added parent tasks: %3$s.', ), ), - '%s removed %s blocking task(s): %s.' => array( + '%s removed %s subtask(s): %s.' => array( array( - '%s removed a blocking task: %3$s.', - '%s removed blocking tasks: %3$s.', + '%s removed a subtask: %3$s.', + '%s removed subtasks: %3$s.', ), ), - '%s removed %s blocked task(s): %s.' => array( + '%s removed %s parent task(s): %s.' => array( array( - '%s removed a blocked task: %3$s.', - '%s removed blocked tasks: %3$s.', + '%s removed a parent task: %3$s.', + '%s removed parent tasks: %3$s.', ), ), - '%s added %s blocking task(s) for %s: %s.' => array( + '%s added %s subtask(s) for %s: %s.' => array( array( - '%s added a blocking task for %3$s: %4$s.', - '%s added blocking tasks for %3$s: %4$s.', + '%s added a subtask for %3$s: %4$s.', + '%s added subtasks for %3$s: %4$s.', ), ), - '%s added %s blocked task(s) for %s: %s.' => array( + '%s added %s parent task(s) for %s: %s.' => array( array( - '%s added a blocked task for %3$s: %4$s.', - '%s added blocked tasks for %3$s: %4$s.', + '%s added a parent task for %3$s: %4$s.', + '%s added parent tasks for %3$s: %4$s.', ), ), - '%s removed %s blocking task(s) for %s: %s.' => array( + '%s removed %s subtask(s) for %s: %s.' => array( array( - '%s removed a blocking task for %3$s: %4$s.', - '%s removed blocking tasks for %3$s: %4$s.', + '%s removed a subtask for %3$s: %4$s.', + '%s removed subtasks for %3$s: %4$s.', ), ), - '%s removed %s blocked task(s) for %s: %s.' => array( + '%s removed %s parent task(s) for %s: %s.' => array( array( - '%s removed a blocked task for %3$s: %4$s.', - '%s removed blocked tasks for %3$s: %4$s.', + '%s removed a parent task for %3$s: %4$s.', + '%s removed parent tasks for %3$s: %4$s.', ), ), - '%s edited blocking task(s), added %s: %s; removed %s: %s.' => - '%s edited blocking tasks, added: %3$s; removed: %5$s.', + '%s edited subtask(s), added %s: %s; removed %s: %s.' => + '%s edited subtasks, added: %3$s; removed: %5$s.', - '%s edited blocking task(s) for %s, added %s: %s; removed %s: %s.' => - '%s edited blocking tasks for %s, added: %4$s; removed: %6$s.', + '%s edited subtask(s) for %s, added %s: %s; removed %s: %s.' => + '%s edited subtasks for %s, added: %4$s; removed: %6$s.', - '%s edited blocked task(s), added %s: %s; removed %s: %s.' => - '%s edited blocked tasks, added: %3$s; removed: %5$s.', + '%s edited parent task(s), added %s: %s; removed %s: %s.' => + '%s edited parent tasks, added: %3$s; removed: %5$s.', - '%s edited blocked task(s) for %s, added %s: %s; removed %s: %s.' => - '%s edited blocked tasks for %s, added: %4$s; removed: %6$s.', + '%s edited parent task(s) for %s, added %s: %s; removed %s: %s.' => + '%s edited parent tasks for %s, added: %4$s; removed: %6$s.', '%s edited answer(s), added %s: %s; removed %d: %s.' => '%s edited answers, added: %3$s; removed: %5$s.',