Hoo man has uploaded a new change for review. https://gerrit.wikimedia.org/r/72724
Change subject: Refactor AbuseFilterView::canEdit* functions ...................................................................... Refactor AbuseFilterView::canEdit* functions Don't use global state in here and centralize the logic for global filters. This also makes the UI for global filters nicer in case the user can't edit them (as all fields are disabled then). Change-Id: Ica4e77536d315d8ef39a45666c6b8834315bee77 --- M Views/AbuseFilterView.php M Views/AbuseFilterViewEdit.php M Views/AbuseFilterViewList.php 3 files changed, 33 insertions(+), 30 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/AbuseFilter refs/changes/24/72724/1 diff --git a/Views/AbuseFilterView.php b/Views/AbuseFilterView.php index c874760..49fdbd2 100644 --- a/Views/AbuseFilterView.php +++ b/Views/AbuseFilterView.php @@ -22,33 +22,36 @@ abstract function show(); /** - * @static + * @param User $user + * * @return bool */ - static function canEdit() { - global $wgUser; - static $canEdit = null; - - if ( is_null( $canEdit ) ) { - $canEdit = $wgUser->isAllowed( 'abusefilter-modify' ); - } - - return $canEdit; + public function canEdit( $user ) { + return $user->isAllowed( 'abusefilter-modify' ); } /** - * @static + * @param User $user + * * @return bool */ - static function canEditGlobal() { - global $wgUser; - static $canEditGlobal = null; + static function canEditGlobal( $user ) { + return $user->isAllowed( 'abusefilter-modify-global' ); + } - if ( is_null( $canEditGlobal ) ) { - $canEditGlobal = $wgUser->isAllowed( 'abusefilter-modify-global' ); - } - - return $canEditGlobal; + /** + * Whether the user can edit the given filter. + * + * @param object $row Filter row + * @param User $user + * + * @return bool + */ + public function canEditFilter( $row, $user ) { + return ( + $this->canEdit( $user ) && + !( isset( $row->af_global ) && $row->af_global == 1 && !$this->canEditGlobal( $user ) ) + ); } /** @@ -60,7 +63,7 @@ static $canView = null; if ( is_null( $canView ) ) { - $canView = self::canEdit() || $wgUser->isAllowed( 'abusefilter-view-private' ); + $canView = $wgUser->isAllowed( 'abusefilter-modify' ) || $wgUser->isAllowed( 'abusefilter-view-private' ); } return $canView; diff --git a/Views/AbuseFilterViewEdit.php b/Views/AbuseFilterViewEdit.php index 5d82db8..3a17ea0 100644 --- a/Views/AbuseFilterViewEdit.php +++ b/Views/AbuseFilterViewEdit.php @@ -29,7 +29,7 @@ } $editToken = $request->getVal( 'wpEditToken' ); - $didEdit = $this->canEdit() + $didEdit = $this->canEdit( $user ) && $user->matchEditToken( $editToken, array( 'abusefilter', $filter ) ); if ( $didEdit ) { @@ -59,9 +59,7 @@ // Don't allow adding a new global rule, or updating a // rule that is currently global, without permissions. - if ( ( $newRow->af_global == 1 || $newRow->mOriginalRow->af_global == 1 ) - && !$this->canEditGlobal() - ) { + if ( !$this->canEditFilter( $newRow, $user ) || !$this->canEditFilter( $newRow->mOriginalRow, $user ) ) { $out->addWikiMsg( 'abusefilter-edit-notallowed-global' ); return; } @@ -335,7 +333,7 @@ $readOnlyAttrib = array(); $cbReadOnlyAttrib = array(); // For checkboxes - if ( !$this->canEdit() || ( isset( $row->af_global ) && $row->af_global == 1 && !$this->canEditGlobal() ) ) { + if ( !$this->canEditFilter( $row, $user ) ) { $readOnlyAttrib['readonly'] = 'readonly'; $cbReadOnlyAttrib['disabled'] = 'disabled'; } @@ -405,7 +403,7 @@ $row->af_pattern, 'wpFilterRules', true, - $this->canEdit() + $this->canEditFilter( $row, $user ) ); $fields['abusefilter-edit-notes'] = Xml::textarea( 'wpFilterNotes', @@ -445,7 +443,7 @@ $dbField = "af_$checkboxId"; $postVar = 'wpFilter' . ucfirst( $checkboxId ); - if ( $checkboxId == 'global' && !$this->canEditGlobal() ) { + if ( $checkboxId == 'global' && !$this->canEditGlobal( $user ) ) { $cbReadOnlyAttrib['disabled'] = 'disabled'; } @@ -519,7 +517,7 @@ $this->buildConsequenceEditor( $row, $actions ) ); - if ( $this->canEdit() ) { + if ( $this->canEditFilter( $row, $user ) ) { $form .= Xml::submitButton( $this->msg( 'abusefilter-edit-save' )->text(), array( 'accesskey' => 's' ) @@ -578,6 +576,7 @@ function buildConsequenceSelector( $action, $set, $parameters, $row ) { global $wgAbuseFilterAvailableActions; + $user = $this->getUser(); if ( !in_array( $action, $wgAbuseFilterAvailableActions ) ) { return ''; } @@ -585,7 +584,7 @@ $readOnlyAttrib = array(); $cbReadOnlyAttrib = array(); // For checkboxes - if ( !$this->canEdit() ) { + if ( !$this->canEditFilter( $row, $user ) ) { $readOnlyAttrib['readonly'] = 'readonly'; $cbReadOnlyAttrib['disabled'] = 'disabled'; } diff --git a/Views/AbuseFilterViewList.php b/Views/AbuseFilterViewList.php index f294e72..0f2cabb 100644 --- a/Views/AbuseFilterViewList.php +++ b/Views/AbuseFilterViewList.php @@ -4,6 +4,7 @@ function show() { $out = $this->getOutput(); $request = $this->getRequest(); + $user = $this->getUser(); // Status info... $this->showStatus(); @@ -11,7 +12,7 @@ $out->addWikiMsg( 'abusefilter-intro' ); // New filter button - if ( $this->canEdit() ) { + if ( $this->canEdit( $user ) ) { $title = $this->getTitle( 'new' ); $link = Linker::link( $title, $this->msg( 'abusefilter-new' )->escaped() ); $links = Xml::tags( 'p', null, $link ) . "\n"; -- To view, visit https://gerrit.wikimedia.org/r/72724 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ica4e77536d315d8ef39a45666c6b8834315bee77 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/AbuseFilter Gerrit-Branch: master Gerrit-Owner: Hoo man <h...@online.de> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits