r44491 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r44490‎ \| r44491 \| r44492 >
Date:	03:48, 12 December 2008
Author:	aaron
Status:	deferred
Tags:
Comment:	* Allow partial reviews * Remove 'overall' aggregate (some issues with it)
Modified paths:	/trunk/extensions/FlaggedRevs/FlaggedRevs.php (modified) (history) /trunk/extensions/FlaggedRevs/FlaggedRevsXML.php (modified) (history) /trunk/extensions/FlaggedRevs/flaggedrevs.js (modified) (history) /trunk/extensions/FlaggedRevs/language/FlaggedRevs.i18n.php (modified) (history) /trunk/extensions/FlaggedRevs/specialpages/LikedPages_body.php (modified) (history) /trunk/extensions/FlaggedRevs/specialpages/ProblemPages_body.php (modified) (history) /trunk/extensions/FlaggedRevs/specialpages/ReaderFeedback_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/FlaggedRevs/FlaggedRevs.php
—	—	@@ -262,7 +262,7 @@
263	263	#########
264	264
265	265	# Bump this number every time you change flaggedrevs.css/flaggedrevs.js
266		~~-$wgFlaggedRevStyleVersion = 45;~~
	266	+$wgFlaggedRevStyleVersion = 46;
267	267
268	268	$wgExtensionFunctions[] = 'efLoadFlaggedRevs';
269	269
Index: trunk/extensions/FlaggedRevs/language/FlaggedRevs.i18n.php
—	—	@@ -174,7 +174,7 @@
175	175	'readerfeedback-npov' => 'Neutrality',
176	176	'readerfeedback-presentation' => 'Presentation',
177	177	'readerfeedback-overall' => 'Overall',
178		~~- 'readerfeedback-level-none' => '(select)',~~
	178	+ 'readerfeedback-level-none' => '(unsure)',
