r104609 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r104608‎ \| r104609 \| r104610 >
Date:	22:58, 29 November 2011
Author:	gregchiasson
Status:	resolved (Comments)
Tags:
Comment:	First (really barebones and hacky) go at AFTv5 feedback page.
Modified paths:	/trunk/extensions/ArticleFeedbackv5/SpecialArticleFeedbackv5.php (modified) (history) /trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5.php (modified) (history) /trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php (added) (history) /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.js
—	—	@@ -3,8 +3,7 @@
4	4
5	5	// TODO: Pass this in from the PHP side. Add it to mwConfig or w/e?
6	6	//$.articleFeedbackv5special.page = mw.config.get( 'wgPageId' );
7		~~- $.articleFeedbackv5special.page = 0;~~
8		~~-console.log('oage id is ' + hackPageId);~~
	7	+ $.articleFeedbackv5special.page = hackPageId;
9	8	$.articleFeedbackv5special.filter = 'visible';
10	9	$.articleFeedbackv5special.sort = 'newest';
11	10	$.articleFeedbackv5special.limit = 5;
Index: trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5.php
—	—	@@ -178,7 +178,7 @@
179	179	if ( $type == 'select' ) {
180	180	$page_prefix = 'afsr_';
181	181	$rev_prefix = 'arfsr_';
182		~~- $select = array( 'aa_field_id', 'aa_response_option_id', 'COUNT(aa_response_option_id) AS earned' );~~
	182	+ $select = array( 'aa_field_id', 'aa_response_option_id', 'COUNT(aa_response_option_id) AS earned', '0 AS submits' );
183	183	$group = array( 'GROUP BY' => 'aa_response_option_id' );
184	184	} else {
185	185	$page_prefix = 'arr_';
—	—	@@ -189,7 +189,11 @@
190	190	}
191	191
192	192	$rows = $dbr->select(
193		~~- array( 'aft_article_answer', 'aft_article_feedback', 'aft_article_field' ),~~
	193	+ array(
	194	+ 'aft_article_answer',
	195	+ 'aft_article_feedback',
	196	+ 'aft_article_field'
	197	+ ),
194	198	$select,
195	199	array(
196	200	'afi_data_type' => $type,
—	—	@@ -202,14 +206,34 @@
203	207	$group
204	208	);
205	209
	210	+ # Fake the select counts, because we want to group by ughhh
	211	+ $totals = array();
	212	+ foreach ( $rows as $row ) {
	213	+ if( !array_key_exists( $row->aa_field_id, $totals ) ) {
	214	+ $totals[$row->aa_field_id] = 0;
	215	+ }
	216	+ $totals[$row->aa_field_id] += $row->earned;
	217	+ }
206	218
207	219	foreach ( $rows as $row ) {
208		~~- if ( !array_key_exists( $row->aa_field_id, $page_data ) ) {~~
209		~~- $page_data[$row->aa_field_id] = array(~~
	220	+ if( $type == 'select' ) {
	221	+ $key = $row->aa_response_option_id;
	222	+ $field = 'option_id';
	223	+ $value = $row->aa_response_option_id;
	224	+ $count = $totals[$row->aa_field_id];
	225	+ } else {
	226	+ $key = $row->aa_field_id;
	227	+ $field = 'rating_id';
	228	+ $value = $row->aa_field_id;
	229	+ $count = $row->submits;
	230	+ }
	231	+
	232	+ if ( !array_key_exists( $key, $page_data ) ) {
	233	+ $page_data[$key] = array(
210	234	$page_prefix . 'page_id' => $pageId,
211	235	$page_prefix . 'total' => 0,
212	236	$page_prefix . 'count' => 0,
213		~~- $page_prefix . ($type == 'select' ? 'option' : 'rating') . '_id' => $row->aa_field_id~~
	237	+ $page_prefix . $field => $value
214	238	);
215	239	}
216	240
—	—	@@ -217,11 +241,12 @@
218	242	$rev_prefix . 'page_id' => $pageId,
219	243	$rev_prefix . 'revision_id' => $revId,
220	244	$rev_prefix . 'total' => $row->earned,
221		~~- $rev_prefix . 'count' => $row->submits,~~
222		~~- $rev_prefix . ($type == 'select' ? 'option' : 'rating') . '_id' => $row->aa_field_id~~
	245	+ $rev_prefix . 'count' => $count,
	246	+ $rev_prefix . $field => $value
223	247	);
224		~~- $page_data[$row->aa_field_id][$page_prefix.'total'] += $row->earned;~~
225		~~- $page_data[$row->aa_field_id][$page_prefix.'count'] += $row->submits;~~
	248	+
	249	+ $page_data[$key][$page_prefix . 'total'] += $row->earned;
	250	+ $page_data[$key][$page_prefix . 'count'] += $count;
226	251	}
227	252
228	253	if ( count( $page_data ) < 1 ) {
Index: trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php
—	—	@@ -0,0 +1,316 @@
	2	+<?php
	3	+/**
	4	+ * ApiViewFeedbackArticleFeedbackv5 class
	5	+ *
	6	+ * @package ArticleFeedback
	7	+ * @subpackage Api
	8	+ * @author Greg Chiasson <greg@omniti.com>
	9	+ */
	10	+
	11	+/**
	12	+ * This class pulls the individual ratings/comments for the feedback page.
	13	+ *
	14	+ * @package ArticleFeedback
	15	+ * @subpackage Api
	16	+ */
	17	+class ApiViewFeedbackArticleFeedbackv5 extends ApiQueryBase {
	18	+
	19	+ /**
	20	+ * Constructor
	21	+ */
	22	+ public function __construct( $query, $moduleName ) {
	23	+ parent::__construct( $query, $moduleName, 'af' );
	24	+ }
	25	+
	26	+ /**
	27	+ * Execute the API call: Pull the requested feedback
	28	+ */
	29	+ public function execute() {
	30	+ $params = $this->extractRequestParams();
	31	+ #error_log(print_r($params,1));
	32	+ $html = '';
	33	+ $result = $this->getResult();
	34	+ $path = array( 'query', $this->getModuleName() );
	35	+ $pageId = $params['pageid'];
	36	+ $feedback = $this->fetchFeedback(
	37	+ $params['pageid'],
	38	+ $params['filter'],
	39	+ $params['sort'],
	40	+ $params['limit'],
	41	+ $params['offset']
	42	+ );
	43	+
	44	+ foreach ( $feedback as $record ) {
	45	+ $html .= $this->renderFeedback($record);
	46	+ }
	47	+
	48	+ $result->addValue( $path, 'feedback', $html );
	49	+
	50	+ $result->setIndexedTagName_internal(
	51	+ array( 'query', $this->getModuleName() ), 'aa'
	52	+ );
	53	+
	54	+ }
	55	+
	56	+ # TODO: When there's an overall rating field, use that.
	57	+ # Using "completeness" for now, because I needed something to test with.
	58	+ public function fetchOverallRating( $pageId ) {
	59	+ $rv = array();
	60	+ $dbr = wfGetDB( DB_SLAVE );
	61	+ $rows = $dbr->select(
	62	+ array( 'aft_article_feedback_ratings_rollup',
	63	+ 'aft_article_field' ),
	64	+ array( 'arr_total / arr_count AS rating',
	65	+ 'afi_name'
	66	+ ),
	67	+ array( 'arr_page_id' => $pageId,
	68	+ 'arr_rating_id = afi_id',
	69	+ "afi_name IN ('found', 'complete')"
	70	+ )
	71	+ );
	72	+
	73	+ foreach( $rows as $row ) {
	74	+ if( $row->afi_name == 'found' ) {
	75	+ $rv['found'] = ( int ) ( 100 * $row->rating );
	76	+ } elseif( $row->afi_name == 'complete' ) {
	77	+ # Or should this be round/ceil/floor/float?
	78	+ $rv['rating'] = ( int ) $row->rating;
	79	+ }
	80	+ }
	81	+
	82	+ return $rv;
	83	+ }
	84	+
	85	+
	86	+ public function fetchFeedback( $pageId,
	87	+ $filter = 'visible', $order = 'newest', $limit = 5, $offset = 0 ) {
	88	+ $dbr = wfGetDB( DB_SLAVE );
	89	+ $ids = array();
	90	+ $rows = array();
	91	+ $rv = array();
	92	+ $order;
	93	+ $where;
	94	+
	95	+ switch($order) {
	96	+ case 'newest':
	97	+ $order = 'af_id DESC';
	98	+ break;
	99	+ default:
	100	+ $order = 'af_id DESC';
	101	+ break;
	102	+ }
	103	+
	104	+ switch($filter) {
	105	+ case 'all':
	106	+ $where = array();
	107	+ break;
	108	+ case 'visible':
	109	+# TODO: add the column to control how a thing is hidden.
	110	+# $where = array(
	111	+# break;
	112	+ default:
	113	+ $where = array();
	114	+ break;
	115	+ }
	116	+
	117	+ $where['af_page_id'] = $pageId;
	118	+
	119	+ /* I'd really love to do this in one big query, but MySQL
	120	+ doesn't support LIMIT inside IN() subselects, and since
	121	+ we don't know the number of answers for each feedback
	122	+ record until we fetch them, this is the only way to make
	123	+ sure we get all answers for the exact IDs we want. */
	124	+ $id_query = $dbr->select(
	125	+ 'aft_article_feedback', 'af_id', $where, __METHOD__,
	126	+ array(
	127	+ 'LIMIT' => $limit,
	128	+ 'OFFSET' => $offset,
	129	+ 'ORDER BY' => $order
	130	+ )
	131	+ );
	132	+ foreach($id_query as $id) {
	133	+ $ids[] = $id->af_id;
	134	+ }
	135	+
	136	+ $rows = $dbr->select(
	137	+ array( 'aft_article_feedback', 'aft_article_answer',
	138	+ 'aft_article_field' ),
	139	+ array( 'af_id', 'af_bucket_id', 'afi_name',
	140	+ 'aa_response_text', 'aa_response_boolean',
	141	+ 'aa_response_rating', 'aa_response_option_id',
	142	+ 'afi_data_type', 'af_created', 'af_user_text'
	143	+ ),
	144	+ array( 'af_id' => $ids ),
	145	+ __METHOD__,
	146	+ array( 'ORDER BY' => $order ),
	147	+ array(
	148	+ 'aft_article_answer' => array(
	149	+ 'LEFT JOIN', 'af_id = aa_feedback_id'
	150	+ ),
	151	+ 'aft_article_field' => array(
	152	+ 'JOIN', 'afi_id = aa_field_id'
	153	+ ),
	154	+ )
	155	+ );
	156	+
	157	+ foreach($rows as $row) {
	158	+ if(!array_key_exists($row->af_id, $rv)) {
	159	+ $rv[$row->af_id] = array();
	160	+ $rv[$row->af_id][0] = $row;
	161	+ }
	162	+ $rv[$row->af_id][$row->afi_name] = $row;
	163	+ }
	164	+
	165	+ return $rv;
	166	+ }
	167	+
	168	+ protected function renderFeedback( $record ) {
	169	+ $rv = "<div class='aft5-feedback'>"
	170	+ ."<p>Feedback #".$record[0]->af_id
	171	+ .', @'.$record[0]->af_created.'</p>';
	172	+ switch( $record[0]->af_bucket_id ) {
	173	+ case 1: $rv .= $this->renderBucket1( $record ); break;
	174	+ case 2: $rv .= $this->renderBucket2( $record ); break;
	175	+ case 3: $rv .= $this->renderBucket3( $record ); break;
	176	+ case 4: $rv .= $this->renderBucket4( $record ); break;
	177	+ case 5: $rv .= $this->renderBucket5( $record ); break;
	178	+ case 6: $rv .= $this->renderBucket6( $record ); break;
	179	+ default: return 'Invalid bucket id';
	180	+ }
	181	+ $rv .= "<p>
	182	+<!--
	183	+ <a href=''>Hide this</a>
	184	+ <a href=''>Flag as abuse</a>
	185	+-->
	186	+ </p>
	187	+ </div><hr>";
	188	+ return $rv;
	189	+ }
	190	+
	191	+ private function renderBucket1( $record ) {
	192	+ $name = $record[0]->af_user_text;
	193	+ $found = $record['found']->aa_response_boolean ? 'found'
	194	+ : 'did not find';
	195	+ return "$name $found what they were looking for:"
	196	+ .'<blockquote>'.$record['comment']->aa_response_text
	197	+ .'</blockquote>';
	198	+ }
	199	+
	200	+ private function renderBucket2( $record ) { }
	201	+ private function renderBucket3( $record ) { }
	202	+ private function renderBucket4( $record ) { }
	203	+
	204	+ private function renderBucket5( $record ) {
	205	+ $name = $record[0]->af_user_text;
	206	+ $rv = "$name had this to say about robocop:<ul>";
	207	+ foreach( $record as $answer ) {
	208	+ if( $answer->afi_data_type == 'rating' ) {
	209	+ $rv .= "<li>".$answer->afi_name.': '.$answer->aa_response_rating."</li>";
	210	+ }
	211	+ }
	212	+ $rv .= "</ul>";
	213	+
	214	+ return $rv;
	215	+ }
	216	+
	217	+ private function renderBucket6( $record ) {
	218	+ return 'User was presented with the CTA-only form.';
	219	+ }
	220	+
	221	+ /**
	222	+ * Gets the allowed parameters
	223	+ *
	224	+ * @return array the params info, indexed by allowed key
	225	+ */
	226	+ public function getAllowedParams() {
	227	+ return array(
	228	+ 'pageid' => array(
	229	+ ApiBase::PARAM_REQUIRED => true,
	230	+ ApiBase::PARAM_ISMULTI => false,
	231	+ ApiBase::PARAM_TYPE => 'integer'
	232	+ ),
	233	+ 'sort' => array(
	234	+ ApiBase::PARAM_REQUIRED => false,
	235	+ ApiBase::PARAM_ISMULTI => false,
	236	+ ApiBase::PARAM_TYPE => array(
	237	+ 'oldest', 'newest', 'etc' )
	238	+ ),
	239	+ 'filter' => array(
	240	+ ApiBase::PARAM_REQUIRED => false,
	241	+ ApiBase::PARAM_ISMULTI => false,
	242	+ ApiBase::PARAM_TYPE => array(
	243	+ 'all', 'hidden', 'visible' )
	244	+ ),
	245	+ 'limit' => array(
	246	+ ApiBase::PARAM_REQUIRED => false,
	247	+ ApiBase::PARAM_ISMULTI => false,
	248	+ ApiBase::PARAM_TYPE => 'integer'
	249	+ ),
	250	+ 'offset' => array(
	251	+ ApiBase::PARAM_REQUIRED => false,
	252	+ ApiBase::PARAM_ISMULTI => false,
	253	+ ApiBase::PARAM_TYPE => 'integer'
	254	+ ),
	255	+ );
	256	+ }
	257	+
	258	+ /**
	259	+ * Gets the parameter descriptions
	260	+ *
	261	+ * @return array the descriptions, indexed by allowed key
	262	+ */
	263	+ public function getParamDescription() {
	264	+ return array(
	265	+ 'pageid' => 'Page ID to get feedback ratings for',
	266	+ 'sort' => 'Key to sort records by',
	267	+ 'filter' => 'What filtering to apply to list',
	268	+ 'limit' => 'Number of records to show',
	269	+ 'offset' => 'How many to skip (for pagination)',
	270	+ );
	271	+ }
	272	+
	273	+ /**
	274	+ * Gets the api descriptions
	275	+ *
	276	+ * @return array the description as the first element in an array
	277	+ */
	278	+ public function getDescription() {
	279	+ return array(
	280	+ 'List article feedback for a specified page'
	281	+ );
	282	+ }
	283	+
	284	+ /**
	285	+ * Gets any possible errors
	286	+ *
	287	+ * @return array the errors
	288	+ */
	289	+ public function getPossibleErrors() {
	290	+ return array_merge( parent::getPossibleErrors(), array(
	291	+ array( 'missingparam', 'anontoken' ),
	292	+ array( 'code' => 'invalidtoken', 'info' => 'The anontoken is not 32 characters' ),
	293	+ )
	294	+ );
	295	+ }
	296	+
	297	+ /**
	298	+ * Gets an example
	299	+ *
	300	+ * @return array the example as the first element in an array
	301	+ */
	302	+ protected function getExamples() {
	303	+ return array(
	304	+ 'api.php?action=query&list=articlefeedbackv5-view-feedback&afpageid=1',
	305	+ );
	306	+ }
	307	+
	308	+ /**
	309	+ * Gets the version info
	310	+ *
	311	+ * @return string the SVN version info
	312	+ */
	313	+ public function getVersion() {
	314	+ return __CLASS__ . ': $Id: ApiViewRatingsArticleFeedbackv5.php 103439 2011-11-17 03:19:01Z rsterbin $';
	315	+ }
	316	+
	317	+}
Property changes on: trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php
___________________________________________________________________
Added: svn:eol-style
1	318	+ native
Index: trunk/extensions/ArticleFeedbackv5/SpecialArticleFeedbackv5.php
—	—	@@ -39,21 +39,6 @@
40	40
41	41	$output .= "\n== Feedback ==\n";
42	42
43		-/*
44		~~- $feedback = $this->api->fetchFeedback( $pageId );~~
45		~~- foreach($feedback as $record) {~~
46		~~- # add a table or something~~
47		~~- $output .= " \n record: ".$record[0]->af_id;~~
48		~~- foreach($record as $answer) {~~
49		~~- $func = 'aa_response_'.$answer->afi_data_type;~~
50		~~- $output .= " \n ".$answer->afi_name.': '~~
51		~~- .$answer->$func;~~
52		~~- }~~
53		~~- $output .= " \n ----------- ";~~
54		-
55		~~- }~~
56		~~-*/~~
57		-
58	43	$wgOut->addWikiText( $output );
59	44
60	45	$wgOut->addHTML(<<<EOH

Comments

#Comment by Johnduhart (talk | contribs) 14:06, 2 December 2011

+	/**
+	 * Gets an example
+	 *
+	 * @return array the example as the first element in an array
+	 */
+	protected function getExamples() {
+		return array(
+			'api.php?action=query&list=articlefeedbackv5-view-feedback&afpageid=1',
+		);
+	}

Should be public.

#Comment by Gregchiasson (talk | contribs) 01:08, 9 December 2011

Fixed in r105014

#Comment by P858snake (talk | contribs) 06:14, 10 December 2011

Can someone un-fixme and manually add the follow up?

Status & tagging log

22:50, 13 December 2011 Catrope (talk | contribs) changed the status of r104609 [removed: fixme added: resolved]
14:06, 2 December 2011 Johnduhart (talk | contribs) changed the status of r104609 [removed: new added: fixme]