Overhaul the interface for hiding AbuseLog entries

The main change is the addition of checkboxes to hide/show multiple
entries at the same time. Also, tweaked some i18n and made the process
return more useful success/error messages.

This patch introduces some technical debt, caused by SpecialAbuseLog and
AbuseLogPager being tightly coupled (which is a pre-existing problem,
but it got worse here).

Bug: T260904
Bug: T144096
Bug: T206945
Bug: T206938
Change-Id: I13f476d8126f81b0417e7509784c83d4f21cf348
This commit is contained in:
Daimona Eaytoy 2018-10-16 21:58:22 +02:00 committed by Jforrester
parent 86f308c6f0
commit 21d7c08aa7
6 changed files with 172 additions and 68 deletions

View file

@ -77,7 +77,7 @@
"abusefilter-log-detailedentry-local": "filter $1",
"abusefilter-log-detailslink": "details",
"abusefilter-log-diff": "diff",
"abusefilter-log-hidelink": "adjust visibility",
"abusefilter-log-hide-entries": "Change visibility of selected entries",
"abusefilter-log-description-not-available": "not available",
"abusefilter-log-details-legend": "Details for log entry $1",
"abusefilter-log-details-var": "Variable",
@ -102,14 +102,19 @@
"abusefilter-log-details-hidden": "You cannot view the details for this entry because it is hidden from public view.",
"abusefilter-log-details-hidden-implicit": "You cannot view the details for this entry because its associated revision is hidden from public view.",
"abusefilter-log-private-not-included": "One or more of the filter IDs you specified are private. Because you are not allowed to view details of private filters, these filters have not been searched for.",
"abusefilter-log-hide-legend": "Hide log entry",
"abusefilter-log-hide-id": "Log entry ID:",
"abusefilter-log-hide-no-selected": "No entries selected",
"abusefilter-log-hide-selected": "{{PLURAL:$1|Selected AbuseLog entry|Selected AbuseLog entries}}:",
"abusefilter-log-hide-legend": "Change visibility",
"abusefilter-log-hide-set-visibility": "Set visibility for the selected entries:",
"abusefilter-log-hide-reason": "Reason:",
"abusefilter-log-hide-reason-other": "Other/additional reason:",
"abusefilter-log-hide-forbidden": "You do not have permission to hide abuse log entries.",
"abusefilter-log-hide-show": "Show",
"abusefilter-log-hide-hide": "Hide",
"abusefilter-log-hide-no-change": "All of the selected IDs already have the desired visibility.",
"abusefilter-log-hide-done": "Visibility updated successfully: $1 {{PLURAL:$1|entry|entries}} $2.",
"abusefilter-log-hide-done-hide": "{{PLURAL:$1|hidden}}",
"abusefilter-log-hide-done-show": "{{PLURAL:$1|unhidden}}",
"abusefilter-log-entry-suppress": "$1 {{GENDER:$2|hid}} $3",
"abusefilter-log-entry-unsuppress": "$1 {{GENDER:$2|unhid}} $3",
"logentry-abusefilter-hit": "$1 {{GENDER:$2|triggered}} $4, {{GENDER:$2|performing}} the action \"$5\" on $3. Actions taken: $6 ($7)",

View file

