| Index: trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5.php |
| — | — | @@ -81,7 +81,7 @@ |
| 82 | 82 | } |
| 83 | 83 | } |
| 84 | 84 | |
| 85 | | - if($error) { |
| | 85 | + if ( $error ) { |
| 86 | 86 | $this->getResult()->addValue( |
| 87 | 87 | null, 'error', $error |
| 88 | 88 | ); |
| — | — | @@ -91,7 +91,7 @@ |
| 92 | 92 | $ctaId = $this->saveUserRatings( $user_answers, $feedbackId, $bucket ); |
| 93 | 93 | $this->updateRollupTables( $pageId, $revisionId, $user_answers ); |
| 94 | 94 | |
| 95 | | - if( $params['email'] ) { |
| | 95 | + if ( $params['email'] ) { |
| 96 | 96 | $this->captureEmail ( $params['email'], FormatJson::encode( |
| 97 | 97 | $email_data |
| 98 | 98 | ) ); |
| — | — | @@ -193,10 +193,10 @@ |
| 194 | 194 | |
| 195 | 195 | /** |
| 196 | 196 | * Cache result of ApiArticleFeedbackv5Utils::getRevisionLimit to avoid |
| 197 | | - * multiple fetches. |
| 198 | | - |
| 199 | | - * @param $pageID int the page id |
| 200 | | - * @return int the oldest revision to still count |
| | 197 | + * multiple fetches. |
| | 198 | + * |
| | 199 | + * @param $pageID int the page id |
| | 200 | + * @return int the oldest revision to still count |
| 201 | 201 | */ |
| 202 | 202 | public function getRevisionLimit( $pageId ) { |
| 203 | 203 | if( $this->revision_limit === null ) { |
| — | — | @@ -212,19 +212,19 @@ |
| 213 | 213 | * @param $revision int the revision id |
| 214 | 214 | * @param $type string the type (rating, select, or boolean) |
| 215 | 215 | * @param $raw_data array the user's validated feedback answers |
| 216 | | - |
| | 216 | + * |
| 217 | 217 | * This should: |
| 218 | 218 | * 0. Attempt to insert a blank revision rollup row for each $data of type $type, based on revId, fieldId. |
| 219 | 219 | * 1. Increment said revision rollup for each $data of type $type, based on revId, fieldId, and value |
| 220 | 220 | * 2. Re-calculate the page value, across the last [X] revisions (an old revision, or more, may have moved outside of the wgArticleFeedbackv5RatingLifetime window, so we can't just increment the page level rollups - revision-level, absolutely) |
| 221 | | - |
| | 221 | + * |
| 222 | 222 | */ |
| 223 | 223 | private function updateRollup( $pageId, $revId, $type, $raw_data ) { |
| 224 | 224 | # sanity check |
| 225 | 225 | if ( $type != 'rating' && $type != 'option_id' && $type != 'boolean' ) { |
| 226 | 226 | return 0; |
| 227 | 227 | } |
| 228 | | - |
| | 228 | + |
| 229 | 229 | // Strip out the data not of this type. |
| 230 | 230 | foreach ( $raw_data as $row ) { |
| 231 | 231 | if ( $row["aa_response_$type"] != null ) { |
| — | — | @@ -240,12 +240,12 @@ |
| 241 | 241 | * @param $revision int the revision id |
| 242 | 242 | * @param $type string the type (rating, select, or boolean) |
| 243 | 243 | * @param $row array a user's validated feedback answer |
| 244 | | - |
| | 244 | + * |
| 245 | 245 | * This should: |
| 246 | 246 | * 0. Attempt to insert a blank revision rollup row, based on revId, fieldId. |
| 247 | 247 | * 1. Increment said revision rollup, based on revId, fieldId, and value |
| 248 | 248 | * 2. Re-calculate the page rolup value, across the last [X] revisions (an old revision, or more, may have moved outside of the wgArticleFeedbackv5RatingLifetime window, so we can't just increment the page level rollups - revision-level, absolutely) |
| 249 | | - |
| | 249 | + * |
| 250 | 250 | */ |
| 251 | 251 | private function updateRollupRow( $pageId, $revId, $type, $row ) { |
| 252 | 252 | $dbr = wfGetDB( DB_SLAVE ); |
| — | — | @@ -258,10 +258,10 @@ |
| 259 | 259 | // Selects are kind of a odd bird. We store one row |
| 260 | 260 | // per option per field, and each one has the number |
| 261 | 261 | // of times that option was chosen, and the number of |
| 262 | | - // times the question was shown in total. So, you'd |
| | 262 | + // times the question was shown in total. So, you'd |
| 263 | 263 | // have 1/10, 2/10, 7/10, eg. We increment the times |
| 264 | 264 | // chosen on the one that was chosen, and the times |
| 265 | | - // shown on all of them. |
| | 265 | + // shown on all of them. |
| 266 | 266 | |
| 267 | 267 | // Fetch all the options for this field. |
| 268 | 268 | $options = $dbr->select( |
| — | — | @@ -271,7 +271,7 @@ |
| 272 | 272 | __METHOD__ |
| 273 | 273 | ); |
| 274 | 274 | |
| 275 | | - // For each option this field has, make sure we have |
| | 275 | + // For each option this field has, make sure we have |
| 276 | 276 | // a row by inserting one - will fail silently if the |
| 277 | 277 | // row already exists. |
| 278 | 278 | foreach( $options as $option ) { |
| — | — | @@ -353,7 +353,7 @@ |
| 354 | 354 | } |
| 355 | 355 | |
| 356 | 356 | // Revision rollups being done, we update the page rollups. |
| 357 | | - // These are built off of the revision rollups, and only |
| | 357 | + // These are built off of the revision rollups, and only |
| 358 | 358 | // count revisions back to the user-specified limit, so |
| 359 | 359 | // they need to be recalculated every time, since we don't |
| 360 | 360 | // know what revision we're dealing with, or how many times |
| — | — | @@ -367,8 +367,8 @@ |
| 368 | 368 | $rows = $dbr->select( |
| 369 | 369 | 'aft_article_revision_feedback_select_rollup', |
| 370 | 370 | array( |
| 371 | | - 'arfsr_option_id', |
| 372 | | - 'SUM(arfsr_total) AS total', |
| | 371 | + 'arfsr_option_id', |
| | 372 | + 'SUM(arfsr_total) AS total', |
| 373 | 373 | 'SUM(arfsr_count) AS count' |
| 374 | 374 | ), |
| 375 | 375 | array( |
| — | — | @@ -387,7 +387,7 @@ |
| 388 | 388 | 'afsr_field_id' => $field, |
| 389 | 389 | 'afsr_option_id' => $row->arfsr_option_id, |
| 390 | 390 | 'afsr_total' => $row->total, |
| 391 | | - 'afsr_count' => $row->count |
| | 391 | + 'afsr_count' => $row->count |
| 392 | 392 | ); |
| 393 | 393 | } |
| 394 | 394 | } else { |
| — | — | @@ -396,7 +396,7 @@ |
| 397 | 397 | $row = $dbr->selectRow( |
| 398 | 398 | 'aft_article_revision_feedback_ratings_rollup', |
| 399 | 399 | array( |
| 400 | | - 'SUM(afrr_total) AS total', |
| | 400 | + 'SUM(afrr_total) AS total', |
| 401 | 401 | 'SUM(afrr_count) AS count' |
| 402 | 402 | ), |
| 403 | 403 | array( |
| — | — | @@ -412,10 +412,10 @@ |
| 413 | 413 | 'arr_page_id' => $pageId, |
| 414 | 414 | 'arr_field_id' => $field, |
| 415 | 415 | 'arr_total' => $row->total, |
| 416 | | - 'arr_count' => $row->count |
| | 416 | + 'arr_count' => $row->count |
| 417 | 417 | ); |
| 418 | 418 | } |
| 419 | | - |
| | 419 | + |
| 420 | 420 | $dbw->begin(); |
| 421 | 421 | // Delete the existing page rollup rows. |
| 422 | 422 | $dbw->delete( $table, array( |
| — | — | @@ -428,7 +428,7 @@ |
| 429 | 429 | $dbw->commit(); |
| 430 | 430 | |
| 431 | 431 | // One way to speed this up would be to purge old rows from |
| 432 | | - // the revision_rollup tables, as soon as they're out of the |
| | 432 | + // the revision_rollup tables, as soon as they're out of the |
| 433 | 433 | // window in which we count them. 30 revisions per page is still |
| 434 | 434 | // a lot, but it'd be better than this, which has no limit and |
| 435 | 435 | // will only get larger over time. |
| — | — | @@ -451,18 +451,23 @@ |
| 452 | 452 | $ip = null; |
| 453 | 453 | |
| 454 | 454 | // Only save IP address if the user isn't logged in. |
| 455 | | - if( !$wgUser->isLoggedIn() ) { |
| 456 | | - $ip = wfGetIP(); |
| | 455 | + if ( !$wgUser->isLoggedIn() ) { |
| | 456 | + $ip = wfGetIP(); |
| 457 | 457 | } |
| 458 | 458 | |
| 459 | 459 | # make sure we have a page/user |
| 460 | 460 | if ( !$params['pageid'] || !$wgUser) { |
| 461 | | - return null; |
| | 461 | + if ( !$feedbackId ) { |
| | 462 | + $this->dieUsage( 'Saving feedback requires a page ID', 'invalidpageid' ); |
| | 463 | + } |
| 462 | 464 | } |
| 463 | 465 | |
| 464 | 466 | # Fetch this if it wasn't passed in |
| 465 | 467 | if ( !$revId ) { |
| 466 | 468 | $title = Title::newFromID( $params['pageid'] ); |
| | 469 | + if ( !$title ) { |
| | 470 | + $this->dieUsage( 'Page ID is invalid', 'invalidpageid' ); |
| | 471 | + } |
| 467 | 472 | $revId = $title->getLatestRevID(); |
| 468 | 473 | } |
| 469 | 474 | |
| — | — | @@ -520,7 +525,7 @@ |
| 521 | 526 | * Gets the anonymous token from the params |
| 522 | 527 | * |
| 523 | 528 | * @param $params array the params |
| 524 | | - * @return string the token, or null if the user is not anonymous |
| | 529 | + * @return string the token, or null if the user is not anonymous |
| 525 | 530 | */ |
| 526 | 531 | public function getAnonToken( $params ) { |
| 527 | 532 | global $wgUser; |
| — | — | @@ -626,6 +631,7 @@ |
| 627 | 632 | array( 'missingparam', 'anontoken' ), |
| 628 | 633 | array( 'code' => 'invalidtoken', 'info' => 'The anontoken is not 32 characters' ), |
| 629 | 634 | array( 'code' => 'invalidpage', 'info' => 'ArticleFeedback is not enabled on this page' ), |
| | 635 | + array( 'code' => 'invalidpageid', 'info' => 'Page ID is missing or invalid' ), |
| 630 | 636 | ) ); |
| 631 | 637 | } |
| 632 | 638 | |