mirror of
https://gerrit.wikimedia.org/r/mediawiki/extensions/AbuseFilter.git
synced 2024-11-23 21:53:35 +00:00
Simplify action arrays
The current form is awkward. They're all like [ actionname => [ 'action' => actionname, 'parameters' => params ] ] This is greatly confusing since adds a nesting level, and just duplicates the actionname information (also, we actually never retrieve it from the internal array). Instead, change all of them to be [ actionname => params ] which is a lot shorter and clearer (and easier to handle). A similar case is handled in I8134ecc41fbecdbed99faf406e9e3ca91b6123b9 (see PS 8..10). Change-Id: I34c040dbeb3ab01158fb3db22496def6ccaf72d9
This commit is contained in:
Daimona Eaytoy
parent
6c6b5afe3b
commit
d8d4750e6a
|
|
@ -2452,12 +2452,12 @@ class AbuseFilter {
|
|||
}
|
||||
|
||||
// If we've activated the 'tag' option, check the arguments for validity.
|
||||
if ( !empty( $actions['tag'] ) ) {
|
||||
if ( count( $actions['tag']['parameters'] ) === 0 ) {
|
||||
if ( isset( $actions['tag'] ) ) {
|
||||
if ( count( $actions['tag'] ) === 0 ) {
|
||||
$validationStatus->error( 'tags-create-no-name' );
|
||||
return $validationStatus;
|
||||
}
|
||||
foreach ( $actions['tag']['parameters'] as $tag ) {
|
||||
foreach ( $actions['tag'] as $tag ) {
|
||||
$status = self::isAllowedTag( $tag );
|
||||
|
||||
if ( !$status->isGood() ) {
|
||||
|
|
@ -2470,17 +2470,17 @@ class AbuseFilter {
|
|||
}
|
||||
|
||||
// Warning and disallow message cannot be empty
|
||||
if ( !empty( $actions['warn'] ) && $actions['warn']['parameters'][0] === '' ) {
|
||||
if ( isset( $actions['warn'] ) && $actions['warn'][0] === '' ) {
|
||||
$validationStatus->error( 'abusefilter-edit-invalid-warn-message' );
|
||||
return $validationStatus;
|
||||
} elseif ( !empty( $actions['disallow'] ) && $actions['disallow']['parameters'][0] === '' ) {
|
||||
} elseif ( isset( $actions['disallow'] ) && $actions['disallow'][0] === '' ) {
|
||||
$validationStatus->error( 'abusefilter-edit-invalid-disallow-message' );
|
||||
return $validationStatus;
|
||||
}
|
||||
|
||||
// If 'throttle' is selected, check its parameters
|
||||
if ( !empty( $actions['throttle'] ) ) {
|
||||
$throttleCheck = self::checkThrottleParameters( $actions['throttle']['parameters'] );
|
||||
if ( isset( $actions['throttle'] ) ) {
|
||||
$throttleCheck = self::checkThrottleParameters( $actions['throttle'] );
|
||||
if ( $throttleCheck !== null ) {
|
||||
$validationStatus->error( $throttleCheck );
|
||||
return $validationStatus;
|
||||
|
|
@ -2524,10 +2524,7 @@ class AbuseFilter {
|
|||
$restrictions = $page->getConfig()->get( 'AbuseFilterRestrictions' );
|
||||
if ( count( array_intersect_key(
|
||||
array_filter( $restrictions ),
|
||||
array_merge(
|
||||
array_filter( $actions ),
|
||||
array_filter( $origActions )
|
||||
)
|
||||
array_merge( $actions, $origActions )
|
||||
) )
|
||||
&& !$page->getUser()->isAllowed( 'abusefilter-modify-restricted' )
|
||||
) {
|
||||
|
|
@ -2604,10 +2601,10 @@ class AbuseFilter {
|
|||
$actionsRows = [];
|
||||
foreach ( array_filter( $availableActions ) as $action => $_ ) {
|
||||
// Check if it's set
|
||||
$enabled = isset( $actions[$action] ) && (bool)$actions[$action];
|
||||
$enabled = isset( $actions[$action] );
|
||||
|
||||
if ( $enabled ) {
|
||||
$parameters = $actions[$action]['parameters'];
|
||||
$parameters = $actions[$action];
|
||||
if ( $action === 'throttle' && $parameters[0] === 'new' ) {
|
||||
// FIXME: Do we really need to keep the filter ID inside throttle parameters?
|
||||
// We'd save space, keep things simpler and avoid this hack. Note: if removing
|
||||
|
|
@ -2631,11 +2628,7 @@ class AbuseFilter {
|
|||
$afh_row[$afh_col] = $newRow[$af_col];
|
||||
}
|
||||
|
||||
$displayActions = [];
|
||||
foreach ( $actions as $action ) {
|
||||
$displayActions[$action['action']] = $action['parameters'];
|
||||
}
|
||||
$afh_row['afh_actions'] = serialize( $displayActions );
|
||||
$afh_row['afh_actions'] = serialize( $actions );
|
||||
|
||||
$afh_row['afh_changed_fields'] = implode( ',', $differences );
|
||||
|
||||
|
|
@ -2740,10 +2733,8 @@ class AbuseFilter {
|
|||
// They're both unset
|
||||
} elseif ( isset( $actions1[$action] ) && isset( $actions2[$action] ) ) {
|
||||
// They're both set. Double check needed, e.g. per T180194
|
||||
if ( array_diff( $actions1[$action]['parameters'],
|
||||
$actions2[$action]['parameters'] ) ||
|
||||
array_diff( $actions2[$action]['parameters'],
|
||||
$actions1[$action]['parameters'] ) ) {
|
||||
if ( array_diff( $actions1[$action], $actions2[$action] ) ||
|
||||
array_diff( $actions2[$action], $actions1[$action] ) ) {
|
||||
// Different parameters
|
||||
$differences[] = 'actions';
|
||||
}
|
||||
|
|
@ -2782,18 +2773,10 @@ class AbuseFilter {
|
|||
}
|
||||
|
||||
// Process actions
|
||||
$actions_raw = unserialize( $row->afh_actions );
|
||||
$actions_output = [];
|
||||
if ( is_array( $actions_raw ) ) {
|
||||
foreach ( $actions_raw as $action => $parameters ) {
|
||||
$actions_output[$action] = [
|
||||
'action' => $action,
|
||||
'parameters' => $parameters
|
||||
];
|
||||
}
|
||||
}
|
||||
$actionsRaw = unserialize( $row->afh_actions );
|
||||
$actionsOutput = is_array( $actionsRaw ) ? $actionsRaw : [];
|
||||
|
||||
return [ $af_row, $actions_output ];
|
||||
return [ $af_row, $actionsOutput ];
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -479,9 +479,7 @@ class AbuseFilterViewEdit extends AbuseFilterView {
|
|||
$output = '';
|
||||
|
||||
foreach ( $enabledActions as $action => $_ ) {
|
||||
Wikimedia\suppressWarnings();
|
||||
$params = $actions[$action]['parameters'];
|
||||
Wikimedia\restoreWarnings();
|
||||
$params = $actions[$action] ?? null;
|
||||
$output .= $this->buildConsequenceSelector(
|
||||
$action, $setActions[$action], $params, $row );
|
||||
}
|
||||
|
|
@ -492,7 +490,7 @@ class AbuseFilterViewEdit extends AbuseFilterView {
|
|||
/**
|
||||
* @param string $action The action to build an editor for
|
||||
* @param bool $set Whether or not the action is activated
|
||||
* @param array $parameters Action parameters
|
||||
* @param array|null $parameters Action parameters
|
||||
* @param stdClass $row abuse_filter row object
|
||||
* @return string|\OOUI\FieldLayout
|
||||
*/
|
||||
|
|
@ -1096,7 +1094,6 @@ class AbuseFilterViewEdit extends AbuseFilterView {
|
|||
}
|
||||
|
||||
// Load the actions
|
||||
$actions = [];
|
||||
$res = $dbr->select(
|
||||
'abuse_filter_action',
|
||||
[ 'afa_consequence', 'afa_parameters' ],
|
||||
|
|
@ -1104,12 +1101,10 @@ class AbuseFilterViewEdit extends AbuseFilterView {
|
|||
__METHOD__
|
||||
);
|
||||
|
||||
$actions = [];
|
||||
foreach ( $res as $actionRow ) {
|
||||
$thisAction = [];
|
||||
$thisAction['action'] = $actionRow->afa_consequence;
|
||||
$thisAction['parameters'] = array_filter( explode( "\n", $actionRow->afa_parameters ) );
|
||||
|
||||
$actions[$actionRow->afa_consequence] = $thisAction;
|
||||
$actions[$actionRow->afa_consequence] =
|
||||
array_filter( explode( "\n", $actionRow->afa_parameters ) );
|
||||
}
|
||||
|
||||
return [ $row, $actions ];
|
||||
|
|
@ -1153,7 +1148,7 @@ class AbuseFilterViewEdit extends AbuseFilterView {
|
|||
$data = FormatJson::decode( $import );
|
||||
|
||||
$importRow = $data->row;
|
||||
$actions = wfObjectToArray( $data->actions );
|
||||
$actions = get_object_vars( $data->actions );
|
||||
|
||||
$copy = [
|
||||
'af_public_comments',
|
||||
|
|
@ -1245,13 +1240,12 @@ class AbuseFilterViewEdit extends AbuseFilterView {
|
|||
}
|
||||
}
|
||||
|
||||
$thisAction = [ 'action' => $action, 'parameters' => $parameters ];
|
||||
$actions[$action] = $thisAction;
|
||||
$actions[$action] = $parameters;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
$row->af_actions = implode( ',', array_keys( array_filter( $actions ) ) );
|
||||
$row->af_actions = implode( ',', array_keys( $actions ) );
|
||||
|
||||
self::$mLoadedRow = $row;
|
||||
self::$mLoadedActions = $actions;
|
||||
|
|
|
|||
|
|
@ -246,7 +246,7 @@ class AbuseFilterSaveTest extends MediaWikiTestCase {
|
|||
$filter = self::$mParameters['id'];
|
||||
$viewEdit = self::getViewEdit( $filter );
|
||||
list( $newRow, $actions ) = $viewEdit->loadRequest( $filter );
|
||||
self::$mParameters['rowActions'] = implode( ',', array_keys( array_filter( $actions ) ) );
|
||||
self::$mParameters['rowActions'] = implode( ',', array_keys( $actions ) );
|
||||
|
||||
// Send data for validation and saving
|
||||
$status = AbuseFilter::saveFilter( $viewEdit, $filter, $newRow, $actions );
|
||||
|
|
|
|||
|
|
@ -521,10 +521,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
'af_group' => 'default'
|
||||
],
|
||||
[
|
||||
'disallow' => [
|
||||
'action' => 'disallow',
|
||||
'parameters' => []
|
||||
]
|
||||
'disallow' => []
|
||||
]
|
||||
],
|
||||
[
|
||||
|
|
@ -539,10 +536,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
'af_group' => 'flow'
|
||||
],
|
||||
[
|
||||
'disallow' => [
|
||||
'action' => 'disallow',
|
||||
'parameters' => []
|
||||
]
|
||||
'disallow' => []
|
||||
]
|
||||
],
|
||||
[
|
||||
|
|
@ -569,10 +563,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
'af_group' => 'default'
|
||||
],
|
||||
[
|
||||
'disallow' => [
|
||||
'action' => 'disallow',
|
||||
'parameters' => []
|
||||
]
|
||||
'disallow' => []
|
||||
]
|
||||
],
|
||||
[
|
||||
|
|
@ -587,10 +578,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
'af_group' => 'default'
|
||||
],
|
||||
[
|
||||
'disallow' => [
|
||||
'action' => 'disallow',
|
||||
'parameters' => []
|
||||
]
|
||||
'disallow' => []
|
||||
]
|
||||
],
|
||||
[]
|
||||
|
|
@ -608,10 +596,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
'af_group' => 'default'
|
||||
],
|
||||
[
|
||||
'disallow' => [
|
||||
'action' => 'disallow',
|
||||
'parameters' => []
|
||||
]
|
||||
'disallow' => []
|
||||
]
|
||||
],
|
||||
[
|
||||
|
|
@ -626,10 +611,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
'af_group' => 'default'
|
||||
],
|
||||
[
|
||||
'degroup' => [
|
||||
'action' => 'degroup',
|
||||
'parameters' => []
|
||||
]
|
||||
'degroup' => []
|
||||
]
|
||||
],
|
||||
[ 'actions' ]
|
||||
|
|
@ -647,10 +629,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
'af_group' => 'default'
|
||||
],
|
||||
[
|
||||
'disallow' => [
|
||||
'action' => 'disallow',
|
||||
'parameters' => []
|
||||
]
|
||||
'disallow' => []
|
||||
]
|
||||
],
|
||||
[
|
||||
|
|
@ -665,10 +644,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
'af_group' => 'flow'
|
||||
],
|
||||
[
|
||||
'blockautopromote' => [
|
||||
'action' => 'blockautopromote',
|
||||
'parameters' => []
|
||||
]
|
||||
'blockautopromote' => []
|
||||
]
|
||||
],
|
||||
[
|
||||
|
|
@ -696,10 +672,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
'af_group' => 'default'
|
||||
],
|
||||
[
|
||||
'disallow' => [
|
||||
'action' => 'disallow',
|
||||
'parameters' => []
|
||||
]
|
||||
'disallow' => []
|
||||
]
|
||||
],
|
||||
[
|
||||
|
|
@ -715,10 +688,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
],
|
||||
[
|
||||
'warn' => [
|
||||
'action' => 'warn',
|
||||
'parameters' => [
|
||||
'abusefilter-warning'
|
||||
]
|
||||
'abusefilter-warning'
|
||||
]
|
||||
]
|
||||
],
|
||||
|
|
@ -738,10 +708,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
],
|
||||
[
|
||||
'warn' => [
|
||||
'action' => 'warn',
|
||||
'parameters' => [
|
||||
'abusefilter-warning'
|
||||
]
|
||||
'abusefilter-warning'
|
||||
]
|
||||
]
|
||||
],
|
||||
|
|
@ -757,10 +724,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
'af_group' => 'default'
|
||||
],
|
||||
[
|
||||
'disallow' => [
|
||||
'action' => 'disallow',
|
||||
'parameters' => []
|
||||
]
|
||||
'disallow' => []
|
||||
]
|
||||
],
|
||||
[ 'actions' ]
|
||||
|
|
@ -779,10 +743,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
],
|
||||
[
|
||||
'warn' => [
|
||||
'action' => 'warn',
|
||||
'parameters' => [
|
||||
'abusefilter-warning'
|
||||
]
|
||||
'abusefilter-warning'
|
||||
]
|
||||
]
|
||||
],
|
||||
|
|
@ -799,15 +760,9 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
],
|
||||
[
|
||||
'warn' => [
|
||||
'action' => 'warn',
|
||||
'parameters' => [
|
||||
'abusefilter-my-best-warning'
|
||||
]
|
||||
'abusefilter-my-best-warning'
|
||||
],
|
||||
'degroup' => [
|
||||
'action' => 'degroup',
|
||||
'parameters' => []
|
||||
]
|
||||
'degroup' => []
|
||||
]
|
||||
],
|
||||
[ 'actions' ]
|
||||
|
|
@ -826,10 +781,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
],
|
||||
[
|
||||
'warn' => [
|
||||
'action' => 'warn',
|
||||
'parameters' => [
|
||||
'abusefilter-warning'
|
||||
]
|
||||
'abusefilter-warning'
|
||||
]
|
||||
]
|
||||
],
|
||||
|
|
@ -846,10 +798,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
],
|
||||
[
|
||||
'warn' => [
|
||||
'action' => 'warn',
|
||||
'parameters' => [
|
||||
'abusefilter-my-best-warning'
|
||||
]
|
||||
'abusefilter-my-best-warning'
|
||||
]
|
||||
]
|
||||
],
|
||||
|
|
@ -874,10 +823,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
],
|
||||
[
|
||||
'warn' => [
|
||||
'action' => 'warn',
|
||||
'parameters' => [
|
||||
'abusefilter-beautiful-warning'
|
||||
]
|
||||
'abusefilter-beautiful-warning'
|
||||
]
|
||||
]
|
||||
],
|
||||
|
|
@ -894,10 +840,7 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
],
|
||||
[
|
||||
'warn' => [
|
||||
'action' => 'warn',
|
||||
'parameters' => [
|
||||
'abusefilter-my-best-warning'
|
||||
]
|
||||
'abusefilter-my-best-warning'
|
||||
]
|
||||
]
|
||||
],
|
||||
|
|
@ -966,14 +909,8 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
'af_enabled' => 1
|
||||
],
|
||||
[
|
||||
'degroup' => [
|
||||
'action' => 'degroup',
|
||||
'parameters' => []
|
||||
],
|
||||
'disallow' => [
|
||||
'action' => 'disallow',
|
||||
'parameters' => []
|
||||
]
|
||||
'degroup' => [],
|
||||
'disallow' => []
|
||||
]
|
||||
]
|
||||
],
|
||||
|
|
@ -1014,16 +951,10 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
],
|
||||
[
|
||||
'warn' => [
|
||||
'action' => 'warn',
|
||||
'parameters' => [
|
||||
'abusefilter-warning',
|
||||
''
|
||||
]
|
||||
'abusefilter-warning',
|
||||
''
|
||||
],
|
||||
'disallow' => [
|
||||
'action' => 'disallow',
|
||||
'parameters' => []
|
||||
]
|
||||
'disallow' => []
|
||||
]
|
||||
]
|
||||
],
|
||||
|
|
@ -1069,23 +1000,14 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
],
|
||||
[
|
||||
'warn' => [
|
||||
'action' => 'warn',
|
||||
'parameters' => [
|
||||
'abusefilter-warning',
|
||||
''
|
||||
]
|
||||
],
|
||||
'disallow' => [
|
||||
'action' => 'disallow',
|
||||
'parameters' => []
|
||||
],
|
||||
'disallow' => [],
|
||||
'block' => [
|
||||
'action' => 'block',
|
||||
'parameters' => [
|
||||
'blocktalk',
|
||||
'8 hours',
|
||||
'infinity'
|
||||
]
|
||||
'blocktalk',
|
||||
'8 hours',
|
||||
'infinity'
|
||||
]
|
||||
]
|
||||
]
|
||||
|
|
@ -1131,19 +1053,13 @@ class AbuseFilterTest extends MediaWikiTestCase {
|
|||
],
|
||||
[
|
||||
'throttle' => [
|
||||
'action' => 'throttle',
|
||||
'parameters' => [
|
||||
'131',
|
||||
'3,60',
|
||||
'user'
|
||||
]
|
||||
'131',
|
||||
'3,60',
|
||||
'user'
|
||||
],
|
||||
'tag' => [
|
||||
'action' => 'tag',
|
||||
'parameters' => [
|
||||
'mytag',
|
||||
'yourtag'
|
||||
]
|
||||
'mytag',
|
||||
'yourtag'
|
||||
]
|
||||
]
|
||||
]
|
||||
|
|
|
|||
Loading…
Reference in a new issue