r102139 MediaWiki - Code Review archive

+	protected $mParams = false;
+	protected $mPosted = false;
+	protected $mRequest = array();
+	protected $mSummary = '';
+	public $mAllowedStatus = array( 'new', 'approved', 'declined' );

Prefixing class member variables with m is so 1999.

+		if( !$wgUser->isAllowed( 'premoderation' ) ) {
+			$this->displayRestrictionError();
+			return;
+		}

Should you also perform checks for read-only mode and if the user trying to perform the action is blocked?

+		$res = $dbr->select(
+			'pm_queue',
+			array( 'pmq_id', 'pmq_page_ns', 'pmq_page_title', 'pmq_user', 'pmq_user_text', 'pmq_timestamp',
+				'pmq_minor', 'pmq_summary', 'pmq_len', 'pmq_status', 'pmq_updated', 'pmq_updated_user_text' ),

Please break lines at 80 characters.

+		if( $wgUser->isAllowed( 'premoderation-viewip' ) ) {
+			$wgOut->addHTML( wfMsg( 'premoderation-private-ip' ) . ' ' . $row['pmq_ip'] );
+		}

I believe that you should pass the $row['pmq_ip'] variable to the message for better international compatibility.

$output .= Xml::closeElement( 'table' );

Personally I'd write </table>, even though the recommended way is to use the Html class.

+	protected function checkInternalConflicts( $db, $id, $ns, $page ) {
+		global $wgOut;
+		
+		$conds = 'pmq_page_ns = ' . $db->addQuotes( $ns ) . ' AND pmq_page_title = ' .
+			$db->addQuotes( $page ) . ' AND pmq_id != ' . $id . ' AND pmq_status != "approved"';
+		
+		$res = $db->select(
+			'pm_queue', '*', $conds, __METHOD__,
+			array( 'ORDER BY' => 'pmq_timestamp DESC', 'LIMIT' => 20 )
+		);

I'd suggest swapping the MySQL-specific != operator to the standard SQL NOT operator <> and writing the query like this:

		$res = $db->select(
			'pm_queue',
			'*',
			array(
				'pmq_page_ns' => $ns,
				'pmq_page_title' => $page,
				"pmq_id <> $id",
				'pmq_status <> "approved"'
			),
			__METHOD__,
			array( 'ORDER BY' => 'pmq_timestamp DESC', 'LIMIT' => 20 )
		);

I'd suggest removing install.php script altogether and writing a function that hooks into the LoadExtensionSchemaUpdates hook; that way the database tables will be created by update.php whenever the user runs it (usually after installing the extension) and you don't need to have a separate installer script. Many social tools, such as Extension:LinkFilter, handle schema changes this way.

+CREATE TABLE /*$wgDBprefix*/pm_queue (
+	pmq_id BIGINT unsigned NOT NULL AUTO_INCREMENT,
[snip]
+	PRIMARY KEY (pmq_id),
+	KEY (pmq_user)
+) /*$wgDBTableOptions*/;

This is not SQLite-compatible. To make it SQLite compatible:

CREATE TABLE /*_*/pm_queue (
	pmq_id BIGINT unsigned NOT NULL PRIMARY KEY AUTO_INCREMENT,
[snip]
) /*$wgDBTableOptions*/;
CREATE INDEX /*i*/pmq_user ON /*_*/pm_queue (pmq_user);

Likewise, to make the pm_whitelist table SQLite-compatible, you will need to move the PRIMARY KEY declaration.

Premoderation.php has exactly one comment. It really could use some comments for the configuration variables. Also, you don't need to use array() in $wgExtensionCredits when there is only one author.

See r102199.