r106733 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r106732‎ | r106733 | r106734 >
Date:00:09, 20 December 2011
Author:gregchiasson
Status:ok (Comments)
Tags:
Comment:
AFTv5 - turns out stuffing a DB result object in mcache was less than successful. Changing it up to be a nested array has better results.
Modified paths:
  • /trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5Utils.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5.php
@@ -56,25 +56,26 @@
5757 );
5858
5959 foreach ( $fields as $field ) {
60 - if ( $field->afi_bucket_id != $bucket ) {
 60+ $field_name = $field->afi_name;
 61+ if ( $field['afi_bucket_id'] != $bucket ) {
6162 continue;
6263 }
63 - if ( isset( $params[$field->afi_name] ) ) {
64 - $value = $params[$field->afi_name];
65 - $type = $field->afi_data_type;
 64+ if ( isset( $params[$field['afi_name']] ) ) {
 65+ $value = $params[$field_name];
 66+ $type = $field['afi_data_type'];
6667 if ( $value === '' ) {
6768 continue;
6869 }
69 - if ( $this->validateParam( $value, $type, $field->afi_id ) ) {
 70+ if ( $this->validateParam( $value, $type, $field['afi_id'] ) ) {
7071 $data = array(
7172 'aa_feedback_id' => $feedbackId,
72 - 'aa_field_id' => $field->afi_id,
 73+ 'aa_field_id' => $field['afi_id'],
7374 );
7475 foreach ( array( 'rating', 'text', 'boolean', 'option_id' ) as $t ) {
7576 $data["aa_response_$t"] = $t == $type ? $value : null;
7677 }
7778 $user_answers[] = $data;
78 - $email_data['ratingData'][$field->afi_name] = $value;
 79+ $email_data['ratingData'][$field_name] = $value;
7980 } else {
8081 $error = 'articlefeedbackv5-error-validation';
8182 }
@@ -577,7 +578,7 @@
578579
579580 $fields = ApiArticleFeedbackv5Utils::getFields();
580581 foreach ( $fields as $field ) {
581 - $ret[$field->afi_name] = array(
 582+ $ret[$field['afi_name']] = array(
582583 ApiBase::PARAM_TYPE => 'string',
583584 ApiBase::PARAM_REQUIRED => false,
584585 ApiBase::PARAM_ISMULTI => false,
@@ -602,7 +603,7 @@
603604 );
604605 $fields = ApiArticleFeedbackv5Utils::getFields();
605606 foreach ( $fields as $f ) {
606 - $ret[$f->afi_name] = 'Optional feedback field, only appears on certain "buckets".';
 607+ $ret[$f['afi_name']] = 'Optional feedback field, only appears on certain "buckets".';
607608 }
608609 return $ret;
609610 }
Index: trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php
@@ -57,12 +57,14 @@
5858
5959 $where['af_page_id'] = $pageId;
6060
61 - // TODO: Not this.
62 - return $dbr->selectField(
63 - array( 'aft_article_feedback' ),
64 - array( 'COUNT(*) AS count' ),
65 - $where
66 - );
 61+ # Until this is done properly, just don't do anything.
 62+ return 0;
 63+
 64+# return $dbr->selectField(
 65+# array( 'aft_article_feedback' ),
 66+# array( 'COUNT(*) AS count' ),
 67+# $where
 68+# );
6769 }
6870
6971 public function fetchFeedback( $pageId,
@@ -184,11 +186,10 @@
185187 }
186188 $rv .= "<p>"
187189 .wfMessage( 'articlefeedbackv5-form-optionid', $record[0]->af_bucket_id )->escaped()
188 - ." | "
 190+ .'</a> '.wfMessage( 'pipe-separator' )->escaped()
189191 ."<a href='#' class='aft5-hide-link' id='aft5-hide-link-$id'>"
190192 .wfMessage( 'articlefeedbackv5-form-hide', $record[0]->af_hide_count )->escaped()
191 - .'</a> | '
192 -//204
 193+ .'</a>'.wfMessage( 'pipe-separator' )->escaped()
193194 ."<a href='#' class='aft5-abuse-link' id='aft5-abuse-link-$id'>"
194195 .wfMessage( 'articlefeedbackv5-form-abuse', $record[0]->af_abuse_count )->escaped()
195196 ."</a></p></div><hr>";
Index: trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5Utils.php
@@ -79,8 +79,9 @@
8080 if( $cached != '' ) {
8181 return $cached;
8282 } else {
83 - $dbr = wfGetDB( DB_SLAVE );
84 - $rv = $dbr->select(
 83+ $rv = array();
 84+ $dbr = wfGetDB( DB_SLAVE );
 85+ $rows = $dbr->select(
8586 'aft_article_field',
8687 array(
8788 'afi_name',
@@ -91,6 +92,16 @@
9293 null,
9394 __METHOD__
9495 );
 96+
 97+ foreach( $rows as $row ) {
 98+ $rv[] = array(
 99+ 'afi_name' => $row->afi_name,
 100+ 'afi_id' => $row->afi_id,
 101+ 'afi_data_type' => $row->afi_data_type,
 102+ 'afi_bucket_id' => $row->afi_bucket_id
 103+ );
 104+ }
 105+
95106 // An hour? That might be reasonable for a cache time.
96107 $wgMemc->set( $key, $rv, 60 * 60 );
97108 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r106741AFTv5 Fix bug introduced in r106733.gregchiasson00:30, 20 December 2011

Comments

#Comment by Catrope (talk | contribs)   12:56, 20 December 2011
+			foreach( $rows as $row ) {
+				$rv[] = array(
+					'afi_name'      => $row->afi_name,
+					'afi_id'        => $row->afi_id, 
+					'afi_data_type' => $row->afi_data_type, 
+					'afi_bucket_id' => $row->afi_bucket_id 
+				);
+			}

You can actually do $rv[] = (array)$row; to convert to an array, or even $rv = iterator_to_array( $rows ); to convert the DB result to an array of row objects (such plain key-value objects are safe to store in memc, it's just resource objects like DB results (which contain references to DB connections and such) that aren't).

Status & tagging log