mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-22 14:52:41 +01:00
Allow Owners packages to be configured to ignore generated paths in Differential
Summary: Depends on D19427. Ref T13130. See PHI251. Support configuring owners packages so they ignore generated paths. This is still a little rough. A couple limitations: - It's hard to figure out how to use this control if you don't know what it's for, but we don't currently have a "CheckboxesEditField". I may add that soon. - The attribute ignore list doesn't apply to Diffusion, only Differential, which isn't obvious. I'll either try to make it work in Diffusion or note this somewhere. - No documentation yet (which could mitigate the other two issues a bit). But the actual behavior seems to work fine. Test Plan: - Set a package to ignore paths with the "generated" attribute. Saw the package stop matching generated paths in Differential. - Removed the attribute from the ignore list. - Tried to set invalid attributes, got sensible errors. - Queried a package with Conduit, got the ignored attribute list. Reviewers: amckinley Reviewed By: amckinley Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13130 Differential Revision: https://secure.phabricator.com/D19428
This commit is contained in:
parent
dc510354c3
commit
4a98e0ff65
7 changed files with 145 additions and 2 deletions
|
@ -0,0 +1,2 @@
|
|||
ALTER TABLE {$NAMESPACE}_owners.owners_package
|
||||
ADD properties LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT};
|
2
resources/sql/autopatches/20180504.owners.04.default.sql
Normal file
2
resources/sql/autopatches/20180504.owners.04.default.sql
Normal file
|
@ -0,0 +1,2 @@
|
|||
UPDATE {$NAMESPACE}_owners.owners_package
|
||||
SET properties = '{}' WHERE properties = '';
|
|
@ -3572,6 +3572,7 @@ phutil_register_library_map(array(
|
|||
'PhabricatorOwnersPackageFerretEngine' => 'applications/owners/search/PhabricatorOwnersPackageFerretEngine.php',
|
||||
'PhabricatorOwnersPackageFulltextEngine' => 'applications/owners/search/PhabricatorOwnersPackageFulltextEngine.php',
|
||||
'PhabricatorOwnersPackageFunctionDatasource' => 'applications/owners/typeahead/PhabricatorOwnersPackageFunctionDatasource.php',
|
||||
'PhabricatorOwnersPackageIgnoredTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageIgnoredTransaction.php',
|
||||
'PhabricatorOwnersPackageNameNgrams' => 'applications/owners/storage/PhabricatorOwnersPackageNameNgrams.php',
|
||||
'PhabricatorOwnersPackageNameTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageNameTransaction.php',
|
||||
'PhabricatorOwnersPackageOwnerDatasource' => 'applications/owners/typeahead/PhabricatorOwnersPackageOwnerDatasource.php',
|
||||
|
@ -9321,6 +9322,7 @@ phutil_register_library_map(array(
|
|||
'PhabricatorOwnersPackageFerretEngine' => 'PhabricatorFerretEngine',
|
||||
'PhabricatorOwnersPackageFulltextEngine' => 'PhabricatorFulltextEngine',
|
||||
'PhabricatorOwnersPackageFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
|
||||
'PhabricatorOwnersPackageIgnoredTransaction' => 'PhabricatorOwnersPackageTransactionType',
|
||||
'PhabricatorOwnersPackageNameNgrams' => 'PhabricatorSearchNgrams',
|
||||
'PhabricatorOwnersPackageNameTransaction' => 'PhabricatorOwnersPackageTransactionType',
|
||||
'PhabricatorOwnersPackageOwnerDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
|
||||
|
|
|
@ -201,6 +201,16 @@ final class PhabricatorOwnersDetailController
|
|||
}
|
||||
$view->addProperty(pht('Auditing'), $auditing);
|
||||
|
||||
$ignored = $package->getIgnoredPathAttributes();
|
||||
$ignored = array_keys($ignored);
|
||||
if ($ignored) {
|
||||
$ignored = implode(', ', $ignored);
|
||||
} else {
|
||||
$ignored = phutil_tag('em', array(), pht('None'));
|
||||
}
|
||||
|
||||
$view->addProperty(pht('Ignored Attributes'), $ignored);
|
||||
|
||||
$description = $package->getDescription();
|
||||
if (strlen($description)) {
|
||||
$description = new PHUIRemarkupView($viewer, $description);
|
||||
|
|
|
@ -162,6 +162,13 @@ EOTEXT
|
|||
->setIsConduitOnly(true)
|
||||
->setValue($object->getStatus())
|
||||
->setOptions($object->getStatusNameMap()),
|
||||
id(new PhabricatorStringListEditField())
|
||||
->setKey('ignored')
|
||||
->setLabel(pht('Ignored Attributes'))
|
||||
->setDescription(pht('Ignore paths with any of these attributes.'))
|
||||
->setTransactionType(
|
||||
PhabricatorOwnersPackageIgnoredTransaction::TRANSACTIONTYPE)
|
||||
->setValue(array_keys($object->getIgnoredPathAttributes())),
|
||||
id(new PhabricatorConduitEditField())
|
||||
->setKey('paths.set')
|
||||
->setLabel(pht('Paths'))
|
||||
|
|
|
@ -20,6 +20,7 @@ final class PhabricatorOwnersPackage
|
|||
protected $viewPolicy;
|
||||
protected $editPolicy;
|
||||
protected $dominion;
|
||||
protected $properties = array();
|
||||
|
||||
private $paths = self::ATTACHABLE;
|
||||
private $owners = self::ATTACHABLE;
|
||||
|
@ -40,6 +41,8 @@ final class PhabricatorOwnersPackage
|
|||
const DOMINION_STRONG = 'strong';
|
||||
const DOMINION_WEAK = 'weak';
|
||||
|
||||
const PROPERTY_IGNORED = 'ignored';
|
||||
|
||||
public static function initializeNewPackage(PhabricatorUser $actor) {
|
||||
$app = id(new PhabricatorApplicationQuery())
|
||||
->setViewer($actor)
|
||||
|
@ -117,6 +120,9 @@ final class PhabricatorOwnersPackage
|
|||
// This information is better available from the history table.
|
||||
self::CONFIG_TIMESTAMPS => false,
|
||||
self::CONFIG_AUX_PHID => true,
|
||||
self::CONFIG_SERIALIZATION => array(
|
||||
'properties' => self::SERIALIZATION_JSON,
|
||||
),
|
||||
self::CONFIG_COLUMN_SCHEMA => array(
|
||||
'name' => 'sort',
|
||||
'description' => 'text',
|
||||
|
@ -137,8 +143,25 @@ final class PhabricatorOwnersPackage
|
|||
}
|
||||
|
||||
public function getMustMatchUngeneratedPaths() {
|
||||
// TODO: For now, there's no way to actually configure this.
|
||||
return false;
|
||||
$ignore_attributes = $this->getIgnoredPathAttributes();
|
||||
return !empty($ignore_attributes['generated']);
|
||||
}
|
||||
|
||||
public function getPackageProperty($key, $default = null) {
|
||||
return idx($this->properties, $key, $default);
|
||||
}
|
||||
|
||||
public function setPackageProperty($key, $value) {
|
||||
$this->properties[$key] = $value;
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function getIgnoredPathAttributes() {
|
||||
return $this->getPackageProperty(self::PROPERTY_IGNORED, array());
|
||||
}
|
||||
|
||||
public function setIgnoredPathAttributes(array $attributes) {
|
||||
return $this->setPackageProperty(self::PROPERTY_IGNORED, $attributes);
|
||||
}
|
||||
|
||||
public function loadOwners() {
|
||||
|
@ -679,6 +702,10 @@ final class PhabricatorOwnersPackage
|
|||
->setKey('dominion')
|
||||
->setType('map<string, wild>')
|
||||
->setDescription(pht('Dominion setting information.')),
|
||||
id(new PhabricatorConduitSearchFieldSpecification())
|
||||
->setKey('ignored')
|
||||
->setType('map<string, wild>')
|
||||
->setDescription(pht('Ignored attribute information.')),
|
||||
);
|
||||
}
|
||||
|
||||
|
@ -732,6 +759,13 @@ final class PhabricatorOwnersPackage
|
|||
'short' => $dominion_short,
|
||||
);
|
||||
|
||||
// Force this to always emit as a JSON object even if empty, never as
|
||||
// a JSON list.
|
||||
$ignored = $this->getIgnoredPathAttributes();
|
||||
if (!$ignored) {
|
||||
$ignored = (object)array();
|
||||
}
|
||||
|
||||
return array(
|
||||
'name' => $this->getName(),
|
||||
'description' => $this->getDescription(),
|
||||
|
@ -740,6 +774,7 @@ final class PhabricatorOwnersPackage
|
|||
'review' => $review,
|
||||
'audit' => $audit,
|
||||
'dominion' => $dominion,
|
||||
'ignored' => $ignored,
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
@ -0,0 +1,85 @@
|
|||
<?php
|
||||
|
||||
final class PhabricatorOwnersPackageIgnoredTransaction
|
||||
extends PhabricatorOwnersPackageTransactionType {
|
||||
|
||||
const TRANSACTIONTYPE = 'owners.ignored';
|
||||
|
||||
public function generateOldValue($object) {
|
||||
return $object->getIgnoredPathAttributes();
|
||||
}
|
||||
|
||||
public function generateNewValue($object, $value) {
|
||||
return array_fill_keys($value, true);
|
||||
}
|
||||
|
||||
public function applyInternalEffects($object, $value) {
|
||||
$object->setIgnoredPathAttributes($value);
|
||||
}
|
||||
|
||||
public function getTitle() {
|
||||
$old = array_keys($this->getOldValue());
|
||||
$new = array_keys($this->getNewValue());
|
||||
|
||||
$add = array_diff($new, $old);
|
||||
$rem = array_diff($old, $new);
|
||||
|
||||
$all_n = new PhutilNumber(count($add) + count($rem));
|
||||
$add_n = phutil_count($add);
|
||||
$rem_n = phutil_count($rem);
|
||||
|
||||
if ($new && $old) {
|
||||
return pht(
|
||||
'%s changed %s ignored attribute(s), added %s: %s; removed %s: %s.',
|
||||
$this->renderAuthor(),
|
||||
$all_n,
|
||||
$add_n,
|
||||
$this->renderValueList($add),
|
||||
$rem_n,
|
||||
$this->renderValueList($rem));
|
||||
} else if ($new) {
|
||||
return pht(
|
||||
'%s changed %s ignored attribute(s), added %s: %s.',
|
||||
$this->renderAuthor(),
|
||||
$all_n,
|
||||
$add_n,
|
||||
$this->rendervalueList($add));
|
||||
} else {
|
||||
return pht(
|
||||
'%s changed %s ignored attribute(s), removed %s: %s.',
|
||||
$this->renderAuthor(),
|
||||
$all_n,
|
||||
$rem_n,
|
||||
$this->rendervalueList($rem));
|
||||
}
|
||||
}
|
||||
|
||||
public function validateTransactions($object, array $xactions) {
|
||||
$errors = array();
|
||||
|
||||
$valid_attributes = array(
|
||||
'generated' => true,
|
||||
);
|
||||
|
||||
foreach ($xactions as $xaction) {
|
||||
$new = $xaction->getNewValue();
|
||||
|
||||
foreach ($new as $attribute) {
|
||||
if (isset($valid_attributes[$attribute])) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$errors[] = $this->newInvalidError(
|
||||
pht(
|
||||
'Changeset attribute "%s" is not valid. Valid changeset '.
|
||||
'attributes are: %s.',
|
||||
$attribute,
|
||||
implode(', ', array_keys($valid_attributes))),
|
||||
$xaction);
|
||||
}
|
||||
}
|
||||
|
||||
return $errors;
|
||||
}
|
||||
|
||||
}
|
Loading…
Reference in a new issue