diff --git a/resources/sql/autopatches/20180910.audit.02.string.sql b/resources/sql/autopatches/20180910.audit.02.string.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20180910.audit.02.string.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_repository.repository_commit + CHANGE auditStatus auditStatus VARCHAR(32) NOT NULL COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20180910.audit.03.status.php b/resources/sql/autopatches/20180910.audit.03.status.php new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20180910.audit.03.status.php @@ -0,0 +1,28 @@ +establishConnection('w'); + +$status_map = array( + 0 => 'none', + 1 => 'needs-audit', + 2 => 'concern-raised', + 3 => 'partially-audited', + 4 => 'audited', + 5 => 'needs-verification', +); + +foreach (new LiskMigrationIterator($table) as $commit) { + $status = $commit->getAuditStatus(); + + if (!isset($status_map[$status])) { + continue; + } + + queryfx( + $conn, + 'UPDATE %T SET auditStatus = %s WHERE id = %d', + $table->getTableName(), + $status_map[$status], + $commit->getID()); +} diff --git a/resources/sql/autopatches/20180910.audit.04.xactions.php b/resources/sql/autopatches/20180910.audit.04.xactions.php new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20180910.audit.04.xactions.php @@ -0,0 +1,48 @@ +establishConnection('w'); + +$status_map = array( + 0 => 'none', + 1 => 'needs-audit', + 2 => 'concern-raised', + 3 => 'partially-audited', + 4 => 'audited', + 5 => 'needs-verification', +); + +$state_type = DiffusionCommitStateTransaction::TRANSACTIONTYPE; + +foreach (new LiskMigrationIterator($table) as $xaction) { + if ($xaction->getTransactionType() !== $state_type) { + continue; + } + + $old_value = $xaction->getOldValue(); + $new_value = $xaction->getNewValue(); + + $any_change = false; + + if (isset($status_map[$old_value])) { + $old_value = $status_map[$old_value]; + $any_change = true; + } + + if (isset($status_map[$new_value])) { + $new_value = $status_map[$new_value]; + $any_change = true; + } + + if (!$any_change) { + continue; + } + + queryfx( + $conn, + 'UPDATE %T SET oldValue = %s, newValue = %s WHERE id = %d', + $table->getTableName(), + phutil_json_encode($old_value), + phutil_json_encode($new_value), + $xaction->getID()); +} diff --git a/src/applications/audit/constants/PhabricatorAuditCommitStatusConstants.php b/src/applications/audit/constants/PhabricatorAuditCommitStatusConstants.php --- a/src/applications/audit/constants/PhabricatorAuditCommitStatusConstants.php +++ b/src/applications/audit/constants/PhabricatorAuditCommitStatusConstants.php @@ -19,18 +19,21 @@ const MODERN_AUDITED = 'audited'; const MODERN_NEEDS_VERIFICATION = 'needs-verification'; - public static function newForLegacyStatus($status) { + public static function newModernKeys(array $values) { $map = self::getMap(); - if (is_int($status) || ctype_digit($status)) { - foreach ($map as $key => $spec) { - if ((int)idx($spec, 'legacy') === (int)$status) { - return self::newForStatus($key); - } + $modern = array(); + foreach ($map as $key => $spec) { + if (isset($spec['legacy'])) { + $modern[$spec['legacy']] = $key; } } - return self::newForStatus($status); + foreach ($values as $key => $value) { + $values[$key] = idx($modern, $value, $value); + } + + return $values; } public static function newForStatus($status) { @@ -58,10 +61,6 @@ return idx($this->spec, 'color'); } - public function getLegacyKey() { - return idx($this->spec, 'legacy'); - } - public function getName() { return idx($this->spec, 'name', pht('Unknown ("%s")', $this->key)); } @@ -96,9 +95,9 @@ public static function getOpenStatusConstants() { $constants = array(); - foreach (self::getMap() as $map) { + foreach (self::getMap() as $key => $map) { if (!$map['closed']) { - $constants[] = $map['legacy']; + $constants[] = $key; } } return $constants; diff --git a/src/applications/diffusion/query/DiffusionCommitQuery.php b/src/applications/diffusion/query/DiffusionCommitQuery.php --- a/src/applications/diffusion/query/DiffusionCommitQuery.php +++ b/src/applications/diffusion/query/DiffusionCommitQuery.php @@ -714,16 +714,12 @@ } if ($this->statuses !== null) { - $statuses = array(); - foreach ($this->statuses as $status) { - $object = PhabricatorAuditCommitStatusConstants::newForLegacyStatus( - $status); - $statuses[] = $object->getLegacyKey(); - } + $statuses = PhabricatorAuditCommitStatusConstants::newModernKeys( + $this->statuses); $where[] = qsprintf( $conn, - 'commit.auditStatus IN (%Ld)', + 'commit.auditStatus IN (%Ls)', $statuses); } diff --git a/src/applications/diffusion/xaction/DiffusionCommitConcernTransaction.php b/src/applications/diffusion/xaction/DiffusionCommitConcernTransaction.php --- a/src/applications/diffusion/xaction/DiffusionCommitConcernTransaction.php +++ b/src/applications/diffusion/xaction/DiffusionCommitConcernTransaction.php @@ -34,7 +34,7 @@ // NOTE: We force the commit directly into "Concern Raised" so that we // override a possible "Needs Verification" state. $object->setAuditStatus( - PhabricatorAuditCommitStatusConstants::CONCERN_RAISED); + PhabricatorAuditCommitStatusConstants::MODERN_CONCERN_RAISED); } public function applyExternalEffects($object, $value) { diff --git a/src/applications/diffusion/xaction/DiffusionCommitStateTransaction.php b/src/applications/diffusion/xaction/DiffusionCommitStateTransaction.php --- a/src/applications/diffusion/xaction/DiffusionCommitStateTransaction.php +++ b/src/applications/diffusion/xaction/DiffusionCommitStateTransaction.php @@ -13,7 +13,7 @@ private function getAuditStatusObject() { $new = $this->getNewValue(); - return PhabricatorAuditCommitStatusConstants::newForLegacyStatus($new); + return PhabricatorAuditCommitStatusConstants::newForStatus($new); } public function getIcon() { diff --git a/src/applications/diffusion/xaction/DiffusionCommitVerifyTransaction.php b/src/applications/diffusion/xaction/DiffusionCommitVerifyTransaction.php --- a/src/applications/diffusion/xaction/DiffusionCommitVerifyTransaction.php +++ b/src/applications/diffusion/xaction/DiffusionCommitVerifyTransaction.php @@ -37,7 +37,7 @@ public function applyInternalEffects($object, $value) { $object->setAuditStatus( - PhabricatorAuditCommitStatusConstants::NEEDS_VERIFICATION); + PhabricatorAuditCommitStatusConstants::MODERN_NEEDS_VERIFICATION); } protected function validateAction($object, PhabricatorUser $viewer) { diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php --- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php +++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php @@ -27,7 +27,7 @@ protected $epoch; protected $mailKey; protected $authorPHID; - protected $auditStatus = PhabricatorAuditCommitStatusConstants::NONE; + protected $auditStatus = PhabricatorAuditCommitStatusConstants::MODERN_NONE; protected $summary = ''; protected $importStatus = 0; @@ -120,7 +120,7 @@ 'authorPHID' => 'phid?', 'authorIdentityPHID' => 'phid?', 'committerIdentityPHID' => 'phid?', - 'auditStatus' => 'uint32', + 'auditStatus' => 'text32', 'summary' => 'text255', 'importStatus' => 'uint32', ), @@ -385,20 +385,22 @@ if ($this->isAuditStatusNeedsVerification()) { // If the change is in "Needs Verification", we keep it there as // long as any auditors still have concerns. - $status = PhabricatorAuditCommitStatusConstants::NEEDS_VERIFICATION; + $status = + PhabricatorAuditCommitStatusConstants::MODERN_NEEDS_VERIFICATION; } else { - $status = PhabricatorAuditCommitStatusConstants::CONCERN_RAISED; + $status = PhabricatorAuditCommitStatusConstants::MODERN_CONCERN_RAISED; } } else if ($any_accept) { if ($any_need) { - $status = PhabricatorAuditCommitStatusConstants::PARTIALLY_AUDITED; + $status = + PhabricatorAuditCommitStatusConstants::MODERN_PARTIALLY_AUDITED; } else { - $status = PhabricatorAuditCommitStatusConstants::FULLY_AUDITED; + $status = PhabricatorAuditCommitStatusConstants::MODERN_AUDITED; } } else if ($any_need) { - $status = PhabricatorAuditCommitStatusConstants::NEEDS_AUDIT; + $status = PhabricatorAuditCommitStatusConstants::MODERN_NEEDS_AUDIT; } else { - $status = PhabricatorAuditCommitStatusConstants::NONE; + $status = PhabricatorAuditCommitStatusConstants::MODERN_NONE; } return $this->setAuditStatus($status); @@ -529,7 +531,7 @@ public function getAuditStatusObject() { $status = $this->getAuditStatus(); - return PhabricatorAuditCommitStatusConstants::newForLegacyStatus($status); + return PhabricatorAuditCommitStatusConstants::newForStatus($status); } public function isAuditStatusNoAudit() {