diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php --- a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php +++ b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php @@ -140,11 +140,11 @@ ->setTransactionType( PhabricatorOwnersPackageAuditingTransaction::TRANSACTIONTYPE) ->setIsCopyable(true) - ->setValue($object->getAuditingEnabled()) + ->setValue($object->getAuditingState()) ->setOptions( array( - '' => pht('Disabled'), - '1' => pht('Enabled'), + PhabricatorOwnersPackage::AUDITING_NONE => pht('No Auditing'), + PhabricatorOwnersPackage::AUDITING_AUDIT => pht('Audit Commits'), )), id(new PhabricatorRemarkupEditField()) ->setKey('description') diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php --- a/src/applications/owners/storage/PhabricatorOwnersPackage.php +++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php @@ -38,6 +38,9 @@ const AUTOREVIEW_BLOCK = 'block'; const AUTOREVIEW_BLOCK_ALWAYS = 'block-always'; + const AUDITING_NONE = 'none'; + const AUDITING_AUDIT = 'audit'; + const DOMINION_STRONG = 'strong'; const DOMINION_WEAK = 'weak'; @@ -564,6 +567,14 @@ return '/owners/package/'.$this->getID().'/'; } + public function getAuditingState() { + if ($this->getAuditingEnabled()) { + return self::AUDITING_AUDIT; + } else { + return self::AUDITING_NONE; + } + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ @@ -720,11 +731,10 @@ 'label' => $review_label, ); + $audit_value = $this->getAuditingState(); if ($this->getAuditingEnabled()) { - $audit_value = 'audit'; $audit_label = pht('Auditing Enabled'); } else { - $audit_value = 'none'; $audit_label = pht('No Auditing'); } diff --git a/src/applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php b/src/applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php --- a/src/applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php +++ b/src/applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php @@ -10,7 +10,15 @@ } public function generateNewValue($object, $value) { - return (int)$value; + switch ($value) { + case PhabricatorOwnersPackage::AUDITING_AUDIT: + return 1; + case '1': + // TODO: Remove, deprecated. + return 1; + default: + return 0; + } } public function applyInternalEffects($object, $value) { @@ -29,4 +37,47 @@ } } + public function validateTransactions($object, array $xactions) { + $errors = array(); + + // See PHI1047. This transaction type accepted some weird stuff. Continue + // supporting it for now, but move toward sensible consistency. + + $modern_options = array( + PhabricatorOwnersPackage::AUDITING_NONE => + sprintf('"%s"', PhabricatorOwnersPackage::AUDITING_NONE), + PhabricatorOwnersPackage::AUDITING_AUDIT => + sprintf('"%s"', PhabricatorOwnersPackage::AUDITING_AUDIT), + ); + + $deprecated_options = array( + '0' => '"0"', + '1' => '"1"', + '' => pht('"" (empty string)'), + ); + + foreach ($xactions as $xaction) { + $new_value = $xaction->getNewValue(); + + if (isset($modern_options[$new_value])) { + continue; + } + + if (isset($deprecated_options[$new_value])) { + continue; + } + + $errors[] = $this->newInvalidError( + pht( + 'Package auditing value "%s" is not supported. Supported options '. + 'are: %s. Deprecated options are: %s.', + $new_value, + implode(', ', $modern_options), + implode(', ', $deprecated_options)), + $xaction); + } + + return $errors; + } + }