r84637 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r84636‎ | r84637 | r84638 >
Date:21:52, 23 March 2011
Author:tparscal
Status:resolved (Comments)
Tags:
Comment:
Fixes more issues in r84535
* User rating expiration is now based on user ratings
* When no ratings are current, the API still shows the user ratings if they exist
* Made the user interface respond to user ratings independent of totals
Modified paths:
  • /trunk/extensions/ArticleFeedback/api/ApiQueryArticleFeedback.php (modified) (history)
  • /trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/jquery.articleFeedback.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/jquery.articleFeedback.js
——@@ -247,6 +247,7 @@
248248 }
249249 }
250250 }
 251+ // Report
251252 if ( typeof ratingData === 'undefined' || ratingData.total == 0 ) {
252253 // Setup in "no ratings" mode
253254 $(this)
——@@ -258,9 +259,7 @@
259260 .end()
260261 .find( '.articleFeedback-rating-count' )
261262 .text( mw.msg( 'articlefeedback-report-empty' ) )
262 - .end()
263 - .find( 'input' )
264 - .attr( 'checked', false );
 263+ .end();
265264 } else {
266265 // Setup using ratingData
267266 var average = ratingData.total / ratingData.count;
——@@ -276,11 +275,24 @@
277276 .text( mw.msg(
278277 'articlefeedback-report-ratings', ratingData.count
279278 ) )
280 - .end()
281 - .find( 'input[value="' + ratingData.userrating + '"]' )
 279+ .end();
 280+ }
 281+ // Form
 282+ if ( typeof ratingData.userrating !== 'undefined' ) {
 283+ if ( ratingData.userrating === 0 ) {
 284+ $(this)
 285+ .find( 'input' )
 286+ .attr( 'checked', false );
 287+ } else {
 288+ $(this)
 289+ .find( 'input[value="' + ratingData.userrating + '"]' )
282290 .attr( 'checked', true );
283 - // If any ratings exist, make sure expertise is enabled so users can suppliment their ratings.
284 - $.articleFeedback.fn.enableExpertise( context.$ui.find( '.articleFeedback-expertise' ) );
 291+ }
 292+ // If any ratings exist, make sure expertise is enabled so users can
 293+ // suppliment their ratings.
 294+ $.articleFeedback.fn.enableExpertise(
 295+ context.$ui.find( '.articleFeedback-expertise' )
 296+ );
285297 }
286298 $.articleFeedback.fn.updateRating.call( $(this) );
287299 } );
Index: trunk/extensions/ArticleFeedback/api/ApiQueryArticleFeedback.php
——@@ -13,7 +13,6 @@
1414
1515 $this->addTables( array( 'article_feedback_pages', 'article_feedback_ratings' ) );
1616 $this->addFields( array(
17 - 'aap_page_id',
1817 'MAX(aap_revision) as aap_revision',
1918 'SUM(aap_total) as aap_total',
2019 'SUM(aap_count) as aap_count',
——@@ -32,59 +31,74 @@
3332 $this->addOption( 'GROUP BY', 'aap_rating_id' );
3433 $this->addOption( 'LIMIT', count( $wgArticleFeedbackRatings ) );
3534
 35+ // Rating counts and totals
3636 $res = $this->select( __METHOD__ );
37 -
38 - if ( $params['userrating'] ) {
39 - $userRatings = $this->getUserRatings( $params );
40 - }
41 -
42 - $ratings = array();
43 -
44 - $userRatedArticle = false;
45 -
46 - $rev = null;
 37+ $ratings = array( $params['pageid'] => array( 'pageid' => $params['pageid'] ) );
4738 foreach ( $res as $i => $row ) {
48 - // All the revs and pageIds will be the same for all rows, these are shortcuts
49 - $pageId = $row->aap_page_id;
50 - // This is special however, because we need this data later for expired calculation
51 - $rev = $row->aap_revision;
52 -
53 - if ( !isset( $ratings[$pageId] ) ) {
54 - $page = array(
55 - 'pageid' => $pageId,
56 - 'revid' => $rev,
57 - );
58 -
59 - if ( $params['userrating'] ) {
60 - if ( isset( $userRatings[$i]['expertise'] ) ) {
61 - $page['expertise'] = $userRatings[$i]['expertise'];
62 - }
63 - }
64 -
65 - $ratings[$pageId] = $page;
 39+ if ( !isset( $ratings[$params['pageid']] ) ) {
 40+ $ratings[$params['pageid']]['revid'] = $row->aap_revision;
6641 }
67 -
68 - $thisRow = array(
 42+ if ( !isset( $ratings[$params['pageid']]['ratings'] ) ) {
 43+ $ratings[$params['pageid']]['ratings'] = array();
 44+ }
 45+ $ratings[$params['pageid']]['ratings'][] = array(
6946 'ratingid' => $row->aap_rating_id,
7047 'ratingdesc' => $row->aar_rating,
7148 'total' => $row->aap_total,
7249 'count' => $row->aap_count,
7350 );
74 -
75 - if ( $params['userrating'] && isset( $userRatings[$i]['value'] ) ) {
76 - $thisRow['userrating'] = $userRatings[$i]['value'];
77 -
78 - $userRatedArticle = true;
 51+ }
 52+
 53+ // User ratings
 54+ $ratings[$params['pageid']]['status'] = 'current';
 55+ if ( $params['userrating'] ) {
 56+ $userRatings = $this->getUserRatings( $params );
 57+ if ( isset( $ratings[$params['pageid']]['ratings'] ) ) {
 58+ // Valid ratings already exist
 59+ foreach ( $ratings[$params['pageid']]['ratings'] as $i => $rating ) {
 60+ // Rating value
 61+ $ratings[$params['pageid']]['ratings'][$i]['userrating'] =
 62+ $userRatings[$rating['ratingid']]['value'];
 63+ // Expertise
 64+ if ( !isset( $ratings[$params['pageid']]['expertise'] ) ) {
 65+ $ratings[$params['pageid']]['expertise'] =
 66+ $userRatings[$rating['ratingid']]['expertise'];
 67+ }
 68+ // Expiration
 69+ if ( $userRatings[$rating['ratingid']]['revision'] < $revisionLimit ) {
 70+ $ratings[$params['pageid']]['status'] = 'expired';
 71+ }
 72+ }
 73+ } else {
 74+ // No valid ratings exist
 75+ if ( count( $userRatings ) ) {
 76+ $ratings[$params['pageid']]['status'] = 'expired';
 77+ }
 78+ foreach ( $userRatings as $ratingId => $userRating ) {
 79+ // Revision
 80+ if ( !isset( $ratings[$params['pageid']]['revid'] ) ) {
 81+ $ratings[$params['pageid']]['revid'] = $userRating['revision'];
 82+ }
 83+ // Ratings
 84+ if ( !isset( $ratings[$params['pageid']]['ratings'] ) ) {
 85+ $ratings[$params['pageid']]['ratings'] = array();
 86+ }
 87+ // Rating value
 88+ $ratings[$params['pageid']]['ratings'][] = array(
 89+ 'ratingid' => $ratingId,
 90+ 'ratingdesc' => $userRating['text'],
 91+ 'total' => 0,
 92+ 'count' => 0,
 93+ 'userrating' => $userRating['value'],
 94+ );
 95+ // Expertise
 96+ if ( !isset( $ratings[$params['pageid']]['expertise'] ) ) {
 97+ $ratings[$params['pageid']]['expertise'] = $userRating['expertise'];
 98+ }
 99+ }
79100 }
80 -
81 - $ratings[$pageId]['ratings'][] = $thisRow;
82101 }
83102
84 - // Ratings can only be expired if the user has rated before
85 - if ( $params['userrating'] && $userRatedArticle ) {
86 - $ratings[$params['pageid']]['status'] = $rev >= $revisionLimit ? 'current' : 'expired';
87 - }
88 -
89103 foreach ( $ratings as $rat ) {
90104 $result->setIndexedTagName( $rat['ratings'], 'r' );
91105 $result->addValue( array( 'query', $this->getModuleName() ), null, $rat );
——@@ -107,11 +121,16 @@
108122 $token = $params['anontoken'];
109123 }
110124
111 - $dbr = wfGetDb( DB_SLAVE );
112 - $res = $dbr->select(
113 - array( 'article_feedback', 'article_feedback_properties' ),
114 - array( 'aa_rating_id', 'aa_rating_value', 'afp_value_text' ),
 125+ $res = $this->getDB()->select(
 126+ array( 'article_feedback', 'article_feedback_properties', 'article_feedback_ratings' ),
115127 array(
 128+ 'aa_rating_id',
 129+ 'aar_rating',
 130+ 'aa_revision',
 131+ 'aa_rating_value',
 132+ 'afp_value_text'
 133+ ),
 134+ array(
116135 'aa_page_id' => $pageId,
117136 'aa_rating_id' => $wgArticleFeedbackRatings,
118137 'aa_user_id' => $wgUser->getId(),
——@@ -121,21 +140,26 @@
122141 'ORDER BY' => 'aa_revision DESC',
123142 'LIMIT' => count( $wgArticleFeedbackRatings )
124143 ),
125 - array( 'article_feedback_properties' => array(
126 - 'LEFT JOIN', array(
127 - 'afp_revision=aa_revision',
128 - 'afp_user_text=aa_user_text',
129 - 'afp_user_anon_token=aa_user_anon_token',
130 - 'aa_user_anon_token' => $token,
131 - 'afp_key' => 'expertise',
132 - )
133 - ) )
 144+ array(
 145+ 'article_feedback_properties' => array(
 146+ 'LEFT JOIN', array(
 147+ 'afp_revision=aa_revision',
 148+ 'afp_user_text=aa_user_text',
 149+ 'afp_user_anon_token=aa_user_anon_token',
 150+ 'aa_user_anon_token' => $token,
 151+ 'afp_key' => 'expertise',
 152+ )
 153+ ),
 154+ 'article_feedback_ratings' => array( 'LEFT JOIN', array( 'aar_id=aa_rating_id' ) )
 155+ )
134156 );
135157 $ratings = array();
136158 foreach ( $res as $row ) {
137159 $ratings[$row->aa_rating_id] = array(
138160 'value' => $row->aa_rating_value,
139 - 'expertise' => $row->afp_value_text
 161+ 'expertise' => $row->afp_value_text,
 162+ 'revision' => $row->aa_revision,
 163+ 'text' => $row->aar_rating,
140164 );
141165 }
142166 return $ratings;
——@@ -150,8 +174,7 @@
151175 protected function getRevisionLimit( $pageId ) {
152176 global $wgArticleFeedbackRatingLifetime;
153177
154 - $dbr = wfGetDb( DB_SLAVE );
155 - $revision = $dbr->selectField(
 178+ $revision = $this->getDB()->selectField(
156179 'revision',
157180 'rev_id',
158181 array( 'rev_page' => $pageId ),

Follow-up revisions

RevisionCommit summaryAuthorDate
r85144Fixed impossible condition - addresses comments on r84637tparscal19:04, 1 April 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r84535Step 2 of 2 for implementing expirations....tparscal17:18, 22 March 2011

Comments

#Comment by Catrope (talk | contribs)   15:39, 28 March 2011 
+			if ( !isset( $ratings[$params['pageid']] ) ) {
+				$ratings[$params['pageid']]['revid'] = $row->aap_revision;

This looks wrong. AFAICT the if condition is impossible to satisfy. Should probably have been !isset( $ratings[$params['pageid']]['revid'] ).

OK otherwise.

Status & tagging log