jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/354518 )
Change subject: Ensure the number of fitlered pages respects all filters
......................................................................
Ensure the number of fitlered pages respects all filters
This is basically done by using the queries in ApiPageTriageList
except doing a COUNT.
Bug: T165738
Change-Id: Icfd6aea82fb834c1ae93d19cabf1b5c5967396b9
---
M SpecialNewPagesFeed.php
M api/ApiPageTriageList.php
M api/ApiPageTriageStats.php
M i18n/en.json
M i18n/qqq.json
M includes/PageTriageUtil.php
M modules/ext.pageTriage.views.list/ext.pageTriage.listControlNav.js
7 files changed, 112 insertions(+), 120 deletions(-)
Approvals:
jenkins-bot: Verified
Kaldari: Looks good to me, approved
diff --git a/SpecialNewPagesFeed.php b/SpecialNewPagesFeed.php
index a598f96..cba88fd 100644
--- a/SpecialNewPagesFeed.php
+++ b/SpecialNewPagesFeed.php
@@ -229,33 +229,40 @@
<b><%=
mw.msg( 'pagetriage-filter-second-show-heading' ) %></b>
</span>
<div
class="mwe-pt-control-options">
- <input
type="radio" name="mwe-pt-filter-radio" id="mwe-pt-filter-no-categories" />
+ <input
type="radio" name="mwe-pt-filter-radio"
+
id="mwe-pt-filter-no-categories" value="no_category" />
<label
for="mwe-pt-filter-no-categories">
<%= mw.msg( 'pagetriage-filter-no-categories' ) %>
</label> <br/>
- <input
type="radio" name="mwe-pt-filter-radio" id="mwe-pt-filter-orphan" />
+ <input
type="radio" name="mwe-pt-filter-radio"
+
id="mwe-pt-filter-orphan" value="no_inbound_links" />
<label
for="mwe-pt-filter-orphan">
<%= mw.msg( 'pagetriage-filter-orphan' ) %>
</label> <br/>
- <input
type="radio" name="mwe-pt-filter-radio" id="mwe-pt-filter-non-autoconfirmed" />
+ <input
type="radio" name="mwe-pt-filter-radio"
+
id="mwe-pt-filter-non-autoconfirmed" value="non_autoconfirmed_users" />
<label
for="mwe-pt-filter-non-autoconfirmed">
<%= mw.msg( 'pagetriage-filter-non-autoconfirmed' ) %>
</label> <br/>
- <input
type="radio" name="mwe-pt-filter-radio" id="mwe-pt-filter-blocked" />
+ <input
type="radio" name="mwe-pt-filter-radio"
+
id="mwe-pt-filter-blocked" value="blocked_users" />
<label
for="mwe-pt-filter-blocked">
<%= mw.msg( 'pagetriage-filter-blocked' ) %>
</label> <br/>
- <input
type="radio" name="mwe-pt-filter-radio" id="mwe-pt-filter-bot-edits" />
+ <input
type="radio" name="mwe-pt-filter-radio"
+
id="mwe-pt-filter-bot-edits" value="showbots" />
<label
for="mwe-pt-filter-bot-edits">
<%= mw.msg( 'pagetriage-filter-bot-edits' ) %>
</label> <br/>
- <input
type="radio" name="mwe-pt-filter-radio" id="mwe-pt-filter-user-selected" />
+ <input
type="radio" name="mwe-pt-filter-radio"
+
id="mwe-pt-filter-user-selected" value="username" />
<label
for="mwe-pt-filter-user-selected">
<%= mw.msg( 'pagetriage-filter-user-heading' ) %>
</label>
<input
type="text" id="mwe-pt-filter-user"
placeholder="<%= mw.msg( 'pagetriage-filter-username' ) %>" /> <br/>
- <input
type="radio" name="mwe-pt-filter-radio" id="mwe-pt-filter-all" />
+ <input
type="radio" name="mwe-pt-filter-radio"
+
id="mwe-pt-filter-all" value="all" />
<label
for="mwe-pt-filter-all">
<%= mw.msg( 'pagetriage-filter-all' ) %>
</label>
diff --git a/api/ApiPageTriageList.php b/api/ApiPageTriageList.php
index 87b4b52..160b35d 100644
--- a/api/ApiPageTriageList.php
+++ b/api/ApiPageTriageList.php
@@ -98,24 +98,27 @@
/**
* Return all the page ids in PageTraige matching the specified filters
- * @param $opts array of filtering options
- * @return array an array of ids
+ * @param $opts array Array of filtering options
+ * @param $count boolean Set to true to return a count instead
+ * @return array|int an array of ids or total number of pages
*
- * @Todo - enforce a range of timestamp to reduce tag record scan
+ * @todo - enforce a range of timestamp to reduce tag record scan
*/
- public static function getPageIds( $opts = [] ) {
+ public static function getPageIds( $opts = [], $count = false ) {
// Initialize required variables
$pages = $options = [];
- // Get the expected limit as defined in getAllowedParams
- $options['LIMIT'] = $opts['limit'] + 1;
+ if ( !$count ) {
+ // Get the expected limit as defined in getAllowedParams
+ $options['LIMIT'] = $opts['limit'] + 1;
- if ( strtolower( $opts['dir'] ) === 'oldestfirst' ) {
- $options['ORDER BY'] = 'ptrp_created ASC, ptrp_page_id
ASC';
- $offsetOperator = ' > ';
- } else {
- $options['ORDER BY'] = 'ptrp_created DESC, ptrp_page_id
DESC';
- $offsetOperator = ' < ';
+ if ( strtolower( $opts['dir'] ) === 'oldestfirst' ) {
+ $options['ORDER BY'] = 'ptrp_created ASC,
ptrp_page_id ASC';
+ $offsetOperator = ' > ';
+ } else {
+ $options['ORDER BY'] = 'ptrp_created DESC,
ptrp_page_id DESC';
+ $offsetOperator = ' < ';
+ }
}
// Start building the massive filter which includes meta data
@@ -166,7 +169,8 @@
if (
array_key_exists( 'offset', $opts ) &&
is_numeric( $opts['offset'] ) &&
- $opts['offset'] > 0
+ $opts['offset'] > 0 &&
+ !$count
) {
$opts['offset'] = $dbr->addQuotes( $dbr->timestamp(
$opts['offset'] ) );
// Offset the list by page ID as well (in case multiple
pages have the same timestamp)
@@ -189,21 +193,30 @@
$tables[] = 'pagetriage_page_tags';
}
- // Pull page IDs from database
- $res = $dbr->select(
- $tables,
- 'ptrp_page_id',
- $conds,
- __METHOD__,
- $options
- );
+ if ( $count ) {
+ $res = $dbr->selectRow(
+ $tables,
+ [ 'COUNT(ptrp_page_id) AS total' ],
+ $conds
+ );
+ return (int) $res->total;
+ } else {
+ // Pull page IDs from database
+ $res = $dbr->select(
+ $tables,
+ 'ptrp_page_id',
+ $conds,
+ __METHOD__,
+ $options
+ );
- // Loop through result set and return ids
- foreach ( $res as $row ) {
- $pages[] = $row->ptrp_page_id;
+ // Loop through result set and return ids
+ foreach ( $res as $row ) {
+ $pages[] = $row->ptrp_page_id;
+ }
+
+ return $pages;
}
-
- return $pages;
}
/**
diff --git a/api/ApiPageTriageStats.php b/api/ApiPageTriageStats.php
index 252b07f..19d54c1 100644
--- a/api/ApiPageTriageStats.php
+++ b/api/ApiPageTriageStats.php
@@ -1,27 +1,28 @@
<?php
class ApiPageTriageStats extends ApiBase {
-
public function execute() {
- $params = $this->extractRequestParams();
+ // Remove empty params. This unforunately means you can't query
for User:0 :(
+ $params = array_filter( $this->extractRequestParams() );
- $filter = [];
- foreach ( $this->getAllowedParams() as $key => $value ) {
- if ( $key !== 'namespace' && $params[$key] ) {
- $filter[$key] = $key;
- }
+ // set default namespace
+ if ( empty( $params['namespace'] ) ) {
+ $params['namespace'] = 0;
}
$data = [
'unreviewedarticle' =>
PageTriageUtil::getUnreviewedArticleStat( $params['namespace'] ),
'reviewedarticle' =>
PageTriageUtil::getReviewedArticleStat( $params['namespace'] ),
- 'filteredarticle' =>
PageTriageUtil::getArticleFilterStat( $filter, $params['namespace'] )
+ 'filteredarticle' =>
PageTriageUtil::getArticleFilterStat( $params ),
];
$result = [ 'result' => 'success', 'stats' => $data ];
$this->getResult()->addValue( null, $this->getModuleName(),
$result );
}
+ /**
+ * @inheritDoc
+ */
public function getAllowedParams() {
return [
'namespace' => [
@@ -39,7 +40,24 @@
'showdeleted' => [
ApiBase::PARAM_TYPE => 'boolean',
],
+ 'showbots' => [
+ ApiBase::PARAM_TYPE => 'boolean',
+ ],
+ 'no_category' => [
+ ApiBase::PARAM_TYPE => 'boolean',
+ ],
+ 'no_inbound_links' => [
+ ApiBase::PARAM_TYPE => 'boolean',
+ ],
+ 'non_autoconfirmed_users' => [
+ ApiBase::PARAM_TYPE => 'boolean',
+ ],
+ 'blocked_users' => [
+ ApiBase::PARAM_TYPE => 'boolean',
+ ],
+ 'username' => [
+ ApiBase::PARAM_TYPE => 'user',
+ ],
];
}
-
}
diff --git a/i18n/en.json b/i18n/en.json
index eca86f8..ac3b5d5 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -399,6 +399,12 @@
"apihelp-pagetriagestats-param-showreviewed": "Whether to include
reviewed.",
"apihelp-pagetriagestats-param-showunreviewed": "Whether to include
unreviewed.",
"apihelp-pagetriagestats-param-showdeleted": "Whether to include
\"proposed for deleted\".",
+ "apihelp-pagetriagestats-param-showbots": "Whether to include only
pages created by bots.",
+ "apihelp-pagetriagestats-param-no_category": "Whether to include only
pages with no category.",
+ "apihelp-pagetriagestats-param-no_inbound_links": "Whether to include
only pages with no inbound links.",
+ "apihelp-pagetriagestats-param-non_autoconfirmed_users": "Whether to
include only pages created by non-autoconfirmed users.",
+ "apihelp-pagetriagestats-param-blocked_users": "Whether to include only
pages created by blocked users.",
+ "apihelp-pagetriagestats-param-username": "Include only pages created
by username.",
"apihelp-pagetriagetagging-description": "Add tags to an article.",
"apihelp-pagetriagetagging-param-pageid": "The article for which to be
tagged.",
"apihelp-pagetriagetagging-param-top": "The tagging text to be added to
the top of an article.",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index e2362f1..8f8f2fd 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -412,6 +412,12 @@
"apihelp-pagetriagestats-param-showreviewed":
"{{doc-apihelp-param|pagetriagestats|showreviewed}}",
"apihelp-pagetriagestats-param-showunreviewed":
"{{doc-apihelp-param|pagetriagestats|showunreviewed}}",
"apihelp-pagetriagestats-param-showdeleted":
"{{doc-apihelp-param|pagetriagestats|showdeleted}}",
+ "apihelp-pagetriagestats-param-showbots":
"{{doc-apihelp-param|pagetriagestats|showbots}}",
+ "apihelp-pagetriagestats-param-no_category":
"{{doc-apihelp-param|pagetriagestats|no_category}}",
+ "apihelp-pagetriagestats-param-no_inbound_links":
"{{doc-apihelp-param|pagetriagestats|no_inbound_links}}",
+ "apihelp-pagetriagestats-param-non_autoconfirmed_users":
"{{doc-apihelp-param|pagetriagestats|non_autoconfirmed_users}}",
+ "apihelp-pagetriagestats-param-blocked_users":
"{{doc-apihelp-param|pagetriagestats|blocked_users}}",
+ "apihelp-pagetriagestats-param-username":
"{{doc-apihelp-param|pagetriagestats|username}}",
"apihelp-pagetriagetagging-description":
"{{doc-apihelp-description|pagetriagetagging}}",
"apihelp-pagetriagetagging-param-pageid":
"{{doc-apihelp-param|pagetriagetagging|pageid}}",
"apihelp-pagetriagetagging-param-top":
"{{doc-apihelp-param|pagetriagetagging|top}}",
diff --git a/includes/PageTriageUtil.php b/includes/PageTriageUtil.php
index d2facfe..6799efd 100755
--- a/includes/PageTriageUtil.php
+++ b/includes/PageTriageUtil.php
@@ -105,68 +105,18 @@
}
/**
- * Get the number of pages based on the selected filter, it's fine to
do this since we have
- * a cron to remove page more than 30 days old from the queue, the
queue volume is not big,
- * we will change this accordingly if potential blocking problems are
spotted
+ * Get the number of pages based on the selected filters.
+ * @param array $filters Associative array of filter names/values.
+ * See ApiPageTriageStats->getAllowedParams() for
possible values,
+ * which are the same that the ApiPageTriageList
endpoint accepts.
+ * @return integer Number of pages based on the selected filters
*/
- public static function getArticleFilterStat( $filter, $namespace = '' )
{
- global $wgMemc;
-
- if ( !isset( $filter['showreviewed'] ) && !isset(
$filter['showunreviewed'] ) ) {
- $filter['showunreviewed'] = 'showunreviewed';
+ public static function getArticleFilterStat( $filters ) {
+ if ( !isset( $filters['showreviewed'] ) && !isset(
$filters['showunreviewed'] ) ) {
+ $filters['showunreviewed'] = 'showunreviewed';
}
- $namespace = self::validatePageNamespace( $namespace );
-
- $key = wfMemcKey(
- 'pagetriage',
- 'filter-article-' . implode( '-', $filter ) . '-' .
$namespace,
- 'stat', self::getCacheVersion()
- );
-
- $data = $wgMemc->get( $key );
- if ( $data !== false ) {
- return $data;
- }
-
- $dbr = wfGetDB( DB_SLAVE );
- $table = [ 'pagetriage_page', 'page' ];
- $conds = [
- 'page_id = ptrp_page_id',
- 'page_namespace' => $namespace
- ];
-
- $ops = '';
- if ( isset( $filter['showreviewed'] ) ) {
- $ops .= '>';
- }
- if ( isset( $filter['showunreviewed'] ) ) {
- $ops .= '=';
- }
-
- $conds[] = 'ptrp_reviewed ' . $ops . ' 0';
-
- if ( !isset( $filter['showdeleted'] ) ) {
- $conds['ptrp_deleted'] = 0;
- }
- if ( !isset( $filter['showredirs'] ) ) {
- $conds['page_is_redirect'] = 0;
- }
-
- $res = $dbr->selectRow(
- $table,
- [ 'COUNT(ptrp_page_id) AS total' ],
- $conds
- );
-
- $total = 0;
- if ( $res ) {
- $total = (int)$res->total;
- }
-
- // make it expire in 10 minutes
- $wgMemc->set( $key, $total, 600 );
- return $total;
+ return ApiPageTriageList::getPageIds( $filters, true );
}
public static function getReviewedArticleStat( $namespace = '' ) {
diff --git a/modules/ext.pageTriage.views.list/ext.pageTriage.listControlNav.js
b/modules/ext.pageTriage.views.list/ext.pageTriage.listControlNav.js
index d3ce519..b367320 100644
--- a/modules/ext.pageTriage.views.list/ext.pageTriage.listControlNav.js
+++ b/modules/ext.pageTriage.views.list/ext.pageTriage.listControlNav.js
@@ -119,22 +119,7 @@
// refresh the stats when a namespace is changed
refreshStats: function () {
- this.options.stats.apiParams = {};
- this.options.stats.setParam( 'namespace', $(
'#mwe-pt-filter-namespace' ).val() );
-
- if ( $( '#mwe-pt-filter-reviewed-edits' ).prop(
'checked' ) ) {
- this.options.stats.setParam( 'showreviewed',
'1' );
- }
- if ( $( '#mwe-pt-filter-unreviewed-edits' ).prop(
'checked' ) ) {
- this.options.stats.setParam( 'showunreviewed',
'1' );
- }
- if ( $( '#mwe-pt-filter-nominated-for-deletion' ).prop(
'checked' ) ) {
- this.options.stats.setParam( 'showdeleted', '1'
);
- }
- if ( $( '#mwe-pt-filter-redirects' ).prop( 'checked' )
) {
- this.options.stats.setParam( 'showredirs', '1'
);
- }
-
+ this.options.stats.apiParams = this.getApiParams();
this.options.stats.fetch();
},
@@ -241,8 +226,7 @@
}
},
- // Sync the filters with the contents of the menu
- filterSync: function () {
+ getApiParams: function () {
// fetch the values from the menu
var apiParams = {};
if ( $( '#mwe-pt-filter-namespace' ).val() ) {
@@ -303,6 +287,14 @@
apiParams.blocked_users = '1';
// jscs: enable
requireCamelCaseOrUpperCaseIdentifiers
}
+
+ return apiParams;
+ },
+
+ // Sync the filters with the contents of the menu
+ filterSync: function () {
+ // fetch the values from the menu
+ var apiParams = this.getApiParams();
// persist the limit and direction parameters
apiParams.limit = this.model.getParam( 'limit' );
@@ -386,7 +378,7 @@
* @param {string} message The message name for the filter.
*/
menuCheckboxUpdate: function ( $checkbox, param, message ) {
- $checkbox.prop( 'checked', parseInt(
this.model.getParam( param ) ) === 1 );
+ $checkbox.prop( 'checked', parseInt(
this.model.getParam( param ), 10 ) === 1 );
if ( this.model.getParam( param ) ) {
this.newFilterStatus.push( mw.msg( message ) );
}
--
To view, visit https://gerrit.wikimedia.org/r/354518
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Icfd6aea82fb834c1ae93d19cabf1b5c5967396b9
Gerrit-PatchSet: 11
Gerrit-Project: mediawiki/extensions/PageTriage
Gerrit-Branch: master
Gerrit-Owner: MusikAnimal <musikani...@wikimedia.org>
Gerrit-Reviewer: Kaldari <rkald...@wikimedia.org>
Gerrit-Reviewer: MusikAnimal <musikani...@wikimedia.org>
Gerrit-Reviewer: Samwilson <s...@samwilson.id.au>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