179	179	'readerfeedback-level-0' => 'Poor',
180	180	'readerfeedback-level-1' => 'Low',
181	181	'readerfeedback-level-2' => 'Fair',
Index: trunk/extensions/FlaggedRevs/specialpages/ProblemPages_body.php
—	—	@@ -55,8 +55,9 @@
56	56
57	57	protected function showPageList() {
58	58	global $wgOut;
	59	+ $tags = FlaggedRevs::getFeedbackTags();
59	60	$pager = new ProblemPagesPager( $this, array(), $this->namespace, $this->tag );
60		~~- if( $pager->getNumRows() ) {~~
	61	+ if( isset($tags[$this->tag]) && $pager->getNumRows() ) {
61	62	$wgOut->addHTML( wfMsgExt('problempages-list', array('parse') ) );
62	63	$wgOut->addHTML( $pager->getNavigationBar() );
63	64	$wgOut->addHTML( $pager->getBody() );
—	—	@@ -118,11 +119,6 @@
119	120	$conds['page_namespace'] = $this->namespace;
120	121	// Has to be bad enough
121	122	$x = 2;
122		~~- if( $this->tag == 'overall' ) {~~
123		~~- global $wgFlaggedRevsFeedbackTags;~~
124		~~- $s = $x*array_sum($wgFlaggedRevsFeedbackTags);~~
125		~~- $x = intval( floor($s/count($wgFlaggedRevsFeedbackTags)) );~~
126		~~- }~~
127	123	$conds[] = "rfp_ave_val < $x";
128	124	// Reasonable sample
129	125	$conds[] = 'rfp_count >= '.READER_FEEDBACK_SIZE;
Index: trunk/extensions/FlaggedRevs/specialpages/ReaderFeedback_body.php
—	—	@@ -49,13 +49,21 @@
50	50	}
51	51	# Get our rating dimensions
52	52	$this->dims = array();
	53	+ $unsureCount = 0;
53	54	foreach( FlaggedRevs::getFeedbackTags() as $tag => $weight ) {
54	55	$this->dims[$tag] = $wgRequest->getIntOrNull( "wp$tag" );
55		~~- if( !self::isValid($this->dims[$tag]) ) {~~
	56	+ if( $this->dims[$tag] === NULL ) { // nothing sent at all :(
56	57	$wgOut->redirect( $this->page->getLocalUrl() );
57	58	return;
	59	+ } else if( $this->dims[$tags] === -1 ) {
	60	+ $unsureCount++;
58	61	}
59	62	}
	63	+ # There must actually be some ratings
	64	+ if( $unsureCount >= count($this->dims) ) {
	65	+ $wgOut->redirect( $this->page->getLocalUrl() );
	66	+ return;
	67	+ }
60	68	# Check validation key
61	69	$this->validatedParams = $wgRequest->getVal('validatedParams');
62	70	if( $this->validatedParams != self::validationKey( $this->oldid, $wgUser->getId() ) ) {
—	—	@@ -96,6 +104,7 @@
97	105	// Make review interface object
98	106	$form = new ReaderFeedback();
99	107	$form->dims = array();
	108	+ $unsureCount = 0;
100	109	$bot = false;
101	110	// Each ajax url argument is of the form param\|val.
102	111	// This means that there is no ugly order dependance.
—	—	@@ -135,15 +144,17 @@
136	145	$p = preg_replace( '/^wp/', '', $par ); // kill any "wp" prefix
137	146	if( array_key_exists( $p, $tags ) ) {
138	147	$form->dims[$p] = intval($val);
139		~~- if( !self::isValid( $form->dims[$p] ) ) {~~
	148	+ if( $form->dims[$p] === NULL ) { // nothing sent at all :(
140	149	return '<err#>' . wfMsg('formerror'); // bad range
	150	+ } else if( $form->dims[$p] === -1 ) {
	151	+ $unsureCount++;
141	152	}
142	153	}
143	154	break;
144	155	}
145	156	}
146	157	// Missing params?
147		~~- if( count($form->dims) != count($tags) ) {~~
	158	+ if( count($form->dims) != count($tags) \|\| $unsureCount >= count($form->dims) ) {
148	159	return '<err#>' . wfMsg('formerror');
149	160	}
150	161	// Doesn't match up?
—	—	@@ -240,6 +251,8 @@
241	252	$dbw = wfGetDB( DB_MASTER );
242	253	# Get date timestamp...
243	254	$date = str_pad( substr( wfTimestampNow(), 0, 8 ), 14, '0' );
	255	+ if( count($this->dims) == 0 )
	256	+ return false;
244	257	$ratings = $this->flattenRatings( $this->dims );
245	258	# Make sure revision is valid!
246	259	$rev = Revision::newFromId( $this->oldid );
—	—	@@ -259,10 +272,10 @@
260	273	'rfb_timestamp' => $dbw->timestamp(),
261	274	'rfb_ratings' => $ratings
262	275	);
263		~~- $dbw->insert( 'reader_feedback', $insertRow, __METHOD__, 'IGNORE' );~~
264	276	# Make sure initial page data is there to begin with...
265	277	$insertRows = array();
266	278	foreach( $this->dims as $tag => $val ) {
	279	+ if( $val < 0 ); // don't store "unsure" votes
267	280	$insertRows[] = array(
268	281	'rfh_page_id' => $rev->getPage(),
269	282	'rfh_tag' => $tag,
—	—	@@ -271,19 +284,23 @@
272	285	'rfh_date' => $date
273	286	);
274	287	}
	288	+ $dbw->insert( 'reader_feedback', $insertRow, __METHOD__, 'IGNORE' );
275	289	$dbw->insert( 'reader_feedback_history', $insertRows, __METHOD__, 'IGNORE' );
276	290	# Update aggregate data for this page over time...
277	291	$touched = $dbw->timestamp( wfTimestampNow() );
278	292	$overall = 0;
279	293	$insertRows = array();
280	294	foreach( $this->dims as $tag => $val ) {
	295	+ if( $val < 0 ) continue; // don't store "unsure" votes
	296	+ # Update daily averages
281	297	$dbw->update( 'reader_feedback_history',
282	298	array( 'rfh_total = rfh_total + '.intval($val),
283	299	'rfh_count = rfh_count + 1'),
284	300	array( 'rfh_page_id' => $rev->getPage(),
285	301	'rfh_tag' => $tag,
286	302	'rfh_date' => $date ),
287		~~- __METHOD__ );~~
	303	+ __METHOD__
	304	+ );
288	305	# Get effective tag values for this page..
289	306	list($aveVal,$n) = FlaggedRevs::getAverageRating( $article, $tag, true );
290	307	$insertRows[] = array(
—	—	@@ -293,19 +310,12 @@
294	311	'rfp_count' => $n,
295	312	'rfp_touched' => $touched
296	313	);
297		~~- $overall += FlaggedRevs::getFeedbackWeight( $tag ) * $aveVal;~~
298	314	}
299		~~- # Get overall data for this page. Used to rank best/worst pages...~~
300		~~- if( isset($n) ) {~~
301		~~- $insertRows[] = array(~~
302		~~- 'rfp_page_id' => $rev->getPage(),~~
303		~~- 'rfp_tag' => 'overall',~~
304		~~- 'rfp_ave_val' => ($overall / count($this->dims)),~~
305		~~- 'rfp_count' => $n,~~
306		~~- 'rfp_touched' => $touched~~
307		~~- );~~
308		~~- }~~
	315	+ # Update recent averages
309	316	$dbw->replace( 'reader_feedback_pages', array( 'PRIMARY' ), $insertRows, __METHOD__ );
	317	+ # Clear out any dead data
	318	+ $dbw->delete( 'reader_feedback_pages', array('rfp_page_id' => $rev->getPage(),
	319	+ 'rfp_tag' => 'overall'), __METHOD__ );
310	320	# For logged in users, box should disappear
311	321	if( $wgUser->getId() ) {
312	322	$this->page->invalidateCache();
Index: trunk/extensions/FlaggedRevs/specialpages/LikedPages_body.php
—	—	@@ -55,8 +55,9 @@
56	56
57	57	protected function showPageList() {
58	58	global $wgOut;
	59	+ $tags = FlaggedRevs::getFeedbackTags();
59	60	$pager = new LikedPagesPager( $this, array(), $this->namespace, $this->tag );
60		~~- if( $pager->getNumRows() ) {~~
	61	+ if( isset($tags[$this->tag]) && $pager->getNumRows() ) {
61	62	$wgOut->addHTML( wfMsgExt('likedpages-list', array('parse') ) );
62	63	$wgOut->addHTML( $pager->getNavigationBar() );
63	64	$wgOut->addHTML( $pager->getBody() );
—	—	@@ -118,11 +119,6 @@
119	120	$conds['page_namespace'] = $this->namespace;
120	121	// Has to be good enough
121	122	$x = 3.5;
122		~~- if( $this->tag == 'overall' ) {~~
123		~~- global $wgFlaggedRevsFeedbackTags;~~
124		~~- $s = $x*array_sum($wgFlaggedRevsFeedbackTags);~~
125		~~- $x = intval( floor($s/count($wgFlaggedRevsFeedbackTags)) );~~
126		~~- }~~
127	123	$conds[] = "rfp_ave_val > $x";
128	124	// Reasonable sample
129	125	$conds[] = 'rfp_count >= '.READER_FEEDBACK_SIZE;
Index: trunk/extensions/FlaggedRevs/FlaggedRevsXML.php
—	—	@@ -92,8 +92,6 @@
93	93	foreach( FlaggedRevs::getFeedbackTags() as $tag => $weight ) {
94	94	$s .= Xml::option( wfMsg( "readerfeedback-$tag" ), $tag, $selected===$tag );
95	95	}
96		~~- # Aggregate for all tags~~
97		~~- $s .= Xml::option( wfMsg( "readerfeedback-overall" ), 'overall', $selected==='overall' );~~
98	96	$s .= Xml::closeElement('select')."\n";
99	97	return $s;
100	98	}
Index: trunk/extensions/FlaggedRevs/flaggedrevs.js
—	—	@@ -94,7 +94,7 @@
95	95	* Update colors when select changes (Opera already does this).
96	96	*/
97	97	function updateFeedbackForm() {
98		~~- var somezero = false;~~
	98	+ var allzero = true;
99	99	var ratingform = document.getElementById('mw-feedbackselects');
100	100	if( !ratingform ) return;
101	101	for( tag in wgFlaggedRevsParams2.tags ) {
—	—	@@ -108,13 +108,13 @@
109	109	if( navigator.appName != 'Opera') {
110	110	levels[0].className = 'fr-rating-option-' + (4 - selectedlevel);
111	111	}
112		~~- if( selectedlevel > 4 ) {~~
113		~~- somezero = true;~~
	112	+ if( selectedlevel <= 4 ) {
	113	+ allzero = false;
114	114	}
115	115	}
116	116	}
117	117	var submit = document.getElementById('submitfeedback');
118		~~- submit.disabled = somezero ? 'disabled' : '';~~
	118	+ submit.disabled = allzero ? 'disabled' : '';
119	119	}
120	120
121	121	addOnloadHook(enable_showhide);

Status & tagging log

21:27, 12 December 2008 Brion VIBBER (talk | contribs) changed the status of r44491 [removed: new added: deferred]