diff --git a/src/applications/maniphest/command/ManiphestPriorityEmailCommand.php b/src/applications/maniphest/command/ManiphestPriorityEmailCommand.php index 85ed148db7..f774578240 100644 --- a/src/applications/maniphest/command/ManiphestPriorityEmailCommand.php +++ b/src/applications/maniphest/command/ManiphestPriorityEmailCommand.php @@ -23,6 +23,9 @@ final class ManiphestPriorityEmailCommand $table[] = '| '.pht('Priority').' | '.pht('Keywords'); $table[] = '|---|---|'; foreach ($keywords as $priority => $words) { + if (ManiphestTaskPriority::isDisabledPriority($priority)) { + continue; + } $words = implode(', ', $words); $table[] = '| '.$names[$priority].' | '.$words; } @@ -63,6 +66,10 @@ final class ManiphestPriorityEmailCommand return array(); } + if (ManiphestTaskPriority::isDisabledPriority($priority)) { + return array(); + } + $xactions[] = $object->getApplicationTransactionTemplate() ->setTransactionType(ManiphestTransaction::TYPE_PRIORITY) ->setNewValue($priority); diff --git a/src/applications/maniphest/command/ManiphestStatusEmailCommand.php b/src/applications/maniphest/command/ManiphestStatusEmailCommand.php index c9fa07494f..aa74e33bec 100644 --- a/src/applications/maniphest/command/ManiphestStatusEmailCommand.php +++ b/src/applications/maniphest/command/ManiphestStatusEmailCommand.php @@ -23,6 +23,10 @@ final class ManiphestStatusEmailCommand $table[] = '| '.pht('Status').' | '.pht('Keywords'); $table[] = '|---|---|'; foreach ($keywords as $status => $words) { + if (ManiphestTaskStatus::isDisabledStatus($status)) { + continue; + } + $words = implode(', ', $words); $table[] = '| '.$names[$status].' | '.$words; } @@ -62,6 +66,10 @@ final class ManiphestStatusEmailCommand return array(); } + if (ManiphestTaskStatus::isDisabledStatus($status)) { + return array(); + } + $xactions[] = $object->getApplicationTransactionTemplate() ->setTransactionType(ManiphestTransaction::TYPE_STATUS) ->setNewValue($status); diff --git a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php index 5a3c05bb22..e4424fef9b 100644 --- a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php +++ b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php @@ -198,6 +198,10 @@ The keys you can provide in a specification are: - `keywords` //Optional list.// Allows you to specify a list of keywords which can be used with `!status` commands in email to select this status. + - `disabled` //Optional bool.// Marks this status as no longer in use so + tasks can not be created or edited to have this status. Existing tasks with + this status will not be affected, but you can batch edit them or let them + die out on their own. Statuses will appear in the UI in the order specified. Note the status marked `special` as `duplicate` is not settable directly and will not appear in UI @@ -280,7 +284,11 @@ EOTEXT ' - `color` A color for this priority, like "red" or "blue".'. ' - `keywords` An optional list of keywords which can '. ' be used to select this priority when using `!priority` '. - ' commands in email.'. + ' commands in email.'."\n". + ' - `disabled` Optional boolean to prevent users from choosing '. + ' this priority when creating or editing tasks. Existing '. + ' tasks will be unaffected, and can be batch edited to a '. + ' different priority or left to eventually die out.'. "\n\n". 'You can choose which priority is the default for newly created '. 'tasks with `%s`.', diff --git a/src/applications/maniphest/constants/ManiphestTaskPriority.php b/src/applications/maniphest/constants/ManiphestTaskPriority.php index 38a56f767b..55ebafe597 100644 --- a/src/applications/maniphest/constants/ManiphestTaskPriority.php +++ b/src/applications/maniphest/constants/ManiphestTaskPriority.php @@ -105,6 +105,11 @@ final class ManiphestTaskPriority extends ManiphestConstants { return 'fa-arrow-right'; } + public static function isDisabledPriority($priority) { + $config = idx(self::getConfig(), $priority, array()); + return idx($config, 'disabled', false); + } + private static function getConfig() { $config = PhabricatorEnv::getEnvConfig('maniphest.priorities'); krsort($config); diff --git a/src/applications/maniphest/constants/ManiphestTaskStatus.php b/src/applications/maniphest/constants/ManiphestTaskStatus.php index 5904b47595..5efc2ea56c 100644 --- a/src/applications/maniphest/constants/ManiphestTaskStatus.php +++ b/src/applications/maniphest/constants/ManiphestTaskStatus.php @@ -167,6 +167,10 @@ final class ManiphestTaskStatus extends ManiphestConstants { return self::getStatusAttribute($status, 'transaction.color'); } + public static function isDisabledStatus($status) { + return self::getStatusAttribute($status, 'disabled'); + } + public static function getStatusIcon($status) { $icon = self::getStatusAttribute($status, 'transaction.icon'); if ($icon) { @@ -274,6 +278,7 @@ final class ManiphestTaskStatus extends ManiphestConstants { 'prefixes' => 'optional list', 'suffixes' => 'optional list', 'keywords' => 'optional list', + 'disabled' => 'optional bool', )); } diff --git a/src/applications/maniphest/editor/ManiphestEditEngine.php b/src/applications/maniphest/editor/ManiphestEditEngine.php index 3dfb740009..235b8b3a4d 100644 --- a/src/applications/maniphest/editor/ManiphestEditEngine.php +++ b/src/applications/maniphest/editor/ManiphestEditEngine.php @@ -51,14 +51,8 @@ final class ManiphestEditEngine } protected function buildCustomEditFields($object) { - // See T4819. - $status_map = ManiphestTaskStatus::getTaskStatusMap(); - $dup_status = ManiphestTaskStatus::getDuplicateStatus(); - if ($object->getStatus() != $dup_status) { - unset($status_map[$dup_status]); - } - - $priority_map = ManiphestTaskPriority::getTaskPriorityMap(); + $status_map = $this->getTaskStatusMap($object); + $priority_map = $this->getTaskPriorityMap($object); if ($object->isClosed()) { $priority_label = null; @@ -124,4 +118,67 @@ final class ManiphestEditEngine return $this->getApplication()->getApplicationURI('editpro/'); } + private function getTaskStatusMap(ManiphestTask $task) { + $status_map = ManiphestTaskStatus::getTaskStatusMap(); + + $current_status = $task->getStatus(); + + // If the current status is something we don't recognize (maybe an older + // status which was deleted), put a dummy entry in the status map so that + // saving the form doesn't destroy any data by accident. + if (idx($status_map, $current_status) === null) { + $status_map[$current_status] = pht('', $current_status); + } + + $dup_status = ManiphestTaskStatus::getDuplicateStatus(); + foreach ($status_map as $status => $status_name) { + // Always keep the task's current status. + if ($status == $current_status) { + continue; + } + + // Don't allow tasks to be changed directly into "Closed, Duplicate" + // status. Instead, you have to merge them. See T4819. + if ($status == $dup_status) { + unset($status_map[$status]); + continue; + } + + // Don't let new or existing tasks be moved into a disabled status. + if (ManiphestTaskStatus::isDisabledStatus($status)) { + unset($status_map[$status]); + continue; + } + } + + return $status_map; + } + + private function getTaskPriorityMap(ManiphestTask $task) { + $priority_map = ManiphestTaskPriority::getTaskPriorityMap(); + $current_priority = $task->getPriority(); + + // If the current value isn't a legitimate one, put it in the dropdown + // anyway so saving the form doesn't cause a side effects. + if (idx($priority_map, $current_priority) === null) { + $priority_map[$current_priority] = pht( + '', + $current_priority); + } + + foreach ($priority_map as $priority => $priority_name) { + // Always keep the current priority. + if ($priority == $current_priority) { + continue; + } + + if (ManiphestTaskPriority::isDisabledPriority($priority)) { + unset($priority_map[$priority]); + continue; + } + } + + return $priority_map; + } + } diff --git a/src/applications/maniphest/typeahead/ManiphestTaskPriorityDatasource.php b/src/applications/maniphest/typeahead/ManiphestTaskPriorityDatasource.php index 4e7dd639d6..217267b8cb 100644 --- a/src/applications/maniphest/typeahead/ManiphestTaskPriorityDatasource.php +++ b/src/applications/maniphest/typeahead/ManiphestTaskPriorityDatasource.php @@ -29,10 +29,16 @@ final class ManiphestTaskPriorityDatasource $priority_map = ManiphestTaskPriority::getTaskPriorityMap(); foreach ($priority_map as $value => $name) { - $results[$value] = id(new PhabricatorTypeaheadResult()) + $result = id(new PhabricatorTypeaheadResult()) ->setIcon(ManiphestTaskPriority::getTaskPriorityIcon($value)) ->setPHID($value) ->setName($name); + + if (ManiphestTaskPriority::isDisabledPriority($value)) { + $result->setClosed(pht('Disabled')); + } + + $results[$value] = $result; } return $results; diff --git a/src/applications/maniphest/typeahead/ManiphestTaskStatusDatasource.php b/src/applications/maniphest/typeahead/ManiphestTaskStatusDatasource.php index 8f7e881a9f..fe59c974e6 100644 --- a/src/applications/maniphest/typeahead/ManiphestTaskStatusDatasource.php +++ b/src/applications/maniphest/typeahead/ManiphestTaskStatusDatasource.php @@ -30,10 +30,16 @@ final class ManiphestTaskStatusDatasource $status_map = ManiphestTaskStatus::getTaskStatusMap(); foreach ($status_map as $value => $name) { - $results[$value] = id(new PhabricatorTypeaheadResult()) + $result = id(new PhabricatorTypeaheadResult()) ->setIcon(ManiphestTaskStatus::getStatusIcon($value)) ->setPHID($value) ->setName($name); + + if (ManiphestTaskStatus::isDisabledStatus($value)) { + $result->setClosed(pht('Disabled')); + } + + $results[$value] = $result; } return $results;