1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-19 20:10:55 +01:00

Add "does not match regexp" to Herald

Summary:
Fixes T10330.

  - Anywhere we support "matches regexp", also allow "does not match regexp". Although you can sometimes write a clever negative regexp, these rules are better expressed with "does not match <simple regexp>" anyway, and sometimes no regexp will work.
  - Always allow "does not contain" when we support "contains".
  - Fix some JS issues with certain rules affecting custom fields.

Test Plan:
  - Wrote an "Affected files do not match regexp" rule that required every diff to touch "MANUALCHANGELOG.md".
  - Tried to diff without the file; rejected.
  - Tried to diff with the file; accepted.
  - Wrote a bunch of "contains" and "does not contain" rules against text fields and custom fields, then edited tasks to trigger/observe them.
  - Swapped the editor into custom text, user, remarkup, etc fields, no more JS errors.

{F1105172}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10330

Differential Revision: https://secure.phabricator.com/D15254
This commit is contained in:
epriestley 2016-02-11 15:17:36 -08:00
parent 8661170819
commit 8934dee543
10 changed files with 78 additions and 8 deletions

View file

@ -14,6 +14,7 @@ abstract class HeraldAdapter extends Phobject {
const CONDITION_IS_ME = 'me';
const CONDITION_IS_NOT_ME = '!me';
const CONDITION_REGEXP = 'regexp';
const CONDITION_NOT_REGEXP = '!regexp';
const CONDITION_RULE = 'conditions';
const CONDITION_NOT_RULE = '!conditions';
const CONDITION_EXISTS = 'exists';
@ -322,6 +323,7 @@ abstract class HeraldAdapter extends Phobject {
self::CONDITION_IS_ME => pht('is myself'),
self::CONDITION_IS_NOT_ME => pht('is not myself'),
self::CONDITION_REGEXP => pht('matches regexp'),
self::CONDITION_NOT_REGEXP => pht('does not match regexp'),
self::CONDITION_RULE => pht('matches:'),
self::CONDITION_NOT_RULE => pht('does not match:'),
self::CONDITION_EXISTS => pht('exists'),
@ -364,16 +366,18 @@ abstract class HeraldAdapter extends Phobject {
switch ($condition_type) {
case self::CONDITION_CONTAINS:
// "Contains" can take an array of strings, as in "Any changed
// filename" for diffs.
case self::CONDITION_NOT_CONTAINS:
// "Contains and "does not contain" can take an array of strings, as in
// "Any changed filename" for diffs.
$result_if_match = ($condition_type == self::CONDITION_CONTAINS);
foreach ((array)$field_value as $value) {
if (stripos($value, $condition_value) !== false) {
return true;
return $result_if_match;
}
}
return false;
case self::CONDITION_NOT_CONTAINS:
return (stripos($field_value, $condition_value) === false);
return !$result_if_match;
case self::CONDITION_IS:
return ($field_value == $condition_value);
case self::CONDITION_IS_NOT:
@ -427,6 +431,9 @@ abstract class HeraldAdapter extends Phobject {
case self::CONDITION_NEVER:
return false;
case self::CONDITION_REGEXP:
case self::CONDITION_NOT_REGEXP:
$result_if_match = ($condition_type == self::CONDITION_REGEXP);
foreach ((array)$field_value as $value) {
// We add the 'S' flag because we use the regexp multiple times.
// It shouldn't cause any troubles if the flag is already there
@ -437,10 +444,10 @@ abstract class HeraldAdapter extends Phobject {
pht('Regular expression is not valid!'));
}
if ($result) {
return true;
return $result_if_match;
}
}
return false;
return !$result_if_match;
case self::CONDITION_REGEXP_PAIR:
// Match a JSON-encoded pair of regular expressions against a
// dictionary. The first regexp must match the dictionary key, and the
@ -509,6 +516,7 @@ abstract class HeraldAdapter extends Phobject {
switch ($condition_type) {
case self::CONDITION_REGEXP:
case self::CONDITION_NOT_REGEXP:
$ok = @preg_match($condition_value, '');
if ($ok === false) {
throw new HeraldInvalidConditionException(

View file

@ -47,6 +47,7 @@ abstract class HeraldField extends Phobject {
HeraldAdapter::CONDITION_IS,
HeraldAdapter::CONDITION_IS_NOT,
HeraldAdapter::CONDITION_REGEXP,
HeraldAdapter::CONDITION_NOT_REGEXP,
);
case self::STANDARD_PHID:
return array(
@ -76,12 +77,16 @@ abstract class HeraldField extends Phobject {
case self::STANDARD_TEXT_LIST:
return array(
HeraldAdapter::CONDITION_CONTAINS,
HeraldAdapter::CONDITION_NOT_CONTAINS,
HeraldAdapter::CONDITION_REGEXP,
HeraldAdapter::CONDITION_NOT_REGEXP,
);
case self::STANDARD_TEXT_MAP:
return array(
HeraldAdapter::CONDITION_CONTAINS,
HeraldAdapter::CONDITION_NOT_CONTAINS,
HeraldAdapter::CONDITION_REGEXP,
HeraldAdapter::CONDITION_NOT_REGEXP,
HeraldAdapter::CONDITION_REGEXP_PAIR,
);
}

View file

@ -1446,5 +1446,19 @@ abstract class PhabricatorCustomField extends Phobject {
return null;
}
public function getHeraldFieldStandardType() {
if ($this->proxy) {
return $this->proxy->getHeraldFieldStandardType();
}
return null;
}
public function getHeraldDatasource() {
if ($this->proxy) {
return $this->proxy->getHeraldDatasource();
}
return null;
}
}

View file

@ -65,8 +65,20 @@ final class PhabricatorCustomFieldHeraldField extends HeraldField {
return $this->getCustomField()->getHeraldFieldConditions();
}
protected function getHeraldFieldStandardType() {
return $this->getCustomField()->getHeraldFieldStandardType();
}
public function getHeraldFieldValueType($condition) {
if ($this->getHeraldFieldStandardType()) {
return parent::getHeraldFieldValueType($condition);
}
return $this->getCustomField()->getHeraldFieldValueType($condition);
}
protected function getDatasource() {
return $this->getCustomField()->getHeraldDatasource();
}
}

View file

@ -129,6 +129,10 @@ final class PhabricatorStandardCustomFieldBool
);
}
public function getHeraldFieldStandardType() {
return HeraldField::STANDARD_BOOL;
}
protected function getHTTPParameterType() {
return new AphrontBoolHTTPParameterType();
}

View file

@ -77,9 +77,14 @@ final class PhabricatorStandardCustomFieldLink
HeraldAdapter::CONDITION_IS,
HeraldAdapter::CONDITION_IS_NOT,
HeraldAdapter::CONDITION_REGEXP,
HeraldAdapter::CONDITION_NOT_REGEXP,
);
}
public function getHeraldFieldStandardType() {
return HeraldField::STANDARD_TEXT;
}
protected function getHTTPParameterType() {
return new AphrontStringHTTPParameterType();
}

View file

@ -241,6 +241,10 @@ abstract class PhabricatorStandardCustomFieldPHIDs
);
}
public function getHeraldFieldStandardType() {
return HeraldField::STANDARD_PHID_NULLABLE;
}
public function getHeraldFieldValue() {
// If the field has a `null` value, make sure we hand an `array()` to
// Herald.

View file

@ -92,9 +92,14 @@ final class PhabricatorStandardCustomFieldRemarkup
HeraldAdapter::CONDITION_IS,
HeraldAdapter::CONDITION_IS_NOT,
HeraldAdapter::CONDITION_REGEXP,
HeraldAdapter::CONDITION_NOT_REGEXP,
);
}
public function getHeraldFieldStandardType() {
return HeraldField::STANDARD_TEXT;
}
protected function getHTTPParameterType() {
return new AphrontStringHTTPParameterType();
}

View file

@ -60,9 +60,14 @@ final class PhabricatorStandardCustomFieldText
HeraldAdapter::CONDITION_IS,
HeraldAdapter::CONDITION_IS_NOT,
HeraldAdapter::CONDITION_REGEXP,
HeraldAdapter::CONDITION_NOT_REGEXP,
);
}
public function getHeraldFieldStandardType() {
return HeraldField::STANDARD_TEXT;
}
protected function getHTTPParameterType() {
return new AphrontStringHTTPParameterType();
}

View file

@ -44,4 +44,12 @@ abstract class PhabricatorStandardCustomFieldTokenizer
->setDatasource($this->getDatasource());
}
public function getHeraldFieldStandardType() {
return HeraldField::STANDARD_PHID_LIST;
}
public function getHeraldDatasource() {
return $this->getDatasource();
}
}