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

Reply via email to