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 <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Ladsgroup <[email protected]>
Gerrit-Reviewer: MarcoAurelio <[email protected]>
Gerrit-Reviewer: Sbisson <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
