r110086 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r110085‎ | r110086 | r110087 >
Date:21:57, 26 January 2012
Author:gregchiasson
Status:ok (Comments)
Tags:aft 
Comment:
AFT5 - SQL statements to rebuild the rollup tables using only bucket 1, and a fix to prevent loading errors if a record with no timestamp comes up (which should be impossible anyway, but has happened on dev, and should be handled either way, instead of breaking the page).
Modified paths:
  • /trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/sql/alter.sql (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/sql/alter.sql
——@@ -66,6 +66,8 @@
6767 ALTER TABLE aft_article_feedback ADD COLUMN af_unhelpful_count integer unsigned NOT NULL DEFAULT 0;
6868
6969 -- added or updated 1/24 (greg)
 70+ALTER TABLE aft_article_feedback ADD COLUMN af_needs_oversight boolean NOT NULL DEFAULT FALSE;
 71+
7072 DELETE FROM aft_article_filter_count;
7173 INSERT INTO aft_article_filter_count(afc_page_id, afc_filter_name, afc_filter_count) SELECT af_page_id, 'helpful', COUNT(*) FROM aft_article_feedback WHERE CONVERT(af_helpful_count, SIGNED) - CONVERT(af_unhelpful_count, SIGNED) > 0 GROUP BY af_page_id;
7274 INSERT INTO aft_article_filter_count(afc_page_id, afc_filter_name, afc_filter_count) SELECT af_page_id, 'abusive', COUNT(*) FROM aft_article_feedback WHERE af_abuse_count > 0 GROUP BY af_page_id;
——@@ -76,4 +78,30 @@
7779 INSERT INTO aft_article_filter_count(afc_page_id, afc_filter_name, afc_filter_count) SELECT af_page_id, 'deleted', COUNT(*) FROM aft_article_feedback WHERE af_delete_count > 0 GROUP BY af_page_id;
7880 INSERT INTO aft_article_filter_count(afc_page_id, afc_filter_name, afc_filter_count) SELECT af_page_id, 'unhelpful', COUNT(*) FROM aft_article_feedback WHERE CONVERT(af_helpful_count, SIGNED) - CONVERT(af_unhelpful_count, SIGNED) < 0 GROUP BY af_page_id;
7981 INSERT INTO aft_article_filter_count(afc_page_id, afc_filter_name, afc_filter_count) SELECT af_page_id, 'needsoversight', COUNT(*) FROM aft_article_feedback WHERE af_needs_oversight IS TRUE GROUP BY af_page_id;
80 -ALTER TABLE aft_article_feedback ADD COLUMN af_needs_oversight boolean NOT NULL DEFAULT FALSE;
 82+
 83+-- added 1/26 (greg) - obviates much of the above from 1/24.
 84+DELETE FROM aft_article_filter_count;
 85+INSERT INTO aft_article_filter_count(afc_page_id, afc_filter_name, afc_filter_count) SELECT af_page_id, 'helpful', COUNT(*) FROM aft_article_feedback WHERE af_bucket_id = 1 AND CONVERT(af_helpful_count, SIGNED) - CONVERT(af_unhelpful_count, SIGNED) > 0 GROUP BY af_page_id;
 86+INSERT INTO aft_article_filter_count(afc_page_id, afc_filter_name, afc_filter_count) SELECT af_page_id, 'abusive', COUNT(*) FROM aft_article_feedback WHERE af_bucket_id = 1 AND af_abuse_count > 0 GROUP BY af_page_id;
 87+INSERT INTO aft_article_filter_count(afc_page_id, afc_filter_name, afc_filter_count) SELECT af_page_id, 'invisible', COUNT(*) FROM aft_article_feedback WHERE af_bucket_id = 1 AND af_hide_count > 0 GROUP BY af_page_id;
 88+INSERT INTO aft_article_filter_count(afc_page_id, afc_filter_name, afc_filter_count) SELECT af_page_id, 'visible', COUNT(*) FROM aft_article_feedback WHERE af_bucket_id = 1 AND af_hide_count = 0 GROUP BY af_page_id;
 89+INSERT INTO aft_article_filter_count(afc_page_id, afc_filter_name, afc_filter_count) SELECT af_page_id, 'all', COUNT(*) FROM aft_article_feedback WHERE af_bucket_id = 1 GROUP BY af_page_id;
 90+INSERT INTO aft_article_filter_count(afc_page_id, afc_filter_name, afc_filter_count) SELECT af_page_id, 'comment', COUNT(*) FROM aft_article_feedback, aft_article_answer WHERE af_bucket_id = 1 AND af_id = aa_feedback_id AND aa_response_text IS NOT NULL GROUP BY af_page_id;
 91+INSERT INTO aft_article_filter_count(afc_page_id, afc_filter_name, afc_filter_count) SELECT af_page_id, 'deleted', COUNT(*) FROM aft_article_feedback WHERE af_bucket_id = 1 AND af_delete_count > 0 GROUP BY af_page_id;
 92+INSERT INTO aft_article_filter_count(afc_page_id, afc_filter_name, afc_filter_count) SELECT af_page_id, 'unhelpful', COUNT(*) FROM aft_article_feedback WHERE af_bucket_id = 1 AND CONVERT(af_helpful_count, SIGNED) - CONVERT(af_unhelpful_count, SIGNED) < 0 GROUP BY af_page_id;
 93+INSERT INTO aft_article_filter_count(afc_page_id, afc_filter_name, afc_filter_count) SELECT af_page_id, 'needsoversight', COUNT(*) FROM aft_article_feedback WHERE af_bucket_id = 1 AND af_needs_oversight IS TRUE GROUP BY af_page_id;
 94+
 95+-- Note that this ignores the select rollups, since bucket 1 doesn't have any
 96+-- selects in it. Those tables can be truncated, or possibly dropped entirely,
 97+-- if bucket 1 remains the only bucket. Holding off on that decision for now.
 98+DELETE FROM aft_article_feedback_ratings_rollup;
 99+DELETE FROM aft_article_revision_feedback_ratings_rollup;
 100+INSERT INTO aft_article_revision_feedback_ratings_rollup (afrr_page_id, afrr_revision_id, afrr_field_id, afrr_total, afrr_count)
 101+SELECT af_page_id, af_revision_id, aa_field_id, SUM(aa_response_boolean), COUNT(aa_response_boolean)
 102+FROM aft_article_feedback, aft_article_answer
 103+WHERE af_bucket_id = 1 AND af_id = aa_feedback_id AND aa_response_boolean IS NOT NULL
 104+GROUP BY af_page_id, af_revision_id;
 105+INSERT INTO aft_article_feedback_ratings_rollup (arr_page_id, arr_field_id, arr_total, arr_count)
 106+SELECT afrr_page_id, afrr_field_id, SUM(afrr_total), SUM(afrr_count)
 107+FROM aft_article_revision_feedback_ratings_rollup
 108+GROUP BY afrr_page_id;
Index: trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php
——@@ -302,8 +302,9 @@
303303 ( $now - $timestamp ), 'avoidseconds'
304304 );
305305 $date = wfMessage( 'articleFeedbackv5-comment-ago', $time )->escaped();
 306+ } elseif( $timestamp ) {
 307+ $date = $wgLang->timeanddate($record[0]->af_created );
306308 } else {
307 - $date = $wgLang->timeanddate($record[0]->af_created );
308309 }
309310
310311 $details = Html::openElement( 'div', array(

Follow-up revisions

RevisionCommit summaryAuthorDate
r110132Fixes issues raised in commits from last night:...gregchiasson15:57, 27 January 2012

Comments

#Comment by Nikerabbit (talk | contribs)   07:38, 27 January 2012

I don't see how this fixes anything. It only leaves $date undefined in some cases.

#Comment by Gregchiasson (talk | contribs)   15:22, 27 January 2012

It stops $wgLang->timeanddate() from throwing an error when it gets an invalid timestamp (see below), which was the original problem. $date's still undefined though, which I guess means I don't have php errors turned up loud enough on my dev environment, because I missed that.

  1. 0 /home/reha/svn/mediawiki/includes/GlobalFunctions.php(1314): MessageCache->get(NULL, true, Object(Language)) #1 /home/reha/svn/mediawiki/includes/GlobalFunctions.php(1439): wfMsgGetKey(NULL, true, Object(Language), false) #2 /home/reha/svn/mediawiki/languages/Language.php(699): wfMsgExt(NULL, Array) #3 /home/reha/svn/mediawiki/languages/Language.php(721): Language->getMessageFromDB(NULL) #4 /home/reha/svn/mediawiki/languages/Language.php(979): Language->getMonthName(false) #5 /home/reha/svn/mediawiki/languages/Language.php(1848): Language->sprintfDate('H:i, j F Y', false) #6 /home/reha/svn/mediawiki/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php(306): Language->timeanddate('2011-12-02 15:') #7 /home/reha/svn/mediawiki/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php(47): ApiViewFeedbackArticleFeedbackv5->renderFeedback(Array) #8 /home/reha/svn/mediawiki/includes/api/ApiQuery.php(266): ApiViewFeedbackArticleFeedbackv5->execute() #9 /home/reha/svn/mediawiki/includes/api/ApiMain.php(705): ApiQuery->execute() #10 /home/reha/svn/mediawiki/includes/api/ApiMain.php(360): ApiMain->executeAction() #11 /home/reha/svn/mediawiki/includes/api/ApiMain.php(344): ApiMain->executeActionWithErrorHandling() #12 /home/reha/svn/mediawiki/api.php(117): ApiMain->execute() #13 {main}
#Comment by Catrope (talk | contribs)   07:31, 31 January 2012

Why do the rollup tables only contain rows for bucket 1 now? Does this mean we'll have to rebuild the rollup tables in production?

#Comment by Gregchiasson (talk | contribs)   16:08, 31 January 2012

There was a requirement from Fabrice to make the Special page only show feedback from bucket 1, which meant that the "X feedback posts on this article" line needed to be updated to only count from bucket 1 (this comes from aft_article_filter_count), as well as the counts in the filter dropdown. Since that doesn't exist on production at the moment it would have to be populated anyway, and this change just populates it properly according to the new requirements.

The main rollup tables probably don't need updating, I just threw that in there to get rid of what are now unused rows (from buckets 2 and 3) for completenesses sake - they already group by field_id, which is a function of bucket ID, so the table can actually be left alone and it shouldn't cause problems.

Status & tagging log