From 7b695aa43bd19400036cefa3a94bd5522f96e1d2 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 11 Aug 2017 16:27:38 -0700 Subject: [PATCH] Migrate revision storage to modern status constants ("accepted") instead of legacy numeric values ("2") Summary: Ref T2543. Rewrites all the storage to use constants. Note that transactions still use legacy values, I'll migrate and update them separately. Test Plan: - Ran migration. - Browsed around, changed revision states, viewed dashboard, etc. - Selected `DISTINCT()` and `GROUP_CONCAT()` of the `status` field in the database, saw sane/expected before and after values. - Verified that old Conduit methods still return numeric constants for compatibility. Reviewers: chad Reviewed By: chad Maniphest Tasks: T2543 Differential Revision: https://secure.phabricator.com/D18418 --- .../20170811.differential.02.modernstatus.sql | 17 +++++++++++++++++ .../constants/DifferentialLegacyQuery.php | 15 --------------- .../query/DifferentialRevisionQuery.php | 4 ++-- .../storage/DifferentialRevision.php | 19 ++++--------------- .../DifferentialRevisionStatusTransaction.php | 3 +-- 5 files changed, 24 insertions(+), 34 deletions(-) create mode 100644 resources/sql/autopatches/20170811.differential.02.modernstatus.sql diff --git a/resources/sql/autopatches/20170811.differential.02.modernstatus.sql b/resources/sql/autopatches/20170811.differential.02.modernstatus.sql new file mode 100644 index 0000000000..a305206411 --- /dev/null +++ b/resources/sql/autopatches/20170811.differential.02.modernstatus.sql @@ -0,0 +1,17 @@ +UPDATE {$NAMESPACE}_differential.differential_revision + SET status = "needs-review" WHERE status = "0"; + +UPDATE {$NAMESPACE}_differential.differential_revision + SET status = "needs-revision" WHERE status = "1"; + +UPDATE {$NAMESPACE}_differential.differential_revision + SET status = "accepted" WHERE status = "2"; + +UPDATE {$NAMESPACE}_differential.differential_revision + SET status = "published" WHERE status = "3"; + +UPDATE {$NAMESPACE}_differential.differential_revision + SET status = "abandoned" WHERE status = "4"; + +UPDATE {$NAMESPACE}_differential.differential_revision + SET status = "changes-planned" WHERE status = "5"; diff --git a/src/applications/differential/constants/DifferentialLegacyQuery.php b/src/applications/differential/constants/DifferentialLegacyQuery.php index ca254918e0..26d2c4aee2 100644 --- a/src/applications/differential/constants/DifferentialLegacyQuery.php +++ b/src/applications/differential/constants/DifferentialLegacyQuery.php @@ -31,21 +31,6 @@ final class DifferentialLegacyQuery return $map[$status]; } - public static function getLegacyValues(array $modern_values) { - $values = array(); - foreach ($modern_values as $status_constant) { - $status_object = DifferentialRevisionStatus::newForStatus( - $status_constant); - - $legacy_key = $status_object->getLegacyKey(); - if ($legacy_key !== null) { - $values[] = $legacy_key; - } - } - - return $values; - } - private static function getMap() { $all = array( DifferentialRevisionStatus::NEEDS_REVIEW, diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php index 39bba8112d..8e6e23776a 100644 --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -695,7 +695,7 @@ final class DifferentialRevisionQuery $where[] = qsprintf( $conn_r, 'r.status in (%Ls)', - DifferentialLegacyQuery::getLegacyValues($this->statuses)); + $this->statuses); } if ($this->isOpen !== null) { @@ -709,7 +709,7 @@ final class DifferentialRevisionQuery $where[] = qsprintf( $conn_r, 'r.status in (%Ls)', - DifferentialLegacyQuery::getLegacyValues($statuses)); + $statuses); } $where[] = $this->buildWhereClauseParts($conn_r); diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index 69334bc01d..eaf856fd80 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -613,26 +613,15 @@ final class DifferentialRevision extends DifferentialDAO } public function setModernRevisionStatus($status) { - $status_object = DifferentialRevisionStatus::newForStatus($status); - - if ($status_object->getKey() != $status) { - throw new Exception( - pht( - 'Trying to set revision to invalid status "%s".', - $status)); - } - - $legacy_status = $status_object->getLegacyKey(); - - return $this->setStatus($legacy_status); + return $this->setStatus($status); } public function getModernRevisionStatus() { - return $this->getStatusObject()->getKey(); + return $this->getStatus(); } public function getLegacyRevisionStatus() { - return $this->getStatus(); + return $this->getStatusObject()->getLegacyKey(); } public function isClosed() { @@ -677,7 +666,7 @@ final class DifferentialRevision extends DifferentialDAO public function getStatusObject() { $status = $this->getStatus(); - return DifferentialRevisionStatus::newForLegacyStatus($status); + return DifferentialRevisionStatus::newForStatus($status); } public function getFlag(PhabricatorUser $viewer) { diff --git a/src/applications/differential/xaction/DifferentialRevisionStatusTransaction.php b/src/applications/differential/xaction/DifferentialRevisionStatusTransaction.php index 0c38bd6d84..75e85bc037 100644 --- a/src/applications/differential/xaction/DifferentialRevisionStatusTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionStatusTransaction.php @@ -14,8 +14,7 @@ final class DifferentialRevisionStatusTransaction } public function getTitle() { - $new = $this->getNewValue(); - $status = DifferentialRevisionStatus::newForLegacyStatus($new); + $status = $this->newStatusObject(); if ($status->isAccepted()) { return pht('This revision is now accepted and ready to land.');