Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13245263
D20124.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
14 KB
Referenced Files
None
Subscribers
None
D20124.diff
View Options
diff --git a/resources/sql/autopatches/20190207.packages.01.state.sql b/resources/sql/autopatches/20190207.packages.01.state.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20190207.packages.01.state.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_owners.owners_package
+ ADD auditingState VARCHAR(32) NOT NULL COLLATE {$COLLATE_TEXT};
diff --git a/resources/sql/autopatches/20190207.packages.02.migrate.sql b/resources/sql/autopatches/20190207.packages.02.migrate.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20190207.packages.02.migrate.sql
@@ -0,0 +1,2 @@
+UPDATE {$NAMESPACE}_owners.owners_package
+ SET auditingState = IF(auditingEnabled = 0, 'none', 'audit');
diff --git a/resources/sql/autopatches/20190207.packages.03.drop.sql b/resources/sql/autopatches/20190207.packages.03.drop.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20190207.packages.03.drop.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_owners.owners_package
+ DROP auditingEnabled;
diff --git a/resources/sql/autopatches/20190207.packages.04.xactions.php b/resources/sql/autopatches/20190207.packages.04.xactions.php
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20190207.packages.04.xactions.php
@@ -0,0 +1,41 @@
+<?php
+
+$table = new PhabricatorOwnersPackageTransaction();
+$conn = $table->establishConnection('w');
+$iterator = new LiskRawMigrationIterator($conn, $table->getTableName());
+
+// Migrate "Auditing State" transactions for Owners Packages from old values
+// (which were "0" or "1", as JSON integer literals, without quotes) to new
+// values (which are JSON strings, with quotes).
+
+foreach ($iterator as $row) {
+ if ($row['transactionType'] !== 'owners.auditing') {
+ continue;
+ }
+
+ $old_value = (int)$row['oldValue'];
+ $new_value = (int)$row['newValue'];
+
+ if (!$old_value) {
+ $old_value = 'none';
+ } else {
+ $old_value = 'audit';
+ }
+
+ if (!$new_value) {
+ $new_value = 'none';
+ } else {
+ $new_value = 'audit';
+ }
+
+ $old_value = phutil_json_encode($old_value);
+ $new_value = phutil_json_encode($new_value);
+
+ queryfx(
+ $conn,
+ 'UPDATE %R SET oldValue = %s, newValue = %s WHERE id = %d',
+ $table,
+ $old_value,
+ $new_value,
+ $row['id']);
+}
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -3668,6 +3668,7 @@
'PhabricatorOwnerPathQuery' => 'applications/owners/query/PhabricatorOwnerPathQuery.php',
'PhabricatorOwnersApplication' => 'applications/owners/application/PhabricatorOwnersApplication.php',
'PhabricatorOwnersArchiveController' => 'applications/owners/controller/PhabricatorOwnersArchiveController.php',
+ 'PhabricatorOwnersAuditRule' => 'applications/owners/constants/PhabricatorOwnersAuditRule.php',
'PhabricatorOwnersConfigOptions' => 'applications/owners/config/PhabricatorOwnersConfigOptions.php',
'PhabricatorOwnersConfiguredCustomField' => 'applications/owners/customfield/PhabricatorOwnersConfiguredCustomField.php',
'PhabricatorOwnersController' => 'applications/owners/controller/PhabricatorOwnersController.php',
@@ -9613,6 +9614,7 @@
'PhabricatorOwnerPathQuery' => 'Phobject',
'PhabricatorOwnersApplication' => 'PhabricatorApplication',
'PhabricatorOwnersArchiveController' => 'PhabricatorOwnersController',
+ 'PhabricatorOwnersAuditRule' => 'Phobject',
'PhabricatorOwnersConfigOptions' => 'PhabricatorApplicationConfigOptions',
'PhabricatorOwnersConfiguredCustomField' => array(
'PhabricatorOwnersCustomField',
diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php
--- a/src/applications/diffusion/controller/DiffusionBrowseController.php
+++ b/src/applications/diffusion/controller/DiffusionBrowseController.php
@@ -566,11 +566,8 @@
$name = idx($spec, 'name', $auto);
$item->addIcon('fa-code', $name);
- if ($package->getAuditingEnabled()) {
- $item->addIcon('fa-check', pht('Auditing Enabled'));
- } else {
- $item->addIcon('fa-ban', pht('No Auditing'));
- }
+ $rule = $package->newAuditingRule();
+ $item->addIcon($rule->getIconIcon(), $rule->getDisplayName());
if ($package->isArchived()) {
$item->setDisabled(true);
diff --git a/src/applications/owners/constants/PhabricatorOwnersAuditRule.php b/src/applications/owners/constants/PhabricatorOwnersAuditRule.php
new file mode 100644
--- /dev/null
+++ b/src/applications/owners/constants/PhabricatorOwnersAuditRule.php
@@ -0,0 +1,101 @@
+<?php
+
+final class PhabricatorOwnersAuditRule
+ extends Phobject {
+
+ const AUDITING_NONE = 'none';
+ const AUDITING_AUDIT = 'audit';
+
+ private $key;
+ private $spec;
+
+ public static function newFromState($key) {
+ $specs = self::newSpecifications();
+ $spec = idx($specs, $key, array());
+
+ $rule = new self();
+ $rule->key = $key;
+ $rule->spec = $spec;
+
+ return $rule;
+ }
+
+ public function getKey() {
+ return $this->key;
+ }
+
+ public function getDisplayName() {
+ return idx($this->spec, 'name', $this->key);
+ }
+
+ public function getIconIcon() {
+ return idx($this->spec, 'icon.icon');
+ }
+
+ public static function newSelectControlMap() {
+ $specs = self::newSpecifications();
+ return ipull($specs, 'name');
+ }
+
+ public static function getStorageValueFromAPIValue($value) {
+ $specs = self::newSpecifications();
+
+ $map = array();
+ foreach ($specs as $key => $spec) {
+ $deprecated = idx($spec, 'deprecated', array());
+ if (isset($deprecated[$value])) {
+ return $key;
+ }
+ }
+
+ return $value;
+ }
+
+ public static function getModernValueMap() {
+ $specs = self::newSpecifications();
+
+ $map = array();
+ foreach ($specs as $key => $spec) {
+ $map[$key] = pht('"%s"', $key);
+ }
+
+ return $map;
+ }
+
+ public static function getDeprecatedValueMap() {
+ $specs = self::newSpecifications();
+
+ $map = array();
+ foreach ($specs as $key => $spec) {
+ $deprecated_map = idx($spec, 'deprecated', array());
+ foreach ($deprecated_map as $deprecated_key => $label) {
+ $map[$deprecated_key] = $label;
+ }
+ }
+
+ return $map;
+ }
+
+ private static function newSpecifications() {
+ return array(
+ self::AUDITING_NONE => array(
+ 'name' => pht('No Auditing'),
+ 'icon.icon' => 'fa-ban',
+ 'deprecated' => array(
+ '' => pht('"" (empty string)'),
+ '0' => '"0"',
+ ),
+ ),
+ self::AUDITING_AUDIT => array(
+ 'name' => pht('Audit Commits'),
+ 'icon.icon' => 'fa-check',
+ 'deprecated' => array(
+ '1' => '"1"',
+ ),
+ ),
+ );
+ }
+
+
+
+}
diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php
--- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php
+++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php
@@ -194,12 +194,8 @@
$name = idx($spec, 'name', $auto);
$view->addProperty(pht('Auto Review'), $name);
- if ($package->getAuditingEnabled()) {
- $auditing = pht('Enabled');
- } else {
- $auditing = pht('Disabled');
- }
- $view->addProperty(pht('Auditing'), $auditing);
+ $rule = $package->newAuditingRule();
+ $view->addProperty(pht('Auditing'), $rule->getDisplayName());
$ignored = $package->getIgnoredPathAttributes();
$ignored = array_keys($ignored);
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
@@ -141,11 +141,7 @@
PhabricatorOwnersPackageAuditingTransaction::TRANSACTIONTYPE)
->setIsCopyable(true)
->setValue($object->getAuditingState())
- ->setOptions(
- array(
- PhabricatorOwnersPackage::AUDITING_NONE => pht('No Auditing'),
- PhabricatorOwnersPackage::AUDITING_AUDIT => pht('Audit Commits'),
- )),
+ ->setOptions(PhabricatorOwnersAuditRule::newSelectControlMap()),
id(new PhabricatorRemarkupEditField())
->setKey('description')
->setLabel(pht('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
@@ -13,7 +13,6 @@
PhabricatorNgramsInterface {
protected $name;
- protected $auditingEnabled;
protected $autoReview;
protected $description;
protected $status;
@@ -21,6 +20,7 @@
protected $editPolicy;
protected $dominion;
protected $properties = array();
+ protected $auditingState;
private $paths = self::ATTACHABLE;
private $owners = self::ATTACHABLE;
@@ -38,9 +38,6 @@
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';
@@ -58,7 +55,7 @@
PhabricatorOwnersDefaultEditCapability::CAPABILITY);
return id(new PhabricatorOwnersPackage())
- ->setAuditingEnabled(0)
+ ->setAuditingState(PhabricatorOwnersAuditRule::AUDITING_NONE)
->setAutoReview(self::AUTOREVIEW_NONE)
->setDominion(self::DOMINION_STRONG)
->setViewPolicy($view_policy)
@@ -129,7 +126,7 @@
self::CONFIG_COLUMN_SCHEMA => array(
'name' => 'sort',
'description' => 'text',
- 'auditingEnabled' => 'bool',
+ 'auditingState' => 'text32',
'status' => 'text32',
'autoReview' => 'text32',
'dominion' => 'text32',
@@ -567,12 +564,8 @@
return '/owners/package/'.$this->getID().'/';
}
- public function getAuditingState() {
- if ($this->getAuditingEnabled()) {
- return self::AUDITING_AUDIT;
- } else {
- return self::AUDITING_NONE;
- }
+ public function newAuditingRule() {
+ return PhabricatorOwnersAuditRule::newFromState($this->getAuditingState());
}
/* -( PhabricatorPolicyInterface )----------------------------------------- */
@@ -731,16 +724,11 @@
'label' => $review_label,
);
- $audit_value = $this->getAuditingState();
- if ($this->getAuditingEnabled()) {
- $audit_label = pht('Auditing Enabled');
- } else {
- $audit_label = pht('No Auditing');
- }
+ $audit_rule = $this->newAuditingRule();
$audit = array(
- 'value' => $audit_value,
- 'label' => $audit_label,
+ 'value' => $audit_rule->getKey(),
+ 'label' => $audit_rule->getDisplayName(),
);
$dominion_value = $this->getDominion();
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
@@ -6,35 +6,29 @@
const TRANSACTIONTYPE = 'owners.auditing';
public function generateOldValue($object) {
- return (int)$object->getAuditingEnabled();
+ return $object->getAuditingState();
}
public function generateNewValue($object, $value) {
- switch ($value) {
- case PhabricatorOwnersPackage::AUDITING_AUDIT:
- return 1;
- case '1':
- // TODO: Remove, deprecated.
- return 1;
- default:
- return 0;
- }
+ return PhabricatorOwnersAuditRule::getStorageValueFromAPIValue($value);
}
public function applyInternalEffects($object, $value) {
- $object->setAuditingEnabled($value);
+ $object->setAuditingState($value);
}
public function getTitle() {
- if ($this->getNewValue()) {
- return pht(
- '%s enabled auditing for this package.',
- $this->renderAuthor());
- } else {
- return pht(
- '%s disabled auditing for this package.',
- $this->renderAuthor());
- }
+ $old_value = $this->getOldValue();
+ $new_value = $this->getNewValue();
+
+ $old_rule = PhabricatorOwnersAuditRule::newFromState($old_value);
+ $new_rule = PhabricatorOwnersAuditRule::newFromState($new_value);
+
+ return pht(
+ '%s changed the audit rule for this package from %s to %s.',
+ $this->renderAuthor(),
+ $this->renderValue($old_rule->getDisplayName()),
+ $this->renderValue($new_rule->getDisplayName()));
}
public function validateTransactions($object, array $xactions) {
@@ -43,18 +37,8 @@
// 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)'),
- );
+ $modern_options = PhabricatorOwnersAuditRule::getModernValueMap();
+ $deprecated_options = PhabricatorOwnersAuditRule::getDeprecatedValueMap();
foreach ($xactions as $xaction) {
$new_value = $xaction->getNewValue();
diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
--- a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
+++ b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
@@ -133,7 +133,8 @@
$revision) {
// Don't trigger an audit if auditing isn't enabled for the package.
- if (!$package->getAuditingEnabled()) {
+ $rule = $package->newAuditingRule();
+ if ($rule->getKey() === PhabricatorOwnersAuditRule::AUDITING_NONE) {
return false;
}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Fri, May 24, 5:53 AM (3 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6272659
Default Alt Text
D20124.diff (14 KB)
Attached To
Mode
D20124: Prepare owners package audit rules to become more flexible
Attached
Detach File
Event Timeline
Log In to Comment