User "Catrope" changed the status of MediaWiki.r89277. Old Status: new New Status: fixme
User "Catrope" also posted a comment on MediaWiki.r89277. Full URL: https://secure.wikimedia.org/wikipedia/mediawiki/wiki/Special:Code/MediaWiki/89277#c17882 Commit summary: Added 'problem articles' view to dashboard; refactored dashboard code (populateAFStatistics.php, primarily); Added new schema sql scripts as well as sql migration script Comment: There's quite a bit wrong with this rev. I'll fix it because Arthur is in fundraising land now. Krinkle reports: <pre> Notice: Undefined variable: cur_ts in /htdocs/SVN/mediawiki/trunk/extensions/ArticleFeedback/populateAFStatistics.php on line 213 Notice: Undefined property: Page::$probematic in /htdocs/SVN/mediawiki/trunk/extensions/ArticleFeedback/populateAFStatistics.php on line 543 </pre> <pre> +CREATE UNIQUE INDEX /*i*/ afs_page_ts_type ON /*_*/ article_feedback_stats( afs_page_id, afs_ts, afs_stats_type_id ); +CREATE INDEX /*i*/ afs_ts_avg_overall ON /*_*/article_feedback_stats (afs_ts, afs_orderable_data); </pre> These indexes are insufficient. Making a note for myself to add proper indexing. <pre> + if ( !$db->tableExists( 'article_feedback_stats_type' )) { </pre> Typo in table name <pre> + array_push( $problems, $page->page_id ); </pre> Per the PHP docs, just use <code>$problems[] = $page->page_id;</code> Problem articles data is built but never inserted. Re-fetches data it just inserted, and does so from the slave, which is guaranteed to fail in a replicated environment. <pre> + array( 'aa_timestamp >= ' . $this->dbr->addQuotes( $ts )), </pre> Needs <code>$this->dbr->timestamp()</code> too. _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
