jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/348169 )
Change subject: Put the "r" behind a preference in non-beta mode
......................................................................
Put the "r" behind a preference in non-beta mode
For RC, this is a new preference; for watchlist and contribs,
use the existing highlight preference.
Also make the "r" always black, never red.
Bug: T162831
Change-Id: Ie5ffefc697d53f1299f772c83d51f151c6ba872e
(cherry picked from commit 17a57bef752ac56f0f705dbe5d0a4c8b20fd5d1c)
---
M i18n/en.json
M i18n/qqq.json
M includes/Hooks.php
M modules/ext.ores.highlighter.css
M tests/phpunit/includes/HooksTest.php
5 files changed, 93 insertions(+), 28 deletions(-)
Approvals:
Niharika29: Looks good to me, approved
jenkins-bot: Verified
diff --git a/i18n/en.json b/i18n/en.json
index d0e24aa..f8904b3 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -15,6 +15,7 @@
"ores-help-damaging-pref": "Sets the level of probability at which the
system flags edits with an \"{{int:ores-damaging-letter}}\" to indicate they
\"need review\" on Recent Changes, Watchlist and Contributions. Also sets the
threshold for \"Show only likely problem edits\".",
"ores-hide-nondamaging-filter": "Hide probably good edits",
"ores-pref-damaging": "ORES sensitivity",
+ "ores-pref-damaging-flag": "Mark likely problem edits with 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-rcfilters-whats-this-link-text": "Learn more",
"ores-rcfilters-ores-conflicts-logactions-global": "The \"Logged
actions\" 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.",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index 60faf08..f965d13 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -21,6 +21,7 @@
"ores-help-damaging-pref": "Help text for \"ORES sensitivity\" in
preferences. \"Show only likely problem edits\" refers to
{{msg-mw|ores-pref-rc-hidenondamaging}} and
{{msg-mw|ores-pref-watchlist-hidenondamaging}}.",
"ores-hide-nondamaging-filter": "Label for Contributions filter, 'only
show'",
"ores-pref-damaging": "Part asking for damaging threshold",
+ "ores-pref-damaging-flag": "Label of preference that controls whether
the \"r\" ({{msg-mw|ores-damaging-letter}}) flag appears next to edits that are
possibly damaging.",
"ores-rcfilters-whats-this-link-text": "Text of a link to more
information about either Contribution Quality Predictions or User Intent
Predictions\n{{Identical|Learn more}}",
"ores-rcfilters-ores-conflicts-logactions-global": "Message shown in
the result area when both an ORES filter and the 'Logged actions' filter are
selected. This indicates that no results will be shown because log actions are
never scored by ORES.\n\n* \"Logged actions\" is
{{msg-mw|rcfilters-filter-logactions-label}}.\n* \"Contribution quality\" is
based on {{msg-mw|Ores-rcfilters-damaging-title}}.\n* \"User intent\" is based
on {{msg-mw|Ores-rcfilters-goodfaith-title}}.\n* \"Active Filters\" is based on
{{msg-mw|rcfilters-activefilters}}.",
"ores-rcfilters-logactions-conflicts-ores": "Tooltip shown when
hovering over a Logged actions filter tag, when an ORES filter is also
selected.\n\n* \"Logged actions\" is
{{msg-mw|rcfilters-filter-logactions-label}}.\n* \"Contribution quality\" is
based on {{msg-mw|Ores-rcfilters-damaging-title}}.\n* \"User intent\" is based
on {{msg-mw|Ores-rcfilters-goodfaith-title}}.",
diff --git a/includes/Hooks.php b/includes/Hooks.php
index b5830d7..05cdcd2 100644
--- a/includes/Hooks.php
+++ b/includes/Hooks.php
@@ -383,18 +383,24 @@
$damaging = self::getScoreRecentChangesList( $rc,
$changesList->getContext() );
if ( $damaging ) {
- $separator = ' <span class="mw-changeslist-separator">.
.</span> ';
- if ( strpos( $s, $separator ) === false ) {
- return;
- }
-
- $classes[] = 'damaging';
+ // Add highlight class
if ( self::isHighlightEnabled( $changesList ) ) {
$classes[] = 'ores-highlight';
}
- $parts = explode( $separator, $s );
- $parts[1] = ChangesList::flag( 'damaging' ) . $parts[1];
- $s = implode( $separator, $parts );
+
+ // Add damaging class and flag
+ if ( self::isDamagingFlagEnabled( $changesList ) ) {
+ $classes[] = 'damaging';
+
+ $separator = ' <span
class="mw-changeslist-separator">. .</span> ';
+ if ( strpos( $s, $separator ) === false ) {
+ return;
+ }
+
+ $parts = explode( $separator, $s );
+ $parts[1] = ChangesList::flag( 'damaging' ) .
$parts[1];
+ $s = implode( $separator, $parts );
+ }
}
return true;
@@ -450,7 +456,10 @@
self::addRowData( $context, $row->rev_id,
(float)$row->ores_damaging_score, 'damaging' );
- if ( $row->ores_damaging_score > $row->ores_damaging_threshold
) {
+ if (
+ self::isDamagingFlagEnabled( $context ) &&
+ $row->ores_damaging_score >
$row->ores_damaging_threshold
+ ) {
// Prepend the "r" flag
array_unshift( $flags, ChangesList::flag( 'damaging' )
);
}
@@ -472,10 +481,11 @@
}
if ( $row->ores_damaging_score > $row->ores_damaging_threshold
) {
- // Add the damaging class
- $classes[] = 'damaging';
- if ( $pager->getUser()->getBoolOption( 'oresHighlight'
) ) {
+ if ( self::isHighlightEnabled( $pager ) ) {
$classes[] = 'ores-highlight';
+ }
+ if ( self::isDamagingFlagEnabled( $pager ) ) {
+ $classes[] = 'damaging';
}
}
}
@@ -522,14 +532,15 @@
IContextSource $context
) {
$damaging = self::getScoreRecentChangesList( $rcObj, $context );
- if ( $damaging ) {
+
+ if ( $damaging && self::isDamagingFlagEnabled( $context ) ) {
$classes[] = 'damaging';
$data['recentChangesFlags']['damaging'] = true;
}
}
/**
- * Check if we should flag a row
+ * Check if we should flag a row. As a side effect, also adds score
data for this row.
* @param RecentChange $rcObj
* @param IContextSource $context
* @return bool
@@ -602,12 +613,19 @@
'help-message' => 'ores-help-damaging-pref',
];
- // highlight damaging edits based on configured sensitivity
if ( $wgOresExtensionStatus !== 'beta' ) {
+ // highlight damaging edits based on configured
sensitivity
$preferences['oresHighlight'] = [
'type' => 'toggle',
'section' => $oresSection,
'label-message' => 'ores-pref-highlight',
+ ];
+
+ // Control whether the "r" appears on RC
+ $preferences['ores-damaging-flag-rc'] = [
+ 'type' => 'toggle',
+ 'section' => 'rc/advancedrc',
+ 'label-message' => 'ores-pref-damaging-flag',
];
}
@@ -704,19 +722,41 @@
}
/**
- * @param Title $title
+ * @param IContextSource $context
+ * @return boolean Whether $context->getTitle() is a RecentChanges page
+ */
+ private static function isRCPage( IContextSource $context ) {
+ return $context->getTitle()->isSpecial( 'Recentchanges' ) ||
+ $context->getTitle()->isSpecial( 'Recentchangeslinked'
);
+ }
+
+ /**
+ * @param IContextSource $title
* @return boolean Whether highlights should be shown
*/
private static function isHighlightEnabled( IContextSource $context ) {
global $wgOresExtensionStatus;
return $wgOresExtensionStatus === 'beta' || (
- $context->getUser()->getBoolOption( 'oresHighlight' ) &&
- !$context->getTitle()->isSpecial( 'Recentchanges' ) &&
- !$context->getTitle()->isSpecial( 'Recentchangeslinked'
)
+ !self::isRCPage( $context ) &&
+ $context->getUser()->getBoolOption( 'oresHighlight' )
);
}
/**
+ * @param IContextSource $context
+ * @return boolean Whether the damaging flag ("r") should be shown
+ */
+ private static function isDamagingFlagEnabled( IContextSource $context
) {
+ global $wgOresExtensionStatus;
+ return $wgOresExtensionStatus === 'beta' ||
+ $context->getUser()->getBoolOption(
+ self::isRCPage( $context ) ?
+ 'ores-damaging-flag-rc' :
+ 'oresHighlight'
+ );
+ }
+
+ /**
* Check whether a given model is enabled in the config
* @param string $model
* @return bool
diff --git a/modules/ext.ores.highlighter.css b/modules/ext.ores.highlighter.css
index 2117e65..fe3f3d4 100644
--- a/modules/ext.ores.highlighter.css
+++ b/modules/ext.ores.highlighter.css
@@ -10,8 +10,3 @@
.damaging.ores-highlight[data-ores-damaging='hard'] {
background-color: #fef6e7;
}
-
-/* Make the "r" red */
-.ores-damaging {
- color: #f00;
-}
diff --git a/tests/phpunit/includes/HooksTest.php
b/tests/phpunit/includes/HooksTest.php
index 5824a97..a4d84c4 100644
--- a/tests/phpunit/includes/HooksTest.php
+++ b/tests/phpunit/includes/HooksTest.php
@@ -15,6 +15,7 @@
use RCCacheEntry;
use RecentChange;
use RequestContext;
+use SpecialPage;
use User;
use WANObjectCache;
@@ -32,8 +33,10 @@
parent::setUp();
$this->user = static::getTestUser()->getUser();
- $this->user->setOption( 'ores-enabled', "1" );
+ $this->user->setOption( 'ores-enabled', 1 );
$this->user->setOption( 'oresDamagingPref', 'soft' );
+ $this->user->setOption( 'oresHighlight', 1 );
+ $this->user->setOption( 'ores-damaging-flag-rc', 1 );
$this->user->saveSettings();
$this->context = self::getContext( $this->user );
@@ -252,6 +255,10 @@
->will( $this->returnValue( $this->user ) );
$ecl->expects( $this->any() )
+ ->method( 'getTitle' )
+ ->will( $this->returnValue( SpecialPage::getTitleFor(
'Recentchanges' ) ) );
+
+ $ecl->expects( $this->any() )
->method( 'getContext' )
->will( $this->returnValue( $this->context ) );
@@ -283,6 +290,10 @@
$cl->expects( $this->any() )
->method( 'getUser' )
->will( $this->returnValue( $this->user ) );
+
+ $cl->expects( $this->any() )
+ ->method( 'getTitle' )
+ ->will( $this->returnValue( SpecialPage::getTitleFor(
'Recentchanges' ) ) );
$cl->expects( $this->any() )
->method( 'getContext' )
@@ -318,6 +329,10 @@
$cl->expects( $this->any() )
->method( 'getUser' )
->will( $this->returnValue( $this->user ) );
+
+ $cl->expects( $this->any() )
+ ->method( 'getTitle' )
+ ->will( $this->returnValue( SpecialPage::getTitleFor(
'Recentchanges' ) ) );
$cl->expects( $this->any() )
->method( 'getContext' )
@@ -384,6 +399,10 @@
->will( $this->returnValue( $this->user ) );
$cp->expects( $this->any() )
+ ->method( 'getTitle' )
+ ->will( $this->returnValue( SpecialPage::getTitleFor(
'Contributions' ) ) );
+
+ $cp->expects( $this->any() )
->method( 'getContext' )
->will( $this->returnValue( $this->context ) );
@@ -448,6 +467,10 @@
->will( $this->returnValue( $this->user ) );
$cp->expects( $this->any() )
+ ->method( 'getTitle' )
+ ->will( $this->returnValue( SpecialPage::getTitleFor(
'Contributions' ) ) );
+
+ $cp->expects( $this->any() )
->method( 'getContext' )
->will( $this->returnValue( $this->context ) );
@@ -461,7 +484,7 @@
ORES\Hooks::onContributionsLineEnding( $cp, $ret, $row,
$classes );
- $this->assertSame( [ 'damaging' ], $classes );
+ $this->assertSame( [ 'ores-highlight', 'damaging' ], $classes );
$this->assertSame( [], $ret );
}
@@ -473,6 +496,10 @@
$cp->expects( $this->any() )
->method( 'getUser' )
->will( $this->returnValue( $this->user ) );
+
+ $cp->expects( $this->any() )
+ ->method( 'getTitle' )
+ ->will( $this->returnValue( SpecialPage::getTitleFor(
'Contributions' ) ) );
$cp->expects( $this->any() )
->method( 'getContext' )
@@ -496,7 +523,7 @@
$prefs = [];
ORES\Hooks::onGetPreferences( $this->user, $prefs );
- $this->assertSame( 4, count( $prefs ) );
+ $this->assertSame( 5, count( $prefs ) );
}
public function testOnGetBetaFeaturePreferences_on() {
@@ -534,6 +561,7 @@
$context->setLanguage( 'en' );
$context->setUser( $user );
+ $context->setTitle( SpecialPage::getTitleFor( 'Recentchanges' )
);
return $context;
}
--
To view, visit https://gerrit.wikimedia.org/r/348169
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie5ffefc697d53f1299f772c83d51f151c6ba872e
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/ORES
Gerrit-Branch: wmf/1.29.0-wmf.20
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Niharika29 <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
