r109934 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r109933‎ \| r109934 \| r109935 >
Date:	17:30, 24 January 2012
Author:	gregchiasson
Status:	ok
Tags:	aft
Comment:	AFT5 - Fix more button, and sort out the filfters - disable the rating one for now, because it doesn't work right just yet.
Modified paths:	/trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.i18n.php (modified) (history) /trunk/extensions/ArticleFeedbackv5/SpecialArticleFeedbackv5.php (modified) (history) /trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.i18n.php
—	—	@@ -54,12 +54,15 @@
55	55	'articlefeedbackv5-form-helpful-votes' => '{{PLURAL:$1\|1 answer\|$1 answers}} ({{PLURAL:$2\|1 yes\|$2 yes}}, {{PLURAL:$3\|1 no\|$3 no}})',
56	56	'articlefeedbackv5-special-add-feedback' => 'Add your feedback',
57	57	'articlefeedbackv5-special-filter-all' => 'All ($1)',
	58	+ 'articlefeedbackv5-special-filter-almostall' => 'All ($1)',
	59	+ 'articlefeedbackv5-special-filter-notall' => 'All ($1)',
58	60	'articlefeedbackv5-special-filter-comment' => 'Comments only ($1)',
59		~~- 'articlefeedbackv5-special-filter-deleted' => 'Deleted ($1)',~~
60	61	'articlefeedbackv5-special-filter-abusive' => 'Abusive ($1)',
61	62	'articlefeedbackv5-special-filter-helpful' => 'Helpful ($1)',
	63	+ 'articlefeedbackv5-special-filter-unhelpful' => 'Unhelpful ($1)',
62	64	'articlefeedbackv5-special-filter-visible' => 'Visible ($1)',
63	65	'articlefeedbackv5-special-filter-invisible' => 'Hidden ($1)',
	66	+ 'articlefeedbackv5-special-filter-deleted' => 'Deleted ($1)',
64	67	'articlefeedbackv5-special-sort-asc' => '^',
65	68	'articlefeedbackv5-special-sort-desc' => 'v',
66	69	'articlefeedbackv5-special-sort-age' => 'Date',
Index: trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php
—	—	@@ -14,6 +14,8 @@
15	15	* @subpackage Api
16	16	*/
17	17	class ApiViewFeedbackArticleFeedbackv5 extends ApiQueryBase {
	18	+ private $continue;
	19	+
18	20	/**
19	21	* Constructor
20	22	*/
—	—	@@ -45,11 +47,9 @@
46	48	$html .= $this->renderFeedback( $record );
47	49	$length++;
48	50	}
49		~~- $tmp = end($feedback);~~
50		~~- if( $tmp ) {~~
51		~~- $continue = $tmp[0]->af_id;~~
52		~~- }~~
53	51
	52	+ $continue = $this->continue;
	53	+
54	54	$result->addValue( $this->getModuleName(), 'length', $length );
55	55	$result->addValue( $this->getModuleName(), 'count', $count );
56	56	$result->addValue( $this->getModuleName(), 'feedback', $html );
—	—	@@ -84,26 +84,30 @@
85	85	$direction = strtolower( $sortOrder ) == 'asc' ? 'ASC' : 'DESC';
86	86	$continueDirection = ( $direction == 'ASC' ? '>' : '<' );
87	87	$order;
88		~~- $continue;~~
	88	+ $continueSql;
89	89	$sortField;
90	90
91	91	switch( $sort ) {
92	92	case 'helpful':
93		~~- $sortField = 'net_helpfulness';~~
	93	+ $sortField = 'net_helpfulness';
	94	+ // Can't use aliases in mysql where clauses.
	95	+ $continueSql = "CONVERT(af_helpful_count, SIGNED) - CONVERT(af_unhelpful_count, SIGNED) $continueDirection";
94	96	break;
95	97	case 'rating':
96		~~- $sortField = 'rating';~~
97		~~- break;~~
	98	+# disable because it's broken
	99	+# $sortField = 'rating';
	100	+# break;
98	101	case 'age':
99	102	# Default field, fall through
100	103	default:
101		~~- $sortField = 'af_id';~~
	104	+ $sortField = 'af_id';
	105	+ $continueSql = "$sortField $continueDirection";
102	106	break;
103	107	}
104	108	$order = "$sortField $direction";
105		~~- $continueSql = "$sortField $continueDirection";~~
106	109
107	110	$where['af_page_id'] = $pageId;
	111	+
