From 7abe9dc4c078532a1cd806e7391a76ef19f96122 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 23 Sep 2013 14:25:28 -0700 Subject: [PATCH] Migrate all Maniphest transaction data to new storage Summary: Ref T2217. This is the risky, hard part; everything after this should be smooth sailing. This is //mostly// clean, except: - The old format would opportunistically combine a comment with some other transaction type if it could. We no longer do that, so: - When migrating, "edit" + "comment" transactions need to be split in two. - When editing now, we should no longer combine these transaction types. - These changes are pretty straightforward and low-impact. - This migration promotes "auxiliary field" data to the new CustomField/StandardField format, so that's not a straight migration either. The formats are very similar, though. Broadly, this takes the same attack that the auth migration did: proxy all the code through to the new storage. `ManiphestTransaction` is now just an API on top of `ManiphestTransactionPro`, which is the new storage format. The two formats are very similar, so this was mostly a straight copy from one table to the other. Test Plan: - Without performing the migration, made a bunch of edits and comments on tasks and verified the new code works correctly. - Droped the test data and performed the migration. - Looked at the resulting data for obvious discrepancies. - Looked at a bunch of tasks and their transaction history. - Used Conduit to pull transaction data. - Edited task description and clicked "View Details" on transaction. - Used batch editor. - Made a bunch more edits. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2217 Differential Revision: https://secure.phabricator.com/D7068 --- .../patches/20130921.xmigratemaniphest.php | 147 +++++++++++++ src/__phutil_library_map__.php | 8 +- .../conduit/ConduitAPI_maniphest_Method.php | 8 +- ...I_maniphest_gettasktransactions_Method.php | 10 +- .../maniphest/constants/ManiphestAction.php | 4 +- .../constants/ManiphestTransactionType.php | 4 +- .../ManiphestBatchEditController.php | 15 +- .../ManiphestTaskEditController.php | 4 +- .../ManiphestTransactionSaveController.php | 22 +- .../editor/ManiphestTransactionEditor.php | 20 +- .../maniphest/mail/ManiphestReplyHandler.php | 2 +- .../query/ManiphestLegacyTransactionQuery.php | 32 ++- .../query/ManiphestTransactionQuery.php | 10 + .../storage/ManiphestTransaction.php | 196 +++++++++++++----- .../storage/ManiphestTransactionComment.php | 5 + .../view/ManiphestTransactionDetailView.php | 16 +- .../patch/PhabricatorBuiltinPatchList.php | 4 + 17 files changed, 398 insertions(+), 109 deletions(-) create mode 100644 resources/sql/patches/20130921.xmigratemaniphest.php create mode 100644 src/applications/maniphest/query/ManiphestTransactionQuery.php diff --git a/resources/sql/patches/20130921.xmigratemaniphest.php b/resources/sql/patches/20130921.xmigratemaniphest.php new file mode 100644 index 0000000000..c768c935a3 --- /dev/null +++ b/resources/sql/patches/20130921.xmigratemaniphest.php @@ -0,0 +1,147 @@ +establishConnection('w'); + +$rows = new LiskRawMigrationIterator($conn_w, 'maniphest_transaction'); +$conn_w->openTransaction(); + +$xaction_table = new ManiphestTransactionPro(); +$comment_table = new ManiphestTransactionComment(); + +foreach ($rows as $row) { + $row_id = $row['id']; + $task_id = $row['taskID']; + + echo "Migrating row {$row_id} (T{$task_id})...\n"; + + $task_row = queryfx_one( + $conn_w, + 'SELECT phid FROM %T WHERE id = %d', + $task_table->getTableName(), + $task_id); + if (!$task_row) { + echo "Skipping, no such task.\n"; + continue; + } + + $task_phid = $task_row['phid']; + + $has_comment = strlen(trim($row['comments'])); + + $xaction_type = $row['transactionType']; + $xaction_old = $row['oldValue']; + $xaction_new = $row['newValue']; + $xaction_source = $row['contentSource']; + $xaction_meta = $row['metadata']; + + // Convert "aux" (auxiliary field) transactions to proper CustomField + // transactions. The formats are very similar, except that the type constant + // is different and the auxiliary key should be prefixed. + if ($xaction_type == 'aux') { + $xaction_meta = @json_decode($xaction_meta, true); + $xaction_meta = nonempty($xaction_meta, array()); + + $xaction_type = PhabricatorTransactions::TYPE_CUSTOMFIELD; + + $aux_key = idx($xaction_meta, 'aux:key'); + if (!preg_match('/^std:maniphest:/', $aux_key)) { + $aux_key = 'std:maniphest:'.$aux_key; + } + + $xaction_meta = array( + 'customfield:key' => $aux_key, + ); + + $xaction_meta = json_encode($xaction_meta); + } + + // If this transaction did something other than just leaving a comment, + // insert a new transaction for that action. If there was a comment (or + // a comment in addition to an action) we'll insert that below. + if ($row['transactionType'] != 'comment') { + $xaction_phid = PhabricatorPHID::generateNewPHID( + PhabricatorApplicationTransactionPHIDTypeTransaction::TYPECONST, + ManiphestPHIDTypeTask::TYPECONST); + + queryfx( + $conn_w, + 'INSERT INTO %T (phid, authorPHID, objectPHID, viewPolicy, editPolicy, + commentPHID, commentVersion, transactionType, oldValue, newValue, + contentSource, metadata, dateCreated, dateModified) + VALUES (%s, %s, %s, %s, %s, %s, %d, %s, %ns, %ns, %s, %s, %d, %d)', + $xaction_table->getTableName(), + $xaction_phid, + $row['authorPHID'], + $task_phid, + 'public', + $row['authorPHID'], + null, + 0, + $xaction_type, + $xaction_old, + $xaction_new, + $xaction_source, + $xaction_meta, + $row['dateCreated'], + $row['dateModified']); + } + + // Now, if the old transaction has a comment, we insert an explicit new + // transaction for it. + if ($has_comment) { + $comment_phid = PhabricatorPHID::generateNewPHID( + PhabricatorPHIDConstants::PHID_TYPE_XCMT, + ManiphestPHIDTypeTask::TYPECONST); + $comment_version = 1; + + $comment_xaction_phid = PhabricatorPHID::generateNewPHID( + PhabricatorApplicationTransactionPHIDTypeTransaction::TYPECONST, + ManiphestPHIDTypeTask::TYPECONST); + + // Insert the comment data. + queryfx( + $conn_w, + 'INSERT INTO %T (phid, transactionPHID, authorPHID, viewPolicy, + editPolicy, commentVersion, content, contentSource, isDeleted, + dateCreated, dateModified) + VALUES (%s, %s, %s, %s, %s, %d, %s, %s, %d, %d, %d)', + $comment_table->getTableName(), + $comment_phid, + $comment_xaction_phid, + $row['authorPHID'], + 'public', + $row['authorPHID'], + $comment_version, + $row['comments'], + $xaction_source, + 0, + $row['dateCreated'], + $row['dateModified']); + + queryfx( + $conn_w, + 'INSERT INTO %T (phid, authorPHID, objectPHID, viewPolicy, editPolicy, + commentPHID, commentVersion, transactionType, oldValue, newValue, + contentSource, metadata, dateCreated, dateModified) + VALUES (%s, %s, %s, %s, %s, %s, %d, %s, %ns, %ns, %s, %s, %d, %d)', + $xaction_table->getTableName(), + $comment_xaction_phid, + $row['authorPHID'], + $task_phid, + 'public', + $row['authorPHID'], + $comment_phid, + $comment_version, + PhabricatorTransactions::TYPE_COMMENT, + $xaction_old, + $xaction_new, + $xaction_source, + $xaction_meta, + $row['dateCreated'], + $row['dateModified']); + } +} + +$conn_w->saveTransaction(); +echo "Done.\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 87f73bcf6c..cc4b7c4b98 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -740,6 +740,7 @@ phutil_register_library_map(array( 'ManiphestTransactionListView' => 'applications/maniphest/view/ManiphestTransactionListView.php', 'ManiphestTransactionPreviewController' => 'applications/maniphest/controller/ManiphestTransactionPreviewController.php', 'ManiphestTransactionPro' => 'applications/maniphest/storage/ManiphestTransactionPro.php', + 'ManiphestTransactionQuery' => 'applications/maniphest/query/ManiphestTransactionQuery.php', 'ManiphestTransactionSaveController' => 'applications/maniphest/controller/ManiphestTransactionSaveController.php', 'ManiphestTransactionType' => 'applications/maniphest/constants/ManiphestTransactionType.php', 'ManiphestView' => 'applications/maniphest/view/ManiphestView.php', @@ -2826,17 +2827,14 @@ phutil_register_library_map(array( 'ManiphestTaskSearchEngine' => 'PhabricatorApplicationSearchEngine', 'ManiphestTaskStatus' => 'ManiphestConstants', 'ManiphestTaskSubscriber' => 'ManiphestDAO', - 'ManiphestTransaction' => - array( - 0 => 'ManiphestDAO', - 1 => 'PhabricatorMarkupInterface', - ), + 'ManiphestTransaction' => 'PhabricatorMarkupInterface', 'ManiphestTransactionComment' => 'PhabricatorApplicationTransactionComment', 'ManiphestTransactionDetailView' => 'ManiphestView', 'ManiphestTransactionEditor' => 'PhabricatorEditor', 'ManiphestTransactionListView' => 'ManiphestView', 'ManiphestTransactionPreviewController' => 'ManiphestController', 'ManiphestTransactionPro' => 'PhabricatorApplicationTransaction', + 'ManiphestTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'ManiphestTransactionSaveController' => 'ManiphestController', 'ManiphestTransactionType' => 'ManiphestConstants', 'ManiphestView' => 'AphrontView', diff --git a/src/applications/maniphest/conduit/ConduitAPI_maniphest_Method.php b/src/applications/maniphest/conduit/ConduitAPI_maniphest_Method.php index 648506e83b..00fdd4aa75 100644 --- a/src/applications/maniphest/conduit/ConduitAPI_maniphest_Method.php +++ b/src/applications/maniphest/conduit/ConduitAPI_maniphest_Method.php @@ -68,7 +68,7 @@ abstract class ConduitAPI_maniphest_Method extends ConduitAPIMethod { $comments = $request->getValue('comments'); if (!$is_new && $comments !== null) { - $changes[ManiphestTransactionType::TYPE_NONE] = null; + $changes[PhabricatorTransactions::TYPE_COMMENT] = null; } $title = $request->getValue('title'); @@ -151,7 +151,7 @@ abstract class ConduitAPI_maniphest_Method extends ConduitAPIMethod { $transaction = clone $template; $transaction->setTransactionType($type); $transaction->setNewValue($value); - if ($type == ManiphestTransactionType::TYPE_NONE) { + if ($type == PhabricatorTransactions::TYPE_COMMENT) { $transaction->setComments($comments); } $transactions[] = $transaction; @@ -169,8 +169,8 @@ abstract class ConduitAPI_maniphest_Method extends ConduitAPIMethod { } $transaction = clone $template; $transaction->setTransactionType( - ManiphestTransactionType::TYPE_AUXILIARY); - $transaction->setMetadataValue('aux:key', $key); + PhabricatorTransactions::TYPE_CUSTOMFIELD); + $transaction->setMetadataValue('customfield:key', $key); $transaction->setOldValue( $field->getOldValueForApplicationTransactions()); $transaction->setNewValue($auxiliary[$key]); diff --git a/src/applications/maniphest/conduit/ConduitAPI_maniphest_gettasktransactions_Method.php b/src/applications/maniphest/conduit/ConduitAPI_maniphest_gettasktransactions_Method.php index 3fbea62996..a790d662c6 100644 --- a/src/applications/maniphest/conduit/ConduitAPI_maniphest_gettasktransactions_Method.php +++ b/src/applications/maniphest/conduit/ConduitAPI_maniphest_gettasktransactions_Method.php @@ -37,16 +37,20 @@ final class ConduitAPI_maniphest_gettasktransactions_Method ->setViewer($request->getUser()) ->withIDs($task_ids) ->execute(); + $tasks = mpull($tasks, null, 'getPHID'); $transactions = ManiphestLegacyTransactionQuery::loadByTasks( $request->getUser(), $tasks); foreach ($transactions as $transaction) { - $task_id = $transaction->getTaskID(); - if (!array_key_exists($task_id, $results)) { - $results[$task_id] = array(); + $task_phid = $transaction->getTaskPHID(); + if (empty($tasks[$task_phid])) { + continue; } + + $task_id = $tasks[$task_phid]->getID(); + $results[$task_id][] = array( 'taskID' => $task_id, 'transactionType' => $transaction->getTransactionType(), diff --git a/src/applications/maniphest/constants/ManiphestAction.php b/src/applications/maniphest/constants/ManiphestAction.php index ed0487161d..9eedc8bb68 100644 --- a/src/applications/maniphest/constants/ManiphestAction.php +++ b/src/applications/maniphest/constants/ManiphestAction.php @@ -14,7 +14,7 @@ final class ManiphestAction extends ManiphestConstants { /* these actions are determined sufficiently by the transaction type and thus we use them here*/ - const ACTION_COMMENT = ManiphestTransactionType::TYPE_NONE; + const ACTION_COMMENT = PhabricatorTransactions::TYPE_COMMENT; const ACTION_CC = ManiphestTransactionType::TYPE_CCS; const ACTION_PRIORITY = ManiphestTransactionType::TYPE_PRIORITY; const ACTION_PROJECT = ManiphestTransactionType::TYPE_PROJECTS; @@ -23,7 +23,7 @@ final class ManiphestAction extends ManiphestConstants { const ACTION_REASSIGN = ManiphestTransactionType::TYPE_OWNER; const ACTION_ATTACH = ManiphestTransactionType::TYPE_ATTACH; const ACTION_EDGE = ManiphestTransactionType::TYPE_EDGE; - const ACTION_AUXILIARY = ManiphestTransactionType::TYPE_AUXILIARY; + const ACTION_AUXILIARY = PhabricatorTransactions::TYPE_CUSTOMFIELD; public static function getActionPastTenseVerb($action) { static $map = array( diff --git a/src/applications/maniphest/constants/ManiphestTransactionType.php b/src/applications/maniphest/constants/ManiphestTransactionType.php index c70a733336..724aec5ea0 100644 --- a/src/applications/maniphest/constants/ManiphestTransactionType.php +++ b/src/applications/maniphest/constants/ManiphestTransactionType.php @@ -5,7 +5,6 @@ */ final class ManiphestTransactionType extends ManiphestConstants { - const TYPE_NONE = 'comment'; const TYPE_STATUS = 'status'; const TYPE_OWNER = 'reassign'; const TYPE_CCS = 'ccs'; @@ -17,11 +16,10 @@ final class ManiphestTransactionType extends ManiphestConstants { const TYPE_TITLE = 'title'; const TYPE_DESCRIPTION = 'description'; - const TYPE_AUXILIARY = 'aux'; public static function getTransactionTypeMap() { return array( - self::TYPE_NONE => pht('Comment'), + PhabricatorTransactions::TYPE_COMMENT => pht('Comment'), self::TYPE_STATUS => pht('Close Task'), self::TYPE_OWNER => pht('Reassign / Claim'), self::TYPE_CCS => pht('Add CCs'), diff --git a/src/applications/maniphest/controller/ManiphestBatchEditController.php b/src/applications/maniphest/controller/ManiphestBatchEditController.php index a5df572be2..eab1a25a16 100644 --- a/src/applications/maniphest/controller/ManiphestBatchEditController.php +++ b/src/applications/maniphest/controller/ManiphestBatchEditController.php @@ -33,7 +33,7 @@ final class ManiphestBatchEditController extends ManiphestController { $task_ids = implode(',', mpull($tasks, 'getID')); return id(new AphrontRedirectResponse()) - ->setURI('/maniphest/query/?ids='.$task_ids); + ->setURI('/maniphest/?ids='.$task_ids); } $handle_phids = mpull($tasks, 'getOwnerPHID'); @@ -147,7 +147,7 @@ final class ManiphestBatchEditController extends ManiphestController { private function buildTransactions($actions, ManiphestTask $task) { $value_map = array(); $type_map = array( - 'add_comment' => ManiphestTransactionType::TYPE_NONE, + 'add_comment' => PhabricatorTransactions::TYPE_COMMENT, 'assign' => ManiphestTransactionType::TYPE_OWNER, 'status' => ManiphestTransactionType::TYPE_STATUS, 'priority' => ManiphestTransactionType::TYPE_PRIORITY, @@ -182,7 +182,7 @@ final class ManiphestBatchEditController extends ManiphestController { $current = $value_map[$type]; } else { switch ($type) { - case ManiphestTransactionType::TYPE_NONE: + case PhabricatorTransactions::TYPE_COMMENT: $current = null; break; case ManiphestTransactionType::TYPE_OWNER: @@ -209,7 +209,7 @@ final class ManiphestBatchEditController extends ManiphestController { $value = $action['value']; switch ($type) { - case ManiphestTransactionType::TYPE_NONE: + case PhabricatorTransactions::TYPE_COMMENT: if (!strlen($value)) { continue 2; } @@ -250,7 +250,7 @@ final class ManiphestBatchEditController extends ManiphestController { // some need to merge the current and edited values (add/remove project). switch ($type) { - case ManiphestTransactionType::TYPE_NONE: + case PhabricatorTransactions::TYPE_COMMENT: if (strlen($current)) { $value = $current."\n\n".$value; } @@ -301,12 +301,15 @@ final class ManiphestBatchEditController extends ManiphestController { // TODO: Set content source to "batch edit". + $template->setContentSource( + PhabricatorContentSource::newFromRequest($this->getRequest())); + foreach ($value_map as $type => $value) { $xaction = clone $template; $xaction->setTransactionType($type); switch ($type) { - case ManiphestTransactionType::TYPE_NONE: + case PhabricatorTransactions::TYPE_COMMENT: $xaction->setComments($value); break; default: diff --git a/src/applications/maniphest/controller/ManiphestTaskEditController.php b/src/applications/maniphest/controller/ManiphestTaskEditController.php index b1e45757a8..cc59ecc5c2 100644 --- a/src/applications/maniphest/controller/ManiphestTaskEditController.php +++ b/src/applications/maniphest/controller/ManiphestTaskEditController.php @@ -226,9 +226,9 @@ final class ManiphestTaskEditController extends ManiphestController { foreach ($aux_fields as $aux_field) { $transaction = clone $template; $transaction->setTransactionType( - ManiphestTransactionType::TYPE_AUXILIARY); + PhabricatorTransactions::TYPE_CUSTOMFIELD); $aux_key = $aux_field->getFieldKey(); - $transaction->setMetadataValue('aux:key', $aux_key); + $transaction->setMetadataValue('customfield:key', $aux_key); $old = idx($old_values, $aux_key); $new = $aux_field->getNewValueForApplicationTransactions(); diff --git a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php index 596e57f84e..4bb0405a99 100644 --- a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php +++ b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php @@ -65,7 +65,6 @@ final class ManiphestTransactionSaveController extends ManiphestController { ->setTransactionType(ManiphestTransactionType::TYPE_ATTACH); $transaction->setNewValue($new); $transactions[] = $transaction; - $file_transaction = $transaction; } // Compute new CCs added by @mentions. Several things can cause CCs to @@ -87,7 +86,6 @@ final class ManiphestTransactionSaveController extends ManiphestController { $transaction = new ManiphestTransaction(); $transaction ->setAuthorPHID($user->getPHID()) - ->setComments($request->getStr('comments')) ->setTransactionType($action); switch ($action) { @@ -124,14 +122,13 @@ final class ManiphestTransactionSaveController extends ManiphestController { case ManiphestTransactionType::TYPE_PRIORITY: $transaction->setNewValue($request->getInt('priority')); break; - case ManiphestTransactionType::TYPE_NONE: case ManiphestTransactionType::TYPE_ATTACH: - // If we have a file transaction, just get rid of this secondary - // transaction and put the comments on it instead. - if ($file_transaction) { - $file_transaction->setComments($transaction->getComments()); - $transaction = null; - } + // Nuke this, we created it above. + $transaction = null; + break; + case PhabricatorTransactions::TYPE_COMMENT: + // Nuke this, we're going to create it below. + $transaction = null; break; default: throw new Exception('unknown action'); @@ -141,6 +138,13 @@ final class ManiphestTransactionSaveController extends ManiphestController { $transactions[] = $transaction; } + if ($request->getStr('comments')) { + $transactions[] = id(new ManiphestTransaction()) + ->setAuthorPHID($user->getPHID()) + ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) + ->setComments($request->getStr('comments')); + } + // When you interact with a task, we add you to the CC list so you get // further updates, and possibly assign the task to you if you took an // ownership action (closing it) but it's currently unowned. We also move diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php index 5a4bfcf496..35651c6a78 100644 --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -38,7 +38,7 @@ final class ManiphestTransactionEditor extends PhabricatorEditor { $value_is_phid_set = false; switch ($type) { - case ManiphestTransactionType::TYPE_NONE: + case PhabricatorTransactions::TYPE_COMMENT: $old = null; break; case ManiphestTransactionType::TYPE_STATUS: @@ -70,11 +70,12 @@ final class ManiphestTransactionEditor extends PhabricatorEditor { $old = $task->getProjectPHIDs(); $value_is_phid_set = true; break; - case ManiphestTransactionType::TYPE_AUXILIARY: - $aux_key = $transaction->getMetadataValue('aux:key'); + case PhabricatorTransactions::TYPE_CUSTOMFIELD: + $aux_key = $transaction->getMetadataValue('customfield:key'); if (!$aux_key) { throw new Exception( - "Expected 'aux:key' metadata on TYPE_AUXILIARY transaction."); + "Expected 'customfield:key' metadata on TYPE_CUSTOMFIELD ". + "transaction."); } // This has already been populated. $old = $transaction->getOldValue(); @@ -120,11 +121,12 @@ final class ManiphestTransactionEditor extends PhabricatorEditor { } else { $transaction->setOldValue(null); $transaction->setNewValue(null); - $transaction->setTransactionType(ManiphestTransactionType::TYPE_NONE); + $transaction->setTransactionType( + PhabricatorTransactions::TYPE_COMMENT); } } else { switch ($type) { - case ManiphestTransactionType::TYPE_NONE: + case PhabricatorTransactions::TYPE_COMMENT: break; case ManiphestTransactionType::TYPE_STATUS: $task->setStatus($new); @@ -160,8 +162,8 @@ final class ManiphestTransactionEditor extends PhabricatorEditor { case ManiphestTransactionType::TYPE_PROJECTS: $task->setProjectPHIDs($new); break; - case ManiphestTransactionType::TYPE_AUXILIARY: - $aux_key = $transaction->getMetadataValue('aux:key'); + case PhabricatorTransactions::TYPE_CUSTOMFIELD: + $aux_key = $transaction->getMetadataValue('customfield:key'); $aux_writes[$aux_key] = $new; break; case ManiphestTransactionType::TYPE_EDGE: @@ -417,7 +419,7 @@ final class ManiphestTransactionEditor extends PhabricatorEditor { case ManiphestTransactionType::TYPE_PRIORITY: $tags[] = MetaMTANotificationType::TYPE_MANIPHEST_PRIORITY; break; - case ManiphestTransactionType::TYPE_NONE: + case PhabricatorTransactions::TYPE_COMMENT: // this is a comment which we will check separately below for // content break; diff --git a/src/applications/maniphest/mail/ManiphestReplyHandler.php b/src/applications/maniphest/mail/ManiphestReplyHandler.php index 50c3114d66..ef815cec29 100644 --- a/src/applications/maniphest/mail/ManiphestReplyHandler.php +++ b/src/applications/maniphest/mail/ManiphestReplyHandler.php @@ -88,7 +88,7 @@ final class ManiphestReplyHandler extends PhabricatorMailReplyHandler { $command = $matches[1]; } - $ttype = ManiphestTransactionType::TYPE_NONE; + $ttype = PhabricatorTransactions::TYPE_COMMENT; $new_value = null; switch ($command) { case 'close': diff --git a/src/applications/maniphest/query/ManiphestLegacyTransactionQuery.php b/src/applications/maniphest/query/ManiphestLegacyTransactionQuery.php index c181f756d9..cfe05aaf05 100644 --- a/src/applications/maniphest/query/ManiphestLegacyTransactionQuery.php +++ b/src/applications/maniphest/query/ManiphestLegacyTransactionQuery.php @@ -6,9 +6,18 @@ final class ManiphestLegacyTransactionQuery { PhabricatorUser $viewer, array $tasks) { - return id(new ManiphestTransaction())->loadAllWhere( - 'taskID IN (%Ld) ORDER BY id ASC', - mpull($tasks, 'getID')); + $xactions = id(new ManiphestTransactionQuery()) + ->setViewer($viewer) + ->withObjectPHIDs(mpull($tasks, 'getPHID')) + ->needComments(true) + ->execute(); + + foreach ($xactions as $key => $xaction) { + $xactions[$key] = ManiphestTransaction::newFromModernTransaction( + $xaction); + } + + return $xactions; } public static function loadByTask( @@ -22,7 +31,22 @@ final class ManiphestLegacyTransactionQuery { PhabricatorUser $viewer, $xaction_id) { - return id(new ManiphestTransaction())->load($xaction_id); + $xaction = id(new ManiphestTransactionPro())->load($xaction_id); + if (!$xaction) { + return null; + } + + $xaction = id(new ManiphestTransactionQuery()) + ->setViewer($viewer) + ->withPHIDs(array($xaction->getPHID())) + ->needComments(true) + ->executeOne(); + + if ($xaction) { + $xaction = ManiphestTransaction::newFromModernTransaction($xaction); + } + + return $xaction; } diff --git a/src/applications/maniphest/query/ManiphestTransactionQuery.php b/src/applications/maniphest/query/ManiphestTransactionQuery.php new file mode 100644 index 0000000000..9451c141b4 --- /dev/null +++ b/src/applications/maniphest/query/ManiphestTransactionQuery.php @@ -0,0 +1,10 @@ +proxy = new ManiphestTransactionPro(); + } + + public function __clone() { + $this->proxy = clone $this->proxy; + } + + public static function newFromModernTransaction( + ManiphestTransactionPro $pro) { + + $obj = new ManiphestTransaction(); + $obj->proxy = $pro; + + return $obj; + } + + public function save() { + $this->proxy->openTransaction(); + $this->proxy + ->setViewPolicy('public') + ->setEditPolicy($this->getAuthorPHID()) + ->save(); + if ($this->pendingComment) { + $comment = id(new ManiphestTransactionComment()) + ->setTransactionPHID($this->proxy->getPHID()) + ->setCommentVersion(1) + ->setAuthorPHID($this->getAuthorPHID()) + ->setViewPolicy('public') + ->setEditPolicy($this->getAuthorPHID()) + ->setContent($this->pendingComment) + ->setContentSource($this->getContentSource()) + ->setIsDeleted(0) + ->save(); + + $this->proxy + ->setCommentVersion(1) + ->setCommentPHID($comment->getPHID()) + ->save(); + + $this->pendingComment = null; + } + $this->proxy->saveTransaction(); - public function setTransactionTask(ManiphestTask $task) { - $this->setTaskID($task->getID()); return $this; } - public function getConfiguration() { - return array( - self::CONFIG_SERIALIZATION => array( - 'oldValue' => self::SERIALIZATION_JSON, - 'newValue' => self::SERIALIZATION_JSON, - 'metadata' => self::SERIALIZATION_JSON, - ), - ) + parent::getConfiguration(); + public function setTransactionTask(ManiphestTask $task) { + $this->proxy->setObjectPHID($task->getPHID()); + return $this; + } + + public function getTaskPHID() { + return $this->proxy->getObjectPHID(); + } + + public function getID() { + return $this->proxy->getID(); + } + + public function setTaskID() { + throw new Exception("No longer supported!"); + } + + public function getTaskID() { + throw new Exception("No longer supported!"); + } + + public function getAuthorPHID() { + return $this->proxy->getAuthorPHID(); + } + + public function setAuthorPHID($phid) { + $this->proxy->setAuthorPHID($phid); + return $this; + } + + public function getOldValue() { + return $this->proxy->getOldValue(); + } + + public function setOldValue($value) { + $this->proxy->setOldValue($value); + return $this; + } + + public function getNewValue() { + return $this->proxy->getNewValue(); + } + + public function setNewValue($value) { + $this->proxy->setNewValue($value); + return $this; + } + + public function getTransactionType() { + return $this->proxy->getTransactionType(); + } + + public function setTransactionType($value) { + $this->proxy->setTransactionType($value); + return $this; + } + + public function setContentSource(PhabricatorContentSource $content_source) { + $this->proxy->setContentSource($content_source); + return $this; + } + + public function getContentSource() { + return $this->proxy->getContentSource(); + } + + public function getMetadataValue($key, $default = null) { + return $this->proxy->getMetadataValue($key, $default); + } + + public function setMetadataValue($key, $value) { + $this->proxy->setMetadataValue($key, $value); + return $this; + } + + public function getComments() { + if ($this->pendingComment) { + return $this->pendingComment; + } + if ($this->proxy->getComment()) { + return $this->proxy->getComment()->getContent(); + } + return null; + } + + public function setComments($comment) { + $this->pendingComment = $comment; + return $this; + } + + public function getDateCreated() { + return $this->proxy->getDateCreated(); + } + + public function getDateModified() { + return $this->proxy->getDateModified(); } public function extractPHIDs() { @@ -78,21 +198,6 @@ final class ManiphestTransaction extends ManiphestDAO return $phids; } - public function getMetadataValue($key, $default = null) { - if (!is_array($this->metadata)) { - return $default; - } - return idx($this->metadata, $key, $default); - } - - public function setMetadataValue($key, $value) { - if (!is_array($this->metadata)) { - $this->metadata = array(); - } - $this->metadata[$key] = $value; - return $this; - } - public function canGroupWith($target) { if ($target->getAuthorPHID() != $this->getAuthorPHID()) { return false; @@ -107,10 +212,10 @@ final class ManiphestTransaction extends ManiphestDAO } if ($target->getTransactionType() == $this->getTransactionType()) { - $aux_type = ManiphestTransactionType::TYPE_AUXILIARY; + $aux_type = PhabricatorTransactions::TYPE_CUSTOMFIELD; if ($this->getTransactionType() == $aux_type) { - $that_key = $target->getMetadataValue('aux:key'); - $this_key = $this->getMetadataValue('aux:key'); + $that_key = $target->getMetadataValue('customfield:key'); + $this_key = $this->getMetadataValue('customfield:key'); if ($that_key == $this_key) { return false; } @@ -126,15 +231,6 @@ final class ManiphestTransaction extends ManiphestDAO return (bool)strlen(trim($this->getComments())); } - public function setContentSource(PhabricatorContentSource $content_source) { - $this->contentSource = $content_source->serialize(); - return $this; - } - - public function getContentSource() { - return PhabricatorContentSource::newFromSerialized($this->contentSource); - } - /* -( Markup Interface )--------------------------------------------------- */ diff --git a/src/applications/maniphest/storage/ManiphestTransactionComment.php b/src/applications/maniphest/storage/ManiphestTransactionComment.php index 110277e05f..3e5a5cc95f 100644 --- a/src/applications/maniphest/storage/ManiphestTransactionComment.php +++ b/src/applications/maniphest/storage/ManiphestTransactionComment.php @@ -3,6 +3,11 @@ final class ManiphestTransactionComment extends PhabricatorApplicationTransactionComment { + public function getTableName() { + // TODO: Remove once the "pro" stuff gets dropped. + return 'maniphest_transaction_comment'; + } + public function getApplicationTransactionObject() { return new ManiphestTransactionPro(); } diff --git a/src/applications/maniphest/view/ManiphestTransactionDetailView.php b/src/applications/maniphest/view/ManiphestTransactionDetailView.php index cc720c51df..dad3da16c7 100644 --- a/src/applications/maniphest/view/ManiphestTransactionDetailView.php +++ b/src/applications/maniphest/view/ManiphestTransactionDetailView.php @@ -231,7 +231,6 @@ final class ManiphestTransactionDetailView extends ManiphestView { if ($this->commentNumber) { $anchor_name = 'comment-'.$this->commentNumber; $anchor_text = - 'T'.$any_transaction->getTaskID(). '#'.$this->commentNumber; $xaction_view->setAnchor($anchor_name, $anchor_text); @@ -343,7 +342,7 @@ final class ManiphestTransactionDetailView extends ManiphestView { $this->renderExpandLink($transaction); } break; - case ManiphestTransactionType::TYPE_NONE: + case PhabricatorTransactions::TYPE_COMMENT: $verb = 'Commented On'; $desc = 'added a comment'; break; @@ -535,14 +534,9 @@ final class ManiphestTransactionDetailView extends ManiphestView { 'removed: '.$rem_desc; } break; - case ManiphestTransactionType::TYPE_AUXILIARY: - $aux_key = $transaction->getMetadataValue('aux:key'); - - // TODO: Migrate all legacy data when everything migrates for T2217. - $aux_field = $this->getAuxiliaryField($aux_key); - if (!$aux_field) { - $aux_field = $this->getAuxiliaryField('std:maniphest:'.$aux_key); - } + case PhabricatorTransactions::TYPE_CUSTOMFIELD: + $aux_key = $transaction->getMetadataValue('customfield:key'); + $aux_field = idx($this->auxiliaryFields, $aux_key); if ($old === null) { $verb = "Set Field"; @@ -568,7 +562,7 @@ final class ManiphestTransactionDetailView extends ManiphestView { break; default: - return array($type, ' brazenly '.$type."'d", $classes); + return array($type, ' brazenly '.$type."'d", $classes, null); } // TODO: [HTML] This code will all be rewritten when we switch to using diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index 62fc5116cf..ea0e94c9ad 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1616,6 +1616,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('20130921.mtransactions.sql'), ), + '20130921.xmigratemaniphest.php' => array( + 'type' => 'php', + 'name' => $this->getPatchPath('20130921.xmigratemaniphest.php'), + ), ); } }