User:Werdna/GlobalBlocking checklist

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.