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 @@ -60,6 +60,30 @@ return idx($this->spec, 'name', pht('Unknown ("%s")', $this->key)); } + public function isNoAudit() { + return ($this->key == self::MODERN_NONE); + } + + public function isConcernRaised() { + return ($this->key == self::MODERN_CONCERN_RAISED); + } + + public function isNeedsVerification() { + return ($this->key == self::MODERN_NEEDS_VERIFICATION); + } + + public function isPartiallyAudited() { + return ($this->key == self::MODERN_PARTIALLY_AUDITED); + } + + public function isAudited() { + return ($this->key == self::MODERN_AUDITED); + } + + public function getIsClosed() { + return idx($this->spec, 'closed'); + } + public static function getStatusNameMap() { $map = self::getMap(); return ipull($map, 'name', 'legacy'); diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -206,12 +206,10 @@ $object->writeImportStatusFlag($import_status_flag); } - $partial_status = PhabricatorAuditCommitStatusConstants::PARTIALLY_AUDITED; - // If the commit has changed state after this edit, add an informational // transaction about the state change. if ($old_status != $new_status) { - if ($new_status == $partial_status) { + if ($object->isAuditStatusPartiallyAudited()) { // This state isn't interesting enough to get a transaction. The // best way we could lead the user forward is something like "This // commit still requires additional audits." but that's redundant and diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -171,13 +171,12 @@ ->setHeaderIcon('fa-code-fork') ->addTag($commit_tag); - if ($commit->getAuditStatus()) { - $icon = PhabricatorAuditCommitStatusConstants::getStatusIcon( - $commit->getAuditStatus()); - $color = PhabricatorAuditCommitStatusConstants::getStatusColor( - $commit->getAuditStatus()); - $status = PhabricatorAuditCommitStatusConstants::getStatusName( - $commit->getAuditStatus()); + if (!$commit->isAuditStatusNoAudit()) { + $status = $commit->getAuditStatusObject(); + + $icon = $status->getIcon(); + $color = $status->getColor(); + $status = $status->getName(); $header->setStatus($icon, $color, $status); } diff --git a/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php b/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php --- a/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php +++ b/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php @@ -31,8 +31,6 @@ // After ApplicationTransactions, we could annotate feed stories more // explicitly. - $fully_audited = PhabricatorAuditCommitStatusConstants::FULLY_AUDITED; - $story = $this->getFeedStory(); $xaction = $story->getPrimaryTransaction(); switch ($xaction->getTransactionType()) { @@ -41,7 +39,7 @@ case PhabricatorAuditActionConstants::CLOSE: return true; case PhabricatorAuditActionConstants::ACCEPT: - if ($object->getAuditStatus() == $fully_audited) { + if ($object->isAuditStatusAudited()) { return true; } break; @@ -165,14 +163,7 @@ } public function isObjectClosed($object) { - switch ($object->getAuditStatus()) { - case PhabricatorAuditCommitStatusConstants::NEEDS_AUDIT: - case PhabricatorAuditCommitStatusConstants::CONCERN_RAISED: - case PhabricatorAuditCommitStatusConstants::PARTIALLY_AUDITED: - return false; - default: - return true; - } + return $object->getAuditStatusObject()->getIsClosed(); } public function getResponsibilityTitle($object) { diff --git a/src/applications/diffusion/engineextension/DiffusionHovercardEngineExtension.php b/src/applications/diffusion/engineextension/DiffusionHovercardEngineExtension.php --- a/src/applications/diffusion/engineextension/DiffusionHovercardEngineExtension.php +++ b/src/applications/diffusion/engineextension/DiffusionHovercardEngineExtension.php @@ -41,12 +41,12 @@ $hovercard->addField(pht('Date'), phabricator_date($commit->getEpoch(), $viewer)); - if ($commit->getAuditStatus() != - PhabricatorAuditCommitStatusConstants::NONE) { + if (!$commit->isAuditStatusNoAudit()) { + $status = $commit->getAuditStatusObject(); - $hovercard->addField(pht('Audit Status'), - PhabricatorAuditCommitStatusConstants::getStatusName( - $commit->getAuditStatus())); + $hovercard->addField( + pht('Audit Status'), + $status->getName()); } } diff --git a/src/applications/diffusion/query/DiffusionCommitRequiredActionResultBucket.php b/src/applications/diffusion/query/DiffusionCommitRequiredActionResultBucket.php --- a/src/applications/diffusion/query/DiffusionCommitRequiredActionResultBucket.php +++ b/src/applications/diffusion/query/DiffusionCommitRequiredActionResultBucket.php @@ -69,14 +69,12 @@ $results = array(); $objects = $this->objects; - $status_concern = PhabricatorAuditCommitStatusConstants::CONCERN_RAISED; - foreach ($objects as $key => $object) { if (empty($phids[$object->getAuthorPHID()])) { continue; } - if ($object->getAuditStatus() != $status_concern) { + if (!$object->isAuditStatusConcernRaised()) { continue; } @@ -91,7 +89,6 @@ $results = array(); $objects = $this->objects; - $status_verify = PhabricatorAuditCommitStatusConstants::NEEDS_VERIFICATION; $has_concern = array( PhabricatorAuditStatusConstants::CONCERNED, ); @@ -102,7 +99,7 @@ continue; } - if ($object->getAuditStatus() != $status_verify) { + if (!$object->isAuditStatusNeedsVerification()) { continue; } @@ -147,10 +144,8 @@ $results = array(); $objects = $this->objects; - $status_concern = PhabricatorAuditCommitStatusConstants::CONCERN_RAISED; - foreach ($objects as $key => $object) { - if ($object->getAuditStatus() != $status_concern) { + if (!$object->isAuditStatusConcernRaised()) { continue; } 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 @@ -54,10 +54,8 @@ // Even if you've already raised a concern, you can raise again as long // as the author requested you verify. - $state_verify = PhabricatorAuditCommitStatusConstants::NEEDS_VERIFICATION; - if ($this->isViewerFullyRejected($object, $viewer)) { - if ($object->getAuditStatus() != $state_verify) { + if (!$object->isAuditStatusNeedsVerification()) { throw new Exception( pht( 'You can not raise a concern with this commit because you have '. 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 @@ -11,29 +11,32 @@ throw new PhutilMethodNotImplementedException(); } - public function getIcon() { + private function getAuditStatusObject() { $new = $this->getNewValue(); - return PhabricatorAuditCommitStatusConstants::getStatusIcon($new); + return PhabricatorAuditCommitStatusConstants::newForLegacyStatus($new); + } + + public function getIcon() { + return $this->getAuditStatusObject()->getIcon(); } public function getColor() { - $new = $this->getNewValue(); - return PhabricatorAuditCommitStatusConstants::getStatusColor($new); + return $this->getAuditStatusObject()->getColor(); } public function getTitle() { - $new = $this->getNewValue(); + $status = $this->getAuditStatusObject(); - switch ($new) { - case PhabricatorAuditCommitStatusConstants::NONE: + switch ($status->getKey()) { + case PhabricatorAuditCommitStatusConstants::MODERN_NONE: return pht('This commit no longer requires audit.'); - case PhabricatorAuditCommitStatusConstants::NEEDS_AUDIT: + case PhabricatorAuditCommitStatusConstants::MODERN_NEEDS_AUDIT: return pht('This commit now requires audit.'); - case PhabricatorAuditCommitStatusConstants::CONCERN_RAISED: + case PhabricatorAuditCommitStatusConstants::MODERN_CONCERN_RAISED: return pht('This commit now has outstanding concerns.'); - case PhabricatorAuditCommitStatusConstants::NEEDS_VERIFICATION: + case PhabricatorAuditCommitStatusConstants::MODERN_NEEDS_VERIFICATION: return pht('This commit now requires verification by auditors.'); - case PhabricatorAuditCommitStatusConstants::FULLY_AUDITED: + case PhabricatorAuditCommitStatusConstants::MODERN_AUDITED: return pht('All concerns with this commit have now been addressed.'); } @@ -41,26 +44,26 @@ } public function getTitleForFeed() { - $new = $this->getNewValue(); + $status = $this->getAuditStatusObject(); - switch ($new) { - case PhabricatorAuditCommitStatusConstants::NONE: + switch ($status->getKey()) { + case PhabricatorAuditCommitStatusConstants::MODERN_NONE: return pht( '%s no longer requires audit.', $this->renderObject()); - case PhabricatorAuditCommitStatusConstants::NEEDS_AUDIT: + case PhabricatorAuditCommitStatusConstants::MODERN_NEEDS_AUDIT: return pht( '%s now requires audit.', $this->renderObject()); - case PhabricatorAuditCommitStatusConstants::CONCERN_RAISED: + case PhabricatorAuditCommitStatusConstants::MODERN_CONCERN_RAISED: return pht( '%s now has outstanding concerns.', $this->renderObject()); - case PhabricatorAuditCommitStatusConstants::NEEDS_VERIFICATION: + case PhabricatorAuditCommitStatusConstants::MODERN_NEEDS_VERIFICATION: return pht( '%s now requires verification by auditors.', $this->renderObject()); - case PhabricatorAuditCommitStatusConstants::FULLY_AUDITED: + case PhabricatorAuditCommitStatusConstants::MODERN_AUDITED: return pht( 'All concerns with %s have now been addressed.', $this->renderObject()); 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 @@ -48,8 +48,7 @@ 'are not the author.')); } - $status = $object->getAuditStatus(); - if ($status != PhabricatorAuditCommitStatusConstants::CONCERN_RAISED) { + if (!$object->isAuditStatusConcernRaised()) { throw new Exception( pht( 'You can not request verification of this commit because no '. 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 @@ -535,6 +535,26 @@ return PhabricatorAuditCommitStatusConstants::newForLegacyStatus($status); } + public function isAuditStatusNoAudit() { + return $this->getAuditStatusObject()->isNoAudit(); + } + + public function isAuditStatusConcernRaised() { + return $this->getAuditStatusObject()->isConcernRaised(); + } + + public function isAuditStatusNeedsVerification() { + return $this->getAuditStatusObject()->isNeedsVerification(); + } + + public function isAuditStatusPartiallyAudited() { + return $this->getAuditStatusObject()->isPartiallyAudited(); + } + + public function isAuditStatusAudited() { + return $this->getAuditStatusObject()->isAudited(); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ public function getCapabilities() {