Page MenuHomePhabricator
Files F14476059

D8579.diff
No OneTemporary
Actions

D8579.diff
View Options

diff --git a/src/applications/maniphest/constants/ManiphestTaskStatus.php b/src/applications/maniphest/constants/ManiphestTaskStatus.php
--- a/src/applications/maniphest/constants/ManiphestTaskStatus.php
+++ b/src/applications/maniphest/constants/ManiphestTaskStatus.php
@@ -1,8 +1,5 @@
<?php
-/**
- * @group maniphest
- */
final class ManiphestTaskStatus extends ManiphestConstants {
const STATUS_OPEN = 0;
@@ -23,7 +20,7 @@
$duplicate = pht('Duplicate');
$spite = pht('Spite');
- return array(
+ $statuses = array(
self::STATUS_OPEN => $open,
self::STATUS_CLOSED_RESOLVED => $resolved,
self::STATUS_CLOSED_WONTFIX => $wontfix,
@@ -31,6 +28,17 @@
self::STATUS_CLOSED_DUPLICATE => $duplicate,
self::STATUS_CLOSED_SPITE => $spite,
);
+
+ $is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business');
+ if (!$is_serious) {
+ $statuses[self::STATUS_CLOSED_SPITE] = pht('Spite');
+ }
+
+ return $statuses;
+ }
+
+ public static function getTaskStatusName($status) {
+ return idx(self::getTaskStatusMap(), $status, pht('Unknown Status'));
}
public static function getTaskStatusFullName($status) {
@@ -103,12 +111,26 @@
return self::STATUS_OPEN;
}
+ public static function getDefaultClosedStatus() {
+ return self::STATUS_CLOSED_RESOLVED;
+ }
+
+ public static function getDuplicateStatus() {
+ return self::STATUS_CLOSED_DUPLICATE;
+ }
+
public static function getOpenStatusConstants() {
return array(
self::STATUS_OPEN,
);
}
+ public static function getClosedStatusConstants() {
+ $all = array_keys(self::getTaskStatusMap());
+ $open = self::getOpenStatusConstants();
+ return array_diff($all, $open);
+ }
+
public static function isOpenStatus($status) {
foreach (self::getOpenStatusConstants() as $constant) {
if ($status == $constant) {
@@ -118,6 +140,35 @@
return false;
}
+ public static function isClosedStatus($status) {
+ return !self::isOpenStatus($status);
+ }
+
+ public static function getStatusActionName($status) {
+ switch ($status) {
+ case self::STATUS_CLOSED_SPITE:
+ return pht('Spited');
+ }
+ return null;
+ }
+
+ public static function getStatusColor($status) {
+ if (self::isOpenStatus($status)) {
+ return 'green';
+ }
+ return 'black';
+ }
+
+ public static function getStatusIcon($status) {
+ switch ($status) {
+ case ManiphestTaskStatus::STATUS_CLOSED_SPITE:
+ return 'dislike';
+ case ManiphestTaskStatus::STATUS_CLOSED_DUPLICATE:
+ return 'delete';
+ }
+ }
+
+
public static function getStatusPrefixMap() {
return array(
'resolve' => self::STATUS_CLOSED_RESOLVED,
diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php
--- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php
+++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php
@@ -151,7 +151,7 @@
$transaction_types = array(
PhabricatorTransactions::TYPE_COMMENT => pht('Comment'),
- ManiphestTransaction::TYPE_STATUS => pht('Close Task'),
+ ManiphestTransaction::TYPE_STATUS => pht('Change Status'),
ManiphestTransaction::TYPE_OWNER => pht('Reassign / Claim'),
ManiphestTransaction::TYPE_CCS => pht('Add CCs'),
ManiphestTransaction::TYPE_PRIORITY => pht('Change Priority'),
@@ -180,21 +180,14 @@
}
}
- if ($task->getStatus() == ManiphestTaskStatus::STATUS_OPEN) {
- $resolution_types = array_select_keys(
- $resolution_types,
- array(
- ManiphestTaskStatus::STATUS_CLOSED_RESOLVED,
- ManiphestTaskStatus::STATUS_CLOSED_WONTFIX,
- ManiphestTaskStatus::STATUS_CLOSED_INVALID,
- ManiphestTaskStatus::STATUS_CLOSED_SPITE,
- ));
- } else {
- $resolution_types = array(
- ManiphestTaskStatus::STATUS_OPEN => 'Reopened',
- );
- $transaction_types[ManiphestTransaction::TYPE_STATUS] =
- 'Reopen Task';
+ // Don't show an option to change to the current status, or to change to
+ // the duplicate status explicitly.
+ unset($resolution_types[$task->getStatus()]);
+ unset($resolution_types[ManiphestTaskStatus::getDuplicateStatus()]);
+
+ // Don't show owner/priority changes for closed tasks, as they don't make
+ // much sense.
+ if ($task->isClosed()) {
unset($transaction_types[ManiphestTransaction::TYPE_PRIORITY]);
unset($transaction_types[ManiphestTransaction::TYPE_OWNER]);
}
@@ -215,12 +208,6 @@
$is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business');
- if ($is_serious) {
- // Prevent tasks from being closed "out of spite" in serious business
- // installs.
- unset($resolution_types[ManiphestTaskStatus::STATUS_CLOSED_SPITE]);
- }
-
$comment_form = new AphrontFormView();
$comment_form
->setUser($user)
@@ -236,7 +223,7 @@
->setID('transaction-action'))
->appendChild(
id(new AphrontFormSelectControl())
- ->setLabel(pht('Resolution'))
+ ->setLabel(pht('Status'))
->setName('resolution')
->setControlID('resolution')
->setControlStyle('display: none')
diff --git a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php
--- a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php
+++ b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php
@@ -147,9 +147,9 @@
$added_ccs[] = $task->getOwnerPHID();
break;
case ManiphestTransaction::TYPE_STATUS:
+ $resolution = $request->getStr('resolution');
if (!$task->getOwnerPHID() &&
- $request->getStr('resolution') !=
- ManiphestTaskStatus::STATUS_OPEN) {
+ ManiphestTaskStatus::isClosedStatus($resolution)) {
// Closing an unassigned task. Assign the user as the owner of
// this task.
$assign = new ManiphestTransaction();
diff --git a/src/applications/maniphest/mail/ManiphestReplyHandler.php b/src/applications/maniphest/mail/ManiphestReplyHandler.php
--- a/src/applications/maniphest/mail/ManiphestReplyHandler.php
+++ b/src/applications/maniphest/mail/ManiphestReplyHandler.php
@@ -82,7 +82,7 @@
switch ($command) {
case 'close':
$ttype = ManiphestTransaction::TYPE_STATUS;
- $new_value = ManiphestTaskStatus::STATUS_CLOSED_RESOLVED;
+ $new_value = ManiphestTaskStatus::getDefaultClosedStatus();
break;
case 'claim':
$ttype = ManiphestTransaction::TYPE_OWNER;
diff --git a/src/applications/maniphest/phid/ManiphestPHIDTypeTask.php b/src/applications/maniphest/phid/ManiphestPHIDTypeTask.php
--- a/src/applications/maniphest/phid/ManiphestPHIDTypeTask.php
+++ b/src/applications/maniphest/phid/ManiphestPHIDTypeTask.php
@@ -38,7 +38,7 @@
$handle->setFullName("T{$id}: {$title}");
$handle->setURI("/T{$id}");
- if (!ManiphestTaskStatus::isOpenStatus($task->getStatus())) {
+ if ($task->isClosed()) {
$handle->setStatus(PhabricatorObjectHandleStatus::STATUS_CLOSED);
}
}
diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php
--- a/src/applications/maniphest/query/ManiphestTaskQuery.php
+++ b/src/applications/maniphest/query/ManiphestTaskQuery.php
@@ -349,9 +349,15 @@
case self::STATUS_ANY:
return null;
case self::STATUS_OPEN:
- return 'status = 0';
+ return qsprintf(
+ $conn,
+ 'status IN (%Ld)',
+ ManiphestTaskStatus::getOpenStatusConstants());
case self::STATUS_CLOSED:
- return 'status > 0';
+ return qsprintf(
+ $conn,
+ 'status IN (%Ld)',
+ ManiphestTaskStatus::getClosedStatusConstants());
default:
$constant = idx($map, $this->status);
if (!$constant) {
diff --git a/src/applications/maniphest/search/ManiphestSearchIndexer.php b/src/applications/maniphest/search/ManiphestSearchIndexer.php
--- a/src/applications/maniphest/search/ManiphestSearchIndexer.php
+++ b/src/applications/maniphest/search/ManiphestSearchIndexer.php
@@ -31,9 +31,9 @@
$task->getDateCreated());
$doc->addRelationship(
- (ManiphestTaskStatus::isOpenStatus($task->getStatus()))
- ? PhabricatorSearchRelationship::RELATIONSHIP_OPEN
- : PhabricatorSearchRelationship::RELATIONSHIP_CLOSED,
+ $task->isClosed()
+ ? PhabricatorSearchRelationship::RELATIONSHIP_CLOSED
+ : PhabricatorSearchRelationship::RELATIONSHIP_OPEN,
$task->getPHID(),
ManiphestPHIDTypeTask::TYPECONST,
time());
diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php
--- a/src/applications/maniphest/storage/ManiphestTask.php
+++ b/src/applications/maniphest/storage/ManiphestTask.php
@@ -156,6 +156,9 @@
return $result;
}
+ public function isClosed() {
+ return ManiphestTaskStatus::isClosedStatus($this->getStatus());
+ }
/* -( Markup Interface )--------------------------------------------------- */
@@ -247,6 +250,7 @@
/* -( PhabricatorTokenReceiverInterface )---------------------------------- */
+
public function getUsersToNotifyOfTokenGiven() {
// Sort of ambiguous who this was intended for; just let them both know.
return array_filter(
diff --git a/src/applications/maniphest/storage/ManiphestTransaction.php b/src/applications/maniphest/storage/ManiphestTransaction.php
--- a/src/applications/maniphest/storage/ManiphestTransaction.php
+++ b/src/applications/maniphest/storage/ManiphestTransaction.php
@@ -139,11 +139,11 @@
}
case self::TYPE_STATUS:
- if ($new == ManiphestTaskStatus::STATUS_OPEN) {
- return 'green';
- } else {
- return 'black';
+ $color = ManiphestTaskStatus::getStatusColor($new);
+ if ($color !== null) {
+ return $color;
}
+ break;
case self::TYPE_PRIORITY:
if ($old == ManiphestTaskPriority::getDefaultPriority()) {
@@ -168,19 +168,24 @@
return pht('Retitled');
case self::TYPE_STATUS:
- switch ($new) {
- case ManiphestTaskStatus::STATUS_OPEN:
- if ($old === null) {
- return pht('Created');
- } else {
- return pht('Reopened');
- }
- case ManiphestTaskStatus::STATUS_CLOSED_SPITE:
- return pht('Spited');
- case ManiphestTaskStatus::STATUS_CLOSED_DUPLICATE:
- return pht('Merged');
- default:
- return pht('Closed');
+ if ($old === null) {
+ return pht('Created');
+ }
+
+ $action = ManiphestTaskStatus::getStatusActionName($new);
+ if ($action) {
+ return $action;
+ }
+
+ $old_closed = ManiphestTaskStatus::isClosedStatus($old);
+ $new_closed = ManiphestTaskStatus::isClosedStatus($new);
+
+ if ($new_closed && !$old_closed) {
+ return pht('Closed');
+ } else if (!$new_closed && $old_closed) {
+ return pht('Reopened');
+ } else {
+ return pht('Changed Status');
}
case self::TYPE_DESCRIPTION:
@@ -238,15 +243,19 @@
return 'edit';
case self::TYPE_STATUS:
- switch ($new) {
- case ManiphestTaskStatus::STATUS_OPEN:
- return 'create';
- case ManiphestTaskStatus::STATUS_CLOSED_SPITE:
- return 'dislike';
- case ManiphestTaskStatus::STATUS_CLOSED_DUPLICATE:
- return 'delete';
- default:
- return 'check';
+ if ($old === null) {
+ return 'create';
+ }
+
+ $action = ManiphestTaskStatus::getStatusIcon($new);
+ if ($action !== null) {
+ return $action;
+ }
+
+ if (ManiphestTaskStatus::isClosedStatus($new)) {
+ return 'check';
+ } else {
+ return 'edit';
}
case self::TYPE_DESCRIPTION:
@@ -299,35 +308,40 @@
$this->renderHandleLink($author_phid));
case self::TYPE_STATUS:
- switch ($new) {
- case ManiphestTaskStatus::STATUS_OPEN:
- if ($old === null) {
- return pht(
- '%s created this task.',
- $this->renderHandleLink($author_phid));
- } else {
- return pht(
- '%s reopened this task.',
- $this->renderHandleLink($author_phid));
- }
-
- case ManiphestTaskStatus::STATUS_CLOSED_SPITE:
- return pht(
- '%s closed this task out of spite.',
- $this->renderHandleLink($author_phid));
- case ManiphestTaskStatus::STATUS_CLOSED_DUPLICATE:
+ if ($old === null) {
+ return pht(
+ '%s created this task.',
+ $this->renderHandleLink($author_phid));
+ }
+
+ $old_closed = ManiphestTaskStatus::isClosedStatus($old);
+ $new_closed = ManiphestTaskStatus::isClosedStatus($new);
+
+ $old_name = ManiphestTaskStatus::getTaskStatusName($old);
+ $new_name = ManiphestTaskStatus::getTaskStatusName($new);
+
+ if ($new_closed && !$old_closed) {
+ if ($new == ManiphestTaskStatus::getDuplicateStatus()) {
return pht(
'%s closed this task as a duplicate.',
$this->renderHandleLink($author_phid));
- default:
- $status_name = idx(
- ManiphestTaskStatus::getTaskStatusMap(),
- $new,
- '???');
+ } else {
return pht(
'%s closed this task as "%s".',
$this->renderHandleLink($author_phid),
- $status_name);
+ $new_name);
+ }
+ } else if (!$new_closed && $old_closed) {
+ return pht(
+ '%s reopened this task as "%s".',
+ $this->renderHandleLink($author_phid),
+ $new_name);
+ } else {
+ return pht(
+ '%s changed the task status from "%s" to "%s".',
+ $this->renderHandleLink($author_phid),
+ $old_name,
+ $new_name);
}
case self::TYPE_OWNER:
@@ -488,40 +502,45 @@
$this->renderHandleLink($object_phid));
case self::TYPE_STATUS:
- switch ($new) {
- case ManiphestTaskStatus::STATUS_OPEN:
- if ($old === null) {
- return pht(
- '%s created %s.',
- $this->renderHandleLink($author_phid),
- $this->renderHandleLink($object_phid));
- } else {
- return pht(
- '%s reopened %s.',
- $this->renderHandleLink($author_phid),
- $this->renderHandleLink($object_phid));
- }
-
- case ManiphestTaskStatus::STATUS_CLOSED_SPITE:
- return pht(
- '%s closed %s out of spite.',
- $this->renderHandleLink($author_phid),
- $this->renderHandleLink($object_phid));
- case ManiphestTaskStatus::STATUS_CLOSED_DUPLICATE:
+ if ($old === null) {
+ return pht(
+ '%s created %s.',
+ $this->renderHandleLink($author_phid),
+ $this->renderHandleLink($object_phid));
+ }
+
+ $old_closed = ManiphestTaskStatus::isClosedStatus($old);
+ $new_closed = ManiphestTaskStatus::isClosedStatus($new);
+
+ $old_name = ManiphestTaskStatus::getTaskStatusName($old);
+ $new_name = ManiphestTaskStatus::getTaskStatusName($new);
+
+ if ($new_closed && !$old_closed) {
+ if ($new == ManiphestTaskStatus::getDuplicateStatus()) {
return pht(
'%s closed %s as a duplicate.',
$this->renderHandleLink($author_phid),
$this->renderHandleLink($object_phid));
- default:
- $status_name = idx(
- ManiphestTaskStatus::getTaskStatusMap(),
- $new,
- '???');
+ } else {
return pht(
'%s closed %s as "%s".',
$this->renderHandleLink($author_phid),
$this->renderHandleLink($object_phid),
- $status_name);
+ $new_name);
+ }
+ } else if (!$new_closed && $old_closed) {
+ return pht(
+ '%s reopened %s as "%s".',
+ $this->renderHandleLink($author_phid),
+ $this->renderHandleLink($object_phid),
+ $new_name);
+ } else {
+ return pht(
+ '%s changed the status of %s from "%s" to "%s".',
+ $this->renderHandleLink($author_phid),
+ $this->renderHandleLink($object_phid),
+ $old_name,
+ $new_name);
}
case self::TYPE_OWNER:
diff --git a/src/applications/maniphest/view/ManiphestTaskListView.php b/src/applications/maniphest/view/ManiphestTaskListView.php
--- a/src/applications/maniphest/view/ManiphestTaskListView.php
+++ b/src/applications/maniphest/view/ManiphestTaskListView.php
@@ -58,7 +58,7 @@
}
$status = $task->getStatus();
- if ($status != ManiphestTaskStatus::STATUS_OPEN) {
+ if ($task->isClosed()) {
$item->setDisabled(true);
}
diff --git a/src/applications/maniphest/view/ManiphestView.php b/src/applications/maniphest/view/ManiphestView.php
--- a/src/applications/maniphest/view/ManiphestView.php
+++ b/src/applications/maniphest/view/ManiphestView.php
@@ -1,8 +1,5 @@
<?php
-/**
- * @group maniphest
- */
abstract class ManiphestView extends AphrontView {
public static function renderTagForTask(ManiphestTask $task) {
diff --git a/src/applications/search/controller/PhabricatorSearchAttachController.php b/src/applications/search/controller/PhabricatorSearchAttachController.php
--- a/src/applications/search/controller/PhabricatorSearchAttachController.php
+++ b/src/applications/search/controller/PhabricatorSearchAttachController.php
@@ -178,7 +178,7 @@
$close_task = id(new ManiphestTransaction())
->setTransactionType(ManiphestTransaction::TYPE_STATUS)
- ->setNewValue(ManiphestTaskStatus::STATUS_CLOSED_DUPLICATE);
+ ->setNewValue(ManiphestTaskStatus::getDuplicateStatus());
$merge_comment = id(new ManiphestTransaction())
->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
diff --git a/webroot/rsrc/js/application/maniphest/behavior-batch-editor.js b/webroot/rsrc/js/application/maniphest/behavior-batch-editor.js
--- a/webroot/rsrc/js/application/maniphest/behavior-batch-editor.js
+++ b/webroot/rsrc/js/application/maniphest/behavior-batch-editor.js
@@ -24,7 +24,7 @@
'add_project': 'Add Projects',
'remove_project' : 'Remove Projects',
'priority': 'Change Priority',
- 'status': 'Open / Close',
+ 'status': 'Change Status',
'add_comment': 'Comment',
'assign': 'Assign',
'add_ccs' : 'Add CCs',

File Metadata

Mime Type
text/plain
Expires
Sat, Dec 28, 11:17 PM (5 h, 37 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6941330
Default Alt Text
D8579.diff (19 KB)

Event Timeline

Phabricator · Built by Phacility