r103372 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r103371‎ | r103372 | r103373 >
Date:20:29, 16 November 2011
Author:gregchiasson
Status:deferred (Comments)
Tags:
Comment:
Enable rollup tables for ratings in AFT5. Ratings still aren't being passed from the JS to the PHP, but once that's worked out this should be functional.
Modified paths:
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.hooks.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5Utils.php (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5.php
——@@ -16,12 +16,11 @@
1717 $token = ApiArticleFeedbackv5Utils::getAnonToken( $params );
1818
1919 // Is feedback enabled on this page check?
20 - if ( !ApiArticleFeedbackv5Utils::isFeedbackEnabled( $params ) ) {
21 - $this->dieUsage( 'ArticleFeedback is not enabled on this page', 'invalidpage' );
22 - }
 20+# if ( !ApiArticleFeedbackv5Utils::isFeedbackEnabled( $params ) ) {
 21+# $this->dieUsage( 'ArticleFeedback is not enabled on this page', 'invalidpage' );
 22+# }
2323
2424 $feedbackId = $this->getFeedbackId($params);
25 -error_log("feedback id is $feedbackId");
2625 $dbr = wfGetDB( DB_SLAVE );
2726 $keys = array();
2827 foreach($params as $key => $unused) { $keys[] = $key; }
——@@ -37,17 +36,13 @@
3837 );
3938
4039 foreach($answers as $answer) {
41 - $type = $answer->afi_data_type;
42 - # TODO: validation
43 - # rating: int between 1 and 5 (inclusive)
44 - # boolean: 1 or 0
45 - # option: option exists
46 - # text: none (maybe xss encode)
47 - if($params[$answer->afi_name]) {
 40+ $type = $answer->afi_data_type;
 41+ $value = $params[$answer->afi_name];
 42+ if($value && $this->validateParam($value, $type)) {
4843 $user_answers[] = array(
4944 'aa_feedback_id' => $feedbackId,
5045 'aa_field_id' => $answer->afi_id,
51 - "aa_response_$type" => $params[$answer->afi_name]
 46+ "aa_response_$type" => $value
5247 );
5348 }
5449 }
——@@ -78,31 +73,131 @@
7974 );
8075 }
8176
82 - public function updateRollupTables($page, $revision) {
83 - $this->updateRatingRollup($page, $revision);
84 - $this->updateSelectRollup($page, $revision);
85 - }
 77+ protected function validateParams($value, $type) {
 78+ # rating: int between 1 and 5 (inclusive)
 79+ # boolean: 1 or 0
 80+ # option: option exists
 81+ # text: none (maybe xss encode)
 82+ switch($type) {
 83+ case 'rating':
 84+ if(preg_match('/^(1|2|3|4|5)$/', $value)) {
 85+ return 1;
 86+ }
 87+ break;
 88+ case 'boolean':
 89+ if(preg_match('/^(1|0)$/', $value)) {
 90+ return 1;
 91+ }
 92+ break;
 93+ case 'option':
 94+ # TODO: check for option existance.
 95+ case 'text':
 96+ return 1;
 97+ break;
 98+ default:
 99+ return 0;
 100+ break;
 101+ }
86102
87 - public function updateRatingRollup($page, $rev) {
88 - $this->__updateRollup($page, $rev, 'ratings', 'page');
89 - $this->__updateRollup($page, $rev, 'ratings', 'revision');
 103+ return 0;
90104 }
91105
92 - public function updateSelectRollup($page, $rev) {
93 - $this->__updateRollup($page, $rev, 'select', 'page');
94 - $this->__updateRollup($page, $rev, 'select', 'revision');
 106+ public function updateRollupTables($page, $revision) {
 107+# foreach( array( 'rating', 'boolean', 'select' ) as $type ) {
 108+# foreach( array( 'rating', 'boolean' ) as $type ) {
 109+ foreach( array( 'rating' ) as $type ) {
 110+ $this->updateRollup( $page, $revision, $type );
 111+ }
95112 }
96113
97 - # page and rev and page and revision ids
98 - # type is either ratings or select, the two rollups we have
99 - # scope is either page or revision
100 - private function __updateRollup($page, $rev, $type, $scope) {
 114+ # TODO: This breaks on the select/option_id type.
 115+ private function updateRollup($pageId, $revId, $type) {
 116+ global $wgArticleFeedbackv5RatingLifetime;
 117+ $dbr = wfGetDB( DB_SLAVE );
 118+ $dbw = wfGetDB( DB_MASTER );
 119+ $limit = ApiArticleFeedbackv5Utils::getRevisionLimit($pageId);
 120+
101121 # sanity check
102 - if($type != 'ratings' && $type != 'select') { return 0; }
103 - if($scope != 'page' && $scope != 'revision') { return 0; }
 122+ if( $type != 'rating' && $type != 'select'
 123+ && $type != 'boolean' ) { return 0; }
104124
105 - # TODO
106 - $table = 'aft_article_'.$rev.'_feedback_'.$type.'_rollup';
 125+ $rows = $dbr->select(
 126+ array( 'aft_article_answer', 'aft_article_feedback',
 127+ 'aft_article_field' ),
 128+ array( 'aa_field_id',
 129+ "SUM(aa_response_$type) AS earned",
 130+ 'COUNT(*) AS submits'),
 131+ array(
 132+ 'afi_data_type' => $type,
 133+ 'af_page_id' => $pageId,
 134+ 'aa_feedback_id = af_id',
 135+ 'afi_id = aa_field_id',
 136+ "af_revision_id >= $limit"
 137+ ),
 138+ __METHOD__,
 139+ array( 'GROUP BY' => 'aa_field_id' )
 140+ );
 141+
 142+ if( $type == 'select' ) {
 143+ $page_prefix = 'afsr_';
 144+ $rev_prefix = 'arfsr_';
 145+ } else {
 146+ $page_prefix = 'arr_';
 147+ $rev_prefix = 'afrr_';
 148+ }
 149+ $page_data = array();
 150+ $rev_data = array();
 151+ $rev_table = 'aft_article_revision_feedback_'
 152+ .( $type == 'select' ? 'select' : 'ratings' ).'_rollup';
 153+ $page_table = 'aft_article_feedback_'
 154+ .( $type == 'select' ? 'select' : 'ratings' ).'_rollup';
 155+
 156+ foreach($rows as $row) {
 157+ if($type == 'rating') {
 158+ $points = $row->submits * 5;
 159+ } else {
 160+ $points = $row->submits;
 161+ }
 162+
 163+ if(!array_key_exists($row->aa_field_id, $page_data)) {
 164+ $page_data[$row->aa_field_id] = array(
 165+ $page_prefix.'page_id' => $pageId,
 166+ $page_prefix.'total' => 0,
 167+ $page_prefix.'count' => 0,
 168+ $page_prefix.($type == 'select' ? 'option' : 'rating').'_id' => $row->aa_field_id
 169+ );
 170+ }
 171+
 172+ $rev_data[] = array(
 173+ $rev_prefix.'page_id' => $pageId,
 174+ $rev_prefix.'revision_id' => $revId,
 175+ $rev_prefix.'total' => $row->earned,
 176+ $rev_prefix.'count' => $points,
 177+ $rev_prefix.($type == 'select' ? 'option' : 'rating').'_id' => $row->aa_field_id
 178+ );
 179+ $page_data[$row->aa_field_id][$page_prefix.'total'] += $row->earned;
 180+ $page_data[$row->aa_field_id][$page_prefix.'count'] += $points;
 181+ }
 182+
 183+ # Hack becuse you can't use array keys or insert barfs.
 184+ $tmp = array();
 185+ foreach($page_data as $p) {
 186+ $tmp[] = $p;
 187+ }
 188+ $page_data = $tmp;
 189+
 190+ $dbw->begin();
 191+ $dbw->delete( $rev_table, array(
 192+ $rev_prefix.'page_id' => $pageId,
 193+ $rev_prefix.'revision_id' => $revId,
 194+ ) );
 195+ $dbw->insert( $rev_table, $rev_data );
 196+ $dbw->delete( $page_table, array(
 197+ $page_prefix.'page_id' => $pageId,
 198+ ) );
 199+ $dbw->insert( $page_table, $page_data );
 200+
 201+ $dbw->commit();
107202 }
108203
109204 public function getFeedbackId($params) {
Index: trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5Utils.php
——@@ -38,6 +38,23 @@
3939 return 1;
4040 }
4141
 42+ public static function getRevisionLimit( $pageId ) {
 43+ global $wgArticleFeedbackv5RatingLifetime;
 44+ $dbr = wfGetDB( DB_SLAVE );
 45+ $revision = $dbr->selectField(
 46+ 'revision', 'rev_id',
 47+ array( 'rev_page' => $pageId ),
 48+ __METHOD__,
 49+ array(
 50+ 'ORDER BY' => 'rev_id DESC',
 51+ 'LIMIT' => 1,
 52+ 'OFFSET' => $wgArticleFeedbackv5RatingLifetime - 1
 53+ )
 54+ );
 55+
 56+ return $revision ? intval($revision) : 0;
 57+ }
 58+
4259 public static function getRevisionId($pageId) {
4360 $dbr = wfGetDB( DB_SLAVE );
4461 $revId = $dbr->selectField(
Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.hooks.php
——@@ -187,13 +187,15 @@
188188 $fields = ApiArticleFeedbackv5Utils::getFields();
189189 $prefix = 'articlefeedbackv5-bucket5-';
190190 foreach( $fields as $field ) {
191 - $resources['messages'][] = $prefix . $field->afi_name . '-label';
192 - $resources['messages'][] = $prefix . $field->afi_name . '-tip';
193 - $resources['messages'][] = $prefix . $field->afi_name . '-tooltip-1';
194 - $resources['messages'][] = $prefix . $field->afi_name . '-tooltip-2';
195 - $resources['messages'][] = $prefix . $field->afi_name . '-tooltip-3';
196 - $resources['messages'][] = $prefix . $field->afi_name . '-tooltip-4';
197 - $resources['messages'][] = $prefix . $field->afi_name . '-tooltip-5';
 191+ if($fielf->afi_data_type == 'rating') {
 192+ $resources['messages'][] = $prefix . $field->afi_name . '-label';
 193+ $resources['messages'][] = $prefix . $field->afi_name . '-tip';
 194+ $resources['messages'][] = $prefix . $field->afi_name . '-tooltip-1';
 195+ $resources['messages'][] = $prefix . $field->afi_name . '-tooltip-2';
 196+ $resources['messages'][] = $prefix . $field->afi_name . '-tooltip-3';
 197+ $resources['messages'][] = $prefix . $field->afi_name . '-tooltip-4';
 198+ $resources['messages'][] = $prefix . $field->afi_name . '-tooltip-5';
 199+ }
198200 }
199201 }
200202 $resourceLoader->register(

Sign-offs

UserFlagDate
Johnduhartinspected03:50, 18 November 2011

Comments

#Comment by Johnduhart (talk | contribs)   03:50, 18 November 2011

Missing some spaces inside of parenthesis.

Status & tagging log