@ -116,7 +116,7 @@
"abusefilter-log-detailedentry-local": "Addition in the abuse filter log detail when a filter rule is a local rule. Parameters:\n* $1 is a local filter ID.\n{{Identical|Filter}}",
"abusefilter-log-detailslink": "Link text for a link to abuse filter log details.\n{{Identical|Detail}}",
"abusefilter-log-diff": "Diff link text to a revision associated with an AbuseFilter log entry\n{{Identical|Diff}}",
"abusefilter-log-hidelink": "Link text in abuse filter log line to hide (when visible) or show (when hidden) a link to the log entry details.",
"abusefilter-log-hide-entries": "Text for a button which allows the user to delete the selected AbuseLog entries.\n\nSee also:\n* {{msg-mw|showhideselectedlogentries}}",
"abusefilter-log-description-not-available": "Placeholder text for when a filter description could not be shown. Used as parameter $7 in {{msg-mw|Abusefilter-log-detailedentry-meta}}.",
"abusefilter-log-details-legend": "Legend for abuse filter log entry details. Parameters:\n* $1 is a filter ID.",
"abusefilter-log-details-var": "Caption of a column on a detail view of [[Special:AbuseLog]]\n{{Identical|Variable}}",
@ -141,14 +141,19 @@
"abusefilter-log-details-hidden": "Message shown instead of log row details when those are hidden.",
"abusefilter-log-details-hidden-implicit": "Message shown instead of log row details when their associated revision is hidden.",
"abusefilter-log-private-not-included": "Message shown when an unauthorized user searches by ID for private filters.",
"abusefilter-log-hide-legend": "Legend for form to hide a log entry.",
"abusefilter-log-hide-id": "Field label in form to hide a log entry.",
"abusefilter-log-hide-no-selected": "Error message when a user tries to enter the form for changing AbuseLog entries visibility without having selected any entry.",
"abusefilter-log-hide-selected": "Legend for a list of selected AbuseLog entries. See also {{mw-msg|logdelete-selected}}.\nParameters:\n* $1 - number of selected AbuseLog entries.",
"abusefilter-log-hide-legend": "Legend for a form to hide or show AbuseLog entries.",
"abusefilter-log-hide-set-visibility": "Description of a form field where users can specify how to change the visibility of selected AbuseLog entries",
"abusefilter-log-hide-reason": "{{Identical|Reason}}",
"abusefilter-log-hide-reason-other": "{{Identical|Other/additional reason}}",
"abusefilter-log-hide-forbidden": "Message shown instead of a \"hide log entry\" form when not having the correct user rights.",
"abusefilter-log-hide-show": "Option allowing to show the selected AbuseLog entries\n{{identical|Show}}",
"abusefilter-log-hide-hide": "Option allowing to hide the selected AbuseLog entries\n{{identical|Hide}}",
"abusefilter-log-hide-no-change": "Error message informing the user that he cannot change visibility for the selected entries because they already have the desired visibility.",
"abusefilter-log-hide-done": "Success message informing the user that AbuseLog entries correctly had their visibility updated.Parameters:\n*$1 The amount of updated entries\n* $2 - Either {{mw-msg|abusefilter-log-hide-done-hide}} or {{mw-msg|abusefilter-log-hide-done-show}}.",
"abusefilter-log-hide-done-hide": "Verb used to indicate that the entries have been hidden.\nUsed in {{mw-msg|abusefilter-log-hide-done}}.\nParameters:\n* $1 - Amount of affected entries for PLURAL",
"abusefilter-log-hide-done-show": "Verb used to indicate that the entries have been unhidden.\nUsed in {{mw-msg|abusefilter-log-hide-done}}.\nParameters:\n* $1 - Amount of affected entries for PLURAL",
"abusefilter-log-entry-suppress": "Log entry when hiding an abuse filter log entry. Parameters:\n* $1 - a link to a user page with a user name as link text, followed by a series of related links\n* $2 - raw username, for GENDER support\n* $3 a link to the log ID.",
"abusefilter-log-entry-unsuppress": "Log entry when unhiding an abuse filter log entry. Parameters:\n* $1 - a link to a user page with a user name as link text, followed by a series of related links\n* $2 - raw username, for GENDER support\n* $3 a link to the log ID.",
"logentry-abusefilter-hit": "This message is for a log entry. Parameters:\n* $1 - user who performed the action\n* $2 - user who performed the action (to be used with GENDER)\n* $3 - link to the page, that the action that triggered the filter was made on\n* $4 - link to filter\n* $5 - action by user, like 'edit', 'move', 'create' etc.\n* $6 - actions taken by the filter\n* $7 - action details link",

View file

