User:Werdna/GlobalBlocking checklist

Jump to navigation Jump to search

Per brion's comments on bug 8707:

  • GlobalBlocking class..
  • GlobalBlocking::getUserPermissionsErrors
  • lack of caching may or may not be worrying
  • conditions...
  • $conds = array( 'gb_address' => $ip, 'gb_timestamp<'.$dbr->addQuotes($dbr->timestamp(wfTimestampNow())) );
  • ok that looks totally broken.
  • no ip range checks
  • timestamp in the past? wtf? we want future blocks?
  • add the range checks
  • remove this extra timestamp check
  • add an expiry check
  • $expiry changes data types (coded timestamp -> text), this is poor practice
  • change this to another var
  • expiry is loaded but not checked
  • expire if we're old!
  • $block->gb_by_wiki is dumped straight into err msg output without a clear data type
  • what's the type of db_by_wiki? dbname or interwiki?
  • GlobalBlocking::getGlobalBlockingMaster()
  • GlobalBlocking::getGlobalBlockingSlave()
  • uses wfGetDB... is this right for the current load balancersystem?
  • GlobalBlocking::buildForm
  • just replace this with wfBuildForm() since it calls through...
  • SpecialGlobalBlock...
  • SpecialGlobalBlock::execute
  • if (!is_array($errors) || count($errors)>0) {
  • wtf? this is some ugly damn code. :D ends up overall just looking very confusing
  • pull apart this if() tree to something legible
  • SpecialGlobalBlock::loadParameters()
  • use getText() for reason
  • quote wpExpiryOther
  • SpecialGlobalBlock::trySubmit()
  • Error-out for dupe blocks is done before expired blocks are purged.
  • purge expired blocks at top
  • range blocks are not saved properly -- missing range start and end fields
  • expiry handling... shouldn't this be encapsulated for us already?
  • check Block for encapsulated infinity handling
  • SpecialGlobalBlock::buildExpirySelector
  • doesn't this duplicate existing code?
  • find and replace with clean call to core code
  • SpecialGlobalBlock::buildForm
  • inputs are too short (compare to 45 chars on Special:Blockip)
  • labels aren't aligned properly
  • SpecialGlobalBlockList
  • SpecialGlobalBlockList::showList
  • -> bad escaping on error messages -- use wfEscapeWikiText() or don't run these through wiki parser
  • validity checks are overlong
  • use IP::isIPAddress
  • SpecialGlobalBlockList::unblockForm
  • same bad escaping on error messages
  • SpecialGlobalBlockList::tryUnblockSubmit
  • overload IP checks again
  • I've seen this pattern many times over:

               $expiry = Block::decodeExpiry( $row->gb_expiry );
               if ($expiry == 'infinity') {
                       $expiry = wfMsg( 'infiniteblock' );
               } else {
                       global $wgLang;
                       $expiry = $wgLang->timeanddate( wfTimestamp( TS_MW, $expiry ), true );

This is probably already encapsulated somewhere in Block-land. If it's not, at least make a local func for it!

  • GlobalBlockListPager::formatRow
  • Comments are getting formatted with the regular wiki parser.

This can be very disruptive -- images, tables, broken code...

  • -> use the comment parser, like all other comments in such lists.