jenkins-bot has submitted this change and it was merged. Change subject: 'goodfaith' filter on Special:RC / Special:Watchlist ......................................................................
'goodfaith' filter on Special:RC / Special:Watchlist This filter is a not a typical show/hide toggle. It is url-driven only for the moment and will be used by the new RC filters (ERI project). It accepts one or many of the following values: good, maybebad, bad Values are separated by a comma. Each value is associated with a range of probabilities that are compared against the goodfaith test. They can be configured using $wgOresGoodfaithLevels. Bug: T149853 Depends-On: If47842f3d91c5999a7c5bf25666b967e9b30a6d7 Change-Id: I28c593f34145d566f803d96ff8220fd685c2f695 --- M extension.json M includes/Hooks.php A includes/Range.php M tests/phpunit/includes/HooksTest.php A tests/phpunit/includes/RangeTest.php 5 files changed, 452 insertions(+), 53 deletions(-) Approvals: Catrope: Looks good to me, approved jenkins-bot: Verified diff --git a/extension.json b/extension.json index c8c143d..d9e7428 100644 --- a/extension.json +++ b/extension.json @@ -13,6 +13,7 @@ "ORES\\Cache": "includes/Cache.php", "ORES\\Hooks": "includes/Hooks.php", "ORES\\FetchScoreJob": "includes/FetchScoreJob.php", + "ORES\\Range": "includes/Range.php", "ORES\\Scoring": "includes/Scoring.php", "ORES\\ApiQueryORES": "includes/ApiQueryORES.php", "ORES\\ApiHooks": "includes/ApiHooks.php", @@ -147,6 +148,11 @@ "soft": 0.70, "hard": 0.50 }, + "OresGoodfaithLevels": { + "good" : { "min": 0.35, "max": 1 }, + "maybebad" : { "min": 0, "max": 0.65 }, + "bad" : { "min": 0, "max": 0.15 } + }, "OresEnabledNamespaces": {}, "OresWikiId": null, "OresRevisionsPerBatch": 50, diff --git a/includes/Hooks.php b/includes/Hooks.php index 31f5de9..979ad18 100644 --- a/includes/Hooks.php +++ b/includes/Hooks.php @@ -93,25 +93,31 @@ ChangesListSpecialPage $clsp, &$filters ) { - if ( !self::oresEnabled( $clsp->getUser() ) || !self::isModelEnabled( 'damaging' ) ) { + if ( !self::oresEnabled( $clsp->getUser() ) ) { return true; } - switch ( $clsp->getName() ) { - case 'Watchlist': - $default = $clsp->getUser()->getOption( 'oresWatchlistHideNonDamaging' ); - break; - case 'Recentchanges': - $default = $clsp->getUser()->getOption( 'oresRCHideNonDamaging' ); - break; - default: - $default = false; + if ( self::isModelEnabled( 'damaging' ) ) { + switch ( $clsp->getName() ) { + case 'Watchlist': + $default = $clsp->getUser()->getOption( 'oresWatchlistHideNonDamaging' ); + break; + case 'Recentchanges': + $default = $clsp->getUser()->getOption( 'oresRCHideNonDamaging' ); + break; + default: + $default = false; + } + + $filters['hidenondamaging'] = [ + 'msg' => 'ores-damaging-filter', + 'default' => $default, + ]; } - $filters['hidenondamaging'] = [ - 'msg' => 'ores-damaging-filter', - 'default' => $default, - ]; + if ( self::isModelEnabled( 'goodfaith' ) ) { + $filters['goodfaith'] = [ 'msg' => false, 'default' => 'all' ]; + } return true; } @@ -138,24 +144,34 @@ } if ( self::isModelEnabled( 'damaging' ) ) { - $hidenondamaging = $opts->getValue( 'hidenondamaging' ); - self::manipulateQuery( - 'damaging', - $wgUser, - 'rc_this_oldid', - $hidenondamaging, + $hideNonDamaging = $opts->getValue( 'hidenondamaging' ); + + self::hideNonDamagingFilter( $tables, $fields, $conds, $query_options, - $join_conds + $join_conds, + $hideNonDamaging, + 'rc_this_oldid', + $wgUser ); - if ( $hidenondamaging ) { - $conds['rc_patrolled'] = 0; + if ( $hideNonDamaging ) { // Performance hack: add STRAIGHT_JOIN (146111) $query_options[] = 'STRAIGHT_JOIN'; } + } + + if ( self::isModelEnabled( 'goodfaith' ) ) { + self::goodfaithFilter( + $tables, + $fields, + $conds, + $query_options, + $join_conds, + $opts->getValue( 'goodfaith' ) + ); } return true; @@ -260,18 +276,19 @@ if ( !self::oresEnabled( $pager->getUser() ) ) { return true; } - $request = $pager->getContext()->getRequest(); - self::manipulateQuery( - 'damaging', - $pager->getUser(), - 'rev_id', - $request->getVal( 'hidenondamaging' ), - $query['tables'], - $query['fields'], - $query['conds'], - $query['options'], - $query['join_conds'] - ); + if ( self::isModelEnabled( 'damaging' ) ) { + $request = $pager->getContext()->getRequest(); + self::hideNonDamagingFilter( + $query['tables'], + $query['fields'], + $query['conds'], + $query['options'], + $query['join_conds'], + $request->getVal( 'hidenondamaging' ), + 'rev_id', + $pager->getUser() + ); + } return true; } @@ -546,21 +563,16 @@ $out->setProperty( 'oresData', $data ); } - private static function manipulateQuery( + private static function joinWithOresTables( $type, - User $user, - $revid_field, $filter, + $revIdField, array &$tables, array &$fields, array &$conds, array &$query_options, array &$join_conds ) { - if ( !self::isModelEnabled( $type ) ) { - return; - } - if ( !ctype_lower( $type ) ) { throw new Exception( "Invalid value for parameter 'type': '$type'. " . @@ -568,14 +580,10 @@ ); } - $dbr = \wfGetDB( DB_REPLICA ); - $threshold = self::getThreshold( $type, $user ); $tables["ores_${type}_mdl"] = 'ores_model'; $tables["ores_${type}_cls"] = 'ores_classification'; $fields["ores_${type}_score"] = "ores_${type}_cls.oresc_probability"; - // Add user-based threshold - $fields["ores_${type}_threshold"] = $dbr->addQuotes( $threshold ); $join_conds["ores_${type}_mdl"] = [ 'LEFT JOIN', [ "ores_${type}_mdl.oresm_is_current" => 1, @@ -583,17 +591,111 @@ ] ]; $join_conds["ores_${type}_cls"] = [ 'LEFT JOIN', [ "ores_${type}_cls.oresc_model = ores_${type}_mdl.oresm_id", - "$revid_field = ores_${type}_cls.oresc_rev", + "$revIdField = ores_${type}_cls.oresc_rev", "ores_${type}_cls.oresc_class" => 1 ] ]; if ( $filter ) { - // Filter out non-damaging edits. - $conds[] = "ores_${type}_cls.oresc_probability > " . $dbr->addQuotes( $threshold ); // Performance hack: override the LEFT JOINs to be INNER JOINs (T137895) $join_conds["ores_${type}_mdl"][0] = 'INNER JOIN'; $join_conds["ores_${type}_cls"][0] = 'INNER JOIN'; } } + private static function hideNonDamagingFilter( + array &$tables, + array &$fields, + array &$conds, + array &$query_options, + array &$join_conds, + $hidenondamaging, + $revIdField, + $user + ) { + self::joinWithOresTables( + 'damaging', + $hidenondamaging, + $revIdField, + $tables, + $fields, + $conds, + $query_options, + $join_conds + ); + + $dbr = \wfGetDB( DB_REPLICA ); + // Add user-based threshold + $threshold = self::getThreshold( 'damaging', $user ); + $fields['ores_damaging_threshold'] = $dbr->addQuotes( $threshold ); + + if ( $hidenondamaging ) { + // Filter out non-damaging edits. + $conds[] = 'ores_damaging_cls.oresc_probability > ' . $dbr->addQuotes( $threshold ); + + $conds['rc_patrolled'] = 0; + } + } + + private static function goodfaithFilter( + &$tables, + &$fields, + &$conds, + &$query_options, + &$join_conds, + $filterValue + ) { + global $wgOresGoodfaithLevels; + + $goodfaithLevels = explode( ',', strtolower( $filterValue ) ); + $goodfaithLevels = array_intersect( $goodfaithLevels, [ 'good', 'bad', 'maybebad' ] ); + + if ( $goodfaithLevels ) { + $ranges = []; + foreach ( $goodfaithLevels as $level ) { + $range = new Range( + $wgOresGoodfaithLevels[$level]['min'], + $wgOresGoodfaithLevels[$level]['max'] + ); + + $result = array_filter( + $ranges, + function ( Range $r ) use ( $range ) { + return $r->overlaps( $range ); + } + ); + $overlap = reset( $result ); + if ( $overlap ) { + $overlap->combineWith( $range ); + } else { + $ranges[] = $range; + } + } + + $betweenConditions = array_map( + function ( Range $range ) { + $min = $range->getMin(); + $max = $range->getMax(); + return "ores_goodfaith_cls.oresc_probability BETWEEN $min AND $max"; + }, + $ranges + ); + + $conds[] = \wfGetDB( DB_REPLICA )->makeList( $betweenConditions, \IDatabase::LIST_OR ); + + self::joinWithOresTables( + 'goodfaith', + count( $goodfaithLevels ), + 'rc_this_oldid', + $tables, + $fields, + $conds, + $query_options, + $join_conds + ); + + // Performance hack: add STRAIGHT_JOIN (146111) + $query_options[] = 'STRAIGHT_JOIN'; + } + } + } diff --git a/includes/Range.php b/includes/Range.php new file mode 100644 index 0000000..0cce96a --- /dev/null +++ b/includes/Range.php @@ -0,0 +1,70 @@ +<?php + +namespace ORES; + +/** + * Represents a range defined by two values: min and max + * + * Class Range + * @package ORES + */ +class Range { + + /** + * @var float + */ + protected $min; + + /** + * @var float + */ + protected $max; + + /** + * Range constructor. + * @param float $min + * @param float $max + */ + public function __construct( $min, $max ) { + if ( $min > $max ) { + throw new \DomainException( '$min must be smaller than or equal to $max' ); + } + $this->min = $min; + $this->max = $max; + } + + /** + * @return float + */ + public function getMin() { + return $this->min; + } + + /** + * @return float + */ + public function getMax() { + return $this->max; + } + + /** + * Check if the current range overlaps with or touches the given range. + * + * @param Range $other + * @return bool + */ + public function overlaps( Range $other ) { + return max( $this->getMin(), $other->getMin() ) <= min( $this->getMax(), $other->getMax() ); + } + + /** + * Expands the current range to include the given range. + * + * @param Range $other + */ + public function combineWith( Range $other ) { + $this->min = min( $this->getMin(), $other->getMin() ); + $this->max = max( $this->getMax(), $other->getMax() ); + } + +} diff --git a/tests/phpunit/includes/HooksTest.php b/tests/phpunit/includes/HooksTest.php index 9834c1f..582ae15 100644 --- a/tests/phpunit/includes/HooksTest.php +++ b/tests/phpunit/includes/HooksTest.php @@ -73,17 +73,25 @@ ORES\Hooks::onChangesListSpecialPageFilters( $clsp, $filters ); $expected = [ - 'hidenondamaging' => [ 'msg' => 'ores-damaging-filter', 'default' => false ] + 'hidenondamaging' => [ 'msg' => 'ores-damaging-filter', 'default' => false ], + 'goodfaith' => [ 'msg' => false, 'default' => 'all' ], ]; $this->assertSame( $expected, $filters ); } - public function testOnChangesListSpecialPageQuery() { - $this->setMwGlobals( 'wgUser', $this->user ); + public function testOnChangesListSpecialPageQuery_hidenondamaging() { + $this->setMwGlobals( [ + 'wgUser' => $this->user, + 'wgOresModels' => [ + 'damaging' => true, + 'goodfaith' => false, + ] + ] ); $opts = new FormOptions(); $opts->add( 'hidenondamaging', true, 2 ); + $opts->add( 'goodfaith', 'all' ); $tables = []; $fields = []; @@ -110,7 +118,7 @@ ], 'conds' => [ "ores_damaging_cls.oresc_probability > '0.7'", - 'rc_patrolled' => 0 + 'rc_patrolled' => 0, ], 'query_options' => [ 'STRAIGHT_JOIN' ], 'join_conds' => [ @@ -136,6 +144,147 @@ $this->assertSame( $expected['join_conds'], $join_conds ); } + public function onChangesListSpecialPageQuery_goodfaith_provider() { + return [ + [ 'good', 0.35, 1 ], + [ 'maybebad', 0, 0.65 ], + [ 'bad', 0, 0.15 ], + [ 'good,maybebad', 0, 1 ], + [ 'maybebad,bad', 0, 0.65 ], + [ 'good,maybebad,bad', 0, 1 ], + ]; + } + + /** + * @dataProvider onChangesListSpecialPageQuery_goodfaith_provider + */ + public function testOnChangesListSpecialPageQuery_goodfaith( + $goodfaithValue, + $expectedMin, + $expectedMax + ) { + $this->setMwGlobals( [ + 'wgUser' => $this->user, + 'wgOresModels' => [ + 'damaging' => false, + 'goodfaith' => true, + ] + ] ); + + $opts = new FormOptions(); + + $opts->add( 'hidenondamaging', false ); + $opts->add( 'goodfaith', $goodfaithValue ); + + $tables = []; + $fields = []; + $conds = []; + $query_options = []; + $join_conds = []; + ORES\Hooks::onChangesListSpecialPageQuery( + '', + $tables, + $fields, + $conds, + $query_options, + $join_conds, + $opts + ); + $expected = [ + 'tables' => [ + 'ores_goodfaith_mdl' => 'ores_model', + 'ores_goodfaith_cls' => 'ores_classification' + ], + 'fields' => [ + 'ores_goodfaith_score' => 'ores_goodfaith_cls.oresc_probability', + ], + 'conds' => [ + "(ores_goodfaith_cls.oresc_probability BETWEEN $expectedMin AND $expectedMax)" + ], + 'join_conds' => [ + 'ores_goodfaith_mdl' => [ 'INNER JOIN', + [ + 'ores_goodfaith_mdl.oresm_is_current' => 1, + 'ores_goodfaith_mdl.oresm_name' => 'goodfaith' + ] + ], + 'ores_goodfaith_cls' => [ 'INNER JOIN', + [ + 'ores_goodfaith_cls.oresc_model = ores_goodfaith_mdl.oresm_id', + 'rc_this_oldid = ores_goodfaith_cls.oresc_rev', + 'ores_goodfaith_cls.oresc_class' => 1 + ] + ] + ], + ]; + $this->assertSame( $expected['tables'], $tables ); + $this->assertSame( $expected['fields'], $fields ); + $this->assertSame( $expected['conds'], $conds ); + $this->assertSame( $expected['join_conds'], $join_conds ); + } + + public function testOnChangesListSpecialPageQuery_goodfaith_goodbad() { + $this->setMwGlobals( [ + 'wgUser' => $this->user, + 'wgOresModels' => [ + 'damaging' => false, + 'goodfaith' => true, + ] + ] ); + + $opts = new FormOptions(); + + $opts->add( 'hidenondamaging', false ); + $opts->add( 'goodfaith', 'good,bad' ); + + $tables = []; + $fields = []; + $conds = []; + $query_options = []; + $join_conds = []; + ORES\Hooks::onChangesListSpecialPageQuery( + '', + $tables, + $fields, + $conds, + $query_options, + $join_conds, + $opts + ); + $expected = [ + 'tables' => [ + 'ores_goodfaith_mdl' => 'ores_model', + 'ores_goodfaith_cls' => 'ores_classification' + ], + 'fields' => [ + 'ores_goodfaith_score' => 'ores_goodfaith_cls.oresc_probability', + ], + 'conds' => [ + "(ores_goodfaith_cls.oresc_probability BETWEEN 0.35 AND 1) OR " . + "(ores_goodfaith_cls.oresc_probability BETWEEN 0 AND 0.15)", + ], + 'join_conds' => [ + 'ores_goodfaith_mdl' => [ 'INNER JOIN', + [ + 'ores_goodfaith_mdl.oresm_is_current' => 1, + 'ores_goodfaith_mdl.oresm_name' => 'goodfaith' + ] + ], + 'ores_goodfaith_cls' => [ 'INNER JOIN', + [ + 'ores_goodfaith_cls.oresc_model = ores_goodfaith_mdl.oresm_id', + 'rc_this_oldid = ores_goodfaith_cls.oresc_rev', + 'ores_goodfaith_cls.oresc_class' => 1 + ] + ] + ], + ]; + $this->assertSame( $expected['tables'], $tables ); + $this->assertSame( $expected['fields'], $fields ); + $this->assertSame( $expected['conds'], $conds ); + $this->assertSame( $expected['join_conds'], $join_conds ); + } + public function testOnEnhancedChangesListModifyLineDataDamaging() { $row = new \stdClass(); $row->ores_damaging_threshold = 0.2; diff --git a/tests/phpunit/includes/RangeTest.php b/tests/phpunit/includes/RangeTest.php new file mode 100644 index 0000000..4cb3c0a --- /dev/null +++ b/tests/phpunit/includes/RangeTest.php @@ -0,0 +1,72 @@ +<?php + +namespace ORES\Tests; + +use ORES; +use ORES\Range; +use PHPUnit_Framework_TestCase; + +/** + * @group ORES + * @covers ORES\Range + */ +class OresRangeTest extends PHPUnit_Framework_TestCase { + + public function testConstructor() { + $r = new Range( 1, 2 ); + + $this->assertEquals( 1, $r->getMin() ); + $this->assertEquals( 2, $r->getMax() ); + } + + /** + * @expectedException \DomainException + */ + public function testConstructorEx() { + $r = new Range( 3, 2 ); + } + + public function overlapsProvider() { + return [ + [ [ 1, 3 ], [ 2, 4 ], true ], + [ [ 1, 2 ], [ 2, 4 ], true ], + [ [ 1, 5 ], [ 2, 3 ], true ], + [ [ 1, 1 ], [ 1, 3 ], true ], + [ [ 0.1, 0.35 ], [ 0.29, 1 ], true ], + [ [ 0.1, 0.35 ], [ 0.56, 1 ], false ], + [ [ 1, 2 ], [ 3, 4 ], false ], + ]; + } + + /** + * @dataProvider overlapsProvider + */ + public function testOverlaps( $r1, $r2, $expectedOverlap ) { + $r1 = new Range( $r1[0], $r1[1] ); + $r2 = new Range( $r2[0], $r2[1] ); + $this->assertEquals( $expectedOverlap, $r1->overlaps( $r2 ) ); + $this->assertEquals( $expectedOverlap, $r2->overlaps( $r1 ) ); + } + + public function combineWithProvider() { + return [ + [ [ 1, 2 ], [ 3, 4 ], [ 1, 4 ] ], + [ [ 4, 44 ], [ 11, 88 ], [ 4, 88 ] ], + [ [ 1, 2 ], [ 4, 5 ], [ 1, 5 ] ], + ]; + } + + /** + * @dataProvider combineWithProvider + */ + public function testCombineWith( $r1, $r2, $expectedRange ) { + $r1 = new Range( $r1[0], $r1[1] ); + $r2 = new Range( $r2[0], $r2[1] ); + + $r1->combineWith( $r2 ); + + $this->assertEquals( $expectedRange[0], $r1->getMin() ); + $this->assertEquals( $expectedRange[1], $r1->getMax() ); + } + +} -- To view, visit https://gerrit.wikimedia.org/r/323328 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I28c593f34145d566f803d96ff8220fd685c2f695 Gerrit-PatchSet: 13 Gerrit-Project: mediawiki/extensions/ORES Gerrit-Branch: master Gerrit-Owner: Sbisson <sbis...@wikimedia.org> Gerrit-Reviewer: Catrope <r...@wikimedia.org> Gerrit-Reviewer: Ladsgroup <ladsgr...@gmail.com> Gerrit-Reviewer: MarcoAurelio <strig...@gmail.com> Gerrit-Reviewer: Sbisson <sbis...@wikimedia.org> Gerrit-Reviewer: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits