From 092f540199b7570a6c931407c85ab20d8a2b8218 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 13 Sep 2013 10:24:46 -0700 Subject: [PATCH] Remove all hardcoded behaviors associated with task priorities Summary: Ref T3583. Currently, we have some hard-coded behaviors associated with the "Unbreak Now" and "Needs Triage" priorities. Remove them: - Users seem somewhat confused by these on occasion, and never seem to think they're cool/useful (that I've seen, at least). - I think they have low utility in general, see T3583. - Saves three queries on the home page, which can no longer use row counting since they must be policy filtered. - Primarily, this paves the way for allowing installs to customize priorities, which is an occasional request. Also deletes a lot of code with no callsites. Test Plan: Mostly `grep`. Loaded home page. Viewed reports and task list. Reviewers: btrahan Reviewed By: btrahan CC: chad, aran Maniphest Tasks: T3583 Differential Revision: https://secure.phabricator.com/D6981 --- src/__celerity_resource_map__.php | 12 +-- .../PhabricatorDirectoryMainController.php | 88 ------------------- .../PhabricatorApplicationManiphest.php | 24 +---- .../constants/ManiphestTaskPriority.php | 36 +------- .../mail/ManiphestCreateMailReceiver.php | 2 +- .../view/ManiphestTransactionDetailView.php | 5 +- .../maniphest/transaction-detail.css | 4 - 7 files changed, 12 insertions(+), 159 deletions(-) diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index cb9b7f4b92..64d5095731 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -2926,7 +2926,7 @@ celerity_register_resource_map(array( ), 'maniphest-transaction-detail-css' => array( - 'uri' => '/res/30242771/rsrc/css/application/maniphest/transaction-detail.css', + 'uri' => '/res/f2c9582c/rsrc/css/application/maniphest/transaction-detail.css', 'type' => 'css', 'requires' => array( @@ -4380,7 +4380,7 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/f32597c9/javelin.pkg.js', 'type' => 'js', ), - '0a9e494f' => + 'ad2ed1da' => array( 'name' => 'maniphest.pkg.css', 'symbols' => @@ -4389,7 +4389,7 @@ celerity_register_resource_map(array( 1 => 'maniphest-transaction-detail-css', 2 => 'phabricator-project-tag-css', ), - 'uri' => '/res/pkg/0a9e494f/maniphest.pkg.css', + 'uri' => '/res/pkg/ad2ed1da/maniphest.pkg.css', 'type' => 'css', ), '83a3853e' => @@ -4505,8 +4505,8 @@ celerity_register_resource_map(array( 'javelin-vector' => 'f32597c9', 'javelin-workflow' => 'f32597c9', 'lightbox-attachment-css' => '1200d176', - 'maniphest-task-summary-css' => '0a9e494f', - 'maniphest-transaction-detail-css' => '0a9e494f', + 'maniphest-task-summary-css' => 'ad2ed1da', + 'maniphest-transaction-detail-css' => 'ad2ed1da', 'phabricator-action-list-view-css' => '1200d176', 'phabricator-application-launch-view-css' => '1200d176', 'phabricator-busy' => '8977e356', @@ -4532,7 +4532,7 @@ celerity_register_resource_map(array( 'phabricator-object-selector-css' => '44bfe40c', 'phabricator-phtize' => '8977e356', 'phabricator-prefab' => '8977e356', - 'phabricator-project-tag-css' => '0a9e494f', + 'phabricator-project-tag-css' => 'ad2ed1da', 'phabricator-property-list-view-css' => '1200d176', 'phabricator-remarkup-css' => '1200d176', 'phabricator-shaped-request' => '5e9e5c4e', diff --git a/src/applications/directory/controller/PhabricatorDirectoryMainController.php b/src/applications/directory/controller/PhabricatorDirectoryMainController.php index b2505ccbbb..c93c9c3b55 100644 --- a/src/applications/directory/controller/PhabricatorDirectoryMainController.php +++ b/src/applications/directory/controller/PhabricatorDirectoryMainController.php @@ -32,12 +32,8 @@ final class PhabricatorDirectoryMainController $maniphest = 'PhabricatorApplicationManiphest'; if (PhabricatorApplication::isClassInstalled($maniphest)) { - $unbreak_panel = $this->buildUnbreakNowPanel(); - $triage_panel = $this->buildNeedsTriagePanel($projects); $tasks_panel = $this->buildTasksPanel(); } else { - $unbreak_panel = null; - $triage_panel = null; $tasks_panel = null; } @@ -54,8 +50,6 @@ final class PhabricatorDirectoryMainController $content = array( $jump_panel, $welcome_panel, - $unbreak_panel, - $triage_panel, $revision_panel, $tasks_panel, $audit_panel, @@ -96,88 +90,6 @@ final class PhabricatorDirectoryMainController } } - private function buildUnbreakNowPanel() { - $user = $this->getRequest()->getUser(); - - $task_query = id(new ManiphestTaskQuery()) - ->setViewer($user) - ->withStatus(ManiphestTaskQuery::STATUS_OPEN) - ->withPriority(ManiphestTaskPriority::PRIORITY_UNBREAK_NOW) - ->setLimit(10); - - $tasks = $task_query->execute(); - - if (!$tasks) { - return $this->renderMiniPanel( - 'No "Unbreak Now!" Tasks', - 'Nothing appears to be critically broken right now.'); - } - - $panel = new AphrontPanelView(); - $panel->setHeader('Unbreak Now!'); - $panel->setCaption('Open tasks with "Unbreak Now!" priority.'); - $panel->addButton( - phutil_tag( - 'a', - array( - 'href' => '/maniphest/view/all/', - 'class' => 'grey button', - ), - "View All Unbreak Now \xC2\xBB")); - - $panel->appendChild($this->buildTaskListView($tasks)); - $panel->setNoBackground(); - - return $panel; - } - - private function buildNeedsTriagePanel(array $projects) { - assert_instances_of($projects, 'PhabricatorProject'); - - $user = $this->getRequest()->getUser(); - - if ($projects) { - $task_query = id(new ManiphestTaskQuery()) - ->setViewer($user) - ->withStatus(ManiphestTaskQuery::STATUS_OPEN) - ->withPriority(ManiphestTaskPriority::PRIORITY_TRIAGE) - ->withAnyProjects(mpull($projects, 'getPHID')) - ->setLimit(10); - $tasks = $task_query->execute(); - } else { - $tasks = array(); - } - - if (!$tasks) { - return $this->renderMiniPanel( - 'No "Needs Triage" Tasks', - hsprintf( - 'No tasks in projects you are a member of '. - 'need triage.')); - } - - $panel = new AphrontPanelView(); - $panel->setHeader('Needs Triage'); - $panel->setCaption(hsprintf( - 'Open tasks with "Needs Triage" priority in '. - 'projects you are a member of.')); - - $panel->addButton( - phutil_tag( - 'a', - array( - // TODO: This should filter to just your projects' need-triage - // tasks? - 'href' => '/maniphest/view/projecttriage/', - 'class' => 'grey button', - ), - "View All Triage \xC2\xBB")); - $panel->appendChild($this->buildTaskListView($tasks)); - $panel->setNoBackground(); - - return $panel; - } - private function buildRevisionPanel() { $user = $this->getRequest()->getUser(); $user_phid = $user->getPHID(); diff --git a/src/applications/maniphest/application/PhabricatorApplicationManiphest.php b/src/applications/maniphest/application/PhabricatorApplicationManiphest.php index b0e87665c8..556f681a20 100644 --- a/src/applications/maniphest/application/PhabricatorApplicationManiphest.php +++ b/src/applications/maniphest/application/PhabricatorApplicationManiphest.php @@ -85,31 +85,13 @@ final class PhabricatorApplicationManiphest extends PhabricatorApplication { $query = id(new ManiphestTaskQuery()) ->setViewer($user) ->withStatus(ManiphestTaskQuery::STATUS_OPEN) - ->withPriority(ManiphestTaskPriority::PRIORITY_UNBREAK_NOW) - ->setLimit(1) - ->setCalculateRows(true); - $query->execute(); + ->withOwners(array($user->getPHID())); + $count = count($query->execute()); - $count = $query->getRowCount(); - $type = PhabricatorApplicationStatusView::TYPE_NEEDS_ATTENTION; - $status[] = id(new PhabricatorApplicationStatusView()) - ->setType($type) - ->setText(pht('%d Unbreak Now Task(s)!', $count)) - ->setCount($count); - - $query = id(new ManiphestTaskQuery()) - ->setViewer($user) - ->withStatus(ManiphestTaskQuery::STATUS_OPEN) - ->withOwners(array($user->getPHID())) - ->setLimit(1) - ->setCalculateRows(true); - $query->execute(); - - $count = $query->getRowCount(); $type = PhabricatorApplicationStatusView::TYPE_WARNING; $status[] = id(new PhabricatorApplicationStatusView()) ->setType($type) - ->setText(pht('%d Assigned Task(s)', $count)) + ->setText(pht('%s Assigned Task(s)', new PhutilNumber($count))) ->setCount($count); return $status; diff --git a/src/applications/maniphest/constants/ManiphestTaskPriority.php b/src/applications/maniphest/constants/ManiphestTaskPriority.php index 846805cffc..aa397a247f 100644 --- a/src/applications/maniphest/constants/ManiphestTaskPriority.php +++ b/src/applications/maniphest/constants/ManiphestTaskPriority.php @@ -44,47 +44,13 @@ final class ManiphestTaskPriority extends ManiphestConstants { ); } - /** - * Get the priorities and some bits for bitwise fun. - * - * @return map Priorities to bits. - */ - public static function getLoadMap() { - return array( - self::PRIORITY_UNBREAK_NOW => 16, - self::PRIORITY_TRIAGE => 8, - self::PRIORITY_HIGH => 4, - self::PRIORITY_NORMAL => 2, - self::PRIORITY_LOW => 1, - self::PRIORITY_WISH => 0, - ); - } - - /** - * Get the lowest defined priority. - * - * @return int The value of the lowest priority constant. - */ - public static function getLowestPriority() { - return self::PRIORITY_WISH; - } - - /** - * Get the highest defined priority. - * - * @return int The value of the highest priority constant. - */ - public static function getHighestPriority() { - return self::PRIORITY_UNBREAK_NOW; - } /** * Return the default priority for this instance of Phabricator. * * @return int The value of the default priority constant. */ public static function getDefaultPriority() { - return PhabricatorEnv::getEnvConfig( - 'maniphest.default-priority'); + return PhabricatorEnv::getEnvConfig('maniphest.default-priority'); } /** diff --git a/src/applications/maniphest/mail/ManiphestCreateMailReceiver.php b/src/applications/maniphest/mail/ManiphestCreateMailReceiver.php index 1e31ccbf17..c295ef7760 100644 --- a/src/applications/maniphest/mail/ManiphestCreateMailReceiver.php +++ b/src/applications/maniphest/mail/ManiphestCreateMailReceiver.php @@ -64,7 +64,7 @@ final class ManiphestCreateMailReceiver extends PhabricatorMailReceiver { $task->setAuthorPHID($sender->getPHID()); $task->setOriginalEmailSource($mail->getHeader('From')); - $task->setPriority(ManiphestTaskPriority::PRIORITY_TRIAGE); + $task->setPriority(ManiphestTaskPriority::getDefaultPriority()); $editor = new ManiphestTransactionEditor(); $editor->setActor($sender); diff --git a/src/applications/maniphest/view/ManiphestTransactionDetailView.php b/src/applications/maniphest/view/ManiphestTransactionDetailView.php index 24eaee9795..7ecd644a1e 100644 --- a/src/applications/maniphest/view/ManiphestTransactionDetailView.php +++ b/src/applications/maniphest/view/ManiphestTransactionDetailView.php @@ -455,7 +455,7 @@ final class ManiphestTransactionDetailView extends ManiphestView { $old_name = ManiphestTaskPriority::getTaskPriorityName($old); $new_name = ManiphestTaskPriority::getTaskPriorityName($new); - if ($old == ManiphestTaskPriority::PRIORITY_TRIAGE) { + if ($old == ManiphestTaskPriority::getDefaultPriority()) { $verb = 'Triaged'; $desc = 'triaged this task as "'.$new_name.'" priority'; } else if ($old > $new) { @@ -467,9 +467,6 @@ final class ManiphestTransactionDetailView extends ManiphestView { $desc = 'raised the priority of this task from "'.$old_name.'" to '. '"'.$new_name.'"'; } - if ($new == ManiphestTaskPriority::PRIORITY_UNBREAK_NOW) { - $classes[] = 'unbreaknow'; - } break; case ManiphestTransactionType::TYPE_ATTACH: if ($this->preview) { diff --git a/webroot/rsrc/css/application/maniphest/transaction-detail.css b/webroot/rsrc/css/application/maniphest/transaction-detail.css index 362366841c..43c3eb13e8 100644 --- a/webroot/rsrc/css/application/maniphest/transaction-detail.css +++ b/webroot/rsrc/css/application/maniphest/transaction-detail.css @@ -47,10 +47,6 @@ border-color: #660099; } -.phabricator-transaction-view .unbreaknow { - border-color: #aa0000; -} - .phabricator-transaction-view .duplicate { border-color: #333333; }