jenkins-bot has submitted this change and it was merged.
Change subject: Implement flagging backend
......................................................................
Implement flagging backend
* add a new table (gather_list_flag) which tracks which user
flagged which list
* add gl_perm_override to track approved/hidden status
* add gl_needs_review to track unreviewed text changes
* add flag/approve modes to the editlist API
* when list is approved, flags become processed - they still prevent
further flags from that user/ip but otherwise have no effect
* when label or description is edited, approved becomes public,
and hidden is flagged as needing review
* hide items with >3 unreviewed flags from public lists, unless they
are approved (limit configurable via $wgGatherAutohideFlagLimit)
* create a review queue (lists API mode=review) which shows lists with
3+ unreviewed flags or a gl_needs_review flag
* expose gl_perm_override and gl_needs_review fields to lists API and
add some derived properties (flagged, hidden)
* add Special:Gather/all/review as a web interface to the review queue
* add UserMerge hooks for gather_list_flag
Bug: T97704
Change-Id: Iae85317f9560d80475b53a66691b474981c13668
---
M extension.json
M i18n/en.json
M i18n/qqq.json
M includes/Gather.hooks.php
M includes/api/ApiEditList.php
M includes/api/ApiMixinListAccess.php
M includes/api/ApiQueryLists.php
M includes/models/CollectionsList.php
M includes/specials/SpecialGather.php
M schema/Updater.hooks.php
A schema/archive/add-gl_list-flag-columns.sql
M schema/gather_list.sql
A schema/gather_list_flag.sql
M tests/phpunit/api/ApiEditListTest.php
M tests/phpunit/api/ApiQueryListPagesTest.php
M tests/phpunit/api/ApiQueryListsTest.php
M tests/phpunit/api/GatherTest.php
M tests/phpunit/api/GatherTestCase.php
18 files changed, 551 insertions(+), 85 deletions(-)
Approvals:
Gergő Tisza: Looks good to me, approved
jenkins-bot: Verified
diff --git a/extension.json b/extension.json
index 2dd3f73..cfb0efa 100644
--- a/extension.json
+++ b/extension.json
@@ -722,7 +722,8 @@
"GatherShouldShowTutorial": true,
"GatherAllowPublicWatchlist": false,
"GatherEnableBetaFeature": false,
- "GatherRecompileTemplates": false
+ "GatherRecompileTemplates": false,
+ "GatherAutohideFlagLimit": 1000000
},
"manifest_version": 1
}
diff --git a/i18n/en.json b/i18n/en.json
index e9e487a..3b18cda 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -110,6 +110,9 @@
"gather-moderation-unhidden": "[[$1|Your collection]] was unhidden.
This means that it can be viewed publicly again. ",
"gather-moderation-unhidden-email-subject": "Your collection $1 was
unhidden.",
"gather-moderation-unhidden-email-body": "Your collection $1 was
unhidden. This means that it can be viewed publicly again.",
+ "gather-moderation-approved": "[[$1|Your collection]] was approved.
This means that it won't be automatically hidden again.",
+ "gather-moderation-approved-email-subject": "Your collection $1 was
approved.",
+ "gather-moderation-approved-email-body": "Your collection $1 was
approved. This means that it won't be automatically hidden again.",
"echo-category-title-gather": "Gather",
"gather-echo-pref-tooltip": "Notify me about changes to my
collections.",
"gather-article-count": "$1 {{PLURAL:$1|page|pages}}",
@@ -140,6 +143,8 @@
"apihelp-editlist-paramvalue-mode-deletelist": "Delete the whole list.",
"apihelp-editlist-paramvalue-mode-hidelist": "Make the list private.
Requires the <kbd>gather-hidelist</kbd> user right (and cannot be undone
without that right).",
"apihelp-editlist-paramvalue-mode-showlist": "Undo a previous hidelist
operation.",
+ "apihelp-editlist-paramvalue-mode-flag": "Flag list as offensive.",
+ "apihelp-editlist-paramvalue-mode-approve": "Mark list as acceptable,
despite any flags. Requires the <kbd>gather-hidelist</kbd> userright.",
"apihelp-query+lists-description": "List collections of a given user.",
"apihelp-query+lists-param-mode": "Show all lists matching certain
criteria (cannot be used with owner/id parameters; if neither those nor mode is
provided, only the lists of the current user are shown).",
"apihelp-query+lists-param-prop": "Extra information to display.",
@@ -149,8 +154,11 @@
"apihelp-query+lists-param-limit": "Limit the number of returned
lists.",
"apihelp-query+lists-paramvalue-mode-allpublic": "Show all (public)
lists",
"apihelp-query+lists-paramvalue-mode-allhidden": "Show all hidden lists
(requires the <kbd>gather-hidelist</kbd> user right; does not show private
lists)",
+ "apihelp-query+lists-paramvalue-mode-review": "Show all lists that need
review (requires the <kbd>gather-hidelist</kbd> userright)",
"apihelp-query+lists-paramvalue-prop-label": "Title of the list",
"apihelp-query+lists-paramvalue-prop-description": "Description of the
list",
+ "apihelp-query+lists-paramvalue-prop-public": "Several
visibility-related properties: perm - <kbd>public</kbd> or <kbd>private</kbd>
(set by owner), private is only visible to owner; perm_override -
<kbd>hidden</kbd> or <kbd>approved</kbd> (or not present), can be set by a
moderator; flagged - the list got enough flags to qualify for moderator
attention, hidden - the list is hidden and only visible to the owenr and
moderators (due to flags or explicit moderator choice)",
+ "apihelp-query+lists-paramvalue-prop-review": "Is the list temporarily
hidden and waiting for moderator review?",
"apihelp-query+lists-paramvalue-prop-image": "Lead image",
"apihelp-query+lists-paramvalue-prop-count": "Number of items in the
list",
"apihelp-query+lists-paramvalue-prop-updated": "Timestamp of last
update",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index 22a893a..11239af 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -111,6 +111,9 @@
"gather-moderation-unhidden": "Label for echo notification explaining
that your collection was unhidden and that it can be viewed again as normal.
Parameters:\n* $1 - Url of collection.",
"gather-moderation-unhidden-email-subject": "Label for echo
notification email subject explaining that your collection was unhidden.
Parameters:\n* $1 - Url of collection.",
"gather-moderation-unhidden-email-body": "Label for echo notification
email body explaining that your collection was unhidden. Parameters:\n* $1 -
Url of collection.",
+ "gather-moderation-approved": "Label for echo notification explaining
that your collection was approved (ie. it won't be automatically hidden when a
lot of users flag it). Parameters:\n* $1 - Url of collection.",
+ "gather-moderation-approved-email-subject": "Label for echo
notification email subject explaining that your collection was approved.
Parameters:\n* $1 - Url of collection.",
+ "gather-moderation-approved-email-body": "Label for echo notification
email body explaining that your collection was approved. Parameters:\n* $1 -
Url of collection.",
"echo-category-title-gather": "A label for the Gather checkboxes in the
Echo notifications preferences.",
"gather-echo-pref-tooltip": "This is a short description of the gather
notification category in the Echo notifications
preferences.\n{{Related|Echo-pref-tooltip}}",
"gather-article-count": "Expression of the number of pages in a
collection. Parameter:\n* $1 - number of pages in the
collection\n{{Identical|Page}}",
@@ -141,6 +144,8 @@
"apihelp-editlist-paramvalue-mode-deletelist":
"{{doc-apihelp-paramvalue||mode|editlist|editlist|deletelist}}",
"apihelp-editlist-paramvalue-mode-hidelist":
"{{doc-apihelp-paramvalue||mode|editlist|editlist|hidelist}}",
"apihelp-editlist-paramvalue-mode-showlist":
"{{doc-apihelp-paramvalue||mode|editlist|editlist|showlist}}",
+ "apihelp-editlist-paramvalue-mode-flag":
"{{doc-apihelp-paramvalue||mode|editlist|editlist|flag}}",
+ "apihelp-editlist-paramvalue-mode-approve":
"{{doc-apihelp-paramvalue||mode|editlist|editlist|approve}}",
"apihelp-query+lists-description":
"{{doc-apihelp-description|lst|lists|query+lists}}",
"apihelp-query+lists-param-mode":
"{{doc-apihelp-param|lst|mode|lists|query+lists}}",
"apihelp-query+lists-param-prop":
"{{doc-apihelp-param|lst|prop|lists|query+lists}}",
@@ -150,8 +155,11 @@
"apihelp-query+lists-param-limit":
"{{doc-apihelp-param|lst|limit|lists|query+lists}}",
"apihelp-query+lists-paramvalue-mode-allpublic":
"{{doc-apihelp-paramvalue|lst|mode|lists|query+lists|allpublic}}",
"apihelp-query+lists-paramvalue-mode-allhidden":
"{{doc-apihelp-paramvalue|lst|mode|lists|query+lists|allhidden}}",
+ "apihelp-query+lists-paramvalue-mode-review":
"{{doc-apihelp-paramvalue|lst|mode|lists|query+lists|review}}",
"apihelp-query+lists-paramvalue-prop-label":
"{{doc-apihelp-paramvalue|lst|prop|lists|query+lists|label}}",
"apihelp-query+lists-paramvalue-prop-description":
"{{doc-apihelp-paramvalue|lst|prop|lists|query+lists|description}}",
+ "apihelp-query+lists-paramvalue-prop-public":
"{{doc-apihelp-paramvalue|lst|prop|lists|query+lists|public}}",
+ "apihelp-query+lists-paramvalue-prop-review":
"{{doc-apihelp-paramvalue|lst|prop|lists|query+lists|review}}",
"apihelp-query+lists-paramvalue-prop-image":
"{{doc-apihelp-paramvalue|lst|prop|lists|query+lists|image}}",
"apihelp-query+lists-paramvalue-prop-count":
"{{doc-apihelp-paramvalue|lst|prop|lists|query+lists|count}}",
"apihelp-query+lists-paramvalue-prop-updated":
"{{doc-apihelp-paramvalue|lst|prop|lists|query+lists|updated}}",
diff --git a/includes/Gather.hooks.php b/includes/Gather.hooks.php
index f66d87b..d21196c 100644
--- a/includes/Gather.hooks.php
+++ b/includes/Gather.hooks.php
@@ -13,6 +13,7 @@
use PageImages;
use BetaFeatures;
use User;
+use EchoEvent;
/**
* Hook handlers for Gather extension
@@ -87,6 +88,17 @@
'email-body-batch-params' => array( 'title' ),
);
+ $notifications['gather-approve'] = array(
+ 'category' => 'gather',
+ 'group' => 'positive',
+ 'title-message' => 'gather-moderation-approved',
+ 'title-params' => array( 'title' ),
+ 'email-subject-message' =>
'gather-moderation-approved-email-subject',
+ 'email-subject-params' => array( 'title' ),
+ 'email-body-batch-message' =>
'gather-moderation-approved-email-batch-body',
+ 'email-body-batch-params' => array( 'title' ),
+ );
+
return true;
}
@@ -100,6 +112,7 @@
switch ( $event->getType() ) {
case 'gather-hide':
case 'gather-unhide':
+ case 'gather-approve':
$extra = $event->getExtra();
if ( !$extra || !isset(
$extra['collection-owner-id'] ) ) {
break;
@@ -339,6 +352,8 @@
public static function onUserMergeAccountFields( array &$updateFields )
{
$updateFields[] = array( 'gather_list', 'gl_user', 'batchKey'
=> 'gl_id',
'options' => array( 'IGNORE' ) );
+ $updateFields[] = array( 'gather_list_flag', 'glf_user_id',
'batchKey' => 'glf_gl_id',
+ 'options' => array( 'IGNORE' ) );
}
/**
@@ -364,5 +379,8 @@
array( 'gl_user' => $newId, "gl_label =
CONCAT(gl_label, $disambiguator )" ),
array( 'gl_user' => $oldId ),
__METHOD__, array( 'IGNORE' ) );
+
+ // If both users flagged a list, just discard the flags made by
the old user.
+ $dbw->delete( 'gather_list_flag', array( 'glf_user_id' =>
$oldId ), __METHOD__ );
}
}
diff --git a/includes/api/ApiEditList.php b/includes/api/ApiEditList.php
index cf9729c..cd6c8f2 100644
--- a/includes/api/ApiEditList.php
+++ b/includes/api/ApiEditList.php
@@ -26,6 +26,7 @@
namespace Gather\api;
+use Gather\models\Collection;
use ManualLogEntry;
use AbuseFilter;
use AbuseFilterVariableHolder;
@@ -57,17 +58,51 @@
const PERM_PRIVATE = 0;
const PERM_PUBLIC = 1;
- const PERM_HIDDEN = 2;
+ const PERM_OVERRIDE_NONE = 0;
+ const PERM_OVERRIDE_HIDDEN = 1; // like private but cannot be published
by owner
+ const PERM_OVERRIDE_APPROVED = 2; // like public but cannot be
autohidden
+
+ /**
+ * Maps API actions (perm parameter values) to PERM_* constants (DB
values);
+ * also DB values to lists API properties
+ * @var array
+ */
+ public static $permMap = array(
+ 'public' => self::PERM_PUBLIC,
+ 'private' => self::PERM_PRIVATE,
+ );
+
+ /**
+ * Maps API actions (mode parameter values) to PERM_OVERRIDE_*
constants (DB values)
+ * @var array
+ */
+ public static $permOverrideMap = array(
+ 'hidelist' => self::PERM_OVERRIDE_HIDDEN,
+ 'showlist' => self::PERM_OVERRIDE_NONE,
+ 'approve' => self::PERM_OVERRIDE_APPROVED,
+ );
+
+ /**
+ * Maps API actions (mode parameter values) to Echo notification types
+ * @var array
+ */
+ public static $permOverrideNotificationMap = array(
+ 'hidelist' => 'gather-hide',
+ 'showlist' => 'gather-unhide',
+ 'approve' => 'gather-approve',
+ );
+
/**
* @param string $type Log type
* @param string $action Log action
* @param Title|null $title Title object or null
- * @param Skin|null $skin Skin object or null. If null, we want to use
the wiki
+ * @param \Skin|null $skin Skin object or null. If null, we want to use
the wiki
* content language, since that will go to the IRC feed.
* @param array $params Parameters
+ * @return string
*/
- public static function getGatherLogFormattedString( $type, $action,
$title, $sk, $params ) {
+ public static function getGatherLogFormattedString( $type, $action,
$title, $skin, $params ) {
return wfMessage( 'gather-checkuser-log-action' )
->rawParams( $params['action'], '[[' .
$title->getPrefixedText() . ']]' )->parse();
}
@@ -116,39 +151,78 @@
case 'deletelist':
// ACTION: delete list (items + list
itself)
$dbw->delete( 'gather_list_item',
array( 'gli_gl_id' => $listId ), __METHOD__ );
+ $dbw->delete( 'gather_list_flag',
array( 'glf_gl_id' => $listId ), __METHOD__ );
$dbw->delete( 'gather_list', array(
'gl_id' => $listId ), __METHOD__ );
$this->setResultStatus( $listId,
'deleted' );
$logEventName = $mode;
break;
case 'hidelist':
case 'showlist':
- $update = array();
- $perm = $mode === 'showlist' ?
self::PERM_PUBLIC : self::PERM_HIDDEN;
- if ( $row->gl_perm !== $perm ) {
- $update['gl_perm'] = $perm;
- }
+ case 'approve':
+ $update = array( 'gl_flag_count' => 0,
'gl_needs_review' => 0 );
+ $permOverride =
self::$permOverrideMap[$mode];
+ $update['gl_perm_override'] =
$permOverride;
+ $update = array_diff_assoc( $update,
(array)$row ); // remove fields with no changes
+
+ $dbw->begin( __METHOD__ );
$this->updateRow( $dbw, $row, $update );
- $logEventName = $mode;
- // Do echo notification
- if ( class_exists( 'EchoEvent' ) ) {
- $eventType = $mode ===
'showlist' ? 'gather-unhide' : 'gather-hide';
- // FIXME: better long term
solution for generating collection urls needed
- // Model currently handles it
which is not accessible from here
- $collectionTitle =
SpecialPage::getTitleFor( 'Gather' )
- ->getSubpage( 'id' )
- ->getSubpage(
$row->gl_id )
- ->getSubpage(
$row->gl_label );
-
- EchoEvent::create( array(
- 'type' => $eventType,
- 'title' =>
$collectionTitle,
- 'extra' => array(
-
'collection-owner-id' => $row->gl_user,
- ),
- 'agent' => $user,
- ) );
-
+ if ( $dbw->affectedRows() ) {
+ $logEventName = $mode;
}
+ $dbw->update( 'gather_list_flag',
+ array( 'glf_reviewed' => 1 ),
+ array( 'glf_gl_id' => $listId ),
+ __METHOD__ );
+ $dbw->commit( __METHOD__ );
+
+ if ( $logEventName ) {
+ // do echo notification, unless
the action was a noop
+ if ( class_exists( 'EchoEvent'
) && in_array( $mode, self::$permOverrideNotificationMap ) ) {
+ $eventType =
self::$permOverrideNotificationMap[$mode];
+ // FIXME: better long
term solution for generating collection urls needed
+ // Model currently
handles it which is not accessible from here
+ $collectionTitle =
SpecialPage::getTitleFor( 'Gather' )
+ ->getSubpage(
'id' )
+ ->getSubpage(
$row->gl_id )
+ ->getSubpage(
$row->gl_label );
+
+ EchoEvent::create(
array(
+ 'type' =>
$eventType,
+ 'title' =>
$collectionTitle,
+ 'extra' =>
array(
+
'collection-owner-id' => $row->gl_user,
+ ),
+ 'agent' =>
$user,
+ ) );
+ }
+ }
+ break;
+ case 'flag':
+ $dbw->begin( __METHOD__ );
+ // lock list to avoid race condition
with a show/hide/approve
+ $dbw->select( 'gather_list', 'gl_id',
array( 'gl_id' => $listId ), __METHOD__,
+ array( 'FOR UPDATE' ) );
+ $dbw->insert( 'gather_list_flag',
+ array(
+ 'glf_user_id' =>
$user->getId(),
+ 'glf_user_ip' =>
$user->getId() ? '' : $this->getRequest()->getIP(),
+ 'glf_gl_id' => $listId,
+ ),
+ __METHOD__,
+ array( 'IGNORE' )
+ );
+ if ( !$dbw->affectedRows() ) {
+ $dbw->rollback( __METHOD__ );
+ $this->dieUsage( "List already
flagged by user",
+ 'alreadyflagged' );
+ }
+ $dbw->update( 'gather_list',
+ array( 'gl_flag_count =
gl_flag_count + 1' ),
+ array( 'gl_id' => $listId ),
+ __METHOD__ );
+ $dbw->commit( __METHOD__ );
+ $this->setResultStatus( $row->gl_id,
'flagged' );
+ $logEventName = $mode;
break;
}
}
@@ -187,8 +261,8 @@
\CheckUserHooks::updateCheckUserData( $rc );
}
- // Surface hide and unhide actions in Special:Log
- if ( $action === 'hidelist' || $action === 'showlist' ) {
+ // Surface hide, unhide and approve actions in Special:Log
+ if ( $action === 'hidelist' || $action === 'showlist' ||
$action === 'approve' ) {
$logId = $entry->insert();
$entry->publish( $logId, 'udp' );
}
@@ -251,11 +325,12 @@
$label = $params['label'];
// These modes cannot change list items or change other params
like label/description/...
// Incidentally, these are also modes that cannot be applied to
the watchlist
- $isNoUpdatesMode = in_array( $mode, array( 'showlist',
'hidelist', 'deletelist' ) );
+ $isNoUpdatesMode = in_array( $mode,
+ array( 'showlist', 'hidelist', 'deletelist', 'approve',
'flag' ) );
- if ( !$user->isLoggedIn() ) {
+ if ( !$user->isLoggedIn() && $mode !== 'flag' ) {
$this->dieUsage( 'You must be logged-in to edit a
list', 'notloggedin' );
- } elseif ( !$user->isAllowed( 'editmywatchlist' ) ) {
+ } elseif ( !$user->isAllowed( 'editmywatchlist' ) && $mode !==
'flag' ) {
$this->dieUsage( 'You don\'t have permission to edit
your list', 'permissiondenied' );
} elseif ( $user->isBlocked() ) {
$this->dieUsage( 'You are blocked from editing your
list', 'blocked' );
@@ -281,15 +356,21 @@
}
}
if ( $isNew ) {
- if ( $isNoUpdatesMode ) {
+ if ( $isNoUpdatesMode || $mode === 'remove' ) {
// These modes are not allowed for the new list
(no ID)
$this->dieUsage( "List must be identified with
{$p}id when {$p}mode=$mode is used",
'invalidparammix' );
- } elseif ( $mode === 'remove' ) {
- $this->dieUsage( "List must be identified with
{$p}id when {$p}mode=remove is used",
- 'invalidparammix' );
} elseif ( $label === null ) {
$this->dieUsage( "List {$p}label must be given
for new lists", 'invalidparammix' );
+ }
+ }
+ if ( $params['perm'] ) {
+ if ( $mode !== 'update' ) {
+ $this->dieUsage( "{$p}mode and {$p}perm cannot
be used together", 'invalidparammix' );
+ }
+ if ( $row && $row->gl_perm_override !==
self::PERM_OVERRIDE_NONE ) {
+ $this->dieUsage( "Cannot change visibility when
it is overriden by an admin",
+ 'invalidparammix' );
}
}
switch ( $mode ) {
@@ -300,13 +381,10 @@
$this->dieUsage( "List {$p}id does not
belong to the current user",
'permissiondenied' );
}
- if ( $row && $params['perm'] !== null &&
$row->gl_perm === self::PERM_HIDDEN ) {
- $this->dieUsage( "Hidden list may not
be made private or public",
- 'permissiondenied' );
- }
break;
case 'hidelist':
case 'showlist':
+ case 'approve':
if ( !$user->isAllowed( 'gather-hidelist' ) ) {
$this->dieUsage( "{$p}mode=$mode
requires gather-hidelist permissions",
'permissiondenied' );
@@ -314,8 +392,11 @@
if ( $row &&
$row->gl_perm === self::PERM_PRIVATE
&& $row->gl_user !== $user->getId()
) {
- $this->dieUsage( "Private list may not
be made hidden", 'invalidparammix' );
+ $this->dieUsage( "Visibility of private
list may only be changed by author",
+ 'invalidparammix' );
}
+ break;
+ case 'flag':
break;
default:
$this->dieDebug( __METHOD__, 'Unknown mode=' .
$mode );
@@ -400,6 +481,8 @@
'deletelist',
'hidelist',
'showlist',
+ 'flag',
+ 'approve',
),
ApiBase::PARAM_HELP_MSG_PER_VALUE => array(),
),
@@ -422,11 +505,14 @@
* Given an info object, update it with arguments from params, and
return JSON str if changed
* @param stdClass $v
* @param Array $params
+ * @param bool $resetPermission true if the list has been edited in
such a way that the
+ * review status is reset (APPROVED becomes plain PUBLIC, HIDDEN
becomes flagged for review)
* @return string JSON encoded info object in case it changed, or NULL
if update is not needed
* @throws \UsageException
*/
- private function updateInfo( stdClass $v, array $params ) {
+ private function updateInfo( stdClass $v, array $params,
&$resetPermission = null ) {
$updated = false;
+ $resetPermission = false;
// Set default
if ( !property_exists( $v, 'description' ) ) {
@@ -440,6 +526,7 @@
if ( $params['description'] !== null && $v->description !==
$params['description'] ) {
$v->description = $params['description'];
$updated = true;
+ $resetPermission = true;
}
if ( $params['image'] !== null && $v->image !==
$params['image'] ) {
if ( $params['image'] === '' ) {
@@ -483,6 +570,7 @@
$label = $isWatchlist ? '' : $params['label'];
$info = $this->updateInfo( new stdClass(), $params );
$createRow = !$isWatchlist || $info || $params['perm'] ===
'public';
+ $perm = $params['perm'] ? self::$permMap[$params['perm']] :
self::PERM_PRIVATE;
if ( $createRow ) {
$id = $dbw->nextSequenceValue( 'gather_list_gl_id_seq'
);
@@ -491,7 +579,7 @@
'gl_user' => $user->getId(),
'gl_label' => $label,
'gl_info' => $info,
- 'gl_perm' => $params['perm'] === 'public' ?
self::PERM_PUBLIC : self::PERM_PRIVATE,
+ 'gl_perm' => $perm,
'gl_updated' => $dbw->timestamp(
wfTimestampNow() ),
), __METHOD__, 'IGNORE' );
$id = $dbw->insertId();
@@ -531,12 +619,20 @@
private function updateListDb( DatabaseBase $dbw, array $params, $row )
{
$update = array();
$info = self::parseListInfo( $row->gl_info, $row->gl_id, true );
- $info = $this->updateInfo( $info, $params );
+ $info = $this->updateInfo( $info, $params, $resetPermission );
if ( $info ) {
$update['gl_info'] = $info;
}
if ( $params['label'] !== null && $row->gl_label !==
$params['label'] ) {
$update['gl_label'] = $params['label'];
+ $resetPermission = true;
+ }
+ if ( $resetPermission ) {
+ if ( $row->gl_perm_override ===
self::PERM_OVERRIDE_HIDDEN ) {
+ $update['gl_needs_review'] = 1;
+ } elseif ( $row->gl_perm_override ===
self::PERM_OVERRIDE_APPROVED ) {
+ $update['gl_perm_override'] =
self::PERM_OVERRIDE_NONE;
+ }
}
if ( $params['perm'] !== null ) {
$perm = $params['perm'] === 'public' ?
@@ -719,7 +815,8 @@
*/
private function getListRow( array $params, DatabaseBase $dbw, array
$conds ) {
$row = self::normalizeRow( $dbw->selectRow( 'gather_list',
- array( 'gl_id', 'gl_user', 'gl_label', 'gl_perm',
'gl_info' ), $conds, __METHOD__ ) );
+ array( 'gl_id', 'gl_user', 'gl_label', 'gl_perm',
'gl_perm_override', 'gl_item_count',
+ 'gl_flag_count', 'gl_needs_review',
'gl_info' ), $conds, __METHOD__ ) );
if ( $row ) {
$this->checkPermissions( $params, $row );
}
@@ -737,6 +834,10 @@
self::normalizeInt( $row, 'gl_id' );
self::normalizeInt( $row, 'gl_user' );
self::normalizeInt( $row, 'gl_perm' );
+ self::normalizeInt( $row, 'gl_perm_override' );
+ self::normalizeInt( $row, 'gl_item_count' );
+ self::normalizeInt( $row, 'gl_flag_count' );
+ self::normalizeInt( $row, 'gl_needs_review' );
}
return $row;
}
diff --git a/includes/api/ApiMixinListAccess.php
b/includes/api/ApiMixinListAccess.php
index ddea2dc..dd02028 100644
--- a/includes/api/ApiMixinListAccess.php
+++ b/includes/api/ApiMixinListAccess.php
@@ -68,6 +68,8 @@
public static function checkListAccess(
DatabaseBase $db, ApiBase $module, array $params,
&$isWatchlist, &$ownerId
) {
+ global $wgGatherAutohideFlagLimit;
+
if ( is_null( $params['owner'] ) !== is_null( $params['token']
) ) {
$p = $module->getModulePrefix();
$module->dieUsage( "Both {$p}owner and {$p}token must
be given or missing",
@@ -85,7 +87,8 @@
// Id was given, this could be public or private list, legacy
watchlist or regular
// Allow access to any public list/watchlist, and to private
with proper owner/self
$listRow = $db->selectRow( 'gather_list',
- array( 'gl_label', 'gl_user', 'gl_perm' ),
+ array( 'gl_label', 'gl_user', 'gl_perm',
'gl_perm_override', 'gl_flag_count',
+ 'gl_needs_review' ),
array( 'gl_id' => $params['id'] ),
__METHOD__ );
if ( $listRow === false ) {
@@ -112,7 +115,12 @@
}
// Check if this is a public list (if required)
- if ( !$showPrivate && $listRow->gl_perm !==
ApiEditList::PERM_PUBLIC ) {
+ if ( !$showPrivate && (
+ $listRow->gl_perm !== ApiEditList::PERM_PUBLIC
+ || $listRow->gl_perm_override ===
ApiEditList::PERM_OVERRIDE_HIDDEN
+ || $listRow->gl_flag_count >= $wgGatherAutohideFlagLimit
+ && $listRow->gl_perm_override !==
ApiEditList::PERM_OVERRIDE_APPROVED
+ ) ) {
$module->dieUsage( 'You have no rights to see this
list', 'badid' );
}
diff --git a/includes/api/ApiQueryLists.php b/includes/api/ApiQueryLists.php
index cc6c104..e732f44 100644
--- a/includes/api/ApiQueryLists.php
+++ b/includes/api/ApiQueryLists.php
@@ -39,11 +39,23 @@
* @ingroup API
*/
class ApiQueryLists extends ApiQueryBase {
+ /**
+ * Maps DB values to API property values
+ * @var array
+ */
+ public static $permOverrideApiMap = array(
+ ApiEditList::PERM_OVERRIDE_NONE => '',
+ ApiEditList::PERM_OVERRIDE_HIDDEN => 'hidden',
+ ApiEditList::PERM_OVERRIDE_APPROVED => 'approved',
+ );
+
public function __construct( ApiQuery $query, $moduleName ) {
parent::__construct( $query, $moduleName, 'lst' );
}
public function execute() {
+ global $wgGatherAutohideFlagLimit;
+
$p = $this->getModulePrefix();
$params = $this->extractRequestParams();
$continue = $params['continue'];
@@ -53,6 +65,7 @@
$fld_label = in_array( 'label', $params['prop'] );
$fld_description = in_array( 'description', $params['prop'] );
$fld_public = in_array( 'public', $params['prop'] );
+ $fld_review = in_array( 'review', $params['prop'] );
$fld_image = in_array( 'image', $params['prop'] );
$fld_updated = in_array( 'updated', $params['prop'] );
$fld_owner = in_array( 'owner', $params['prop'] );
@@ -111,6 +124,9 @@
}
$this->addFieldsIf( 'gl_updated', $fld_updated || $mode );
$this->addFieldsIf( 'gl_perm', $fld_public );
+ $this->addFieldsIf( 'gl_perm_override', $fld_public );
+ $this->addFieldsIf( 'gl_flag_count', $fld_public );
+ $this->addFieldsIf( 'gl_needs_review', $fld_review );
if ( $fld_owner && !$owner ) {
// Join user table to get user name
@@ -149,9 +165,33 @@
}
if ( $mode === 'allhidden' ) {
- $this->addWhereFld( 'gl_perm', ApiEditList::PERM_HIDDEN
);
+ // lists made private by their owners are not
considered hidden
+ $this->addWhereFld( 'gl_perm', ApiEditList::PERM_PUBLIC
);
+ $this->addWhere( $db->makeList( array(
+ 'gl_perm_override' =>
ApiEditList::PERM_OVERRIDE_HIDDEN,
+ $db->makeList( array(
+ 'gl_flag_count >= ' .
$wgGatherAutohideFlagLimit,
+ 'gl_perm_override != ' .
ApiEditList::PERM_OVERRIDE_APPROVED,
+ ), LIST_AND ),
+ ), LIST_OR ) );
+ } elseif ( $mode === 'review' ) {
+ $this->addWhereFld( 'gl_perm', ApiEditList::PERM_PUBLIC
);
+ $this->addWhere( $db->makeList( array(
+ 'gl_needs_review' => 1,
+ 'gl_flag_count >= ' .
$wgGatherAutohideFlagLimit,
+ ), LIST_OR ) );
+
} elseif ( $showPrivate !== true ) {
- $cond = array( 'gl_perm' => ApiEditList::PERM_PUBLIC );
+ $cond = array();
+ $cond[] = $db->makeList( array(
+ 'gl_perm' => ApiEditList::PERM_PUBLIC,
+ 'gl_perm_override' =>
ApiEditList::PERM_OVERRIDE_APPROVED,
+ ), LIST_AND );
+ $cond[] = $db->makeList( array(
+ 'gl_perm' => ApiEditList::PERM_PUBLIC,
+ 'gl_perm_override != ' .
ApiEditList::PERM_OVERRIDE_HIDDEN,
+ 'gl_flag_count < ' . $wgGatherAutohideFlagLimit,
+ ), LIST_AND );
if ( $showPrivate === null ) {
$cond['gl_user'] = $this->getUser()->getId();
}
@@ -243,8 +283,8 @@
// The very first DB row already has a
label, so inject a fake
if ( !$this->processRow(
null, $count, $mode, $limit,
$useInfo, $title, $fld_label,
- $fld_description, $fld_public,
$fld_image, $fld_updated, $fld_owner, $path,
- $owner,
$getContinueEnumParameter
+ $fld_description, $fld_public,
$fld_review, $fld_image, $fld_updated,
+ $fld_owner, $path, $owner,
$getContinueEnumParameter
) ) {
break;
}
@@ -253,7 +293,7 @@
}
if ( !$this->processRow(
$row, $count, $mode, $limit, $useInfo, $title,
$fld_label, $fld_description,
- $fld_public, $fld_image, $fld_updated,
$fld_owner, $path, $owner,
+ $fld_public, $fld_review, $fld_image,
$fld_updated, $fld_owner, $path, $owner,
$getContinueEnumParameter
) ) {
break;
@@ -264,7 +304,7 @@
// There are no records in the database, and we need to
inject watchlist row
$this->processRow(
null, $count, $mode, $limit, $useInfo, $title,
$fld_label, $fld_description,
- $fld_public, $fld_image, $fld_updated,
$fld_owner, $path, $owner,
+ $fld_public, $fld_review, $fld_image,
$fld_updated, $fld_owner, $path, $owner,
$getContinueEnumParameter
);
}
@@ -297,6 +337,7 @@
'label',
'description',
'public',
+ 'review',
'image',
'count',
'updated',
@@ -326,7 +367,7 @@
ApiBase::PARAM_TYPE => 'limit',
ApiBase::PARAM_MIN => 1,
ApiBase::PARAM_MAX => ApiBase::LIMIT_BIG1,
- ApiBase::PARAM_MAX2 => ApiBase::LIMIT_BIG2
+ ApiBase::PARAM_MAX2 => ApiBase::LIMIT_BIG2,
),
'continue' => array(
ApiBase::PARAM_HELP_MSG =>
'api-help-param-continue',
@@ -356,6 +397,7 @@
* @param bool $fld_label True if the label property is requested.
* @param bool $fld_description True if the description property is
requested.
* @param bool $fld_public True if the public property is requested.
+ * @param bool $fld_review True if the review property is requested.
* @param bool $fld_image True if the image property is requested.
* @param bool $fld_updated True if the updated property is requested.
* @param bool $fld_owner True if the owner property is requested.
@@ -367,14 +409,19 @@
*/
private function processRow(
$row, &$count, $mode, $limit, $useInfo, $title, $fld_label,
$fld_description, $fld_public,
- $fld_image, $fld_updated, $fld_owner, $path, $owner,
$getContinueEnumParameter
+ $fld_review, $fld_image, $fld_updated, $fld_owner, $path,
$owner, $getContinueEnumParameter
) {
+ global $wgGatherAutohideFlagLimit;
+
if ( $row === null ) {
// Fake watchlist row
$row = (object) array(
'gl_id' => 0,
'gl_label' => '',
'gl_perm' => ApiEditList::PERM_PRIVATE,
+ 'gl_perm_override' =>
ApiEditList::PERM_OVERRIDE_NONE,
+ 'gl_needs_review' => 0,
+ 'gl_flag_count' => 0,
'gl_updated' => '',
'gl_info' => '',
);
@@ -412,19 +459,34 @@
}
}
if ( $fld_public ) {
- switch ( $row->gl_perm ) {
- case ApiEditList::PERM_PRIVATE:
- $data['perm'] = 'private';
- break;
- case ApiEditList::PERM_PUBLIC:
- $data['perm'] = 'public';
- break;
- case ApiEditList::PERM_HIDDEN:
- $data['perm'] = 'hidden';
- break;
- default:
- $this->dieDebug( __METHOD__,
- "Unknown gather
perm={$row->gl_perm} for id {$row->gl_id}" );
+ $permMap = array_flip( ApiEditList::$permMap );
+ $permOverrideMap = self::$permOverrideApiMap;
+ if ( !array_key_exists( $row->gl_perm, $permMap ) ) {
+ $this->dieDebug( __METHOD__,
+ "Unknown gather perm={$row->gl_perm}
for id {$row->gl_id}" );
+ }
+ if ( !array_key_exists( $row->gl_perm_override,
$permOverrideMap ) ) {
+ $this->dieDebug( __METHOD__, "Unknown gather "
+ .
"perm_override={$row->gl_perm_override} for id {$row->gl_id}" );
+ }
+ $data['perm'] = $permMap[$row->gl_perm];
+ if ( $permOverrideMap[$row->gl_perm_override] ) {
+ $data['perm_override'] =
$permOverrideMap[$row->gl_perm_override];
+ }
+ if ( $row->gl_flag_count >= $wgGatherAutohideFlagLimit
) {
+ $data['flagged'] = true;
+ }
+ if (
+ $row->gl_perm_override ===
ApiEditList::PERM_OVERRIDE_HIDDEN
+ || $row->gl_perm_override ===
ApiEditList::PERM_OVERRIDE_NONE
+ && $row->gl_flag_count >=
$wgGatherAutohideFlagLimit
+ ) {
+ $data['hidden'] = true;
+ }
+ }
+ if ( $fld_review ) {
+ if ( $row->gl_needs_review || $row->gl_flag_count >=
$wgGatherAutohideFlagLimit ) {
+ $data['review'] = true;
}
}
if ( $useInfo ) {
diff --git a/includes/models/CollectionsList.php
b/includes/models/CollectionsList.php
index 4346242..e4c61ca 100644
--- a/includes/models/CollectionsList.php
+++ b/includes/models/CollectionsList.php
@@ -145,12 +145,13 @@
* @param boolean [$includePrivate] if the list should show private
collections or not
* @param string|boolean [$memberTitle] title of member to check for
* @param array [$params] generate collection list with additional api
parameters
- * @param string [$mode] to run
+ * @param string [$mode] listing mode (public/private/review)
* @param integer [$limit] of number of collections to show
* @return models\CollectionsList List of collections.
*/
public static function newFromApi( $user = null, $includePrivate =
false,
- $memberTitle = false, $params = array(), $mode = null, $limit =
50 ) {
+ $memberTitle = false, $params = array(), $mode = null, $limit =
50
+ ) {
$collectionsList = new CollectionsList( $user, $includePrivate
);
$query = array_merge( $params, array(
'action' => 'query',
@@ -166,7 +167,7 @@
$query['lstowner'] = $user->getName();
}
if ( $mode ) {
- $query['lstmode'] = "all$mode";
+ $query['lstmode'] = ( $mode === 'review' ) ? $mode :
"all$mode";
} else {
$mode = '';
}
diff --git a/includes/specials/SpecialGather.php
b/includes/specials/SpecialGather.php
index 7882586..73e3765 100644
--- a/includes/specials/SpecialGather.php
+++ b/includes/specials/SpecialGather.php
@@ -156,19 +156,23 @@
SpecialPage::getTitleFor( 'Gather'
)->getSubPage( 'id' )->getSubPage( $id )->getLocalURL()
);
} elseif ( preg_match( '/^all(\/(?<cond>[^\/]+))?\/?$/',
$subpage, $matches ) ) {
- // All collections. Public or hidden
+ // All collections. Public or hidden or pending review
// /all = /all/ = /all/public = /all/public/
// /all/hidden = /all/hidden/
// /all/recent = /all/recent/
+ // /all/review = /all/review/
$apiParams = array();
$displayAsTable = true;
// Mode defaults to public
+ $allowedModes = array( 'public', 'hidden', 'recent',
'review' );
if ( !isset( $matches['cond'] ) ) {
$mode = 'public';
- } else {
+ } elseif ( in_array( $matches['cond'], $allowedModes )
) {
$mode = $matches['cond'];
+ } else {
+ $mode = 'error';
}
$originalMode = $mode;
@@ -182,7 +186,7 @@
// Active is a concrete view of public with
different params.
$mode = 'public';
- } elseif ( $mode === 'hidden' ) {
+ } elseif ( $mode === 'hidden' || $mode === 'review' ) {
// Table list of hidden collections.
// If user doesn't have permissions, bail out.
@@ -207,7 +211,7 @@
$req = $this->getRequest();
$apiParams = array_merge( $apiParams, $req->getValues()
);
- $cList = models\CollectionsList::newFromApi( null,
$mode === 'hidden',
+ $cList = models\CollectionsList::newFromApi( null,
$mode,
false, $apiParams, $mode, 100 );
$cList->setMode( $originalMode );
if ( $displayAsTable ) {
diff --git a/schema/Updater.hooks.php b/schema/Updater.hooks.php
index d33e12f..610ac79 100644
--- a/schema/Updater.hooks.php
+++ b/schema/Updater.hooks.php
@@ -18,6 +18,7 @@
$du->addExtensionTable( 'gather_list', "$dir/gather_list.sql" );
$du->addExtensionTable( 'gather_list_item',
"$dir/gather_list_item.sql" );
+ $du->addExtensionTable( 'gather_list_flag',
"$dir/gather_list_flag.sql" );
return true;
}
@@ -33,6 +34,10 @@
$du->addExtensionIndex( 'gather_list_item', 'gli_id_ns_title',
__DIR__ . '/archive/add-gli_id_ns_title.sql' );
+ // adds gl_flag_count, gl_perm_override and gl_needs_review at
the same time
+ $du->addExtensionField( 'gather_list', 'gl_flag_count',
+ __DIR__ . '/archive/add-gl_list-flag-columns.sql' );
+
return true;
}
}
diff --git a/schema/archive/add-gl_list-flag-columns.sql
b/schema/archive/add-gl_list-flag-columns.sql
new file mode 100644
index 0000000..c053f00
--- /dev/null
+++ b/schema/archive/add-gl_list-flag-columns.sql
@@ -0,0 +1,12 @@
+ALTER TABLE /*_*/gather_list
+ ADD gl_perm_override TINYINT UNSIGNED NOT NULL DEFAULT 0 AFTER gl_perm,
+ ADD gl_needs_review TINYINT UNSIGNED NOT NULL DEFAULT 0 AFTER
gl_perm_override,
+ ADD gl_flag_count INT UNSIGNED NOT NULL DEFAULT 0 AFTER gl_needs_review;
+
+-- migrate PERM_HIDDEN to PERM_PUBLIC + PERM_OVERRIDE_HIDDEN
+UPDATE /*_*/gather_list
+ SET gl_perm = 1, gl_perm_override = 1
+ WHERE gl_perm = 2;
+
+DROP INDEX /*i*/gl_perm_updated ON /*_*/gather_list;
+CREATE INDEX /*i*/gl_visibility_updated ON /*_*/gather_list (gl_perm,
gl_perm_override, gl_flag_count, gl_needs_review, gl_updated, gl_id);
diff --git a/schema/gather_list.sql b/schema/gather_list.sql
index 7d4bc5c..68df082 100644
--- a/schema/gather_list.sql
+++ b/schema/gather_list.sql
@@ -16,11 +16,21 @@
-- Must be unique per user - makes it easier for querying/looking up
gl_label VARCHAR(255) BINARY NOT NULL,
- -- The list permissions type (PRIVATE=0, PUBLIC=1, HIDDEN=2, ...)
+ -- The list permissions type (PRIVATE=0, PUBLIC=1)
gl_perm TINYINT UNSIGNED NOT NULL,
+
+ -- The list permissions type (NONE=0, HIDDEN=1, APPROVED=2)
+ gl_perm_override TINYINT UNSIGNED NOT NULL DEFAULT 0,
-- The number of items (pages) in this collection
gl_item_count INT UNSIGNED NOT NULL DEFAULT 0,
+
+ -- The number of unreviewed flags (glf_reviewed=0) on this collection
+ gl_flag_count INT UNSIGNED NOT NULL DEFAULT 0,
+
+ -- True if the list is in a state which is the result of admin review, but
changed since
+ -- the last review and needs to be reevaluated
+ gl_needs_review BOOL NOT NULL DEFAULT false,
-- The timestamp is updated whenever the list's meta data is modified.
-- It is possible we might update this field when modifying watchlist / list
pages
@@ -38,5 +48,5 @@
CREATE UNIQUE INDEX /*i*/gl_user_label ON /*_*/gather_list (gl_user, gl_label);
-- Show all public lists, sorted by the last updated timestamp
--- gl_id is included to allow for safe continuation of the query
-CREATE INDEX /*i*/gl_perm_updated ON /*_*/gather_list (gl_perm, gl_updated);
+CREATE INDEX /*i*/gl_visibility_updated ON /*_*/gather_list (gl_perm,
gl_perm_override, gl_flag_count, gl_needs_review, gl_updated, gl_id);
+
diff --git a/schema/gather_list_flag.sql b/schema/gather_list_flag.sql
new file mode 100644
index 0000000..ca83da5
--- /dev/null
+++ b/schema/gather_list_flag.sql
@@ -0,0 +1,20 @@
+-- Licence: CC0
+-- Run with maintenance/sql.php for automatic replacement of /*_*/ etc
+
+CREATE TABLE /*_*/gather_list_flag (
+ -- ID of the collection which has been flagged
+ glf_gl_id INT UNSIGNED NOT NULL,
+
+ -- Foreign key to user.user_id, 0 for anonymous
+ glf_user_id INT UNSIGNED NOT NULL DEFAULT 0,
+
+ -- IP address for anonymous users
+ glf_user_ip VARBINARY(40) NOT NULL DEFAULT '',
+
+ -- Marks flags which have been seen by an admin already and should not be
used to autohide
+ glf_reviewed BOOL NOT NULL DEFAULT false
+) /*$wgDBTableOptions*/;
+
+CREATE UNIQUE INDEX /*i*/glf_list_user_ip ON /*_*/gather_list_flag (glf_gl_id,
glf_user_id, glf_user_ip);
+CREATE INDEX /*i*/glf_list_reviewed ON /*_*/gather_list_flag (glf_gl_id,
glf_reviewed);
+
diff --git a/tests/phpunit/api/ApiEditListTest.php
b/tests/phpunit/api/ApiEditListTest.php
index e0d6856..02f3eff 100644
--- a/tests/phpunit/api/ApiEditListTest.php
+++ b/tests/phpunit/api/ApiEditListTest.php
@@ -30,6 +30,33 @@
$this->assertItemCount( 5, $listId );
}
+ public function testPermissionOverrideChangeOnEdit() {
+ $listId = $this->createList( 'gatherUser', array( 'P1' ) );
+ $this->setListPermissionOverride( $listId, 'approve' );
+ $this->doApiRequestWithWatchToken( array(
+ 'action' => 'editlist',
+ 'id' => $listId,
+ 'label' => 'new test label',
+ ), null, false, static::$users['gatherUser']->getUser() );
+
+ $listData = $this->getListData( $listId );
+ $this->assertArrayNotHasKey( 'perm_override', $listData );
+ }
+
+ public function testOneFlagPerUser() {
+ $this->addTestUsers( 'flagger' );
+ $user = static::$users['flagger']->getUser();
+ $listId = $this->createList( 'gatherUser', array( 'P1' ) );
+ $params = array(
+ 'action' => 'editlist',
+ 'mode' => 'flag',
+ 'id' => $listId,
+ );
+
+ $this->doApiRequestWithWatchToken( $params, null, false, $user
);
+ $this->assertRequestFails( $params, $user, true );
+ }
+
/**
* Verifies (via direct DB access) gather_list.gl_item_count
* @param int $expected
diff --git a/tests/phpunit/api/ApiQueryListPagesTest.php
b/tests/phpunit/api/ApiQueryListPagesTest.php
index 2023cd4..12d04e9 100644
--- a/tests/phpunit/api/ApiQueryListPagesTest.php
+++ b/tests/phpunit/api/ApiQueryListPagesTest.php
@@ -10,6 +10,11 @@
class ApiQueryListPagesTest extends GatherTestCase {
protected $tablesUsed = array( 'gather_list', 'gather_list_item' );
+ public function setUp() {
+ parent::setUp();
+ $this->setMwGlobals( 'wgGatherAutohideFlagLimit', 3 );
+ }
+
public function testSort() {
$listId = $this->createList( 'gatherUser', array( 'P1', 'P2',
'Talk:P3', 'Help:P4', 'P5' ) );
@@ -36,6 +41,29 @@
$this->assertArrayEquals( array( 'P1', 'P2', 'P5', 'Talk:P3',
'Help:P4' ), $pages, true );
}
+ // also tests access control for the listmembership API as both use
ApiMixinListAccess
+ public function testAccessControl() {
+ $listId = $this->createList( 'gatherUser', array( 'P1' ) );
+ $this->addTestUsers( 'flagger1', 'flagger2', 'flagger3' );
+ $request = array(
+ 'action' => 'query',
+ 'list' => 'listpages',
+ 'lspid' => $listId,
+ );
+
+ $pages = $this->getPagenamesFromResults( $this->doApiRequest(
$request ) );
+ $this->assertArrayEquals( array( 'P1' ), $pages );
+
+ $this->setListPermissionOverride( $listId, 'hidelist' );
+ $this->assertRequestFails( $request );
+
+ $this->setListPermissionOverride( $listId, 'showlist' );
+ $this->flagList( 'flagger1', $listId );
+ $this->flagList( 'flagger2', $listId );
+ $this->flagList( 'flagger3', $listId );
+ $this->assertRequestFails( $request );
+ }
+
/**
* Returns page names (without namespace) from a listpages API query
result
* @param array $ret Return value of doApiRequest()
diff --git a/tests/phpunit/api/ApiQueryListsTest.php
b/tests/phpunit/api/ApiQueryListsTest.php
index 74b075f..2b81190 100644
--- a/tests/phpunit/api/ApiQueryListsTest.php
+++ b/tests/phpunit/api/ApiQueryListsTest.php
@@ -10,6 +10,11 @@
class ApiQueryListsTest extends GatherTestCase {
protected $tablesUsed = array( 'gather_list', 'gather_list_item' );
+ public function setUp() {
+ parent::setUp();
+ $this->setMwGlobals( 'wgGatherAutohideFlagLimit', 3 );
+ }
+
public function testMinitems() {
$list1 = $this->createList( 'gatherUser', array( 'P1', 'P2' ) );
$list2 = $this->createList( 'gatherUser', array( 'P1', 'P2',
'P3' ) );
@@ -24,6 +29,67 @@
$this->assertArrayEquals( array( $list2, $list3 ), $minitems3 );
}
+ public function testVisibility() {
+ $this->addTestUsers( 'flagger1', 'flagger2', 'flagger3' );
+
+ // normal list
+ $list1 = $this->createList( 'gatherUser', array( 'P1' ) );
+
+ // hidden list
+ $list2 = $this->createList( 'gatherUser', array( 'P1' ) );
+ $this->setListPermissionOverride( $list2, 'hidelist' );
+
+ // normal list with 3 flags
+ $list3 = $this->createList( 'gatherUser', array( 'P1' ) );
+ $this->flagList( 'flagger1', $list3 );
+ $this->flagList( 'flagger2', $list3 );
+ $this->flagList( 'flagger3', $list3 );
+
+ // approved list with 3 flags
+ $list4 = $this->createList( 'gatherUser', array( 'P1' ) );
+ $this->setListPermissionOverride( $list4, 'approve' );
+ $this->flagList( 'flagger1', $list4 );
+ $this->flagList( 'flagger2', $list4 );
+ $this->flagList( 'flagger3', $list4 );
+
+ // normal list with 3 flags which have been reviewed
+ $list5 = $this->createList( 'gatherUser', array( 'P1' ) );
+ $this->flagList( 'flagger1', $list5 );
+ $this->flagList( 'flagger2', $list5 );
+ $this->flagList( 'flagger3', $list5 );
+ $this->setListPermissionOverride( $list5, 'showlist' );
+
+ // hidden list which needs review
+ $list6 = $this->createList( 'gatherUser', array( 'P1' ) );
+ $this->setListPermissionOverride( $list6, 'hidelist' );
+ $this->doApiRequestWithWatchToken( array(
+ 'action' => 'editlist',
+ 'id' => $list6,
+ 'label' => 'new test label',
+ ), null, false, static::$users['gatherUser']->getUser() );
+
+ $normalList = $this->getListIdsFromResults(
$this->doApiRequest( array(
+ 'action' => 'query',
+ 'list' => 'lists',
+ 'lstmode' => 'allpublic',
+ ) ) );
+ $this->assertArrayEquals( array( $list1, $list4, $list5 ),
$normalList );
+
+ $hiddenlList = $this->getListIdsFromResults(
$this->doApiRequest( array(
+ 'action' => 'query',
+ 'list' => 'lists',
+ 'lstmode' => 'allhidden',
+ ), null, false, static::$users['sysop']->getUser() ) );
+ $this->assertArrayEquals( array( $list2, $list3, $list6 ),
$hiddenlList );
+
+ $reviewList = $this->getListIdsFromResults(
$this->doApiRequest( array(
+ 'action' => 'query',
+ 'list' => 'lists',
+ 'lstmode' => 'review',
+ ), null, false, static::$users['sysop']->getUser() ) );
+ $this->assertArrayEquals( array( $list3, $list4, $list6 ),
$reviewList );
+ }
+
/**
* Returns Gather list IDs from a lists API query result
* @param array $ret Return value of doApiRequest()
diff --git a/tests/phpunit/api/GatherTest.php b/tests/phpunit/api/GatherTest.php
index 989e529..1d211ea 100644
--- a/tests/phpunit/api/GatherTest.php
+++ b/tests/phpunit/api/GatherTest.php
@@ -548,7 +548,8 @@
$res = $this->editList( 'ed-o2', $usrS, $tokenS, $idBs . ',
"mode":"hidelist"' );
$this->getVal( 'ed-o2', '"status"', $res, 'updated' );
$this->getVal( 'ed-o2', '"id"', $res, $idB );
- $expListsB2->perm = 'hidden';
+ $expListsB2->perm_override = 'hidden';
+ $expListsB2->hidden = true;
$this->assertPages( 'ed-o3', $usr, $idB, $expPagesB );
$this->assertLists( 'ed-o4', $usr, $idB, $expListsB,
$expListsB2 );
@@ -586,7 +587,8 @@
$res = $this->editList( 'ed-p1', $usrS, $tokenS, $idBs . ',
"mode":"showlist"' );
$this->getVal( 'ed-p1', '"status"', $res, 'updated' );
$this->getVal( 'ed-p1', '"id"', $res, $idB );
- $expListsB2->perm = 'public';
+ unset( $expListsB2->perm_override );
+ unset( $expListsB2->hidden );
$this->assertPages( 'ed-p2', $usr, $idB, $expPagesB );
$this->assertLists( 'ed-p3', $usr, $idB, $expListsB,
$expListsB2 );
diff --git a/tests/phpunit/api/GatherTestCase.php
b/tests/phpunit/api/GatherTestCase.php
index 61d7165..56fa3fe 100644
--- a/tests/phpunit/api/GatherTestCase.php
+++ b/tests/phpunit/api/GatherTestCase.php
@@ -6,10 +6,26 @@
public function setUp() {
parent::setUp();
- if ( !array_key_exists( 'gatherUser', self::$gatherUsers ) ) {
- self::$gatherUsers['gatherUser'] = new TestUser(
'gatherUser' );
- }
static::$users = array_merge( static::$users,
self::$gatherUsers );
+ $this->addTestUsers( 'gatherUser', 'loggedInUser' );
+ }
+
+ /**
+ * @param string $name Username of the test user. Additional usernames
can be passed as
+ * additional arguments.
+ */
+ protected function addTestUsers( $name /* ... */ ) {
+ // This is a bit overcomplicated but I ran into trouble
whenever I tried to implement it
+ // in saner ways. $users is reset in setUp() but the user DB
does not so simply extending
+ // $users would result in a duplicate key error.
+ foreach ( func_get_args() as $name ) {
+ if ( !array_key_exists( $name, self::$gatherUsers ) ) {
+ if ( array_key_exists( $name, static::$users )
) {
+ throw new Exception( 'reserved
username: ' . $name );
+ }
+ static::$users[$name] =
self::$gatherUsers[$name] = new TestUser( $name );
+ }
+ }
}
protected function doApiRequestWithWatchToken(
@@ -47,12 +63,47 @@
$params = array_merge( array(
'action' => 'editlist',
'label' => $label,
+ 'perm' => 'public',
'titles' => implode( '|', $pages ),
), $properties );
$result = $this->getFromResults(
$this->doApiRequestWithWatchToken( $params, null, false,
$user ), 'editlist' );
$this->assertEquals( 'created', $result['status'] );
return $result['id'];
+ }
+
+ /**
+ * @param User|string $user Flagging user (as User object or $users
index)
+ * @param int $listId
+ */
+ protected function flagList( $user, $listId ) {
+ if ( is_string( $user ) ) {
+ $user = static::$users[$user]->getUser();
+ }
+ $params = array(
+ 'action' => 'editlist',
+ 'mode' => 'flag',
+ 'id' => $listId,
+ );
+ $result = $this->getFromResults(
$this->doApiRequestWithWatchToken( $params, null, false,
+ $user ), 'editlist' );
+ $this->assertEquals( 'flagged', $result['status'] );
+ }
+
+ /**
+ * @param int $listId
+ * @param string $mode editlist API mode parameter
+ */
+ protected function setListPermissionOverride( $listId, $mode ) {
+ $user = static::$users['sysop']->getUser();
+ $params = array(
+ 'action' => 'editlist',
+ 'mode' => $mode,
+ 'id' => $listId,
+ );
+ $result = $this->getFromResults(
$this->doApiRequestWithWatchToken( $params, null, false,
+ $user ), 'editlist' );
+ $this->assertContains( $result['status'], array( 'updated',
'unchanged' ) );
}
/**
@@ -76,8 +127,42 @@
}
}
+ protected function getListData( $listId, $user = null ) {
+ $lists = $this->getFromResults( $this->doApiRequest( array(
+ 'action' => 'query',
+ 'list' => 'lists',
+ 'lstid' => $listId,
+ 'lstprop' =>
'label|description|public|review|image|count|updated|owner',
+ ), null, false, $user ), 'lists' );
+ $this->assertNotEmpty( $lists, "List $listId not found" );
+ return $lists[0];
+ }
+
protected function debug( $enable = true ) {
global $wgDebugLogFile;
$wgDebugLogFile = $enable ?
'/vagrant/logs/mediawiki-phpunit-debug.log' : null;
}
+
+ /**
+ * Asserts that an API request failed (e.g. due to lack of permissions)
+ * @param array $params Request parameters
+ * @param User $user
+ * @param bool $withWatchToken Set to true if the API request requires
a watch token.
+ */
+ protected function assertRequestFails( $params, $user = null,
$withWatchToken = false ) {
+ try {
+ if ( $withWatchToken ) {
+ $this->doApiRequestWithWatchToken( $params,
null, false, $user );
+ } else {
+ $this->doApiRequest( $params, null, false,
$user );
+ }
+ } catch ( UsageException $e ) {
+ $this->assertTrue( true );
+ return;
+ }
+ $this->fail( 'Failed to assert that API request fails: '
+ . 'user: ' . ( $user ? $user->getName() : 'anon' ) . ',
'
+ . 'params: ' . json_encode( $params )
+ );
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/212593
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iae85317f9560d80475b53a66691b474981c13668
Gerrit-PatchSet: 40
Gerrit-Project: mediawiki/extensions/Gather
Gerrit-Branch: master
Gerrit-Owner: Gergő Tisza <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>
Gerrit-Reviewer: BarryTheBrowserTestBot <[email protected]>
Gerrit-Reviewer: Gergő Tisza <[email protected]>
Gerrit-Reviewer: Jdlrobson <[email protected]>
Gerrit-Reviewer: MaxSem <[email protected]>
Gerrit-Reviewer: Siebrand <[email protected]>
Gerrit-Reviewer: Springle <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
