mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-28 00:10:57 +01:00
Route all ManiphestTransaction reads through a single class
Summary: Ref T2217. I'm going to do the fake-double-writes ("double reads"?) thing where we proxy the storage that worked pretty well for auth. That is: - (Some more cleanup diffs next, maybe?) - Move all the data to the new storage, and make `ManiphestTransaction` read and write by wrapping `ManiphestTransactionPro`. - If nothing breaks, it's a straight shot to nuking ManiphestTransaction callsite by callsite. I think Maniphest is way easier than Differential, because there are very few query sites and no inline comments. Test Plan: `grep` to find callsites. Loaded task view, called Conduit. Reviewers: btrahan Reviewed By: btrahan CC: aran, chad Maniphest Tasks: T2217 Differential Revision: https://secure.phabricator.com/D7067
This commit is contained in:
parent
4c2bd2ca8e
commit
17e1732152
8 changed files with 57 additions and 13 deletions
|
@ -706,6 +706,7 @@ phutil_register_library_map(array(
|
|||
'ManiphestExcelFormat' => 'applications/maniphest/export/ManiphestExcelFormat.php',
|
||||
'ManiphestExportController' => 'applications/maniphest/controller/ManiphestExportController.php',
|
||||
'ManiphestHovercardEventListener' => 'applications/maniphest/event/ManiphestHovercardEventListener.php',
|
||||
'ManiphestLegacyTransactionQuery' => 'applications/maniphest/query/ManiphestLegacyTransactionQuery.php',
|
||||
'ManiphestNameIndex' => 'applications/maniphest/storage/ManiphestNameIndex.php',
|
||||
'ManiphestNameIndexEventListener' => 'applications/maniphest/event/ManiphestNameIndexEventListener.php',
|
||||
'ManiphestPHIDTypeTask' => 'applications/maniphest/phid/ManiphestPHIDTypeTask.php',
|
||||
|
@ -733,6 +734,7 @@ phutil_register_library_map(array(
|
|||
'ManiphestTaskStatus' => 'applications/maniphest/constants/ManiphestTaskStatus.php',
|
||||
'ManiphestTaskSubscriber' => 'applications/maniphest/storage/ManiphestTaskSubscriber.php',
|
||||
'ManiphestTransaction' => 'applications/maniphest/storage/ManiphestTransaction.php',
|
||||
'ManiphestTransactionComment' => 'applications/maniphest/storage/ManiphestTransactionComment.php',
|
||||
'ManiphestTransactionDetailView' => 'applications/maniphest/view/ManiphestTransactionDetailView.php',
|
||||
'ManiphestTransactionEditor' => 'applications/maniphest/editor/ManiphestTransactionEditor.php',
|
||||
'ManiphestTransactionListView' => 'applications/maniphest/view/ManiphestTransactionListView.php',
|
||||
|
@ -2829,6 +2831,7 @@ phutil_register_library_map(array(
|
|||
0 => 'ManiphestDAO',
|
||||
1 => 'PhabricatorMarkupInterface',
|
||||
),
|
||||
'ManiphestTransactionComment' => 'PhabricatorApplicationTransactionComment',
|
||||
'ManiphestTransactionDetailView' => 'ManiphestView',
|
||||
'ManiphestTransactionEditor' => 'PhabricatorEditor',
|
||||
'ManiphestTransactionListView' => 'ManiphestView',
|
||||
|
|
|
@ -33,9 +33,14 @@ final class ConduitAPI_maniphest_gettasktransactions_Method
|
|||
return $results;
|
||||
}
|
||||
|
||||
$transactions = id(new ManiphestTransaction())->loadAllWhere(
|
||||
'taskID IN (%Ld) ORDER BY id ASC',
|
||||
$task_ids);
|
||||
$tasks = id(new ManiphestTaskQuery())
|
||||
->setViewer($request->getUser())
|
||||
->withIDs($task_ids)
|
||||
->execute();
|
||||
|
||||
$transactions = ManiphestLegacyTransactionQuery::loadByTasks(
|
||||
$request->getUser(),
|
||||
$tasks);
|
||||
|
||||
foreach ($transactions as $transaction) {
|
||||
$task_id = $transaction->getTaskID();
|
||||
|
|
|
@ -32,7 +32,9 @@ final class ManiphestTaskDescriptionChangeController
|
|||
}
|
||||
|
||||
$transaction_id = $this->getTransactionID();
|
||||
$transaction = id(new ManiphestTransaction())->load($transaction_id);
|
||||
$transaction = ManiphestLegacyTransactionQuery::loadByTransactionID(
|
||||
$user,
|
||||
$transaction_id);
|
||||
if (!$transaction) {
|
||||
return new Aphront404Response();
|
||||
}
|
||||
|
|
|
@ -31,9 +31,9 @@ final class ManiphestTaskDetailController extends ManiphestController {
|
|||
$parent_task = id(new ManiphestTask())->load($workflow);
|
||||
}
|
||||
|
||||
$transactions = id(new ManiphestTransaction())->loadAllWhere(
|
||||
'taskID = %d ORDER BY id ASC',
|
||||
$task->getID());
|
||||
$transactions = ManiphestLegacyTransactionQuery::loadByTask(
|
||||
$user,
|
||||
$task);
|
||||
|
||||
$field_list = PhabricatorCustomField::getObjectFields(
|
||||
$task,
|
||||
|
|
|
@ -194,7 +194,7 @@ final class ManiphestTransactionEditor extends PhabricatorEditor {
|
|||
}
|
||||
|
||||
foreach ($transactions as $transaction) {
|
||||
$transaction->setTaskID($task->getID());
|
||||
$transaction->setTransactionTask($task);
|
||||
$transaction->save();
|
||||
}
|
||||
|
||||
|
@ -463,7 +463,7 @@ final class ManiphestTransactionEditor extends PhabricatorEditor {
|
|||
$new_ccs = array_merge($current_ccs, array($user->getPHID()));
|
||||
|
||||
$transaction = new ManiphestTransaction();
|
||||
$transaction->setTaskID($task->getID());
|
||||
$transaction->setTransactionTask($task);
|
||||
$transaction->setAuthorPHID($user->getPHID());
|
||||
$transaction->setTransactionType(ManiphestTransactionType::TYPE_CCS);
|
||||
$transaction->setNewValue(array_unique($new_ccs));
|
||||
|
@ -481,7 +481,7 @@ final class ManiphestTransactionEditor extends PhabricatorEditor {
|
|||
$new_ccs = array_diff($current_ccs, array($user->getPHID()));
|
||||
|
||||
$transaction = new ManiphestTransaction();
|
||||
$transaction->setTaskID($task->getID());
|
||||
$transaction->setTransactionTask($task);
|
||||
$transaction->setAuthorPHID($user->getPHID());
|
||||
$transaction->setTransactionType(ManiphestTransactionType::TYPE_CCS);
|
||||
$transaction->setNewValue(array_unique($new_ccs));
|
||||
|
|
|
@ -0,0 +1,29 @@
|
|||
<?php
|
||||
|
||||
final class ManiphestLegacyTransactionQuery {
|
||||
|
||||
public static function loadByTasks(
|
||||
PhabricatorUser $viewer,
|
||||
array $tasks) {
|
||||
|
||||
return id(new ManiphestTransaction())->loadAllWhere(
|
||||
'taskID IN (%Ld) ORDER BY id ASC',
|
||||
mpull($tasks, 'getID'));
|
||||
}
|
||||
|
||||
public static function loadByTask(
|
||||
PhabricatorUser $viewer,
|
||||
ManiphestTask $task) {
|
||||
|
||||
return self::loadByTasks($viewer, array($task));
|
||||
}
|
||||
|
||||
public static function loadByTransactionID(
|
||||
PhabricatorUser $viewer,
|
||||
$xaction_id) {
|
||||
|
||||
return id(new ManiphestTransaction())->load($xaction_id);
|
||||
}
|
||||
|
||||
|
||||
}
|
|
@ -38,9 +38,9 @@ final class ManiphestSearchIndexer
|
|||
time());
|
||||
}
|
||||
|
||||
$transactions = id(new ManiphestTransaction())->loadAllWhere(
|
||||
'taskID = %d',
|
||||
$task->getID());
|
||||
$transactions = ManiphestLegacyTransactionQuery::loadByTask(
|
||||
$this->getViewer(),
|
||||
$task);
|
||||
|
||||
$current_ccs = $task->getCCPHIDs();
|
||||
$owner = null;
|
||||
|
|
|
@ -18,6 +18,11 @@ final class ManiphestTransaction extends ManiphestDAO
|
|||
protected $metadata = array();
|
||||
protected $contentSource;
|
||||
|
||||
public function setTransactionTask(ManiphestTask $task) {
|
||||
$this->setTaskID($task->getID());
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function getConfiguration() {
|
||||
return array(
|
||||
self::CONFIG_SERIALIZATION => array(
|
||||
|
|
Loading…
Reference in a new issue