From 83efa6e1c5c8df1ee72a02cd7c6dd869a393db6c Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Fri, 2 Dec 2011 16:20:00 -0800 Subject: [PATCH] make arc diff link maniphest tasks with revisions Summary: add "Maniphest Task:" or "Maniphest Tasks:" followed by text that has TX in it. foreach TX the task will be attached to the revision and the revision will be attached to the task. parsing is pretty... ummm, robust such that it will pick up any TX substring and parse that as a Maniphest Task just fine. it errors out if there is not an actual task for TX and otherwise churns along pretty nicely. Also, make sure the PhabricatorObjectHandle loads the task ID as the alternateID since we need that here and it should be that way anyhoo. Test Plan: made a diff and in the commit message added Maniphest Task(s): TX combination. Tried various combinations of TX -- single, multiple with commas, multiple many lines, single bad, multiple bad, multiple mix of bad and good. verified that the good tasks were attached to the diff and diff was attached to the good tasks. Maniphest Tasks: T137 Reviewers: epriestley Reviewed By: epriestley CC: aran, btrahan, epriestley Differential Revision: 1165 --- ...entialManiphestTasksFieldSpecification.php | 122 ++++++++++++++++++ .../specification/maniphesttasks/__init__.php | 7 + .../data/PhabricatorObjectHandleData.php | 1 + 3 files changed, 130 insertions(+) diff --git a/src/applications/differential/field/specification/maniphesttasks/DifferentialManiphestTasksFieldSpecification.php b/src/applications/differential/field/specification/maniphesttasks/DifferentialManiphestTasksFieldSpecification.php index 3f2f9d4b8a..bd1f9c298d 100644 --- a/src/applications/differential/field/specification/maniphesttasks/DifferentialManiphestTasksFieldSpecification.php +++ b/src/applications/differential/field/specification/maniphesttasks/DifferentialManiphestTasksFieldSpecification.php @@ -19,6 +19,8 @@ final class DifferentialManiphestTasksFieldSpecification extends DifferentialFieldSpecification { + private $maniphestTasks = array(); + public function shouldAppearOnRevisionView() { return PhabricatorEnv::getEnvConfig('maniphest.enabled'); } @@ -51,4 +53,124 @@ final class DifferentialManiphestTasksFieldSpecification PhabricatorPHIDConstants::PHID_TYPE_TASK); } + /** + * Attach the revision to the task(s) and the task(s) to the revision. + * + * @return void + */ + public function willWriteRevision(DifferentialRevisionEditor $editor) { + // 1 -- revision => tasks + $revision = $editor->getRevision(); + $revision->setAttachedPHIDs(PhabricatorPHIDConstants::PHID_TYPE_TASK, + $this->maniphestTasks); + + // 2 -- tasks => revision + $maniphest_editor = new ManiphestTransactionEditor(); + $user = $this->getUser(); + $type = ManiphestTransactionType::TYPE_ATTACH; + $attach_type = PhabricatorPHIDConstants::PHID_TYPE_DREV; + $attach_data = array($revision->getPHID() => array()); + + $tasks = id(new ManiphestTask()) + ->loadAllWhere('phid IN (%Ld)', $this->maniphestTasks); + + foreach ($tasks as $task) { + $transaction = new ManiphestTransaction(); + $transaction->setAuthorPHID($user->getPHID()); + $transaction->setTransactionType($type); + + $new = $task->getAttached(); + $new[$attach_type] = $attach_data; + + $transaction->setNewValue($new); + $maniphest_editor->applyTransactions($task, array($transaction)); + } + } + + protected function didSetRevision() { + $this->maniphestTasks = $this->getManiphestTaskPHIDs(); + } + + public function getRequiredHandlePHIDsForCommitMessage() { + return $this->maniphestTasks; + } + + public function shouldAppearOnCommitMessageTemplate() { + return PhabricatorEnv::getEnvConfig('maniphest.enabled'); + } + + public function shouldAppearOnCommitMessage() { + return PhabricatorEnv::getEnvConfig('maniphest.enabled'); + } + + public function getCommitMessageKey() { + return 'maniphestTaskPHIDs'; + } + + public function setValueFromParsedCommitMessage($value) { + $this->maniphestTasks = nonempty($value, array()); + return $this; + } + + public function renderLabelForCommitMessage() { + return 'Maniphest Tasks'; + } + + public function getSupportedCommitMessageLabels() { + return array( + 'Maniphest Task', + 'Maniphest Tasks', + ); + } + + public function renderValueForCommitMessage($is_edit) { + if (!$this->maniphestTasks) { + return null; + } + + $names = array(); + foreach ($this->maniphestTasks as $phid) { + $handle = $this->getHandle($phid); + $names[] = 'T'.$handle->getAlternateID(); + } + return implode(', ', $names); + } + + public function parseValueFromCommitMessage($value) { + $matches = null; + preg_match_all('/T(\d+)/', $value, $matches); + if (empty($matches[0])) { + return array(); + } + + + $task_ids = $matches[1]; + $tasks = id(new ManiphestTask()) + ->loadAllWhere('id in (%Ld)', $task_ids); + + $task_phids = array(); + $invalid = array(); + foreach ($task_ids as $task_id) { + $task = idx($tasks, $task_id); + if (empty($task)) { + $invalid[] = 'T'.$task_id; + } else { + $task_phids[] = $task->getPHID(); + } + } + + if ($invalid) { + if (count($invalid) > 1) { + $what = 'Maniphest Tasks'; + } else { + $what = 'Maniphest Task'; + } + $invalid = implode(', ', $invalid); + throw new DifferentialFieldParseException( + "Commit message references nonexistent {$what}: {$invalid}."); + } + + return $task_phids; + } + } diff --git a/src/applications/differential/field/specification/maniphesttasks/__init__.php b/src/applications/differential/field/specification/maniphesttasks/__init__.php index 54c0ba2d66..4f9b037319 100644 --- a/src/applications/differential/field/specification/maniphesttasks/__init__.php +++ b/src/applications/differential/field/specification/maniphesttasks/__init__.php @@ -6,9 +6,16 @@ +phutil_require_module('phabricator', 'applications/differential/field/exception/parse'); phutil_require_module('phabricator', 'applications/differential/field/specification/base'); +phutil_require_module('phabricator', 'applications/maniphest/constants/transactiontype'); +phutil_require_module('phabricator', 'applications/maniphest/editor/transaction'); +phutil_require_module('phabricator', 'applications/maniphest/storage/task'); +phutil_require_module('phabricator', 'applications/maniphest/storage/transaction'); phutil_require_module('phabricator', 'applications/phid/constants'); phutil_require_module('phabricator', 'infrastructure/env'); +phutil_require_module('phutil', 'utils'); + phutil_require_source('DifferentialManiphestTasksFieldSpecification.php'); diff --git a/src/applications/phid/handle/data/PhabricatorObjectHandleData.php b/src/applications/phid/handle/data/PhabricatorObjectHandleData.php index 91bf8096ea..74d9a433da 100644 --- a/src/applications/phid/handle/data/PhabricatorObjectHandleData.php +++ b/src/applications/phid/handle/data/PhabricatorObjectHandleData.php @@ -296,6 +296,7 @@ class PhabricatorObjectHandleData { $handle->setURI('/T'.$task->getID()); $handle->setFullName('T'.$task->getID().': '.$task->getTitle()); $handle->setComplete(true); + $handle->setAlternateID($task->getID()); if ($task->getStatus() != ManiphestTaskStatus::STATUS_OPEN) { $closed = PhabricatorObjectHandleStatus::STATUS_CLOSED; $handle->setStatus($closed);