Christopher Johnson (WMDE) has submitted this change and it was merged.
Change subject: Refactor SprintBoardTaskEditController
......................................................................
Refactor SprintBoardTaskEditController
Fix BurndownException
Remove dead code
Change-Id: If4d2b6ed9cf5b8c4ecdd8c2a3365c4da553a4f9c
---
M src/controller/SprintBoardColumnDetailController.php
M src/controller/SprintBoardColumnEditController.php
M src/controller/SprintBoardColumnHideController.php
M src/controller/SprintBoardImportController.php
M src/controller/SprintBoardReorderController.php
M src/controller/SprintBoardTaskEditController.php
M src/controller/SprintBoardViewController.php
M src/controller/SprintController.php
M src/query/SprintQuery.php
M src/view/BurndownDataView.php
M src/view/TasksTableView.php
11 files changed, 328 insertions(+), 235 deletions(-)
Approvals:
Christopher Johnson (WMDE): Verified; Looks good to me, approved
diff --git a/src/controller/SprintBoardColumnDetailController.php
b/src/controller/SprintBoardColumnDetailController.php
index 26e4f28..5ab0b94 100644
--- a/src/controller/SprintBoardColumnDetailController.php
+++ b/src/controller/SprintBoardColumnDetailController.php
@@ -46,7 +46,7 @@
->withObjectPHIDs(array($column->getPHID()))
->execute();
- $engine = id(new PhabricatorMarkupEngine())
+ id(new PhabricatorMarkupEngine())
->setViewer($viewer);
$timeline = id(new PhabricatorApplicationTransactionView())
diff --git a/src/controller/SprintBoardColumnEditController.php
b/src/controller/SprintBoardColumnEditController.php
index bc4f236..3f0f1f1 100644
--- a/src/controller/SprintBoardColumnEditController.php
+++ b/src/controller/SprintBoardColumnEditController.php
@@ -98,7 +98,7 @@
->setNewValue($v_limit);
try {
- $editor = id(new PhabricatorProjectColumnTransactionEditor())
+ id(new PhabricatorProjectColumnTransactionEditor())
->setActor($viewer)
->setContinueOnNoEffect(true)
->setContentSourceFromRequest($request)
diff --git a/src/controller/SprintBoardColumnHideController.php
b/src/controller/SprintBoardColumnHideController.php
index 3aaaeb3..28ff6cf 100644
--- a/src/controller/SprintBoardColumnHideController.php
+++ b/src/controller/SprintBoardColumnHideController.php
@@ -42,8 +42,6 @@
return new Aphront404Response();
}
- $column_phid = $column->getPHID();
-
$view_uri = $this->getApplicationURI('/board/'.$this->projectID.'/');
$view_uri = new PhutilURI($view_uri);
foreach ($request->getPassthroughRequestData() as $key => $value) {
@@ -71,7 +69,7 @@
->setNewValue($new_status),
);
- $editor = id(new PhabricatorProjectColumnTransactionEditor())
+ id(new PhabricatorProjectColumnTransactionEditor())
->setActor($viewer)
->setContinueOnNoEffect(true)
->setContentSourceFromRequest($request)
diff --git a/src/controller/SprintBoardImportController.php
b/src/controller/SprintBoardImportController.php
index 41bd803..6c21631 100644
--- a/src/controller/SprintBoardImportController.php
+++ b/src/controller/SprintBoardImportController.php
@@ -56,7 +56,7 @@
if ($import_column->isHidden()) {
continue;
}
- $new_column = PhabricatorProjectColumn::initializeNewColumn($viewer)
+ PhabricatorProjectColumn::initializeNewColumn($viewer)
->setSequence($import_column->getSequence())
->setProjectPHID($project->getPHID())
->setName($import_column->getName())
diff --git a/src/controller/SprintBoardReorderController.php
b/src/controller/SprintBoardReorderController.php
index d34ac8e..301458f 100644
--- a/src/controller/SprintBoardReorderController.php
+++ b/src/controller/SprintBoardReorderController.php
@@ -54,7 +54,6 @@
return new Aphront404Response();
}
- $target_column = $columns[$column_phid];
$new_sequence = $request->getInt('sequence');
if ($new_sequence < 0) {
return new Aphront404Response();
diff --git a/src/controller/SprintBoardTaskEditController.php
b/src/controller/SprintBoardTaskEditController.php
index d94fa99..0770f3e 100644
--- a/src/controller/SprintBoardTaskEditController.php
+++ b/src/controller/SprintBoardTaskEditController.php
@@ -15,30 +15,15 @@
$response_type = $request->getStr('responseType', 'task');
$order = $request->getStr('order',
PhabricatorProjectColumn::DEFAULT_ORDER);
- $can_edit_assign = $this->hasApplicationCapability(
- ManiphestEditAssignCapability::CAPABILITY);
- $can_edit_policies = $this->hasApplicationCapability(
- ManiphestEditPoliciesCapability::CAPABILITY);
- $can_edit_priority = $this->hasApplicationCapability(
- ManiphestEditPriorityCapability::CAPABILITY);
- $can_edit_projects = $this->hasApplicationCapability(
- ManiphestEditProjectsCapability::CAPABILITY);
- $can_edit_status = $this->hasApplicationCapability(
- ManiphestEditStatusCapability::CAPABILITY);
+ list($can_edit_assign,$can_edit_policies,$can_edit_priority,
+ $can_edit_projects, $can_edit_status)
+ = $this->setApplicationCapabilities();
$parent_task = null;
$template_id = null;
if ($this->id) {
- $task = id(new ManiphestTaskQuery())
- ->setViewer($user)
- ->requireCapabilities(
- array(
- PhabricatorPolicyCapability::CAN_VIEW,
- PhabricatorPolicyCapability::CAN_EDIT,
- ))
- ->withIDs(array($this->id))
- ->executeOne();
+ $task = $this->getTask($user);
if (!$task) {
return new Aphront404Response();
}
@@ -58,28 +43,10 @@
$projects = $request->getStr('projects');
if ($projects) {
$tokens = $request->getStrList('projects');
-
$type_project = PhabricatorProjectProjectPHIDType::TYPECONST;
- foreach ($tokens as $key => $token) {
- if (phid_get_type($token) == $type_project) {
- // If this is formatted like a PHID, leave it as-is.
- continue;
- }
- if (preg_match('/^#/', $token)) {
- // If this already has a "#", leave it as-is.
- continue;
- }
-
- // Add a "#" prefix.
- $tokens[$key] = '#'.$token;
- }
-
- $default_projects = id(new PhabricatorObjectQuery())
- ->setViewer($user)
- ->withNames($tokens)
- ->execute();
- $default_projects = mpull($default_projects, 'getPHID');
+ $tokens = $this->setTokens($tokens, $type_project);
+ $default_projects = $this->getDefaultProjects($user, $tokens);
if ($default_projects) {
$task->attachProjectPHIDs($default_projects);
@@ -102,16 +69,7 @@
if ($can_edit_assign) {
$assign = $request->getStr('assign');
if (strlen($assign)) {
- $assign_user = id(new PhabricatorPeopleQuery())
- ->setViewer($user)
- ->withUsernames(array($assign))
- ->executeOne();
- if (!$assign_user) {
- $assign_user = id(new PhabricatorPeopleQuery())
- ->setViewer($user)
- ->withPHIDs(array($assign))
- ->executeOne();
- }
+ $assign_user = $this->assignUser($user, $assign);
if ($assign_user) {
$task->setOwnerPHID($assign_user->getPHID());
@@ -138,13 +96,7 @@
$errors = array();
$e_title = true;
- $field_list = PhabricatorCustomField::getObjectFields(
- $task,
- PhabricatorCustomField::ROLE_EDIT);
- $field_list->setViewer($user);
- $field_list->readFieldsFromStorage($task);
-
- $aux_fields = $field_list->getFields();
+ list($aux_fields, $field_list) = $this->getAuxFields($task, $user);
if ($request->isFormPost()) {
$changes = array();
@@ -194,68 +146,16 @@
// hack but shouldn't be long for this world.
$placeholder_editor = new PhabricatorUserProfileEditor();
- $field_errors = $aux_field->validateApplicationTransactions(
- $placeholder_editor,
- PhabricatorTransactions::TYPE_CUSTOMFIELD,
- array(
- id(new ManiphestTransaction())
- ->setOldValue($aux_old_value)
- ->setNewValue($aux_new_value),
- ));
-
- foreach ($field_errors as $error) {
- $errors[] = $error->getMessage();
- }
+ $errors = $this->setFieldErrors($aux_field, $placeholder_editor,
$aux_old_value, $aux_new_value, $errors);
$old_values[$aux_field->getFieldKey()] = $aux_old_value;
}
if ($errors) {
- $task->setTitle($new_title);
- $task->setDescription($new_desc);
- $task->setPriority($request->getInt('priority'));
- $task->setOwnerPHID($owner_phid);
- $task->setCCPHIDs($request->getArr('cc'));
- $task->attachProjectPHIDs($request->getArr('projects'));
+ $task = $this->setTaskError($task, $new_title, $new_desc,
+ $request, $owner_phid);
} else {
-
- if ($can_edit_priority) {
- $changes[ManiphestTransaction::TYPE_PRIORITY] =
- $request->getInt('priority');
- }
- if ($can_edit_assign) {
- $changes[ManiphestTransaction::TYPE_OWNER] = $owner_phid;
- }
-
- $changes[ManiphestTransaction::TYPE_CCS] = $request->getArr('cc');
-
- if ($can_edit_projects) {
- $projects = $request->getArr('projects');
- $changes[ManiphestTransaction::TYPE_PROJECTS] =
- $projects;
- $column_phid = $request->getStr('columnPHID');
- // allow for putting a task in a project column at creation -only-
- if (!$task->getID() && $column_phid && $projects) {
- $column = id(new PhabricatorProjectColumnQuery())
- ->setViewer($user)
- ->withProjectPHIDs($projects)
- ->withPHIDs(array($column_phid))
- ->executeOne();
- if ($column) {
- $changes[ManiphestTransaction::TYPE_PROJECT_COLUMN] =
- array(
- 'new' => array(
- 'projectPHID' => $column->getProjectPHID(),
- 'columnPHIDs' => array($column_phid),
- ),
- 'old' => array(
- 'projectPHID' => $column->getProjectPHID(),
- 'columnPHIDs' => array(),
- ),
- );
- }
- }
- }
+ $changes = $this->setTaskChanges($request, $owner_phid, $user, $task);
if ($can_edit_policies) {
$changes[PhabricatorTransactions::TYPE_VIEW_POLICY] =
@@ -264,84 +164,17 @@
$request->getStr('editPolicy');
}
- $template = new ManiphestTransaction();
- $transactions = array();
-
- foreach ($changes as $type => $value) {
- $transaction = clone $template;
- $transaction->setTransactionType($type);
- if ($type == ManiphestTransaction::TYPE_PROJECT_COLUMN) {
- $transaction->setNewValue($value['new']);
- $transaction->setOldValue($value['old']);
- } else if ($type == ManiphestTransaction::TYPE_PROJECTS) {
- // TODO: Gross.
- $project_type =
- PhabricatorProjectObjectHasProjectEdgeType::EDGECONST;
- $transaction
- ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
- ->setMetadataValue('edge:type', $project_type)
- ->setNewValue(
- array(
- '=' => array_fuse($value),
- ));
- } else {
- $transaction->setNewValue($value);
- }
- $transactions[] = $transaction;
- }
+ $template = new ManiphestTransaction();
+ $transactions = $this->setTransactions($template, $changes);
if ($aux_fields) {
- foreach ($aux_fields as $aux_field) {
- $transaction = clone $template;
- $transaction->setTransactionType(
- PhabricatorTransactions::TYPE_CUSTOMFIELD);
- $aux_key = $aux_field->getFieldKey();
- $transaction->setMetadataValue('customfield:key', $aux_key);
- $old = idx($old_values, $aux_key);
- $new = $aux_field->getNewValueForApplicationTransactions();
-
- $transaction->setOldValue($old);
- $transaction->setNewValue($new);
-
- $transactions[] = $transaction;
- }
- }
+ $transactions = $this->setAuxFieldTransactions($aux_fields,
$template, $old_values);
+ }
if ($transactions) {
$is_new = !$task->getID();
-
- $event = new PhabricatorEvent(
- PhabricatorEventType::TYPE_MANIPHEST_WILLEDITTASK,
- array(
- 'task' => $task,
- 'new' => $is_new,
- 'transactions' => $transactions,
- ));
- $event->setUser($user);
- $event->setAphrontRequest($request);
- PhutilEventEngine::dispatchEvent($event);
-
- $task = $event->getValue('task');
- $transactions = $event->getValue('transactions');
-
- $editor = id(new ManiphestTransactionEditor())
- ->setActor($user)
- ->setContentSourceFromRequest($request)
- ->setContinueOnNoEffect(true)
- ->applyTransactions($task, $transactions);
-
- $event = new PhabricatorEvent(
- PhabricatorEventType::TYPE_MANIPHEST_DIDEDITTASK,
- array(
- 'task' => $task,
- 'new' => $is_new,
- 'transactions' => $transactions,
- ));
- $event->setUser($user);
- $event->setAphrontRequest($request);
- PhutilEventEngine::dispatchEvent($event);
+ $task = $this->createEvent($task, $is_new, $transactions, $user,
$request);
}
-
if ($parent_task) {
// TODO: This should be transactional now.
@@ -440,20 +273,10 @@
$user->getPHID(),
));
if ($template_id) {
- $template_task = id(new ManiphestTaskQuery())
- ->setViewer($user)
- ->withIDs(array($template_id))
- ->executeOne();
+ $template_task = $this->getTemplateTask($user, $template_id);
if ($template_task) {
- $cc_phids = array_unique(array_merge(
- $template_task->getCCPHIDs(),
- array($user->getPHID())));
- $task->setCCPHIDs($cc_phids);
- $task->attachProjectPHIDs($template_task->getProjectPHIDs());
- $task->setOwnerPHID($template_task->getOwnerPHID());
- $task->setPriority($template_task->getPriority());
- $task->setViewPolicy($template_task->getViewPolicy());
- $task->setEditPolicy($template_task->getEditPolicy());
+ $cc_phids = $this->setCCPHIDS($template_task, $user);
+ $task = $this->setTemplateTask($task, $template_task, $cc_phids);
$template_fields = PhabricatorCustomField::getObjectFields(
$template_task,
@@ -484,17 +307,7 @@
}
}
- $phids = array_merge(
- array($task->getOwnerPHID()),
- $task->getCCPHIDs(),
- $task->getProjectPHIDs());
-
- if ($parent_task) {
- $phids[] = $parent_task->getPHID();
- }
-
- $phids = array_filter($phids);
- $phids = array_unique($phids);
+ $phids = $this->getTaskPHIDS($task, $parent_task);
$handles = $this->loadViewerHandles($phids);
@@ -752,5 +565,273 @@
'pageObjects' => $page_objects,
));
}
+ private function setApplicationCapabilities() {
+ $can_edit_assign = $this->hasApplicationCapability(
+ ManiphestEditAssignCapability::CAPABILITY);
+ $can_edit_policies = $this->hasApplicationCapability(
+ ManiphestEditPoliciesCapability::CAPABILITY);
+ $can_edit_priority = $this->hasApplicationCapability(
+ ManiphestEditPriorityCapability::CAPABILITY);
+ $can_edit_projects = $this->hasApplicationCapability(
+ ManiphestEditProjectsCapability::CAPABILITY);
+ $can_edit_status = $this->hasApplicationCapability(
+ ManiphestEditStatusCapability::CAPABILITY);
+ return array($can_edit_assign,$can_edit_policies,$can_edit_priority,
+ $can_edit_projects,$can_edit_status);
+ }
+
+ private function setTokens($tokens, $type_project) {
+ foreach ($tokens as $key => $token) {
+ if (phid_get_type($token) == $type_project) {
+ // If this is formatted like a PHID, leave it as-is.
+ continue;
+ }
+
+ if (preg_match('/^#/', $token)) {
+ // If this already has a "#", leave it as-is.
+ continue;
+ }
+
+ // Add a "#" prefix.
+ $tokens[$key] = '#'.$token;
+ }
+ return $tokens;
+ }
+
+ private function getDefaultProjects($user, $tokens) {
+ $default_projects = id(new PhabricatorObjectQuery())
+ ->setViewer($user)
+ ->withNames($tokens)
+ ->execute();
+ $default_projects = mpull($default_projects, 'getPHID');
+ return $default_projects;
+ }
+
+ private function getTask($user) {
+ $task = id(new ManiphestTaskQuery())
+ ->setViewer($user)
+ ->requireCapabilities(
+ array(
+ PhabricatorPolicyCapability::CAN_VIEW,
+ PhabricatorPolicyCapability::CAN_EDIT,
+ ))
+ ->withIDs(array($this->id))
+ ->executeOne();
+ return $task;
+ }
+
+ private function getAuxFields($task, $user){
+ $field_list = PhabricatorCustomField::getObjectFields(
+ $task,
+ PhabricatorCustomField::ROLE_EDIT);
+ $field_list->setViewer($user);
+ $field_list->readFieldsFromStorage($task);
+ $aux_fields = $field_list->getFields();
+ return array($aux_fields, $field_list);
+ }
+
+ private function setTaskError($task, $new_title, $new_desc, $request,
$owner_phid) {
+ $task->setTitle($new_title)
+ ->setDescription($new_desc)
+ ->setPriority($request->getInt('priority'))
+ ->setOwnerPHID($owner_phid)
+ ->setCCPHIDs($request->getArr('cc'))
+ ->attachProjectPHIDs($request->getArr('projects'));
+ return $task;
+ }
+
+ private function setFieldErrors($aux_field, $placeholder_editor,
$aux_old_value, $aux_new_value, $errors) {
+ $field_errors = $aux_field->validateApplicationTransactions(
+ $placeholder_editor,
+ PhabricatorTransactions::TYPE_CUSTOMFIELD,
+ array(
+ id(new ManiphestTransaction())
+ ->setOldValue($aux_old_value)
+ ->setNewValue($aux_new_value),
+ ));
+
+ foreach ($field_errors as $error) {
+ $errors[] = $error->getMessage();
+ }
+ return $errors;
+ }
+
+ private function setTaskChanges($request, $owner_phid, $user, $task) {
+ list($can_edit_assign,$can_edit_policies,$can_edit_priority,
+ $can_edit_projects, $can_edit_status)
+ = $this->setApplicationCapabilities();
+
+ if ($can_edit_priority) {
+ $changes[ManiphestTransaction::TYPE_PRIORITY] =
+ $request->getInt('priority');
+ }
+ if ($can_edit_assign) {
+ $changes[ManiphestTransaction::TYPE_OWNER] = $owner_phid;
+ }
+
+ $changes[ManiphestTransaction::TYPE_CCS] = $request->getArr('cc');
+
+ if ($can_edit_projects) {
+ $projects = $request->getArr('projects');
+ $changes[ManiphestTransaction::TYPE_PROJECTS] =
+ $projects;
+ $column_phid = $request->getStr('columnPHID');
+ // allow for putting a task in a project column at creation -only-
+ if (!$task->getID() && $column_phid && $projects) {
+ $column = id(new PhabricatorProjectColumnQuery())
+ ->setViewer($user)
+ ->withProjectPHIDs($projects)
+ ->withPHIDs(array($column_phid))
+ ->executeOne();
+ if ($column) {
+ $changes[ManiphestTransaction::TYPE_PROJECT_COLUMN] =
+ array(
+ 'new' => array(
+ 'projectPHID' => $column->getProjectPHID(),
+ 'columnPHIDs' => array($column_phid),
+ ),
+ 'old' => array(
+ 'projectPHID' => $column->getProjectPHID(),
+ 'columnPHIDs' => array(),
+ ),
+ );
+ }
+ }
+ }
+ return $changes;
+ }
+
+ private function createEvent($task, $is_new, $transactions, $user, $request)
{
+ $event = new PhabricatorEvent(
+ PhabricatorEventType::TYPE_MANIPHEST_WILLEDITTASK,
+ array(
+ 'task' => $task,
+ 'new' => $is_new,
+ 'transactions' => $transactions,
+ ));
+ $event->setUser($user);
+ $event->setAphrontRequest($request);
+ PhutilEventEngine::dispatchEvent($event);
+
+ $task = $event->getValue('task');
+ $transactions = $event->getValue('transactions');
+ id(new ManiphestTransactionEditor())
+ ->setActor($user)
+ ->setContentSourceFromRequest($request)
+ ->setContinueOnNoEffect(true)
+ ->applyTransactions($task, $transactions);
+ $event = new PhabricatorEvent(
+ PhabricatorEventType::TYPE_MANIPHEST_DIDEDITTASK,
+ array(
+ 'task' => $task,
+ 'new' => $is_new,
+ 'transactions' => $transactions,
+ ));
+ $event->setUser($user);
+ $event->setAphrontRequest($request);
+ PhutilEventEngine::dispatchEvent($event);
+ return $task;
+ }
+
+ private function setTransactions($template, $changes) {
+ $transactions = array();
+
+ foreach ($changes as $type => $value) {
+ $transaction = clone $template;
+ $transaction->setTransactionType($type);
+ if ($type == ManiphestTransaction::TYPE_PROJECT_COLUMN) {
+ $transaction->setNewValue($value['new']);
+ $transaction->setOldValue($value['old']);
+ } else if ($type == ManiphestTransaction::TYPE_PROJECTS) {
+ // TODO: Gross.
+ $project_type =
+ PhabricatorProjectObjectHasProjectEdgeType::EDGECONST;
+ $transaction
+ ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
+ ->setMetadataValue('edge:type', $project_type)
+ ->setNewValue(
+ array(
+ '=' => array_fuse($value),
+ ));
+ } else {
+ $transaction->setNewValue($value);
+ }
+ $transactions[] = $transaction;
+ }
+ return array ($transactions);
+ }
+
+ private function setAuxFieldTransactions($aux_fields, $template,
$old_values) {
+ foreach ($aux_fields as $aux_field) {
+ $transaction = clone $template;
+ $transaction->setTransactionType(
+ PhabricatorTransactions::TYPE_CUSTOMFIELD);
+ $aux_key = $aux_field->getFieldKey();
+ $transaction->setMetadataValue('customfield:key', $aux_key);
+ $old = idx($old_values, $aux_key);
+ $new = $aux_field->getNewValueForApplicationTransactions();
+
+ $transaction->setOldValue($old);
+ $transaction->setNewValue($new);
+
+ $transactions[] = $transaction;
+ }
+ return $transactions;
+ }
+
+ private function assignUser($user, $assign) {
+ $assign_user = id(new PhabricatorPeopleQuery())
+ ->setViewer($user)
+ ->withUsernames(array($assign))
+ ->executeOne();
+ if (!$assign_user) {
+ $assign_user = id(new PhabricatorPeopleQuery())
+ ->setViewer($user)
+ ->withPHIDs(array($assign))
+ ->executeOne();
+ }
+ return $assign_user;
+ }
+
+ private function getTemplateTask($user, $template_id) {
+ $template_task = id(new ManiphestTaskQuery())
+ ->setViewer($user)
+ ->withIDs(array($template_id))
+ ->executeOne();
+ return $template_task;
+ }
+
+ private function setCCPHIDS($template_task, $user) {
+ $cc_phids = array_unique(array_merge(
+ $template_task->getCCPHIDs(),
+ array($user->getPHID())));
+ return $cc_phids;
+ }
+
+ private function setTemplateTask($task, $template_task, $cc_phids) {
+ $task->setCCPHIDs($cc_phids);
+ $task->attachProjectPHIDs($template_task->getProjectPHIDs());
+ $task->setOwnerPHID($template_task->getOwnerPHID());
+ $task->setPriority($template_task->getPriority());
+ $task->setViewPolicy($template_task->getViewPolicy());
+ $task->setEditPolicy($template_task->getEditPolicy());
+ return $task;
+ }
+
+ private function getTaskPHIDS($task, $parent_task) {
+ $phids = array_merge(
+ array($task->getOwnerPHID()),
+ $task->getCCPHIDs(),
+ $task->getProjectPHIDs());
+
+ if ($parent_task) {
+ $phids[] = $parent_task->getPHID();
+ }
+
+ $phids = array_filter($phids);
+ $phids = array_unique($phids);
+
+ return $phids;
+ }
}
diff --git a/src/controller/SprintBoardViewController.php
b/src/controller/SprintBoardViewController.php
index 69108d5..362ca2d 100644
--- a/src/controller/SprintBoardViewController.php
+++ b/src/controller/SprintBoardViewController.php
@@ -87,10 +87,8 @@
return id(new AphrontRedirectResponse())
->setURI(
$this->getApplicationURI('board/'.$project->getID().'/import/'));
- break;
default:
return $this->initializeWorkboardDialog($project);
- break;
}
}
@@ -350,8 +348,11 @@
$base_uri = $this->getURIWithState();
+
$items = array();
foreach ($named as $key => $name) {
+ $active_order = null;
+
$is_selected = ($key == $sort_key);
if ($is_selected) {
$active_order = $name;
@@ -411,6 +412,8 @@
$items = array();
foreach ($named as $key => $name) {
+ $active_filter = null;
+
$is_selected = ($key == $query_key);
if ($is_selected) {
$active_filter = $name;
diff --git a/src/controller/SprintController.php
b/src/controller/SprintController.php
index b8f0a46..709a264 100644
--- a/src/controller/SprintController.php
+++ b/src/controller/SprintController.php
@@ -15,11 +15,11 @@
}
public function getUser() {
- return $user = $this->getRequest()->getUser();
+ return $this->getRequest()->getUser();
}
public function setApplicationURI() {
- return $uri= new PhutilURI($this->getApplicationURI());
+ return new PhutilURI($this->getApplicationURI());
}
public function buildApplicationMenu() {
diff --git a/src/query/SprintQuery.php b/src/query/SprintQuery.php
index 7e93e4d..709d1f5 100644
--- a/src/query/SprintQuery.php
+++ b/src/query/SprintQuery.php
@@ -68,12 +68,12 @@
}
public function checkNull($start, $end, $tasks) {
+ $mword = SprintConstants::MAGIC_WORD;
if (!$start OR !$end) {
- throw new BurndownException("This project is not set up for Burndowns, "
- . "make sure it has 'Sprint' in the name, and then edit it to add
the "
- . "sprint start and end date.");
+ throw new BurndownException("This project is not set up for Sprints. "
+ . "Check that it has a start date and end date. And has an "
+ . "'{$mword}' in the name.");
}
-
if (!$tasks) {
throw new BurndownException("This project has no tasks.");
}
diff --git a/src/view/BurndownDataView.php b/src/view/BurndownDataView.php
index d39e5da..7a44321 100644
--- a/src/view/BurndownDataView.php
+++ b/src/view/BurndownDataView.php
@@ -53,12 +53,13 @@
$start = $query->getStartDate($aux_fields);
$end = $query->getEndDate($aux_fields);
$stats = id(new SprintBuildStats());
+ $tasks = $query->getTasks();
+ $query->checkNull($start, $end, $tasks);
+
$timezone = $stats->setTimezone($this->viewer);
$dates = $stats->buildDateArray($start, $end, $timezone);
$this->timeseries = $stats->buildTimeSeries($start, $end);
- $tasks = $query->getTasks();
- $query->checkNull($start, $end, $tasks);
$xactions = $query->getXactions($tasks);
$events = $query->getEvents($xactions, $tasks);
diff --git a/src/view/TasksTableView.php b/src/view/TasksTableView.php
index 961da90..aca9aad 100644
--- a/src/view/TasksTableView.php
+++ b/src/view/TasksTableView.php
@@ -170,6 +170,18 @@
return $map;
}
+ private function setOwnerLink($handles, $task) {
+ // Get the owner object so we can render the owner username/link
+ $owner = $handles[$task->getOwnerPHID()];
+
+ if ($owner instanceof PhabricatorObjectHandle) {
+ $owner_link = $task->getOwnerPHID() ? $owner->renderLink() : 'none
assigned';
+ } else {
+ $owner_link = 'none assigned';
+ }
+ return $owner_link;
+ }
+
private function getTaskPoints($task) {
$query = id(new SprintQuery())
->setProject($this->project)
@@ -180,6 +192,11 @@
return $points;
}
+ private function getPriorityName($task) {
+ $priority_name = new ManiphestTaskPriority;
+ return $priority_name->getTaskPriorityName($task->getPriority());
+ }
+
private function addTaskToTree($output, $task, $tasks, $map, $handles,
$depth = 0) {
static $included = array();
@@ -187,21 +204,15 @@
$repeat = isset($included[$task->getPHID()]);
$points = $this->getTaskPoints($task);
- $priority_name = new ManiphestTaskPriority();
+
$status = $this->setTaskStatus($task);
$depth_indent = '';
for ($i = 0; $i < $depth; $i++) {
$depth_indent .= ' ';
}
- // Get the owner object so we can render the owner username/link
- $owner = $handles[$task->getOwnerPHID()];
-
- if ($owner instanceof PhabricatorObjectHandle) {
- $owner_link = $task->getOwnerPHID() ? $owner->renderLink() : 'none
assigned';
- } else {
- $owner_link = 'none assigned';
- }
+ $owner_link = $this->setOwnerLink($handles, $task);
+ $priority_name = $this->getPriorityName($task);
// Build the row
$output[] = array(
@@ -217,7 +228,7 @@
) . ($repeat ? ' <em title="This task is a child of
more than one task in this list. Children are only shown on ' .
'the first occurance">[Repeat]</em>' : '')),
$owner_link,
- $priority_name->getTaskPriorityName($task->getPriority()),
+ $priority_name,
$points,
$status,
);
--
To view, visit https://gerrit.wikimedia.org/r/176859
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: If4d2b6ed9cf5b8c4ecdd8c2a3365c4da553a4f9c
Gerrit-PatchSet: 3
Gerrit-Project: phabricator/extensions/Sprint
Gerrit-Branch: master
Gerrit-Owner: Christopher Johnson (WMDE) <[email protected]>
Gerrit-Reviewer: Christopher Johnson (WMDE) <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
