From d9c360e0a4618b291c10a3af30674f1f5a80db4c Mon Sep 17 00:00:00 2001 From: Roan Kattouw Date: Tue, 3 Apr 2018 22:53:24 +0200 Subject: [PATCH] RCFilters: Convert patrolled filter to three states Unpatrolled, manual and auto. This moves the old hidepatrolled/hideunpatrolled filters to a legacy group, and adds a new string options group with three options. Also adds code mapping the old parameters to the new ones, and handling for the hidepatrolled preference. Bug: T190408 Change-Id: Ic1f181d3704c1d998696617a0d10270a87f22a62 --- .../specialpage/ChangesListSpecialPage.php | 106 ++++++++++++++---- includes/specials/SpecialRecentchanges.php | 8 +- includes/specials/SpecialWatchlist.php | 8 +- languages/i18n/en.json | 10 +- languages/i18n/qqq.json | 10 +- .../ChangesListSpecialPageTest.php | 28 ++++- 6 files changed, 137 insertions(+), 33 deletions(-) diff --git a/includes/specialpage/ChangesListSpecialPage.php b/includes/specialpage/ChangesListSpecialPage.php index 14b824f7d9..845e4949a1 100644 --- a/includes/specialpage/ChangesListSpecialPage.php +++ b/includes/specialpage/ChangesListSpecialPage.php @@ -87,9 +87,12 @@ abstract class ChangesListSpecialPage extends SpecialPage { // Same format as filterGroupDefinitions, but for a single group (reviewStatus) // that is registered conditionally. + private $legacyReviewStatusFilterGroupDefinition; + + // Single filter group registered conditionally private $reviewStatusFilterGroupDefinition; - // Single filter registered conditionally + // Single filter group registered conditionally private $hideCategorizationFilterDefinition; /** @@ -301,7 +304,7 @@ abstract class ChangesListSpecialPage extends SpecialPage { ] ], - // reviewStatus (conditional) + // significance (conditional) [ 'name' => 'significance', @@ -457,17 +460,14 @@ abstract class ChangesListSpecialPage extends SpecialPage { ]; - $this->reviewStatusFilterGroupDefinition = [ + $this->legacyReviewStatusFilterGroupDefinition = [ [ - 'name' => 'reviewStatus', + 'name' => 'legacyReviewStatus', 'title' => 'rcfilters-filtergroup-reviewstatus', 'class' => ChangesListBooleanFilterGroup::class, - 'priority' => -5, 'filters' => [ [ 'name' => 'hidepatrolled', - 'label' => 'rcfilters-filter-patrolled-label', - 'description' => 'rcfilters-filter-patrolled-description', // rcshowhidepatr-show, rcshowhidepatr-hide // wlshowhidepatr 'showHideSuffix' => 'showhidepatr', @@ -477,27 +477,75 @@ abstract class ChangesListSpecialPage extends SpecialPage { ) { $conds[] = 'rc_patrolled = 0'; }, - 'cssClassSuffix' => 'patrolled', - 'isRowApplicableCallable' => function ( $ctx, $rc ) { - return $rc->getAttribute( 'rc_patrolled' ); - }, + 'isReplacedInStructuredUi' => true, ], [ 'name' => 'hideunpatrolled', - 'label' => 'rcfilters-filter-unpatrolled-label', - 'description' => 'rcfilters-filter-unpatrolled-description', 'default' => false, 'queryCallable' => function ( $specialClassName, $ctx, $dbr, &$tables, &$fields, &$conds, &$query_options, &$join_conds ) { $conds[] = 'rc_patrolled != 0'; }, - 'cssClassSuffix' => 'unpatrolled', + 'isReplacedInStructuredUi' => true, + ], + ], + ] + ]; + + $this->reviewStatusFilterGroupDefinition = [ + [ + 'name' => 'reviewStatus', + 'title' => 'rcfilters-filtergroup-reviewstatus', + 'class' => ChangesListStringOptionsFilterGroup::class, + 'isFullCoverage' => true, + 'priority' => -5, + 'filters' => [ + [ + 'name' => 'unpatrolled', + 'label' => 'rcfilters-filter-reviewstatus-unpatrolled-label', + 'description' => 'rcfilters-filter-reviewstatus-unpatrolled-description', + 'cssClassSuffix' => 'reviewstatus-unpatrolled', + 'isRowApplicableCallable' => function ( $ctx, $rc ) { + return $rc->getAttribute( 'rc_patrolled' ) == RecentChange::PRC_UNPATROLLED; + }, + ], + [ + 'name' => 'manual', + 'label' => 'rcfilters-filter-reviewstatus-manual-label', + 'description' => 'rcfilters-filter-reviewstatus-manual-description', + 'cssClassSuffix' => 'reviewstatus-manual', 'isRowApplicableCallable' => function ( $ctx, $rc ) { - return !$rc->getAttribute( 'rc_patrolled' ); + return $rc->getAttribute( 'rc_patrolled' ) == RecentChange::PRC_PATROLLED; + }, + ], + [ + 'name' => 'auto', + 'label' => 'rcfilters-filter-reviewstatus-auto-label', + 'description' => 'rcfilters-filter-reviewstatus-auto-description', + 'cssClassSuffix' => 'reviewstatus-auto', + 'isRowApplicableCallable' => function ( $ctx, $rc ) { + return $rc->getAttribute( 'rc_patrolled' ) == RecentChange::PRC_AUTOPATROLLED; }, ], ], + 'default' => ChangesListStringOptionsFilterGroup::NONE, + 'queryCallable' => function ( $specialPageClassName, $ctx, $dbr, + &$tables, &$fields, &$conds, &$query_options, &$join_conds, $selected + ) { + if ( $selected === [] ) { + return; + } + $rcPatrolledValues = [ + 'unpatrolled' => RecentChange::PRC_UNPATROLLED, + 'manual' => RecentChange::PRC_PATROLLED, + 'auto' => RecentChange::PRC_AUTOPATROLLED, + ]; + // e.g. rc_patrolled IN (0, 2) + $conds['rc_patrolled'] = array_map( function ( $s ) use ( $rcPatrolledValues ) { + return $rcPatrolledValues[ $s ]; + }, $selected ); + } ] ]; @@ -910,6 +958,7 @@ abstract class ChangesListSpecialPage extends SpecialPage { // information to all users just because the user that saves the edit can // patrol or is logged in) if ( !$this->including() && $this->getUser()->useRCPatrol() ) { + $this->registerFiltersFromDefinitions( $this->legacyReviewStatusFilterGroupDefinition ); $this->registerFiltersFromDefinitions( $this->reviewStatusFilterGroupDefinition ); } @@ -1339,7 +1388,7 @@ abstract class ChangesListSpecialPage extends SpecialPage { } /** - * Replace old options 'hideanons' or 'hideliu' with structured UI equivalent + * Replace old options with their structured UI equivalents * * @param FormOptions $opts * @return bool True if the change was made @@ -1349,21 +1398,40 @@ abstract class ChangesListSpecialPage extends SpecialPage { return false; } + $changed = false; + // At this point 'hideanons' and 'hideliu' cannot be both true, // because fixBackwardsCompatibilityOptions resets (at least) 'hideanons' in such case if ( $opts[ 'hideanons' ] ) { $opts->reset( 'hideanons' ); $opts[ 'userExpLevel' ] = 'registered'; - return true; + $changed = true; } if ( $opts[ 'hideliu' ] ) { $opts->reset( 'hideliu' ); $opts[ 'userExpLevel' ] = 'unregistered'; - return true; + $changed = true; } - return false; + if ( $this->getFilterGroup( 'legacyReviewStatus' ) ) { + if ( $opts[ 'hidepatrolled' ] ) { + $opts->reset( 'hidepatrolled' ); + $opts[ 'reviewStatus' ] = 'unpatrolled'; + $changed = true; + } + + if ( $opts[ 'hideunpatrolled' ] ) { + $opts->reset( 'hideunpatrolled' ); + $opts[ 'reviewStatus' ] = implode( + ChangesListStringOptionsFilterGroup::SEPARATOR, + [ 'manual', 'auto' ] + ); + $changed = true; + } + } + + return $changed; } /** diff --git a/includes/specials/SpecialRecentchanges.php b/includes/specials/SpecialRecentchanges.php index d6d4c2723f..42edecb8aa 100644 --- a/includes/specials/SpecialRecentchanges.php +++ b/includes/specials/SpecialRecentchanges.php @@ -208,8 +208,12 @@ class SpecialRecentChanges extends ChangesListSpecialPage { $reviewStatus = $this->getFilterGroup( 'reviewStatus' ); if ( $reviewStatus !== null ) { // Conditional on feature being available and rights - $hidePatrolled = $reviewStatus->getFilter( 'hidepatrolled' ); - $hidePatrolled->setDefault( $user->getBoolOption( 'hidepatrolled' ) ); + if ( $user->getBoolOption( 'hidepatrolled' ) ) { + $reviewStatus->setDefault( 'unpatrolled' ); + $legacyReviewStatus = $this->getFilterGroup( 'legacyReviewStatus' ); + $legacyHidePatrolled = $legacyReviewStatus->getFilter( 'hidepatrolled' ); + $legacyHidePatrolled->setDefault( true ); + } } $changeType = $this->getFilterGroup( 'changeType' ); diff --git a/includes/specials/SpecialWatchlist.php b/includes/specials/SpecialWatchlist.php index 7b3f25c838..a94b2298a2 100644 --- a/includes/specials/SpecialWatchlist.php +++ b/includes/specials/SpecialWatchlist.php @@ -264,8 +264,12 @@ class SpecialWatchlist extends ChangesListSpecialPage { $reviewStatus = $this->getFilterGroup( 'reviewStatus' ); if ( $reviewStatus !== null ) { // Conditional on feature being available and rights - $hidePatrolled = $reviewStatus->getFilter( 'hidepatrolled' ); - $hidePatrolled->setDefault( $user->getBoolOption( 'watchlisthidepatrolled' ) ); + if ( $user->getBoolOption( 'watchlisthidepatrolled' ) ) { + $reviewStatus->setDefault( 'unpatrolled' ); + $legacyReviewStatus = $this->getFilterGroup( 'legacyReviewStatus' ); + $legacyHidePatrolled = $legacyReviewStatus->getFilter( 'hidepatrolled' ); + $legacyHidePatrolled->setDefault( true ); + } } $authorship = $this->getFilterGroup( 'authorship' ); diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 27f9814f37..e9199d6b14 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -1441,10 +1441,12 @@ "rcfilters-filter-humans-label": "Human (not bot)", "rcfilters-filter-humans-description": "Edits made by human editors.", "rcfilters-filtergroup-reviewstatus": "Review status", - "rcfilters-filter-patrolled-label": "Patrolled", - "rcfilters-filter-patrolled-description": "Edits marked as patrolled.", - "rcfilters-filter-unpatrolled-label": "Unpatrolled", - "rcfilters-filter-unpatrolled-description": "Edits not marked as patrolled.", + "rcfilters-filter-reviewstatus-unpatrolled-description": "Edits not manually or automatically marked as patrolled.", + "rcfilters-filter-reviewstatus-unpatrolled-label": "Unpatrolled", + "rcfilters-filter-reviewstatus-manual-description": "Edits manually marked as patrolled.", + "rcfilters-filter-reviewstatus-manual-label": "Manually patrolled", + "rcfilters-filter-reviewstatus-auto-description": "Edits by advanced users whose work is automatically marked as patrolled.", + "rcfilters-filter-reviewstatus-auto-label": "Autopatrolled", "rcfilters-filtergroup-significance": "Significance", "rcfilters-filter-minor-label": "Minor edits", "rcfilters-filter-minor-description": "Edits the author labeled as minor.", diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index c22b3c6d4a..170bdfbffc 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -1638,10 +1638,12 @@ "rcfilters-filter-humans-label": "Label for the filter for showing edits made by human editors.", "rcfilters-filter-humans-description": "Description for the filter for showing edits made by human editors.", "rcfilters-filtergroup-reviewstatus": "Title for the filter group about review status (in core this is whether it's been patrolled)", - "rcfilters-filter-patrolled-label": "Label for the filter for showing patrolled edits", - "rcfilters-filter-patrolled-description": "Label for the filter showing patrolled edits", - "rcfilters-filter-unpatrolled-label": "Label for the filter for showing unpatrolled edits", - "rcfilters-filter-unpatrolled-description": "Description for the filter for showing unpatrolled edits", + "rcfilters-filter-reviewstatus-manual-description": "Description for the filter showing manually patrolled edits", + "rcfilters-filter-reviewstatus-manual-label": "Label for the filter showing manually patrolled edits", + "rcfilters-filter-reviewstatus-auto-description": "Description for the filter showing automatically patrolled edits", + "rcfilters-filter-reviewstatus-auto-label": "Label for the filter showing automatically patrolled edits", + "rcfilters-filter-reviewstatus-unpatrolled-description": "Description for the filter for showing unpatrolled edits", + "rcfilters-filter-reviewstatus-unpatrolled-label": "Label for the filter for showing unpatrolled edits", "rcfilters-filtergroup-significance": "Title for the filter group for edit significance.\n{{Identical|Significance}}", "rcfilters-filter-minor-label": "Label for the filter for showing edits marked as minor.", "rcfilters-filter-minor-description": "Description for the filter for showing edits marked as minor.", diff --git a/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php b/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php index 51182180a9..aeaa1aee9e 100644 --- a/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php +++ b/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php @@ -347,7 +347,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase $user = $this->getTestSysop()->getUser(); $this->assertConditions( [ # expected - 'rc_patrolled = 0', + 'rc_patrolled' => 0, ], [ 'hidepatrolled' => 1, @@ -361,7 +361,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase $user = $this->getTestSysop()->getUser(); $this->assertConditions( [ # expected - 'rc_patrolled != 0', + 'rc_patrolled' => [ 1, 2 ], ], [ 'hideunpatrolled' => 1, @@ -371,6 +371,30 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase ); } + public function testRcReviewStatusFilter() { + $user = $this->getTestSysop()->getUser(); + $this->assertConditions( + [ #expected + 'rc_patrolled' => 1, + ], + [ + 'reviewStatus' => 'manual' + ], + "rc conditions: reviewStatus=manual", + $user + ); + $this->assertConditions( + [ #expected + 'rc_patrolled' => [ 0, 2 ], + ], + [ + 'reviewStatus' => 'unpatrolled;auto' + ], + "rc conditions: reviewStatus=unpatrolled;auto", + $user + ); + } + public function testRcHideminorFilter() { $this->assertConditions( [ # expected -- 2.20.1