1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-22 21:40:55 +01:00

Use ManiphestTaskQuery in nearly all interfaces

Summary:
Ref T603. Make almost every task read policy-aware. Notable exceptions are:

  - Edge editor -- this stuff is prescreened and should be moved to ApplicationTransactions eventually anyway.
  - Search/attach stuff -- this stuff needs some general work. The actual list should be fine since you can't pull handles. There may be a very indirect hole here where you could attach an object you can't see (but do know the ID of) to an object you can see. Pretty fluff.
  - The "Tasks" field in Differential will let you reference objects you can't see. Possibly this is desirable, in the case of commandeering revisions. Mostly, it was inconvenient to get a viewer (I think).

Test Plan:
  - Called `maniphest.info`.
  - Called `maniphest.update`.
  - Batch edited tasks.
  - Dragged and dropped tasks to change subpriority.
  - Subscribed and unsubscribed from a task.
  - Edited a task.
  - Created a task.
  - Created a task with a parent.
  - Created a task with a template.
  - Previewed a task update.
  - Commented on a task.
  - Added a dependency.
  - Searched for "T33" in object search dialog.
  - Created a branch "T33", ran `arc diff`, verified link.
  - Pushed a commit with "Fixes T33", verified close.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7119
This commit is contained in:
epriestley 2013-09-25 13:44:14 -07:00
parent 475afe4a5b
commit 3a87a95e11
17 changed files with 98 additions and 26 deletions

View file

