"Catrope" changed the status of MediaWiki.r100165 to "fixme" and commented it.
URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/100165#c28221
Old Status: new
> New Status: fixme
Commit summary for MediaWiki.r100165:
Adds API Module by cryptocoryne
Bug 31723
Catrope's comment:
<pre>
+ $time = wfTimestamp( TS_MW, strtotime('now') - (
strtotime($params['timecond'] ? $params['timecond'] : '2 weeks') -
strtotime('now')));
</pre>
WTF?
# Computing <code>$now - ($timecond - $now)</code> doesn't make much sense to
me. What are you trying to do here?
# You should just be setting a PARAM_DEFAULT for 'timecond' rather than using a
default value of 2 weeks implicitly
<pre>
+ $this->addWhere( "cuc_timestamp > $time" );
</pre>
$time needs to be quoted, preferably with $this->getDB()->addQuotes() so
escaping is done too.
<pre>
+ $ips[$ip]['end'] = $timestamp;
[...]
+ $edits[$count]['timestamp'] = $row->cuc_timestamp;
</pre>
Timestamps for output should be formatted as RFC2822. There are more
occurrences of this all over the query module.
<pre>
+ $edits[$count]['ns'] = $row->cuc_namespace;
+ $edits[$count]['title'] = $row->cuc_title;
</pre>
You should run the namespace through intval() so it'll be output as an integer
in JSON and in any other output formats that might differentiate between
integers and strings. Also, this means that User talk:Catrope looks like
<code>namespace:3, title:'Catrope'</code> , is that what you intended? The most
common format used in the API is <code>namespace:3, title:"User
talk:Catrope"</code> .
<pre>
+ if( $row->cuc_actiontext ) {
[...]
+ } elseif( $row->cuc_comment ) {
</pre>
What are you really trying to test for here? Emptiness (<code><nowiki>!==
''</nowiki></code>)? Null-ness (<code>!== null</code>)? Casting to a boolean
like this may give false negatives for things like <code>'0'</code> .
<pre>
+ ApiBase::PARAM_MAX => 5000,
+ ApiBase::PARAM_MAX2 => 5000
</pre>
You should set these to <code>ApiBase::LIMIT_BIG1</code> (=500) and
<code>ApiBase::LIMIT_BIG2</code> (=5000) respectively. QueryCheckUserLog does
do this correctly.
_______________________________________________
MediaWiki-CodeReview mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
