User "Jack Phoenix" posted a comment on MediaWiki.r102139.
Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/102139#c25594
Commit summary:
Added experimental version of new extension
Comment:
<pre><nowiki>
+ protected $mParams = false;
+ protected $mPosted = false;
+ protected $mRequest = array();
+ protected $mSummary = '';
+ public $mAllowedStatus = array( 'new', 'approved', 'declined' );
</nowiki></pre>
Prefixing class member variables with <tt>m</tt> is ''so'' 1999.
<pre><nowiki>
+ if( !$wgUser->isAllowed( 'premoderation' ) ) {
+ $this->displayRestrictionError();
+ return;
+ }
</nowiki></pre>
Should you also perform checks for read-only mode and if the user trying to
perform the action is blocked?
<pre><nowiki>
+ $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' ),
</nowiki></pre>
Please break lines at 80 characters.
<pre><nowiki>
+ if( $wgUser->isAllowed( 'premoderation-viewip' ) ) {
+ $wgOut->addHTML( wfMsg( 'premoderation-private-ip' ) .
' ' . $row['pmq_ip'] );
+ }
</nowiki></pre>
I believe that you should pass the $row['pmq_ip'] variable to the message for
better international compatibility.
<pre><nowiki>
$output .= Xml::closeElement( 'table' );
</nowiki></pre>
''Personally'' I'd write </table>, even though the recommended way is to
use the Html class.
<pre><nowiki>
+ 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 )
+ );
</nowiki></pre>
I'd suggest swapping the MySQL-specific <tt>!=</tt> operator to the standard
SQL NOT operator <tt><></tt> and writing the query like this:
<source lang="php">
$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 )
);
</source>
I'd suggest removing install.php script altogether and writing a function that
hooks into the
[[Manual:Hooks/LoadExtensionSchemaUpdates|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.
<pre><nowiki>
+CREATE TABLE /*$wgDBprefix*/pm_queue (
+ pmq_id BIGINT unsigned NOT NULL AUTO_INCREMENT,
[snip]
+ PRIMARY KEY (pmq_id),
+ KEY (pmq_user)
+) /*$wgDBTableOptions*/;
</nowiki></pre>
This is not [[Manual:SQLite|SQLite]]-compatible. To make it SQLite compatible:
<source lang="sql">
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);
</source>
Likewise, to make the <tt>pm_whitelist</tt> 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.
_______________________________________________
MediaWiki-CodeReview mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