@ -56,7 +56,10 @@ final class DifferentialBranchFieldSpecification
$match = null;
if (preg_match('/^T(\d+)/i', $branch, $match)) { // No $ to allow T123_demo.
list(, $task_id) = $match;
$task = id(new ManiphestTask())->load($task_id);
$task = id(new ManiphestTaskQuery())
->setViewer($editor->requireActor())
->withIDs(array($task_id))
->executeOne();
if ($task) {
id(new PhabricatorEdgeEditor())
->setActor($this->getUser())

View file

@ -150,8 +150,10 @@ abstract class DifferentialFreeformFieldSpecification
$tasks = $this->findMentionedTasks($message);
if ($tasks) {
$tasks = id(new ManiphestTask())
->loadAllWhere('id IN (%Ld)', array_keys($tasks));
$tasks = id(new ManiphestTaskQuery())
->setViewer($editor->getActor())
->withIDs(array_keys($tasks))
->execute();
$this->saveFieldEdges(
$editor->getRevision(),
PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK,

View file

@ -128,6 +128,7 @@ final class DifferentialManiphestTasksFieldSpecification
return array();
}
// TODO: T603 Get a viewer here so we can issue the right query.
$task_ids = $matches[1];
$tasks = id(new ManiphestTask())

View file

@ -30,7 +30,10 @@ final class ConduitAPI_maniphest_info_Method
protected function execute(ConduitAPIRequest $request) {
$task_id = $request->getValue('task_id');
$task = id(new ManiphestTask())->load($task_id);
$task = id(new ManiphestTaskQuery())
->setViewer($request->getUser())
->withIDs(array($task_id))
->executeOne();
if (!$task) {
throw new ConduitException('ERR_BAD_TASK');
}

View file

@ -35,11 +35,15 @@ final class ConduitAPI_maniphest_update_Method
}
if ($id) {
$task = id(new ManiphestTask())->load($id);
$task = id(new ManiphestTaskQuery())
->setViewer($request->getUser())
->withIDs(array($id))
->executeOne();
} else {
$task = id(new ManiphestTask())->loadOneWhere(
'phid = %s',
$phid);
$task = id(new ManiphestTaskQuery())
->setViewer($request->getUser())
->withPHIDs(array($phid))
->executeOne();
}
$params = $request->getAllParameters();

View file

@ -11,9 +11,15 @@ final class ManiphestBatchEditController extends ManiphestController {
$user = $request->getUser();
$task_ids = $request->getArr('batch');
$tasks = id(new ManiphestTask())->loadAllWhere(
'id IN (%Ld)',
$task_ids);
$tasks = id(new ManiphestTaskQuery())
->setViewer($user)
->withIDs($task_ids)
->requireCapabilities(
array(
PhabricatorPolicyCapability::CAN_VIEW,
PhabricatorPolicyCapability::CAN_EDIT,
))
->execute();
$actions = $request->getStr('actions');
if ($actions) {

View file

@ -13,13 +13,24 @@ final class ManiphestSubpriorityController extends ManiphestController {
return new Aphront403Response();
}
$task = id(new ManiphestTask())->load($request->getInt('task'));
$task = id(new ManiphestTaskQuery())
->setViewer($user)
->withIDs(array($request->getInt('task')))
->requireCapabilities(
array(
PhabricatorPolicyCapability::CAN_VIEW,
PhabricatorPolicyCapability::CAN_EDIT,
))
->executeOne();
if (!$task) {
return new Aphront404Response();
}
if ($request->getInt('after')) {
$after_task = id(new ManiphestTask())->load($request->getInt('after'));
$after_task = id(new ManiphestTaskQuery())
->setViewer($user)
->withIDs(array($request->getInt('after')))
->executeOne();
if (!$after_task) {
return new Aphront404Response();
}

View file

@ -15,7 +15,10 @@ final class ManiphestSubscribeController extends ManiphestController {
$request = $this->getRequest();
$user = $request->getUser();
$task = id(new ManiphestTask())->load($this->id);
$task = id(new ManiphestTaskQuery())
->setViewer($user)
->withIDs(array($this->id))
->executeOne();
if (!$task) {
return new Aphront404Response();
}

View file

@ -20,7 +20,10 @@ final class ManiphestTaskDetailController extends ManiphestController {
$priority_map = ManiphestTaskPriority::getTaskPriorityMap();
$task = id(new ManiphestTask())->load($this->id);
$task = id(new ManiphestTaskQuery())
->setViewer($user)
->withIDs(array($this->id))
->executeOne();
if (!$task) {
return new Aphront404Response();
}
@ -28,7 +31,10 @@ final class ManiphestTaskDetailController extends ManiphestController {
$workflow = $request->getStr('workflow');
$parent_task = null;
if ($workflow && is_numeric($workflow)) {
$parent_task = id(new ManiphestTask())->load($workflow);
$parent_task = id(new ManiphestTaskQuery())
->setViewer($user)
->withIDs(array($workflow))
->executeOne();
}
$transactions = id(new ManiphestTransactionQuery())

View file

@ -21,7 +21,15 @@ final class ManiphestTaskEditController extends ManiphestController {
$template_id = null;
if ($this->id) {
$task = id(new ManiphestTask())->load($this->id);
$task = id(new ManiphestTaskQuery())
->setViewer($user)
->requireCapabilities(
array(
PhabricatorPolicyCapability::CAN_VIEW,
PhabricatorPolicyCapability::CAN_EDIT,
))
->withIDs(array($this->id))
->executeOne();
if (!$task) {
return new Aphront404Response();
}
@ -74,7 +82,10 @@ final class ManiphestTaskEditController extends ManiphestController {
// You can only have a parent task if you're creating a new task.
$parent_id = $request->getInt('parent');
if ($parent_id) {
$parent_task = id(new ManiphestTask())->load($parent_id);
$parent_task = id(new ManiphestTaskQuery())
->setViewer($user)
->withIDs(array($parent_id))
->executeOne();
if (!$template_id) {
$template_id = $parent_id;
}
@ -274,7 +285,10 @@ final class ManiphestTaskEditController extends ManiphestController {
$user->getPHID(),
));
if ($template_id) {
$template_task = id(new ManiphestTask())->load($template_id);
$template_task = id(new ManiphestTaskQuery())
->setViewer($user)
->withIDs(array($template_id))
->executeOne();
if ($template_task) {
$task->setCCPHIDs($template_task->getCCPHIDs());
$task->setProjectPHIDs($template_task->getProjectPHIDs());

View file

@ -18,7 +18,10 @@ final class ManiphestTransactionPreviewController extends ManiphestController {
$comments = $request->getStr('comments');
$task = id(new ManiphestTask())->load($this->id);
$task = id(new ManiphestTaskQuery())
->setViewer($user)
->withIDs(array($this->id))
->executeOne();
if (!$task) {
return new Aphront404Response();
}

View file

@ -9,7 +9,14 @@ final class ManiphestTransactionSaveController extends ManiphestController {
$request = $this->getRequest();
$user = $request->getUser();
$task = id(new ManiphestTask())->load($request->getStr('taskID'));
// TODO: T603 This doesn't require CAN_EDIT because non-editors can still
// leave comments, probably? For now, this just nondisruptive. Smooth this
// out once policies are more clear.
$task = id(new ManiphestTaskQuery())
->setViewer($user)
->withIDs(array($request->getStr('taskID')))
->executeOne();
if (!$task) {
return new Aphront404Response();
}

View file

@ -15,6 +15,9 @@ final class ManiphestTransactionEditor extends PhabricatorEditor {
public static function getNextSubpriority($pri, $sub) {
// TODO: T603 Figure out what the policies here should be once this gets
// cleaned up.
if ($sub === null) {
$next = id(new ManiphestTask())->loadOneWhere(
'priority = %d ORDER BY subpriority ASC LIMIT 1',

View file

@ -36,6 +36,9 @@ final class ManiphestEdgeEventListener extends PhutilEventListener {
$edges = $this->loadAllEdges($event);
$tasks = array();
if ($edges) {
// TODO: T603 This should probably all get nuked. Until then, this isn't
// realllllly a policy issue since callers are (or should be) doing
// policy checks anyway.
$tasks = id(new ManiphestTask())->loadAllWhere(
'phid IN (%Ls)',
array_keys($edges));

View file

@ -149,9 +149,10 @@ final class PhabricatorSearchAttachController
return $response;
}
$targets = id(new ManiphestTask())->loadAllWhere(
'phid in (%Ls) ORDER BY id ASC',
array_keys($phids));
$targets = id(new ManiphestTaskQuery())
->setViewer($user)
->withPHIDs(array_keys($phids))
->execute();
if (empty($targets)) {
return $response;

View file

@ -98,6 +98,8 @@ final class PhabricatorSearchSelectController
$object_ids);
break;
case ManiphestPHIDTypeTask::TYPECONST:
// TODO: (T603) Clean this up. This should probably all run through
// ObjectQuery?
$objects = id(new ManiphestTask())->loadAllWhere(
'id IN (%Ld)',
$object_ids);

View file

@ -10,11 +10,11 @@ abstract class PhabricatorEditor extends Phobject {
return $this;
}
final protected function getActor() {
final public function getActor() {
return $this->actor;
}
final protected function requireActor() {
final public function requireActor() {
$actor = $this->getActor();
if (!$actor) {
throw new Exception('You must setActor()!');