jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/392452 )
Change subject: Split WL and RC prefs for ORES
......................................................................
Split WL and RC prefs for ORES
- Make ORES preferences separate for Watchlist and RecentChanges,
so both include:
* Prediction Threshold
* "Highlight likely problem edits" control
* "Show only likely problem edits" control
- Provide both sections with separate titles, not mentioning the other one.
- Change instruction text displayed on help icon click.
Bug: T180866
Change-Id: Ifbc5a6b3a5f5e634cc308d769360365f30fceecc
---
M extension.json
M i18n/en.json
M i18n/qqq.json
M includes/Hooks.php
M includes/Hooks/ApiHooksHandler.php
M includes/Hooks/ChangesListHooksHandler.php
M includes/Hooks/ContributionsHooksHandler.php
M includes/Hooks/PreferencesHookHandler.php
M tests/phpunit/includes/Hooks/ChangesListHooksHandlerTest.php
M tests/phpunit/includes/Hooks/ContributionsHookHandlerTest.php
M tests/phpunit/includes/HooksTest.php
11 files changed, 97 insertions(+), 64 deletions(-)
Approvals:
Catrope: Looks good to me, approved
jenkins-bot: Verified
diff --git a/extension.json b/extension.json
index 63f226b..e8c6af3 100644
--- a/extension.json
+++ b/extension.json
@@ -206,6 +206,7 @@
"DefaultUserOptions": {
"ores-damaging-flag-rc": false,
"oresDamagingPref": "soft",
+ "rcOresDamagingPref": "soft",
"oresHighlight": false,
"oresRCHideNonDamaging": false,
"oresWatchlistHideNonDamaging": false
diff --git a/i18n/en.json b/i18n/en.json
index 5cd0889..1e136c3 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -11,11 +11,11 @@
"ores-damaging-legend": "This edit may have problems and should be
reviewed ([[:mw:Special:MyLanguage/ORES review tool|more info]])",
"ores-hide-nondamaging-filter": "Hide probably good edits",
"ores-pref-damaging": "Prediction threshold",
- "ores-pref-damaging-flag": "Highlight likely problem edits with colors
and an \"{{int:ores-damaging-letter}}\" for \"needs review\" (to set the level
at which edits are marked, use the \"{{int:ores-pref-damaging}}\" setting in
the Watchlist preferences)",
+ "ores-pref-damaging-flag": "Highlight likely problem edits with colors
and an \"{{int:ores-damaging-letter}}\" for \"needs review\"",
"ores-damaging-maybebad": "May have problems (flags most problem edits
but includes many false positives)",
"ores-damaging-likelybad": "Likely have problems (medium probability)",
"ores-damaging-verylikelybad": "Very likely have problems (flags few
false positives but finds a smaller % of problem edits)",
- "ores-help-damaging-pref": "Sets the sensitivity of the \"Highlight
likely problem edits...\" and \"Show only likely problem edits...\" options on
Recent Changes and Watchlist.",
+ "ores-help-damaging-pref": "Change the \"threshold\" setting to make
the options below broader or more selective.",
"ores-rcfilters-whats-this-link-text": "Learn more",
"ores-rcfilters-ores-conflicts-logactions-global": "The
\"{{int:rcfilters-filter-logactions-label}}\" filter conflicts with one or more
Contribution Quality or User Intent filters. Quality and Intent predictions are
not available for logged actions. The conflicting filters are marked in the
Active Filters area, above.",
"ores-rcfilters-logactions-conflicts-ores": "This filter conflicts with
one or more Contribution Quality or User Intent filters. Quality and Intent
predictions are not available for logged actions.",
@@ -48,7 +48,8 @@
"ores-pref-highlight": "Highlight likely problem edits with colors and
an \"{{int:ores-damaging-letter}}\" for \"needs review\"",
"ores-pref-rc-hidenondamaging": "Show only likely problem edits (and
hide probably good edits)",
"ores-pref-watchlist-hidenondamaging": "Show only likely problem edits
(and hide probably good edits)",
- "prefs-ores" : "Revision scoring",
+ "prefs-ores-wl": "Revision scoring on Watchlist",
+ "prefs-ores-rc": "Revision scoring on Recent changes, Related changes,
and Contributions",
"apihelp-query+ores-description": "Return ORES configuration and model
data for this wiki.",
"apihelp-query+ores-summary": "Return ORES configuration and model data
for this wiki.",
"apihelp-query+ores-example-simple": "Fetch ORES data:",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index 48caa78..c33972d 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -55,7 +55,8 @@
"ores-pref-highlight": "Display message for user preference to enable
highlighting of damaging edits.",
"ores-pref-rc-hidenondamaging": "Display message for user preferences
to make hidenondamaging default in recent changes",
"ores-pref-watchlist-hidenondamaging": "Display message for user
preferences to make hidenondamaging default in the watchlist.\nAn algorithm
classifies the modifications according to a degree of probability. So we are
talking about \"probably problematic\" because a calculation is done.",
- "prefs-ores": "Name of ORES section in preferences",
+ "prefs-ores-wl": "Name of ORES section in Watchlist tab on preferences",
+ "prefs-ores-rc": "Name of ORES section in Recent Changes tab on
preferences",
"apihelp-query+ores-description":
"{{doc-apihelp-description|query+ores}}",
"apihelp-query+ores-summary": "{{doc-apihelp-summary|query+ores}}",
"apihelp-query+ores-example-simple":
"{{doc-apihelp-example|query+ores}}",
diff --git a/includes/Hooks.php b/includes/Hooks.php
index 04d1ac1..e0a743c 100644
--- a/includes/Hooks.php
+++ b/includes/Hooks.php
@@ -29,6 +29,7 @@
use SpecialRecentChanges;
use SpecialWatchlist;
use User;
+use Title;
class Hooks {
// The oresDamagingPref preference uses these names for historical
reasons
@@ -126,10 +127,15 @@
* Internal helper to get damaging level preference
* with backward compatibility for old level names
* @param User $user
+ * @param Title $title
* @return string 'maybebad', 'likelybad', or 'verylikelybad'
*/
- public static function getDamagingLevelPreference( User $user ) {
- $pref = $user->getOption( 'oresDamagingPref' );
+ public static function getDamagingLevelPreference( User $user, Title
$title = null ) {
+ $option = !$title || self::isWLPage( $title ) ?
+ 'oresDamagingPref' :
+ 'rcOresDamagingPref';
+
+ $pref = $user->getOption( $option );
if ( isset( self::$damagingPrefMap[ $pref ] ) ) {
$pref = self::$damagingPrefMap[ $pref ];
}
@@ -140,12 +146,13 @@
* Internal helper to get threshold
* @param string $type
* @param User $user
+ * @param Title|null $title
* @return float|null Threshold, or null if not set
* @throws Exception When $type is not recognized
*/
- public static function getThreshold( $type, User $user ) {
+ public static function getThreshold( $type, User $user, Title $title =
null ) {
if ( $type === 'damaging' ) {
- $pref = self::getDamagingLevelPreference( $user );
+ $pref = self::getDamagingLevelPreference( $user, $title
);
$thresholds = self::getDamagingThresholds();
if ( isset( $thresholds[ $pref ] ) ) {
return $thresholds[ $pref ];
@@ -246,20 +253,20 @@
}
/**
- * @param IContextSource $context
- * @return bool Whether $context->getTitle() is a RecentChanges page
+ * @param Title $title
+ * @return bool Whether $title is a RecentChanges page
*/
- private static function isRCPage( IContextSource $context ) {
- return $context->getTitle()->isSpecial( 'Recentchanges' ) ||
- $context->getTitle()->isSpecial( 'Recentchangeslinked'
);
+ private static function isRCPage( Title $title ) {
+ return $title->isSpecial( 'Recentchanges' ) ||
+ $title->isSpecial( 'Recentchangeslinked' );
}
/**
- * @param IContextSource $context
- * @return bool Whether $context->getTitle() is the Watchlist page
+ * @param Title $title
+ * @return bool Whether $title is the Watchlist page
*/
- private static function isWLPage( IContextSource $context ) {
- return $context->getTitle()->isSpecial( 'Watchlist' );
+ private static function isWLPage( Title $title ) {
+ return $title->isSpecial( 'Watchlist' );
}
public static function isRCStructuredUiEnabled( IContextSource $context
) {
@@ -295,12 +302,12 @@
return false;
}
- if ( self::isRCPage( $context ) ) {
+ if ( self::isRCPage( $context->getTitle() ) ) {
return !self::isRCStructuredUiEnabled( $context ) &&
$user->getBoolOption( 'ores-damaging-flag-rc' );
}
- if ( self::isWLPage( $context ) ) {
+ if ( self::isWLPage( $context->getTitle() ) ) {
return !self::isWLStructuredUiEnabled( $context ) &&
$user->getBoolOption( 'oresHighlight' );
}
@@ -368,11 +375,12 @@
array &$fields,
array &$conds,
$hidenondamaging,
- $user
+ User $user,
+ Title $title = null
) {
$dbr = \wfGetDB( DB_REPLICA );
// Add user-based threshold
- $threshold = self::getThreshold( 'damaging', $user );
+ $threshold = self::getThreshold( 'damaging', $user, $title );
if ( $threshold === null ) {
return;
}
diff --git a/includes/Hooks/ApiHooksHandler.php
b/includes/Hooks/ApiHooksHandler.php
index 48cb7cc..1385ae2 100644
--- a/includes/Hooks/ApiHooksHandler.php
+++ b/includes/Hooks/ApiHooksHandler.php
@@ -134,7 +134,7 @@
}
}
- $threshold = Hooks::getThreshold( 'damaging',
$module->getUser() );
+ $threshold = Hooks::getThreshold( 'damaging',
$module->getUser(), $module->getTitle() );
$dbr = \wfGetDB( DB_REPLICA );
$tables[] = 'ores_model';
diff --git a/includes/Hooks/ChangesListHooksHandler.php
b/includes/Hooks/ChangesListHooksHandler.php
index c5d7344..4e4a9eb 100644
--- a/includes/Hooks/ChangesListHooksHandler.php
+++ b/includes/Hooks/ChangesListHooksHandler.php
@@ -51,10 +51,13 @@
if ( Hooks::isModelEnabled( 'damaging' ) ) {
if ( $clsp instanceof SpecialRecentChanges ) {
$damagingDefault = $clsp->getUser()->getOption(
'oresRCHideNonDamaging' );
+ $highlightDefault =
$clsp->getUser()->getBoolOption( 'ores-damaging-flag-rc' );
} elseif ( $clsp instanceof SpecialWatchlist ) {
$damagingDefault = $clsp->getUser()->getOption(
'oresWatchlistHideNonDamaging' );
+ $highlightDefault =
$clsp->getUser()->getBoolOption( 'oresHighlight' );
} else {
$damagingDefault = false;
+ $highlightDefault = false;
}
$damagingLevels = $stats->getThresholds( 'damaging' );
@@ -170,18 +173,23 @@
}
if ( $damagingDefault ) {
- $newDamagingGroup->setDefault(
Hooks::getDamagingLevelPreference( $clsp->getUser
- () ) );
+ $newDamagingGroup->setDefault(
Hooks::getDamagingLevelPreference(
+ $clsp->getUser(),
+ $clsp->getPageTitle()
+ ) );
}
- if ( $clsp->getUser()->getBoolOption(
'oresHighlight' ) ) {
+ if ( $highlightDefault ) {
$levelsColors = [
'maybebad' => 'c3',
'likelybad' => 'c4',
'verylikelybad' => 'c5',
];
- $prefLevel =
Hooks::getDamagingLevelPreference( $clsp->getUser() );
+ $prefLevel =
Hooks::getDamagingLevelPreference(
+ $clsp->getUser(),
+ $clsp->getPageTitle()
+ );
$allLevels = array_keys( $levelsColors
);
$applicableLevels = array_slice(
$allLevels, array_search( $prefLevel, $allLevels ) );
$applicableLevels = array_intersect(
$applicableLevels, array_keys( $filters ) );
@@ -209,7 +217,7 @@
'default' => $damagingDefault,
'queryCallable' => function (
$specialClassName, $ctx, $dbr, &$tables,
&$fields,
&$conds, &$query_options, &$join_conds ) {
-
Hooks::hideNonDamagingFilter( $fields, $conds, true, $ctx->getUser() );
+
Hooks::hideNonDamagingFilter( $fields, $conds, true, $ctx->getUser(),
$ctx->getTitle() );
// Filter out
incompatible types; log actions and external rows are not scorable
$conds[] = 'rc_type NOT
IN (' . $dbr->makeList( [ RC_LOG, RC_EXTERNAL ] ) . ')';
// Filter out patrolled
edits: the 'r' doesn't appear for them
@@ -493,10 +501,9 @@
* @return bool
*/
public static function getScoreRecentChangesList( $rcObj,
IContextSource $context ) {
- global $wgUser;
$threshold = $rcObj->getAttribute( 'ores_damaging_threshold' );
if ( $threshold === null ) {
- $threshold = Hooks::getThreshold( 'damaging', $wgUser );
+ $threshold = Hooks::getThreshold( 'damaging',
$context->getUser(), $context->getTitle() );
}
$score = $rcObj->getAttribute( 'ores_damaging_score' );
$patrolled = $rcObj->getAttribute( 'rc_patrolled' );
diff --git a/includes/Hooks/ContributionsHooksHandler.php
b/includes/Hooks/ContributionsHooksHandler.php
index 45721f2..bb5d5d4 100644
--- a/includes/Hooks/ContributionsHooksHandler.php
+++ b/includes/Hooks/ContributionsHooksHandler.php
@@ -22,6 +22,7 @@
use ORES\Hooks;
use RequestContext;
use SpecialContributions;
+use IContextSource;
use Xml;
class ContributionsHooksHandler {
@@ -41,8 +42,6 @@
}
if ( Hooks::isModelEnabled( 'damaging' ) ) {
- $request = $pager->getContext()->getRequest();
-
Hooks::joinWithOresTables(
'damaging',
'rev_id',
@@ -54,8 +53,9 @@
Hooks::hideNonDamagingFilter(
$query['fields'],
$query['conds'],
- $request->getVal( 'hidenondamaging' ),
- $pager->getUser()
+ self::hideNonDamagingPreference(
$pager->getContext() ),
+ $pager->getUser(),
+ $pager->getTitle()
);
}
}
@@ -131,10 +131,28 @@
$page->msg( 'ores-hide-nondamaging-filter'
)->text(),
'hidenondamaging',
'ores-hide-nondamaging',
- $page->getRequest()->getVal( 'hidenondamaging'
),
+ self::hideNonDamagingPreference(
$page->getContext() ),
[ 'class' => 'mw-input' ]
)
);
}
+ /**
+ * Get user preference for hiding non-damaging edits, either by:
+ * - URL parameter 'hidenondamaging' or
+ * - User preference 'oresRCHideNonDamaging'
+ *
+ * @param IContextSource $context
+ * @return string|boolean $option URL string param or boolean user
preference
+ */
+ private static function hideNonDamagingPreference( IContextSource
$context ) {
+ $option = $context->getRequest()->getVal( 'hidenondamaging' );
+
+ if ( $option === null ) {
+ $option = $context->getUser()->getOption(
'oresRCHideNonDamaging' );
+ }
+
+ return $option;
+ }
+
}
diff --git a/includes/Hooks/PreferencesHookHandler.php
b/includes/Hooks/PreferencesHookHandler.php
index 19fb13e..64a2f50 100644
--- a/includes/Hooks/PreferencesHookHandler.php
+++ b/includes/Hooks/PreferencesHookHandler.php
@@ -17,9 +17,6 @@
namespace ORES\Hooks;
use ORES\Hooks;
-use DerivativeContext;
-use DerivativeRequest;
-use RequestContext;
use User;
class PreferencesHookHandler {
@@ -32,7 +29,7 @@
* @param string[] &$preferences
*/
public static function onGetPreferences( User $user, array
&$preferences ) {
- global $wgOresFiltersThresholds, $wgOresExtensionStatus,
$wgHiddenPrefs;
+ global $wgOresFiltersThresholds, $wgOresExtensionStatus;
if ( !Hooks::oresUiEnabled( $user ) || !Hooks::isModelEnabled(
'damaging' ) ) {
return;
@@ -53,11 +50,19 @@
$options[ $text ] = $prefName;
}
}
- $oresSection = $wgOresExtensionStatus === 'beta' ? 'rc/ores' :
'watchlist/ores';
+
$preferences['oresDamagingPref'] = [
'type' => 'select',
'label-message' => 'ores-pref-damaging',
- 'section' => $oresSection,
+ 'section' => 'watchlist/ores-wl',
+ 'options' => $options,
+ 'help-message' => 'ores-help-damaging-pref',
+ ];
+
+ $preferences['rcOresDamagingPref'] = [
+ 'type' => 'select',
+ 'label-message' => 'ores-pref-damaging',
+ 'section' => 'rc/ores-rc',
'options' => $options,
'help-message' => 'ores-help-damaging-pref',
];
@@ -66,14 +71,14 @@
// highlight damaging edits based on configured
sensitivity
$preferences['oresHighlight'] = [
'type' => 'toggle',
- 'section' => $oresSection,
+ 'section' => 'watchlist/ores-wl',
'label-message' => 'ores-pref-highlight',
];
// Control whether the "r" appears on RC
$preferences['ores-damaging-flag-rc'] = [
'type' => 'toggle',
- 'section' => 'rc/advancedrc',
+ 'section' => 'rc/ores-rc',
'label-message' => 'ores-pref-damaging-flag',
];
}
@@ -81,26 +86,14 @@
// Make hidenondamaging default
$preferences['oresWatchlistHideNonDamaging'] = [
'type' => 'toggle',
- 'section' => 'watchlist/ores',
+ 'section' => 'watchlist/ores-wl',
'label-message' =>
'ores-pref-watchlist-hidenondamaging',
];
$preferences['oresRCHideNonDamaging'] = [
'type' => 'toggle',
- 'section' => 'rc/advancedrc',
+ 'section' => 'rc/ores-rc',
'label-message' => 'ores-pref-rc-hidenondamaging',
];
-
- // Hide RC/wL prefs if enhanced filters are enabled
- $context = new DerivativeContext( RequestContext::getMain() );
- $context->setUser( $user );
- $context->setRequest( new DerivativeRequest(
$context->getRequest(), [] ) );
- $rcFiltersEnabled = Hooks::isRCStructuredUiEnabled( $context );
- // HACK: Note that this only hides the preferences on the
preferences page,
- // it does not cause them to behave as if they're set to their
default value,
- // because this hook only runs on the preferences page.
- if ( $rcFiltersEnabled ) {
- $wgHiddenPrefs[] = 'ores-damaging-flag-rc';
- }
}
}
diff --git a/tests/phpunit/includes/Hooks/ChangesListHooksHandlerTest.php
b/tests/phpunit/includes/Hooks/ChangesListHooksHandlerTest.php
index 3880651..aacafdf 100644
--- a/tests/phpunit/includes/Hooks/ChangesListHooksHandlerTest.php
+++ b/tests/phpunit/includes/Hooks/ChangesListHooksHandlerTest.php
@@ -40,9 +40,10 @@
$this->user = static::getTestUser()->getUser();
$this->user->setOption( 'ores-enabled', 1 );
- $this->user->setOption( 'oresDamagingPref', 'maybebad' );
+ $this->user->setOption( 'rcOresDamagingPref', 'maybebad' );
$this->user->setOption( 'oresHighlight', 1 );
$this->user->setOption( 'ores-damaging-flag-rc', 1 );
+ $this->user->setOption( 'rcenhancedfilters-disable', true );
$this->user->saveSettings();
$this->context = self::getContext( $this->user );
@@ -308,7 +309,7 @@
$config = $this->getMockBuilder( Config::class )->getMock();
$config->expects( $this->any() )
->method( 'get' )
- ->will( $this->returnValue( false ) );
+ ->will( $this->returnValue( true ) );
$cl = $this->getMockBuilder( ChangesList::class )
->disableOriginalConstructor()
diff --git a/tests/phpunit/includes/Hooks/ContributionsHookHandlerTest.php
b/tests/phpunit/includes/Hooks/ContributionsHookHandlerTest.php
index 77fc6fb..9d01a50 100644
--- a/tests/phpunit/includes/Hooks/ContributionsHookHandlerTest.php
+++ b/tests/phpunit/includes/Hooks/ContributionsHookHandlerTest.php
@@ -34,9 +34,11 @@
$this->user = static::getTestUser()->getUser();
$this->user->setOption( 'ores-enabled', 1 );
- $this->user->setOption( 'oresDamagingPref', 'maybebad' );
+ $this->user->setOption( 'rcOresDamagingPref', 'maybebad' );
$this->user->setOption( 'oresHighlight', 1 );
$this->user->setOption( 'ores-damaging-flag-rc', 1 );
+ $this->user->setOption( 'rcenhancedfilters', null );
+ $this->user->setOption( 'rcenhancedfilters-disable', true );
$this->user->saveSettings();
$this->context = self::getContext( $this->user );
diff --git a/tests/phpunit/includes/HooksTest.php
b/tests/phpunit/includes/HooksTest.php
index fd374a2..60d988d 100644
--- a/tests/phpunit/includes/HooksTest.php
+++ b/tests/phpunit/includes/HooksTest.php
@@ -46,22 +46,23 @@
$preferences = [];
PreferencesHookHandler::onGetPreferences( $this->user,
$preferences );
$this->assertArrayHasKey( 'oresDamagingPref', $preferences );
+ $this->assertArrayHasKey( 'rcOresDamagingPref', $preferences );
$this->assertArrayHasKey( 'oresWatchlistHideNonDamaging',
$preferences );
$this->assertArrayHasKey( 'oresRCHideNonDamaging', $preferences
);
}
public function testGetThreshold() {
- $this->user->setOption( 'oresDamagingPref', 'maybebad' );
+ $this->user->setOption( 'rcOresDamagingPref', 'maybebad' );
$this->assertEquals(
0.16,
- Hooks::getThreshold( 'damaging', $this->user )
+ Hooks::getThreshold( 'damaging', $this->user,
$this->context->getTitle() )
);
// b/c
- $this->user->setOption( 'oresDamagingPref', 'soft' );
+ $this->user->setOption( 'rcOresDamagingPref', 'soft' );
$this->assertEquals(
0.56,
- Hooks::getThreshold( 'damaging', $this->user )
+ Hooks::getThreshold( 'damaging', $this->user,
$this->context->getTitle() )
);
}
@@ -69,7 +70,7 @@
$prefs = [];
PreferencesHookHandler::onGetPreferences( $this->user, $prefs );
- $this->assertSame( 5, count( $prefs ) );
+ $this->assertSame( 6, count( $prefs ) );
}
public function testOnGetBetaFeaturePreferences_on() {
--
To view, visit https://gerrit.wikimedia.org/r/392452
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifbc5a6b3a5f5e634cc308d769360365f30fceecc
Gerrit-PatchSet: 10
Gerrit-Project: mediawiki/extensions/ORES
Gerrit-Branch: master
Gerrit-Owner: Petar.petkovic <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Nikerabbit <[email protected]>
Gerrit-Reviewer: Petar.petkovic <[email protected]>
Gerrit-Reviewer: Sbisson <[email protected]>
Gerrit-Reviewer: Siebrand <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
