https://bugzilla.wikimedia.org/show_bug.cgi?id=15129
--- Comment #14 from Roan Kattouw <[email protected]> 2009-01-14 22:17:43 UTC --- (In reply to comment #11) > Created an attachment (id=5676) --> (https://bugzilla.wikimedia.org/attachment.cgi?id=5676) [details] > Patch to add CheckUser API module > > I have written some bare code enabling this functionality (in other words, it > needs review). > Here you are: * By convention, class names of API modules start with 'API', so use e.g. ApiCheckUser as class name (and hence ApiCheckUser.php as filename) * 'cantcheckuser' message: "don't" misspelled as "dont" * Parameters: ** Use ApiBase::PARAM_TYPE => 'user' for the user parameter (accepts IPs too) so you won't have to do username normalization ** Use ApiBase::PARAM_TYPE => array('allowed', 'values', 'here') for the type parameter ** If the duration parameter is a timestamp (not clear to me), use ApiBase::PARAM_TYPE => 'timestamp' ** If it's a number of days, use ApiBase::PARAM_TYPE => 'integer' and set ApiBase::PARAM_MIN and ApiBase::PARAM_MAX to appropriate values ** Use ApiBase::PARAM_DFLT => '' for the reason parameter rather than doing if(!isset($params['reason'])) $reason = ''; * You're using regexes to check for IPv4 and IPv6 IP's while you shouldn't: the /xff thing should go (like Bryan said) and the rest will be handled for you when you change the user parameter to be of type 'user' (In reply to comment #13) > A few general comments on this: > * You are basically duplicating the CheckUser interface. This is bad because > changes in one part of the CheckUser code will not affect other parts. Code > duplication should be avoided when possible. Bryan's right. You should call existing functions in CheckUser, not copy them. If CheckUser doesn't have nice functions for your purposes, create them and/or adapt existing ones to be nice to you. > * You are constructing raw sql, whereas using the wrapper functions is > strongly > recommended Eek! Looks like that's part of the code copied from CheckUser, though. > * Limits are hardcoded Your module should accept a 'limit' parameter: 'limit' => array ( ApiBase :: PARAM_DFLT => 10, ApiBase :: PARAM_TYPE => 'limit', ApiBase :: PARAM_MIN => 1, ApiBase :: PARAM_MAX => ApiBase :: LIMIT_BIG1, ApiBase :: PARAM_MAX2 => ApiBase :: LIMIT_BIG2 ) > * XFF is added as /xff. In the api you will simply want to use a boolean xff > parameter > 'xff' => false will do that. $params['xff'] will be a simple boolean then, so no need for isset($params['xff']) checks. -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ Wikibugs-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
