r46091 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r46090‎ | r46091 | r46092 >
Date:19:23, 23 January 2009
Author:werdna
Status:deferred
Tags:
Comment:
Remove dependency on change-tagging being branch-merged to trunk for now, and fix a few miscellaneous related bugs that came up in final testing.
Modified paths:
  • /trunk/extensions/AbuseFilter/AbuseFilter.class.php (modified) (history)
  • /trunk/extensions/AbuseFilter/AbuseFilter.hooks.php (modified) (history)
  • /trunk/extensions/AbuseFilter/AbuseFilter.php (modified) (history)
  • /trunk/extensions/AbuseFilter/Views/AbuseFilterViewEdit.php (modified) (history)

Diff [purge]

Index: trunk/extensions/AbuseFilter/AbuseFilter.php
——@@ -49,8 +49,8 @@
5050 $wgHooks['ArticleDelete'][] = 'AbuseFilterHooks::onArticleDelete';
5151 $wgHooks['LoadExtensionSchemaUpdates'][] = 'AbuseFilterHooks::onSchemaUpdate';
5252 $wgHooks['AbortDeleteQueueNominate'][] = 'AbuseFilterHooks::onAbortDeleteQueueNominate';
53 -$wgHooks['RecentChange_save'][] = 'AbuseFilterHooks::onRecentChangeSave';
54 -$wgHooks['ListDefinedTags'][] = 'AbuseFilterHooks::onListDefinedTags';
 53+// $wgHooks['RecentChange_save'][] = 'AbuseFilterHooks::onRecentChangeSave';
 54+// $wgHooks['ListDefinedTags'][] = 'AbuseFilterHooks::onListDefinedTags';
5555
5656 $wgAvailableRights[] = 'abusefilter-modify';
5757 $wgAvailableRights[] = 'abusefilter-log-detail';
——@@ -58,7 +58,7 @@
5959 $wgAvailableRights[] = 'abusefilter-log';
6060 $wgAvailableRights[] = 'abusefilter-private';
6161
62 -$wgAbuseFilterAvailableActions = array( 'flag', 'throttle', 'warn', 'disallow', 'blockautopromote', 'block', 'degroup', 'rangeblock', 'tag' );
 62+$wgAbuseFilterAvailableActions = array( 'flag', 'throttle', 'warn', 'disallow', 'blockautopromote', 'block', 'degroup', 'rangeblock' /*, 'tag' Disabled for now to avoid trunk changes. */ );
6363
6464 // Conditions take about 4ms to check, so 100 conditions would take 400ms
6565 // Currently, has no effect.
Index: trunk/extensions/AbuseFilter/Views/AbuseFilterViewEdit.php
——@@ -109,7 +109,7 @@
110110
111111 // Do the update
112112 $dbw->insert( 'abuse_filter_history', $afh_row, __METHOD__ );
113 - $dbw->delete( 'abuse_filter_action', array( 'afa_filter' => $filter, 'afa_consequence' => $deadActions ), __METHOD__ );
 113+ $dbw->delete( 'abuse_filter_action', array( 'afa_filter' => $filter ), __METHOD__ );
