Matěj Grabovský has uploaded a new change for review.
https://gerrit.wikimedia.org/r/134827
Change subject: Make constructor of Block accept array of options
......................................................................
Make constructor of Block accept array of options
Block::__construct now accepts an array of options instead of a myriad
of optional parameters. Also deprecates the old calling style.
Rename Block's protected variables $isHardblock and $isAutoblock to
prevent confusion with namesake functions.
Change-Id: I6ccd4df569ab49ad841a1ad591e23cafb1715841
---
M includes/Block.php
M tests/phpunit/includes/BlockTest.php
M tests/phpunit/includes/TitlePermissionTest.php
3 files changed, 137 insertions(+), 75 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/27/134827/1
diff --git a/includes/Block.php b/includes/Block.php
index 3896369..ea598ca 100644
--- a/includes/Block.php
+++ b/includes/Block.php
@@ -23,15 +23,16 @@
/** @var string */
public $mReason;
- /** @var bool|string */
+ /** @var string */
public $mTimestamp;
- /** @var int */
+ /** @var bool */
public $mAuto;
- /** @var bool|string */
+ /** @var bool */
public $mExpiry;
+ /** @var bool */
public $mHideName;
/** @var int */
@@ -65,10 +66,10 @@
protected $blocker;
/** @var bool */
- protected $isHardblock = true;
+ protected $mHardblock;
/** @var bool */
- protected $isAutoblocking = true;
+ protected $mAutoblock;
# TYPE constants
const TYPE_USER = 1;
@@ -78,45 +79,96 @@
const TYPE_ID = 5;
/**
- * @todo FIXME: Don't know what the best format to have for this
constructor
- * is, but fourteen optional parameters certainly isn't it.
+ * Create a new block with specified parameters on a user, IP or IP
range.
+ *
+ * @param array $options Parameters of the block:
+ * - address : Target user name, User object, IP address or IP range
+ * - user : Override target user ID (for foreign users)
+ * - by : User ID of the blocker
+ * - byText : Username of the blocker (for foreign users)
+ * - reason : Reason of the block
+ * - timestamp : The time at which the block comes into effect
+ * - auto : Is this an automatic block?
+ * - hideName : Hide the target user name
+ * - anonOnly : Only disallow anonymous actions
+ * - enableAutoblock : Enable automatic blocking
+ * - blockEmail : Disallow sending emails
+ * - allowUsertalk : Allow the target to edit its own talk page
+ * - createAccount : Disallow creation of new accounts
+ * - expiry : Timestamp of expiration of the block or 'infinity'
*/
- function __construct( $address = '', $user = 0, $by = 0, $reason = '',
- $timestamp = 0, $auto = 0, $expiry = '', $anonOnly = 0,
$createAccount = 0, $enableAutoblock = 0,
- $hideName = 0, $blockEmail = 0, $allowUsertalk = 0, $byText = ''
- ) {
- if ( $timestamp === 0 ) {
- $timestamp = wfTimestampNow();
+ function __construct( array $options = array() ) {
+ if ( func_num_args() > 1 ) {
+ wfDeprecated( __METHOD__ . ' with multiple arguments',
'1.24' );
+ $options = array_combine(
+ array( 'address', 'user', 'by', 'reason',
'timestamp', 'auto',
+ 'expiry', 'anonOnly', 'createAccount',
'enableAutoblock',
+ 'hideName', 'blockEmail',
'allowUsertalk', 'byText'
+ ),
+ func_get_args()
+ );
}
- if ( count( func_get_args() ) > 0 ) {
- # Soon... :D
- # wfDeprecated( __METHOD__ . " with arguments" );
+ if ( isset( $options['address'] ) ) {
+ $this->setTarget( $options['address'] );
}
- $this->setTarget( $address );
- if ( $this->target instanceof User && $user ) {
- $this->forcedTargetID = $user; // needed for foreign
users
+ if ( $this->target instanceof User && isset( $options['user'] )
) {
+ // Needed for foreign users
+ $this->forcedTargetID = $options['user'];
}
- if ( $by ) { // local user
- $this->setBlocker( User::newFromID( $by ) );
- } else { // foreign user
- $this->setBlocker( $byText );
+
+ if ( isset( $options['by'] ) ) {
+ // Local user
+ $this->setBlocker( User::newFromID( $options['by'] ) );
+ } elseif ( isset( $options['byText'] ) ) {
+ // Foreign user
+ $this->setBlocker( $options['byText'] );
}
- $this->mReason = $reason;
- $this->mTimestamp = wfTimestamp( TS_MW, $timestamp );
- $this->mAuto = $auto;
- $this->isHardblock( !$anonOnly );
- $this->prevents( 'createaccount', $createAccount );
- if ( $expiry == 'infinity' || $expiry == wfGetDB( DB_SLAVE
)->getInfinity() ) {
- $this->mExpiry = 'infinity';
+
+ $this->mReason = isset( $options['reason'] )
+ ? (string)$options['reason']
+ : '';
+ $this->mTimestamp = isset( $options['timestamp'] )
+ ? wfTimestamp( TS_MW, $options['timestamp'] )
+ : wfTimestampNow();
+
+ // Boolean settings
+ $this->mAuto = isset( $options['auto'] )
+ ? (bool)$options['auto']
+ : false;
+ $this->mHideName = isset( $options['hideName'] )
+ ? (bool)$options['hideName']
+ : false;
+ $this->isHardblock( isset( $options['anonOnly'] )
+ ? !$options['anonOnly']
+ : true );
+ $this->isAutoblocking( isset( $options['enableAutoblock'] )
+ ? (bool)$options['enableAutoblock']
+ : false );
+
+ // Prevention measures
+ $this->prevents( 'sendemail', isset( $options['blockEmail'] )
+ ? (bool)$options['blockEmail']
+ : false );
+ $this->prevents( 'editownusertalk', isset(
$options['allowUsertalk'] )
+ ? !$options['allowUsertalk']
+ : true );
+ $this->prevents( 'createaccount', isset(
$options['createAccount'] )
+ ? (bool)$options['createAccount']
+ : true );
+
+ if ( isset( $options['expiry'] ) ) {
+ if ( ( $options['expiry'] == 'infinity' ||
+ $options['expiry'] == wfGetDB( DB_SLAVE
)->getInfinity() ) )
+ {
+ $this->mExpiry = 'infinity';
+ } else {
+ $this->mExpiry = $options['expiry'];
+ }
} else {
- $this->mExpiry = wfTimestamp( TS_MW, $expiry );
+ $this->mExpiry = wfTimestampNow();
}
- $this->isAutoblocking( $enableAutoblock );
- $this->mHideName = $hideName;
- $this->prevents( 'sendemail', $blockEmail );
- $this->prevents( 'editownusertalk', !$allowUsertalk );
$this->mFromMaster = false;
}
@@ -885,21 +937,21 @@
* @return bool
*/
public function isHardblock( $x = null ) {
- wfSetVar( $this->isHardblock, $x );
+ wfSetVar( $this->mHardblock, $x );
# You can't *not* hardblock a user
return $this->getType() == self::TYPE_USER
? true
- : $this->isHardblock;
+ : $this->mHardblock;
}
public function isAutoblocking( $x = null ) {
- wfSetVar( $this->isAutoblocking, $x );
+ wfSetVar( $this->mAutoblocking, $x );
# You can't put an autoblock on an IP or range as we don't have
any history to
# look over to get more IPs from
return $this->getType() == self::TYPE_USER
- ? $this->isAutoblocking
+ ? $this->mAutoblocking
: false;
}
diff --git a/tests/phpunit/includes/BlockTest.php
b/tests/phpunit/includes/BlockTest.php
index b248d24..ef2e893 100644
--- a/tests/phpunit/includes/BlockTest.php
+++ b/tests/phpunit/includes/BlockTest.php
@@ -38,9 +38,13 @@
$oldBlock->delete();
}
- $this->block = new Block( 'UTBlockee', $user->getID(), 0,
- 'Parce que', 0, false, time() + 100500
+ $blockOptions = array(
+ 'address' => 'UTBlockee',
+ 'user' => $user->getID(),
+ 'reason' => 'Parce que',
+ 'expiry' => time() + 100500,
);
+ $this->block = new Block( $blockOptions );
$this->madeAt = wfTimestamp( TS_MW );
$this->block->insert();
@@ -150,22 +154,19 @@
);
// Foreign perspective (blockee not on current wiki)...
- $block = new Block(
- /* $address */ $username,
- /* $user */ 14146,
- /* $by */ 0,
- /* $reason */ 'crosswiki block...',
- /* $timestamp */ wfTimestampNow(),
- /* $auto */ false,
- /* $expiry */ $this->db->getInfinity(),
- /* anonOnly */ false,
- /* $createAccount */ true,
- /* $enableAutoblock */ true,
- /* $hideName (ipb_deleted) */ true,
- /* $blockEmail */ true,
- /* $allowUsertalk */ false,
- /* $byName */ 'MetaWikiUser'
+ $blockOptions = array(
+ 'address' => $username,
+ 'user' => 14146,
+ 'reason' => 'crosswiki block...',
+ 'timestamp' => wfTimestampNow(),
+ 'expiry' => $this->db->getInfinity(),
+ 'createAccount' => true,
+ 'enableAutoblock' => true,
+ 'hideName' => true,
+ 'blockEmail' => true,
+ 'byText' => 'MetaWikiUser',
);
+ $block = new Block( $blockOptions);
$block->insert();
// Reload block from DB
@@ -201,22 +202,19 @@
}
// Foreign perspective (blockee not on current wiki)...
- $block = new Block(
- /* $address */ 'UserOnForeignWiki',
- /* $user */ 14146,
- /* $by */ 0,
- /* $reason */ 'crosswiki block...',
- /* $timestamp */ wfTimestampNow(),
- /* $auto */ false,
- /* $expiry */ $this->db->getInfinity(),
- /* anonOnly */ false,
- /* $createAccount */ true,
- /* $enableAutoblock */ true,
- /* $hideName (ipb_deleted) */ true,
- /* $blockEmail */ true,
- /* $allowUsertalk */ false,
- /* $byName */ 'MetaWikiUser'
+ $blockOptions = array(
+ 'address' => 'UserOnForeignWiki',
+ 'user' => 14146,
+ 'reason' => 'crosswiki block...',
+ 'timestamp' => wfTimestampNow(),
+ 'expiry' => $this->db->getInfinity(),
+ 'createAccount' => true,
+ 'enableAutoblock' => true,
+ 'hideName' => true,
+ 'blockEmail' => true,
+ 'byText' => 'MetaWikiUser',
);
+ $block = new Block( $blockOptions );
$res = $block->insert( $this->db );
$this->assertTrue( (bool)$res['id'], 'Block succeeded' );
diff --git a/tests/phpunit/includes/TitlePermissionTest.php
b/tests/phpunit/includes/TitlePermissionTest.php
index ac80a9a..bf6bedd 100644
--- a/tests/phpunit/includes/TitlePermissionTest.php
+++ b/tests/phpunit/includes/TitlePermissionTest.php
@@ -736,8 +736,14 @@
$prev = time();
$now = time() + 120;
$this->user->mBlockedby = $this->user->getId();
- $this->user->mBlock = new Block( '127.0.8.1', 0,
$this->user->getId(),
- 'no reason given', $prev + 3600, 1, 0 );
+ $this->user->mBlock = new Block( array(
+ 'address' => '127.0.8.1',
+ 'by' => $this->user->getId(),
+ 'reason' => 'no reason given',
+ 'timestamp' => $prev + 3600,
+ 'auto' => true,
+ 'expiry' => 0
+ ) );
$this->user->mBlock->mTimestamp = 0;
$this->assertEquals( array( array( 'autoblockedtext',
'[[User:Useruser|Useruser]]', 'no reason
given', '127.0.0.1',
@@ -753,8 +759,14 @@
global $wgLocalTZoffset;
$wgLocalTZoffset = -60;
$this->user->mBlockedby = $this->user->getName();
- $this->user->mBlock = new Block( '127.0.8.1', 0,
$this->user->getId(),
- 'no reason given', $now, 0, 10 );
+ $this->user->mBlock = new Block( array(
+ 'address' => '127.0.8.1',
+ 'by' => $this->user->getId(),
+ 'reason' => 'no reason given',
+ 'timestamp' => $now,
+ 'auto' => false,
+ 'expiry' => 10,
+ ) );
$this->assertEquals( array( array( 'blockedtext',
'[[User:Useruser|Useruser]]', 'no reason
given', '127.0.0.1',
'Useruser', null, '23:00, 31 December 1969',
'127.0.8.1',
--
To view, visit https://gerrit.wikimedia.org/r/134827
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6ccd4df569ab49ad841a1ad591e23cafb1715841
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Matěj Grabovský <mgrabov...@yahoo.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
