diff --git a/resources/sql/autopatches/20180208.maniphest.01.close.sql b/resources/sql/autopatches/20180208.maniphest.01.close.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20180208.maniphest.01.close.sql @@ -0,0 +1,5 @@ +ALTER TABLE {$NAMESPACE}_maniphest.maniphest_task + ADD closedEpoch INT UNSIGNED; + +ALTER TABLE {$NAMESPACE}_maniphest.maniphest_task + ADD closerPHID VARBINARY(64); diff --git a/resources/sql/autopatches/20180208.maniphest.02.populate.php b/resources/sql/autopatches/20180208.maniphest.02.populate.php new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20180208.maniphest.02.populate.php @@ -0,0 +1,65 @@ +establishConnection('w'); +$viewer = PhabricatorUser::getOmnipotentUser(); + +foreach (new LiskMigrationIterator($table) as $task) { + if ($task->getClosedEpoch()) { + // Task already has a closed date. + continue; + } + + $status = $task->getStatus(); + if (!ManiphestTaskStatus::isClosedStatus($status)) { + // Task isn't closed. + continue; + } + + // Look through the transactions from newest to oldest until we find one + // where the task was closed. A merge also counts as a close, even though + // it doesn't currently produce a separate transaction. + + $type_merge = ManiphestTaskStatusTransaction::TRANSACTIONTYPE; + $type_status = ManiphestTaskMergedIntoTransaction::TRANSACTIONTYPE; + + $xactions = id(new ManiphestTransactionQuery()) + ->setViewer($viewer) + ->withObjectPHIDs(array($task->getPHID())) + ->withTransactionTypes( + array( + $type_merge, + $type_status, + )) + ->execute(); + foreach ($xactions as $xaction) { + $old = $xaction->getOldValue(); + $new = $xaction->getNewValue(); + + $type = $xaction->getTransactionType(); + + // If this is a status change, but is not a close, don't use it. + // (We always use merges, even though it's possible to merge a task which + // was previously closed: we can't tell when this happens very easily.) + if ($type === $type_status) { + if (!ManiphestTaskStatus::isClosedStatus($new)) { + continue; + } + + if ($old && ManiphestTaskStatus::isClosedStatus($old)) { + continue; + } + } + + queryfx( + $conn, + 'UPDATE %T SET closedEpoch = %d, closerPHID = %ns + WHERE id = %d', + $table->getTableName(), + $xaction->getDateCreated(), + $xaction->getAuthorPHID(), + $task->getID()); + + break; + } +} 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 @@ -44,6 +44,9 @@ protected $points; protected $subtype; + protected $closedEpoch; + protected $closerPHID; + private $subscriberPHIDs = self::ATTACHABLE; private $groupByProjectPHID = self::ATTACHABLE; private $customFields = self::ATTACHABLE; @@ -90,6 +93,8 @@ 'points' => 'double?', 'bridgedObjectPHID' => 'phid?', 'subtype' => 'text64', + 'closedEpoch' => 'epoch?', + 'closerPHID' => 'phid?', ), self::CONFIG_KEY_SCHEMA => array( 'key_phid' => null, @@ -131,6 +136,12 @@ 'key_subtype' => array( 'columns' => array('subtype'), ), + 'key_closed' => array( + 'columns' => array('closedEpoch'), + ), + 'key_closer' => array( + 'columns' => array('closerPHID', 'closedEpoch'), + ), ), ) + parent::getConfiguration(); } diff --git a/src/applications/maniphest/xaction/ManiphestTaskMergedIntoTransaction.php b/src/applications/maniphest/xaction/ManiphestTaskMergedIntoTransaction.php --- a/src/applications/maniphest/xaction/ManiphestTaskMergedIntoTransaction.php +++ b/src/applications/maniphest/xaction/ManiphestTaskMergedIntoTransaction.php @@ -10,7 +10,7 @@ } public function applyInternalEffects($object, $value) { - $object->setStatus(ManiphestTaskStatus::getDuplicateStatus()); + $this->updateStatus($object, ManiphestTaskStatus::getDuplicateStatus()); } public function getActionName() { diff --git a/src/applications/maniphest/xaction/ManiphestTaskStatusTransaction.php b/src/applications/maniphest/xaction/ManiphestTaskStatusTransaction.php --- a/src/applications/maniphest/xaction/ManiphestTaskStatusTransaction.php +++ b/src/applications/maniphest/xaction/ManiphestTaskStatusTransaction.php @@ -10,7 +10,7 @@ } public function applyInternalEffects($object, $value) { - $object->setStatus($value); + $this->updateStatus($object, $value); } public function shouldHide() { diff --git a/src/applications/maniphest/xaction/ManiphestTaskTransactionType.php b/src/applications/maniphest/xaction/ManiphestTaskTransactionType.php --- a/src/applications/maniphest/xaction/ManiphestTaskTransactionType.php +++ b/src/applications/maniphest/xaction/ManiphestTaskTransactionType.php @@ -3,4 +3,27 @@ abstract class ManiphestTaskTransactionType extends PhabricatorModularTransactionType { + protected function updateStatus($object, $new_value) { + $old_value = $object->getStatus(); + $object->setStatus($new_value); + + // If this status change closes or opens the task, update the closed + // date and actor PHID. + $old_closed = ManiphestTaskStatus::isClosedStatus($old_value); + $new_closed = ManiphestTaskStatus::isClosedStatus($new_value); + + $is_close = ($new_closed && !$old_closed); + $is_open = (!$new_closed && $old_closed); + + if ($is_close) { + $object + ->setClosedEpoch(PhabricatorTime::getNow()) + ->setCloserPHID($this->getActingAsPHID()); + } else if ($is_open) { + $object + ->setClosedEpoch(null) + ->setCloserPHID(null); + } + } + }