114114 $dbw->insert( 'abuse_filter_action', $actionsRows, __METHOD__ );
115115
116116 $dbw->commit();
——@@ -293,70 +293,80 @@
294294
295295 $output = '';
296296
297 - // Special case: flagging - always on.
298 - $checkbox = Xml::checkLabel( wfMsg( 'abusefilter-edit-action-flag' ), 'wpFilterActionFlag', 'wpFilterActionFlag', true, array( 'disabled' => '1' ) );
299 - $output .= Xml::tags( 'p', null, $checkbox );
300 -
301 - // Special case: throttling
302 - $throttleSettings = Xml::checkLabel( wfMsg( 'abusefilter-edit-action-throttle' ), 'wpFilterActionThrottle', 'wpFilterActionThrottle', $setActions['throttle'] );
303 - $throttleFields = array();
304 -
305 - if ($setActions['throttle']) {
306 - array_shift( $actions['throttle']['parameters'] );
307 - $throttleRate = explode(',',$actions['throttle']['parameters'][0]);
308 - $throttleCount = $throttleRate[0];
309 - $throttlePeriod = $throttleRate[1];
310 -
311 - $throttleGroups = implode("\n", array_slice($actions['throttle']['parameters'], 1 ) );
312 - } else {
313 - $throttleCount = 3;
314 - $throttlePeriod = 60;
315 -
316 - $throttleGroups = "user\n";
 297+ foreach( $wgAbuseFilterAvailableActions as $action ) {
 298+ $output .= $this->buildConsequenceSelector( $action, $setActions[$action], @$actions[$action]['parameters'] );
317299 }
318300
319 - $throttleFields['abusefilter-edit-throttle-count'] = Xml::input( 'wpFilterThrottleCount', 20, $throttleCount );
320 - $throttleFields['abusefilter-edit-throttle-period'] = wfMsgExt( 'abusefilter-edit-throttle-seconds', array( 'parseinline', 'replaceafter' ), array(Xml::input( 'wpFilterThrottlePeriod', 20, $throttlePeriod ) ) );
321 - $throttleFields['abusefilter-edit-throttle-groups'] = Xml::textarea( 'wpFilterThrottleGroups', $throttleGroups."\n" );
322 - $throttleSettings .= Xml::buildForm( $throttleFields );
323 - $output .= Xml::tags( 'p', null, $throttleSettings );
 301+ return $output;
 302+ }