@ -41,6 +41,11 @@ class AbuseLogPager extends ReverseChronologicalPager {
/** @var string */
private $basePageName;
/**
* @var string[] Map of [ id => show|hide ], for entries that we're currently (un)hiding
*/
private $hideEntries;
/**
* @param IContextSource $context
* @param LinkRenderer $linkRenderer
@ -49,6 +54,7 @@ class AbuseLogPager extends ReverseChronologicalPager {
* @param PermissionManager $permManager
* @param AbuseFilterPermissionManager $afPermissionManager
* @param string $basePageName
* @param string[] $hideEntries
*/
public function __construct(
IContextSource $context,
@ -57,7 +63,8 @@ class AbuseLogPager extends ReverseChronologicalPager {
LinkBatchFactory $linkBatchFactory,
PermissionManager $permManager,
AbuseFilterPermissionManager $afPermissionManager,
string $basePageName
string $basePageName,
array $hideEntries = []
) {
parent::__construct( $context, $linkRenderer );
$this->mConds = $conds;
@ -65,6 +72,7 @@ class AbuseLogPager extends ReverseChronologicalPager {
$this->permissionManager = $permManager;
$this->afPermissionManager = $afPermissionManager;
$this->basePageName = $basePageName;
$this->hideEntries = $hideEntries;
}
/**
@ -214,17 +222,6 @@ class AbuseLogPager extends ReverseChronologicalPager {
$actionLinks[] = $diffLink;
}
if ( $this->afPermissionManager->canHideAbuseLog( $user ) ) {
$hideLink = $linkRenderer->makeKnownLink(
SpecialPage::getTitleFor( $this->basePageName, 'hide' ),
$this->msg( 'abusefilter-log-hidelink' )->text(),
[],
[ 'id' => $row->afl_id ]
);
$actionLinks[] = $hideLink;
}
if ( $global ) {
$centralDb = $this->getConfig()->get( 'AbuseFilterCentralDB' );
$linkMsg = $this->msg( 'abusefilter-log-detailedentry-global' )
@ -273,13 +270,22 @@ class AbuseLogPager extends ReverseChronologicalPager {
}
$attribs = null;
if ( $isHidden === true ) {
if (
$this->isHidingEntry( $row ) === true ||
// If isHidingEntry is false, we've just unhidden the row
( $this->isHidingEntry( $row ) === null && $isHidden === true )
) {
$attribs = [ 'class' => 'mw-abusefilter-log-hidden-entry' ];
} elseif ( $isHidden === 'implicit' ) {
}
if ( $isHidden === 'implicit' ) {
$description .= ' ' .
$this->msg( 'abusefilter-log-hidden-implicit' )->parse();
}
if ( !$this->hideEntries && $this->afPermissionManager->canHideAbuseLog( $user ) ) {
$description = Xml::check( 'hideids[' . $row->afl_id . ']' ) . $description;
}
if ( $isListItem ) {
return Xml::tags( 'li', $attribs, $description );
} else {
@ -408,6 +414,21 @@ class AbuseLogPager extends ReverseChronologicalPager {
$result->seek( 0 );
}
/**
* Check whether the entry passed in is being currently hidden/unhidden.
* This is used to format the entries list shown when updating visibility, and is necessary because
* when we decide whether to display the entry as hidden the DB hasn't been updated yet.
*
* @param stdClass $row
* @return bool|null True if just hidden, false if just unhidden, null if untouched
*/
private function isHidingEntry( stdClass $row ) : ?bool {
if ( isset( $this->hideEntries[ $row->afl_id ] ) ) {
return $this->hideEntries[ $row->afl_id ] === 'hide';
}
return null;
}
/**
* @return string
*/

View file

@ -32,7 +32,7 @@ abstract class AbuseFilterView extends ContextSource {
protected $linkRenderer;
/** @var string */
private $basePageName;
protected $basePageName;
/**
* @param AbuseFilterPermissionManager $afPermManager

View file

@ -2,19 +2,23 @@
namespace MediaWiki\Extension\AbuseFilter\View;
use DeferredUpdates;
use Html;
use HTMLForm;
use IContextSource;
use LogEventsList;
use LogPage;
use ManualLogEntry;
use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager;
use MediaWiki\Extension\AbuseFilter\Pager\AbuseLogPager;
use MediaWiki\Linker\LinkRenderer;
use MediaWiki\MediaWikiServices;
use Xml;
class HideAbuseLog extends AbuseFilterView {
/** @var int */
private $hideID;
/** @var int[] */
private $hideIDs;
/**
* @param AbuseFilterPermissionManager $afPermManager
@ -29,7 +33,7 @@ class HideAbuseLog extends AbuseFilterView {
string $basePageName
) {
parent::__construct( $afPermManager, $context, $linkRenderer, $basePageName, [] );
$this->hideID = $this->getRequest()->getIntOrNull( 'id' );
$this->hideIDs = array_keys( $this->getRequest()->getArray( 'hideids', [] ) );
}
/**
@ -44,20 +48,37 @@ class HideAbuseLog extends AbuseFilterView {
return;
}
$dbr = wfGetDB( DB_REPLICA );
$deleted = $dbr->selectField(
'abuse_filter_log',
'afl_deleted',
[ 'afl_id' => $this->hideID ],
__METHOD__
);
if ( $deleted === false ) {
$output->addWikiMsg( 'abusefilter-log-nonexistent' );
if ( !$this->hideIDs ) {
$output->addWikiMsg( 'abusefilter-log-hide-no-selected' );
return;
}
// TODO DI
$pager = new AbuseLogPager(
$this->getContext(),
MediaWikiServices::getInstance()->getLinkRenderer(),
[ 'afl_id' => $this->hideIDs ],
MediaWikiServices::getInstance()->getLinkBatchFactory(),
MediaWikiServices::getInstance()->getPermissionManager(),
$this->afPermManager,
$this->basePageName,
array_fill_keys( $this->hideIDs, $this->getRequest()->getVal( 'wpshoworhide' ) )
);
$pager->doQuery();
if ( $pager->getResult()->numRows() === 0 ) {
$output->addWikiMsg( 'abusefilter-log-hide-no-selected' );
return;
}
$output->wrapWikiMsg(
"<strong>$1</strong>",
[
'abusefilter-log-hide-selected',
$this->getLanguage()->formatNum( count( $this->hideIDs ) )
]
);
$output->addHTML( Xml::tags( 'ul', [ 'class' => 'plainlinks' ], $pager->getBody() ) );
$hideReasonsOther = $this->msg( 'revdelete-reasonotherlist' )->text();
$hideReasons = $this->msg( 'revdelete-reason-dropdown-suppress' )->inContentLanguage()->text();
$hideReasons = Xml::listDropDownOptions( $hideReasons, [ 'other' => $hideReasonsOther ] );
@ -70,14 +91,9 @@ class HideAbuseLog extends AbuseFilterView {
'abusefilter-log-hide-show' => 'show',
'abusefilter-log-hide-hide' => 'hide'
],
'default' => (int)$deleted === 0 ? 'show' : 'hide',
'default' => 'hide',
'flatlist' => true
],
'logid' => [
'type' => 'info',
'default' => (string)$this->hideID,
'label-message' => 'abusefilter-log-hide-id',
],
'dropdownreason' => [
'type' => 'select',
'options' => $hideReasons,
@ -89,51 +105,90 @@ class HideAbuseLog extends AbuseFilterView {
],
];
$actionURL = $this->getTitle( 'hide' )->getFullURL( [ 'hideids' => array_fill_keys( $this->hideIDs, 1 ) ] );
HTMLForm::factory( 'ooui', $formInfo, $this->getContext() )
->setAction( $this->getTitle( 'hide' )->getFullURL( [ 'id' => $this->hideID ] ) )
->setAction( $actionURL )
->setWrapperLegend( $this->msg( 'abusefilter-log-hide-legend' )->text() )
->addHiddenField( 'hide', $this->hideID )
->setSubmitCallback( [ $this, 'saveHideForm' ] )
->showAlways();
// Show suppress log for this entry
$suppressLogPage = new LogPage( 'suppress' );
$output->addHTML( "<h2>" . $suppressLogPage->getName()->escaped() . "</h2>\n" );
LogEventsList::showLogExtract( $output, 'suppress', $this->getTitle( (string)$this->hideID ) );
// Show suppress log for this entry. Hack: since every suppression is performed on a
// totally different page (i.e. Special:AbuseLog/xxx), we use showLogExtract without
// specifying a title and then adding it in conds.
// This isn't shown if the request was posted because we update visibility in a DeferredUpdate, so it would
// display outdated info that might confuse the user.
// TODO Can we improve this somehow?
if ( !$this->getRequest()->wasPosted() ) {
$suppressLogPage = new LogPage( 'suppress' );
$output->addHTML( "<h2>" . $suppressLogPage->getName()->escaped() . "</h2>\n" );
$searchTitles = [];
foreach ( $this->hideIDs as $id ) {
$searchTitles[] = $this->getTitle( (string)$id )->getDBKey();
}
$conds = [ 'log_namespace' => NS_SPECIAL, 'log_title' => $searchTitles ];
LogEventsList::showLogExtract( $output, 'suppress', '', '', [ 'conds' => $conds ] );
}
}
/**
* Process the hide form after submission. This performs the actual visibility update. Used as callback by HTMLForm
*
* @param array $fields
* @return bool
* @return bool|array True on success, array of error message keys otherwise
*/
public function saveHideForm( array $fields ) : bool {
$logid = $this->getRequest()->getVal( 'hide' );
$newValue = $fields['showorhide'] === 'hide' ? 1 : 0;
public function saveHideForm( array $fields ) {
// Determine which rows actually have to be changed
$dbw = wfGetDB( DB_MASTER );
$newValue = $fields['showorhide'] === 'hide' ? 1 : 0;
$actualIDs = $dbw->selectFieldValues(
'abuse_filter_log',
'afl_id',
[ 'afl_id' => $this->hideIDs, "afl_deleted != $newValue" ],
__METHOD__
);
if ( !count( $actualIDs ) ) {
return [ 'abusefilter-log-hide-no-change' ];
}
$dbw->update(
'abuse_filter_log',
[ 'afl_deleted' => $newValue ],
[ 'afl_id' => $logid ],
[ 'afl_id' => $actualIDs ],
__METHOD__
);
$reason = $fields['dropdownreason'];
if ( $reason === 'other' ) {
$reason = $fields['reason'];
} elseif ( $fields['reason'] !== '' ) {
$reason .=
$this->msg( 'colon-separator' )->inContentLanguage()->text() . $fields['reason'];
}
// Log in a DeferredUpdates to avoid potential flood
DeferredUpdates::addCallableUpdate( function () use ( $fields, $actualIDs ) {
$reason = $fields['dropdownreason'];
if ( $reason === 'other' ) {
$reason = $fields['reason'];
} elseif ( $fields['reason'] !== '' ) {
$reason .=
$this->msg( 'colon-separator' )->inContentLanguage()->text() . $fields['reason'];
}
$action = $fields['showorhide'] === 'hide' ? 'hide-afl' : 'unhide-afl';
$logEntry = new ManualLogEntry( 'suppress', $action );
$logEntry->setPerformer( $this->getUser() );
$logEntry->setTarget( $this->getTitle( $logid ) );
$logEntry->setComment( $reason );
$logEntry->insert();
$action = $fields['showorhide'] === 'hide' ? 'hide-afl' : 'unhide-afl';
foreach ( $actualIDs as $logid ) {
$logEntry = new ManualLogEntry( 'suppress', $action );
$logEntry->setPerformer( $this->getUser() );
$logEntry->setTarget( $this->getTitle( $logid ) );
$logEntry->setComment( $reason );
$logEntry->insert();
}
} );
$count = count( $actualIDs );
$this->getOutput()->prependHTML(
Html::successBox(
$this->msg( 'abusefilter-log-hide-done' )->params(
$this->getLanguage()->formatNum( $count ),
// Messages used: abusefilter-log-hide-done-hide, abusefilter-log-hide-done-show
$this->msg( 'abusefilter-log-hide-done-' . $fields['showorhide'] )->numParams( $count )->text()
)->escaped()
)
);
return true;
}
}

View file

@ -9,6 +9,7 @@ use MediaWiki\Extension\AbuseFilter\View\HideAbuseLog;
use MediaWiki\Logger\LoggerFactory;
use MediaWiki\MediaWikiServices;
use MediaWiki\Permissions\PermissionManager;
use OOUI\ButtonInputWidget;
class SpecialAbuseLog extends AbuseFilterSpecialPage {
/**
@ -558,9 +559,11 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
'form',
[
'method' => 'GET',
'action' => $this->getPageTitle()->getLocalURL()
'action' => $this->getPageTitle( 'hide' )->getLocalURL()
],
Xml::tags( 'ul', [ 'class' => 'plainlinks' ], $pager->getBody() )
$this->getDeleteButton() .
Xml::tags( 'ul', [ 'class' => 'plainlinks' ], $pager->getBody() ) .
$this->getDeleteButton()
);
if ( $result && $result->numRows() !== 0 ) {
@ -570,6 +573,21 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
}
}
/**
* Returns the HTML for a button to hide selected entries
*
* @return string|ButtonInputWidget
*/
private function getDeleteButton() {
if ( !$this->afPermissionManager->canHideAbuseLog( $this->getUser() ) ) {
return '';
}
return new ButtonInputWidget( [
'label' => $this->msg( 'abusefilter-log-hide-entries' )->text(),
'type' => 'submit'
] );
}
/**
* @param string|int $id
* @suppress SecurityCheck-SQLInjection