108	112	# This join is needed for the comment filter.
109	113	$where[] = 'af_id = aa_feedback_id';
110	114
—	—	@@ -135,6 +139,7 @@
136	140
137	141	foreach ( $id_query as $id ) {
138	142	$ids[] = $id->af_id;
	143	+ $this->continue = $id->$sortField;
139	144	}
140	145
141	146	if ( !count( $ids ) ) {
—	—	@@ -191,27 +196,6 @@
192	197	return $rv;
193	198	}
194	199
195		~~- private function getContinue( $sort, $sortOrder ) {~~
196		~~- $continue;~~
197		~~- $direction = strtolower( $sortOrder ) == 'asc' ? '<' : '>';~~
198		-
199		~~- switch( $sort ) {~~
200		~~- case 'helpful':~~
201		~~- $continue = 'net_helpfulness <';~~
202		~~- break;~~
203		~~- case 'rating':~~
204		~~- # For now, just fall through. Not specced out.~~
205		~~- break;~~
206		~~- case 'age':~~
207		~~- # Default sort is by age.~~
208		~~- default:~~
209		~~- $continue = 'af_id >';~~
210		~~- break;~~
211		~~- }~~
212		-
213		~~- return $continue;~~
214		~~- }~~
215		-
216	200	private function getFilterCriteria( $filter, $filterValue = null ) {
217	201	global $wgUser;
218	202	$where = array();
—	—	@@ -229,33 +213,53 @@
230	214
231	215	// Always limit this to non-hidden records, unless they
232	216	// specifically ask to see them.
233		~~- if( $filter != 'invisible' ) {~~
234		~~- $where[ 'af_hide_count' ] = 0;~~
	217	+ if( in_array( $filter, array( 'invisible', 'all', 'almostall' ) ) ) {
	218	+ $where['af_hide_count'] = 0;
235	219	}
236	220
237	221	// Same, but for deleted/supressed/oversight-only records.
238		~~- if( $filter != 'deleted' ) {~~
239		~~- $where[ 'af_delete_count' ] = 0;~~
	222	+ if( in_array( $filter, array( 'deleted', 'all' ) ) ) {
	223	+ $where['af_delete_count'] = 0;
240	224	}
241	225
242	226	switch( $filter ) {
	227	+ case 'id':
	228	+ # Used for permalinks.
	229	+ $where[ 'af_id' ] = $filterValue;
	230	+ break;
243	231	case 'all':
244		~~- # no op~~
	232	+ # oversight, real 'all' - no filtering done
245	233	break;
	234	+ case 'almostall':
	235	+ # non-oversight 'all' - no deleted feedback
	236	+ $where['af_delete_count'] = 0;
	237	+ break;
	238	+ case 'notall':
	239	+ # non-moderator 'all' - no hidden/deleted
	240	+ $where['af_delete_count'] = 0;
	241	+ $where['af_hide_count'] = 0;
	242	+ break;
	243	+ case 'visible':
	244	+ $where['af_delete_count'] = 0;
	245	+ $where['af_hide_count'] = 0;
	246	+ break;
246	247	case 'invisible':
247	248	$where[] = 'af_hide_count > 0';
248	249	break;
	250	+ case 'abusive':
	251	+ $where[] = 'af_abuse_count > 0';
	252	+ break;
	253	+ case 'helpful':
	254	+ $where[] = 'net_helpfulness > 0';
	255	+ break;
	256	+ case 'unhelpful':
	257	+ $where[] = 'net_helpfulness <= 0';
	258	+ break;
249	259	case 'comment':
250	260	$where[] = 'aa_response_text IS NOT NULL';
251	261	break;
252		~~- case 'id':~~
253		~~- $where[ 'af_id' ] = $filterValue;~~
254		~~- break;~~
255		~~- case 'visible':~~
256		~~- $where[ 'af_hide_count' ] = 0;~~
257		~~- break;~~
258	262	case 'deleted':
259		~~- $where[ 'af_delete_count' ] = 0;~~
	263	+ $where[] = 'af_delete_count > 0';
260	264	break;
261	265	default:
262	266	break;
—	—	@@ -529,7 +533,7 @@
530	534	ApiBase::PARAM_REQUIRED => false,
531	535	ApiBase::PARAM_ISMULTI => false,
532	536	ApiBase::PARAM_TYPE => array(
533		~~- 'all', 'invisible', 'visible', 'comment', 'id' )~~
	537	+ 'all', 'invisible', 'visible', 'comment', 'id', 'helpful', 'unhelpful', 'abusive', 'almostall', 'notall' )
534	538	),
535	539	'filtervalue' => array(
536	540	ApiBase::PARAM_REQUIRED => false,
Index: trunk/extensions/ArticleFeedbackv5/SpecialArticleFeedbackv5.php
—	—	@@ -17,7 +17,7 @@
18	18	class SpecialArticleFeedbackv5 extends SpecialPage {
19	19	private $filters = array(
20	20	'comment',
21		~~- 'visible'~~
	21	+ 'helpful'
22	22	);
23	23	private $sorts = array(
24	24	'age',
—	—	@@ -33,13 +33,29 @@
34	34	parent::__construct( 'ArticleFeedbackv5' );
35	35
36	36	if( $wgUser->isAllowed( 'aftv5-see-hidden-feedback' ) ) {
37		~~- $this->filters[] = 'invisible';~~
38		~~- $this->filters[] = 'all';~~
	37	+ array_push( $this->filters,
	38	+ 'invisible', 'unhelpful', 'abusive'
	39	+ );
39	40	}
40	41
41	42	if( $wgUser->isAllowed( 'aftv5-see-deleted-feedback' ) ) {
42		~~- $this->filters[] = 'deleted';~~
	43	+ array_push( $this->filters, 'deleted', 'all' );
43	44	}
	45	+
	46	+ // The 'all' option actually displays different things based
	47	+ // on the users role, which is why we do this.
	48	+ // - deleted-all is actually everything
	49	+ // - hidden-all is 'visible + hidden'
	50	+ // - regular non-admin all is just 'all visible'
	51	+ if( !$wgUser->isAllowed( 'aftv5-see-deleted-feedback' )
	52	+ && $wgUser->isAllowed( 'aftv5-see-hidden-feedback' ) ) {
	53	+ $this->filters[] = 'almostall';
	54	+ }
	55	+
	56	+ if( !$wgUser->isAllowed( 'aftv5-see-deleted-feedback' )
	57	+ && !$wgUser->isAllowed( 'aftv5-see-hidden-feedback' ) ) {
	58	+ $this->filters[] = 'notall';
	59	+ }
44	60	}
45	61
46	62	/**
—	—	@@ -180,7 +196,7 @@
181	197	$opts = array();
182	198	$counts = $this->getFilterCounts( $pageId );
183	199	foreach( $this->filters as $filter ) {
184		~~- $count = isset($counts[$filter]) ? $counts[$filter] : 0;~~
	200	+ $count = array_key_exists( $filter, $counts ) ? $counts[$filter] : 0;
185	201	$key = $this->msg( 'articlefeedbackv5-special-filter-'.$filter, $count )->escaped();
186	202	$opts[ (string) $key ] = $filter;
187	203	}

Status & tagging log

01:28, 13 June 2012 Krinkle (talk | contribs) changed the tags for r109934 [removed: nodeploy]
06:39, 31 January 2012 Catrope (talk | contribs) changed the status of r109934 [removed: new added: ok]
23:31, 26 January 2012 MarkAHershberger (talk | contribs) changed the tags for r109934 [added: nodeploy]
18:52, 24 January 2012 MarkAHershberger (talk | contribs) changed the tags for r109934 [added: aft]