324303
325 - // Special case: Warning
326 - $checkbox = Xml::checkLabel( wfMsg( 'abusefilter-edit-action-warn' ), 'wpFilterActionWarn', 'wpFilterActionWarn', $setActions['warn'] );
327 - $output .= Xml::tags( 'p', null, $checkbox );
 304+ function buildConsequenceSelector( $action, $set, $parameters ) {
 305+ global $wgAbuseFilterAvailableActions;
328306
329 - $warnMsg = empty($setActions['warn']) ? 'abusefilter-warning' : $actions['warn']['parameters'][0];
330 - $warnFields['abusefilter-edit-warn-message'] = Xml::input( 'wpFilterWarnMessage', 45, $warnMsg );
331 - $output .= Xml::tags( 'p', null, Xml::buildForm( $warnFields ) );
332 -
333 - // Special case: tagging
334 - if ($setActions['tag']) {
335 - $tags = $actions['tag']['parameters'];
336 - } else {
337 - $tags = array();
 307+ if ( !in_array( $action, $wgAbuseFilterAvailableActions ) ) {
 308+ return;
338309 }
 310+
 311+ switch( $action ) {
 312+ case 'throttle':
 313+ $throttleSettings = Xml::checkLabel( wfMsg( 'abusefilter-edit-action-throttle' ), 'wpFilterActionThrottle', 'wpFilterActionThrottle', $set );
 314+ $throttleFields = array();
339315
340 - $checkbox = Xml::checkLabel( wfMsg('abusefilter-edit-action-tag'), 'wpFilterActionTag', 'wpFilterActionTag', $setActions['tag'] );
341 - $output .= Xml::tags( 'p', null, $checkbox );
 316+ if ($set) {
 317+ array_shift( $parameters );
 318+ $throttleRate = explode(',', $parameters[0]);
 319+ $throttleCount = $throttleRate[0];
 320+ $throttlePeriod = $throttleRate[1];
342321
343 - $tagFields['abusefilter-edit-tag-tag'] = Xml::textarea( 'wpFilterTags', implode( "\n", $tags ) );
344 - $output .= Xml::tags( 'p', null, Xml::buildForm( $tagFields ) );
 322+ $throttleGroups = implode("\n", array_slice($parameters, 1 ) );
 323+ } else {
 324+ $throttleCount = 3;
 325+ $throttlePeriod = 60;
345326
346 - // The remainder are just toggles
347 - $remainingActions = array_diff( $wgAbuseFilterAvailableActions, array( 'flag', 'throttle', 'warn', 'tag' ) );
 327+ $throttleGroups = "user\n";
 328+ }
348329
349 - foreach( $remainingActions as $action ) {
350 - $message = 'abusefilter-edit-action-'.$action;
351 - $form_field = 'wpFilterAction' . ucfirst($action);
352 - $status = $setActions[$action];
 330+ $throttleFields['abusefilter-edit-throttle-count'] = Xml::input( 'wpFilterThrottleCount', 20, $throttleCount );
 331+ $throttleFields['abusefilter-edit-throttle-period'] = wfMsgExt( 'abusefilter-edit-throttle-seconds', array( 'parseinline', 'replaceafter' ), array(Xml::input( 'wpFilterThrottlePeriod', 20, $throttlePeriod ) ) );
 332+ $throttleFields['abusefilter-edit-throttle-groups'] = Xml::textarea( 'wpFilterThrottleGroups', $throttleGroups."\n" );
 333+ $throttleSettings .= Xml::buildForm( $throttleFields );
 334+ return Xml::tags( 'p', null, $throttleSettings );
 335+ case 'flag':
 336+ $checkbox = Xml::checkLabel( wfMsg( 'abusefilter-edit-action-flag' ), 'wpFilterActionFlag', 'wpFilterActionFlag', true, array( 'disabled' => '1' ) );
 337+ return Xml::tags( 'p', null, $checkbox );
 338+ case 'warn':
 339+ $output = '';
 340+ $checkbox = Xml::checkLabel( wfMsg( 'abusefilter-edit-action-warn' ), 'wpFilterActionWarn', 'wpFilterActionWarn', $set );
 341+ $output .= Xml::tags( 'p', null, $checkbox );
353342
354 - $thisAction = Xml::checkLabel( wfMsg( $message ), $form_field, $form_field, $status );
355 - $thisAction = Xml::tags( 'p', null, $thisAction );
 343+ $warnMsg = empty($set) ? 'abusefilter-warning' : $parameters[0];
 344+ $warnFields['abusefilter-edit-warn-message'] = Xml::input( 'wpFilterWarnMessage', 45, $warnMsg );
 345+ $output .= Xml::tags( 'p', null, Xml::buildForm( $warnFields ) );
 346+ return $output;
 347+ // Commented out to avoid trunk changes for now.
 348+// case 'tag':
 349+// if ($set) {
 350+// $tags = $parameters;
 351+// } else {
 352+// $tags = array();
 353+// }
 354+// $output = '';
 355+//
 356+// $checkbox = Xml::checkLabel( wfMsg('abusefilter-edit-action-tag'), 'wpFilterActionTag', 'wpFilterActionTag', $set );
 357+// $output .= Xml::tags( 'p', null, $checkbox );
 358+//
 359+// $tagFields['abusefilter-edit-tag-tag'] = Xml::textarea( 'wpFilterTags', implode( "\n", $tags ) );
 360+// $output .= Xml::tags( 'p', null, Xml::buildForm( $tagFields ) );
 361+// return $output;
 362+ default:
 363+ $message = 'abusefilter-edit-action-'.$action;
 364+ $form_field = 'wpFilterAction' . ucfirst($action);
 365+ $status = $set;
356366
357 - $output .= $thisAction;
 367+ $thisAction = Xml::checkLabel( wfMsg( $message ), $form_field, $form_field, $status );
 368+ $thisAction = Xml::tags( 'p', null, $thisAction );
 369+ return $thisAction;
358370 }
359 -
360 - return $output;
361371 }
362372
363373 function loadFilterData( $id ) {
Index: trunk/extensions/AbuseFilter/AbuseFilter.class.php
——@@ -197,6 +197,26 @@
198198
199199 foreach( $actionsByFilter as $filter => $actions ) {
200200 // Special-case handling for warnings.
 201+
 202+ if ( !empty( $actions['throttle'] ) ) {
 203+ $parameters = $actions['throttle']['parameters'];
 204+ $throttleId = array_shift( $parameters );
 205+ list( $rateCount, $ratePeriod ) = explode( ',', array_shift( $parameters ) );
 206+
 207+ $hitThrottle = false;
 208+
 209+ // The rest are throttle-types.
 210+ foreach( $parameters as $throttleType ) {
 211+ $hitThrottle = $hitThrottle || self::isThrottled( $throttleId, $throttleType, $title, $rateCount, $ratePeriod );
 212+ }
 213+
 214+ unset( $actions['throttle'] );
 215+ if (!$hitThrottle) {
 216+ $actionsTaken[$filter][] = 'throttle';
 217+ continue;
 218+ }
 219+ }
 220+
201221 if ( !empty( $actions['warn'] ) ) {
202222 $parameters = $actions['warn']['parameters'];
203223 $warnKey = 'abusefilter-warned-'.$title->getPrefixedText();
——@@ -207,7 +227,7 @@
208228 $msg = ( !empty($parameters[0]) && strlen($parameters[0]) ) ? $parameters[0] : 'abusefilter-warning';
209229 $messages[] = wfMsgNoTrans( $msg, self::$filters[$filter]->af_public_comments ) . "<br />\n";
210230
211 - $actionsTaken[] = 'warn';
 231+ $actionsTaken[$filter][] = 'warn';
212232
213233 continue; // Don't do anything else.
214234 } else {
——@@ -218,25 +238,6 @@
219239 unset( $actions['warn'] );
220240 }
221241
222 - if ( !empty( $actions['throttle'] ) ) {
223 - $parameters = $actions['throttle']['parameters'];
224 - $throttleId = array_shift( $parameters );
225 - list( $rateCount, $ratePeriod ) = explode( ',', array_shift( $parameters ) );
226 -
227 - $hitThrottle = false;
228 -
229 - // The rest are throttle-types.
230 - foreach( $parameters as $throttleType ) {
231 - $hitThrottle = $hitThrottle || self::isThrottled( $throttleId, $throttleType, $title, $rateCount, $ratePeriod );
232 - }
233 -
234 - unset( $actions['throttle'] );
235 - if (!$hitThrottle) {
236 - $actionsTaken[] = 'throttle';
237 - continue;
238 - }
239 - }
240 -
241242 // Do the rest of the actions
242243 foreach( $actions as $action => $info ) {
243244 $newMsg = self::takeConsequenceAction( $action, $info['parameters'], $title, $vars, self::$filters[$filter]->af_public_comments );
——@@ -259,7 +260,6 @@
260261
261262 // Short-cut any remaining code if no filters were hit.
262263 if ( count( array_filter( $filter_matched) ) == 0 ) {
263 - die( var_dump( $filter_matched ) );
264264 return true;
265265 }
266266
——@@ -284,7 +284,7 @@
285285 $thisLog = $log_template;
286286 $thisLog['afl_filter'] = $filter;
287287 $thisLog['afl_action'] = $action;
288 - $thisLog['afl_actions'] = implode( ',', $actions_taken[$filter] );
 288+ $thisLog['afl_actions'] = implode( ',', $actions );
289289
290290 // Don't log if we were only throttling.
291291 if ($thisLog['afl_actions'] != 'throttle') {
——@@ -418,16 +418,16 @@
419419 // Do nothing. Here for completeness.
420420 break;
421421
422 - case 'tag':
423 - // Mark with a tag on recentchanges.
424 - global $wgUser;
425 -
426 - $actionID = implode( '-', array(
427 - $title->getPrefixedText(), $wgUser->getName(), $vars['ACTION']
428 - ) );
429 -
430 - AbuseFilter::$tagsToSet[$actionID] = $parameters;
431 - break;
 422+// case 'tag':
 423+// // Mark with a tag on recentchanges.
 424+// global $wgUser;
 425+//
 426+// $actionID = implode( '-', array(
 427+// $title->getPrefixedText(), $wgUser->getName(), $vars['ACTION']
 428+// ) );
 429+//
 430+// AbuseFilter::$tagsToSet[$actionID] = $parameters;
 431+// break;
432432 }
433433
434434 return $display;
——@@ -442,7 +442,6 @@
443443 if ($count > 0) {
444444 $wgMemc->incr( $key );
445445 if ($count > $rateCount) {
446 - $wgMemc->delete( $key );
447446 return true; // THROTTLED
448447 }
449448 } else {
Index: trunk/extensions/AbuseFilter/AbuseFilter.hooks.php
——@@ -165,30 +165,31 @@
166166 return $filter_result == '' || $filter_result === true;
167167 }
168168
169 - public static function onRecentChangeSave( $recentChange ) {
170 - $title = Title::makeTitle( $recentChange->mAttribs['rc_namespace'], $recentChange->mAttribs['rc_title'] );
171 - $action = $recentChange->mAttribs['rc_log_type'] ? $recentChange->mAttribs['rc_log_type'] : 'edit';
172 - $actionID = implode( '-', array(
173 - $title->getPrefixedText(), $recentChange->mAttribs['rc_user_text'], $action
174 - ) );
 169+// Commented out to avoid trunk changes for now.
 170+// public static function onRecentChangeSave( $recentChange ) {
 171+// $title = Title::makeTitle( $recentChange->mAttribs['rc_namespace'], $recentChange->mAttribs['rc_title'] );
 172+// $action = $recentChange->mAttribs['rc_log_type'] ? $recentChange->mAttribs['rc_log_type'] : 'edit';
 173+// $actionID = implode( '-', array(
 174+// $title->getPrefixedText(), $recentChange->mAttribs['rc_user_text'], $action
 175+// ) );
 176+//
 177+// if ( !empty( AbuseFilter::$tagsToSet[$actionID] ) && count( $tags = AbuseFilter::$tagsToSet[$actionID]) ) {
 178+// ChangeTags::addTags( $tags, $recentChange->mAttribs['rc_id'], $recentChange->mAttribs['rc_this_oldid'], $recentChange->mAttribs['rc_logid'] );
 179+// }
 180+//
 181+// return true;
 182+// }
175183
176 - if ( !empty( AbuseFilter::$tagsToSet[$actionID] ) && count( $tags = AbuseFilter::$tagsToSet[$actionID]) ) {
177 - ChangeTags::addTags( $tags, $recentChange->mAttribs['rc_id'], $recentChange->mAttribs['rc_this_oldid'], $recentChange->mAttribs['rc_logid'] );
178 - }
179 -
180 - return true;
181 - }
182 -
183 - public static function onListDefinedTags( &$emptyTags ) {
184 - ## This is a pretty awful hack.
185 - $dbr = wfGetDB( DB_SLAVE );
186 -
187 - $res = $dbr->select( 'abuse_filter_action', 'afa_parameters', array( 'afa_consequence' => 'tag' ), __METHOD__ );
188 -
189 - while( $row = $res->fetchObject() ) {
190 - $emptyTags = array_filter( array_merge( explode( "\n", $row->afa_parameters ), $emptyTags ) );
191 - }
192 -
193 - return true;
194 - }
 184+// public static function onListDefinedTags( &$emptyTags ) {
 185+// ## This is a pretty awful hack.
 186+// $dbr = wfGetDB( DB_SLAVE );
 187+//
 188+// $res = $dbr->select( 'abuse_filter_action', 'afa_parameters', array( 'afa_consequence' => 'tag' ), __METHOD__ );
 189+//
 190+// while( $row = $res->fetchObject() ) {
 191+// $emptyTags = array_filter( array_merge( explode( "\n", $row->afa_parameters ), $emptyTags ) );
 192+// }
 193+//
 194+// return true;
 195+// }
195196 }

Status & tagging log