r51338 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r51337‎ \| r51338 \| r51339 >
Date:	12:26, 2 June 2009
Author:	jan
Status:	deferred (Comments)
Tags:
Comment:	* Add a log for Polls * Add messages for the log
Modified paths:	/trunk/extensions/Poll/Poll.i18n.php (modified) (history) /trunk/extensions/Poll/Poll.php (modified) (history) /trunk/extensions/Poll/Poll_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Poll/Poll_body.php
—	—	@@ -274,7 +274,7 @@
275	275	}
276	276
277	277	public function submit( $pid ) {
278		~~- global $wgRequest, $wgOut, $wgUser, $wgTitle;~~
	278	+ global $wgRequest, $wgOut, $wgUser, $wgTitle;
279	279
280	280	$type = $_POST['type'];
281	281
—	—	@@ -307,6 +307,10 @@
308	308	'alternative_3' => $alternative_3, 'alternative_4' => $alternative_4, 'alternative_5' => $alternative_5,
309	309	'alternative_6' => $alternative_6, 'creater' => $user, 'dis' => $dis ) );
310	310
	311	+ $log = new LogPage( "poll" );
	312	+ $title = $wgTitle;
	313	+ $log->addEntry( "poll", $title, wfMsg( 'poll-log-create', "[[Benutzer:".htmlentities( $user, ENT_QUOTES, 'UTF-8' )."]]", htmlentities( $question, ENT_QUOTES, 'UTF-8' ) ) );
	314	+
311	315	$wgOut->addWikiMsg( 'poll-create-pass' );
312	316	$wgOut->addHtml( '<a href="'.$wgTitle->getFullURL('action=list').'">'.wfMsg('poll-back').'</a>' );
313	317	}
—	—	@@ -369,15 +373,20 @@
370	374
371	375	$controll_change_right = $wgUser->isAllowed( 'poll-admin' );
372	376	$controll_change_blocked = $wgUser->isBlocked();
373		~~- if ( ($controll_change_right != true) OR ($creater == $user) ) {~~
374		~~- $wgOut->addWikiMsg( 'poll-change-right-error' );~~
	377	+
	378	+ $wgOut->addHtml( '<a href="'.$wgTitle->getFullURL('action=list').'">'.wfMsg('poll-back').'</a>' );
	379	+
	380	+ if ( ( $creater != $user ) AND ( $controll_change_right == false ) ) {
	381	+ $wgOut->addWikiMsg( 'poll-change-right-error' );
375	382	$wgOut->addHtml( '<a href="'.$wgTitle->getFullURL('action=list').'">'.wfMsg('poll-back').'</a>' );
376		~~- }~~
377		~~- elseif ( $controll_change_blocked == true ) {~~
	383	+ }
	384	+
	385	+ if ( isset($controll_delete_blocked) AND ( $controll_change_blocked == true ) ) {
378	386	$wgOut->addWikiMsg( 'poll-change-block-error' );
379	387	$wgOut->addHtml( '<a href="'.$wgTitle->getFullURL('action=list').'">'.wfMsg('poll-back').'</a>' );
380	388	}
381		~~- else {~~
	389	+
	390	+ if ( ( ( $creater == $user ) OR ( $controll_change_right == true ) ) AND ( $controll_change_blocked != true ) ) {
382	391	$dbw = wfGetDB( DB_MASTER );
383	392	$question = $_POST['question'];
384	393	$alternative_1 = $_POST['poll_alternative_1'];
—	—	@@ -393,6 +402,10 @@
394	403	'alternative_3' => $alternative_3, 'alternative_4' => $alternative_4, 'alternative_5' => $alternative_5,
395	404	'alternative_6' => $alternative_6, 'creater' => $user, 'dis' => $dis ), array( 'id' => $pid ) );
396	405
	406	+ $log = new LogPage( "poll" );
	407	+ $title = $wgTitle;
	408	+ $log->addEntry( "poll", $title, wfMsg( 'poll-log-change', "[[Benutzer:".htmlentities( $user, ENT_QUOTES, 'UTF-8' )."]]", htmlentities( $question, ENT_QUOTES, 'UTF-8' ) ) );
	409	+
397	410	$wgOut->addWikiMsg( 'poll-change-pass' );
398	411	$wgOut->addHtml( '<a href="'.$wgTitle->getFullURL('action=list').'">'.wfMsg('poll-back').'</a>' );
399	412	}
—	—	@@ -400,35 +413,47 @@
401	414
402	415	if($type == 'delete') {
403	416	$dbr = wfGetDB( DB_SLAVE );
404		~~- $query = $dbr->select( 'poll', 'creater', array( 'id' => $pid ) );~~
	417	+ $query = $dbr->select( 'poll', 'creater, question', array( 'id' => $pid ) );
405	418	$user = $wgUser->getName();
406	419
407	420	while( $row = $dbr->fetchObject( $query ) ) {
408	421	$creater = htmlentities( $row->creater );
	422	+ $question = $row->question;
409	423	}
410	424
411		~~- $controll_delete_right = $wgUser->isAllowed( 'poll-admin' );~~
	425	+ $controll_delete_right = $wgUser->isAllowed( 'poll-admin' );
	426	+ $controll_delete_blocked = $wgUser->isBlocked();
412	427
413		~~- $wgOut->addHtml( $controll_delete_right );~~
	428	+ if ( ( $creater != $user ) AND ( $controll_delete_right == false ) ) {
	429	+ $wgOut->addWikiMsg( 'poll-delete-right-error' );
	430	+ $wgOut->addHtml( '<a href="'.$wgTitle->getFullURL('action=list').'">'.wfMsg('poll-back').'</a>' );
	431	+ }
414	432
415		~~- $controll_delete_blocked = $wgUser->isBlocked();~~
416		~~- if ( ($controll_delete_right != true) OR ($creater == $user) ) {~~
417		~~- $wgOut->addWikiMsg( 'poll-delete-right-error' );~~
418		~~- $wgOut->addHtml( '<a href="'.$wgTitle->getFullURL('action=list').'">'.wfMsg('poll-back').'</a>' );~~
419		~~- }~~
420		~~- elseif ( $controll_delete_blocked == true ) {~~
	433	+ if ( isset($controll_delete_blocked) AND ( $controll_delete_blocked == true ) ) {
421	434	$wgOut->addWikiMsg( 'poll-delete-block-error' );
422	435	$wgOut->addHtml( '<a href="'.$wgTitle->getFullURL('action=list').'">'.wfMsg('poll-back').'</a>' );
423	436	}
424		~~- else {~~
	437	+
	438	+ if ( ( ( $creater == $user ) OR ( $controll_delete_right == true ) ) AND ( $controll_delete_blocked != true ) ) {
	439	+ if( isset($_POST['controll_delete']) AND $_POST['controll_delete'] == 1 ) {
425	440	$dbw = wfGetDB( DB_MASTER );
426	441	$user = $wgUser->getName();
427	442
428	443	$dbw->delete( 'poll', array( 'id' => $pid ) );
429	444	$dbw->delete( 'poll_answer', array( 'uid' => $pid ) );
	445	+
	446	+ $log = new LogPage( "poll" );
	447	+ //$title = "Delete Poll";
	448	+ $title = $wgTitle;
	449	+ $log->addEntry( "poll", $title, wfMsg( 'poll-log-delete', "[[Benutzer:".htmlentities( $user, ENT_QUOTES, 'UTF-8' )."]]", htmlentities( $question, ENT_QUOTES, 'UTF-8' ) ) );
430	450
431	451	$wgOut->addWikiMsg( 'poll-delete-pass' );
432	452	$wgOut->addHtml( '<a href="'.$wgTitle->getFullURL('action=list').'">'.wfMsg('poll-back').'</a>' );
	453	+ }
	454	+ else {
	455	+ $wgOut->addWikiMsg( 'poll-delete-cancel' );
	456	+ $wgOut->addHtml( '<a href="'.$wgTitle->getFullURL('action=list').'">'.wfMsg('poll-back').'</a>' );
	457	+ }
433	458	}
434	459	}
435	460
Index: trunk/extensions/Poll/Poll.i18n.php
—	—	@@ -48,6 +48,13 @@
49	49	'poll-delete-right-error' => 'You must be the creater of the Poll or have the "poll-admin" right to delete this Poll',
50	50	'poll-delete-block-error' => 'You are not allowed to delete a poll because you use a blocked user',
51	51	'poll-delete-pass' => 'Deleted!',
	52	+ 'poll-delete-cancel' => 'Poll don\'t delete(Checkbox don\'t set)!',
	53	+ 'poll-logpage' => 'Poll-Log',
	54	+ 'poll-logpagetext' => 'This is a log of changes to Polls.',
	55	+ 'poll-log-create' => '$1 created "$2"!',
	56	+ 'poll-log-change' => '$1 changed "$2"!',
	57	+ 'poll-log-delete' => '$1 deleted "$2"!',
	58	+ 'poll-logentry' => 'Polls changed',
52	59	);
53	60
54	61	/** German (Deutsch)
—	—	@@ -90,4 +97,11 @@
91	98	'poll-delete-right-error' => 'Du musst der Autor dieser Umfrage sein oder die "poll-admin"-Gruppenberechtigung haben, um diese Umfrage zu löschen',
92	99	'poll-delete-block-error' => 'Leider darfst du keine Umfrage löschen, weil du einen gesperten Benutzer benutzt',
93	100	'poll-delete-pass' => 'Umfrage erfolgreich gelöscht',
	101	+ 'poll-delete-cancel' => 'Umfrage wurde nicht gelöscht(Häckchen nicht gesetzt)!',
	102	+ 'poll-logpage' => 'Umfrage-Logbuch',
	103	+ 'poll-logpagetext' => 'Dieses Logbuch zeigt Änderungen an den Umfragen.',
	104	+ 'poll-log-create' => '$1 hat "$2" erstellt!',
	105	+ 'poll-log-change' => '$1 hat "$2" geändert!',
	106	+ 'poll-log-delete' => '$1 hat "$2" gelöscht!',
	107	+ 'poll-logentry' => 'Änderung an den Umfragen wurde vorgenommen',
94	108	);
Index: trunk/extensions/Poll/Poll.php
—	—	@@ -56,6 +56,13 @@
57	57	$wgSpecialPages['Poll'] = 'Poll'; # Let MediaWiki know about your new special page.
58	58	$wgSpecialPageGroups['Poll'] = 'other';
59	59
	60	+# Log
	61	+$wgLogTypes[] = 'poll';
	62	+$wgLogNames['poll'] = 'poll-logpage';
	63	+$wgLogHeaders['poll'] = 'poll-logpagetext';
	64	+$wgLogActions['poll/poll'] = 'poll-logentry';
	65	+
	66	+
60	67	# Schema changes
61	68	$wgHooks['LoadExtensionSchemaUpdates'][] = 'efPollSchemaUpdates';
62	69
—	—	@@ -67,7 +74,7 @@
68	75	$wgExtNewFields[] = array( 'poll', 'creater', "$base/archives/patch-creater.sql" ); // Add creater
69	76	$wgExtNewFields[] = array( 'poll', 'dis', "$base/archives/patch-dis.sql" ); // Add dis
70	77	$wgExtNewTables[] = array( 'poll_answer', "$base/archives/Poll-answer.sql" ); // Initial answer tables
71		~~- $wgExtNewFields[] = array( 'poll_answer', 'user', "$base/archives/patch-user.sql" ); // Add dis~~
	78	+ $wgExtNewFields[] = array( 'poll_answer', 'user', "$base/archives/patch-user.sql" ); // Add user
72	79	}
73	80	return true;
74	81	}

Follow-up revisions

Revision	Commit summary	Author	Date
r51600	Fixes for r51338:...	jan	17:58, 8 June 2009

Comments

#Comment by Nikerabbit (talk | contribs) 16:42, 2 June 2009

$wgOut->addHtml( '<a href="'.$wgTitle->getFullURL('action=list').'">'.wfMsg('poll-back').'</a>' );

Raw html messages are discouraged. wfMsgHTML should be suitable in this case.

$controll_delete_blocked = $wgUser->isBlocked();
...
if ( isset($controll_delete_blocked) AND ( $controll_delete_blocked == true ) ) {

The variable cannot be in unset state.

if( isset($_POST['controll_delete']) AND $_POST['controll_delete'] == 1 ) {

Accessing request variables directly is discouraged. There is $wgRequest for that. I'd also be wary of the == comparisons, which may lead to unexpected results when one of the values looks like an integer. Explicit type conversion and === or !== makes the intention clear.

#Comment by Jan Luca (talk | contribs) 19:15, 4 June 2009

What do you meen with "Raw html messages are discouraged. wfMsgHTML should be suitable in this case." ?

Status & tagging log

04:13, 6 June 2009 Tim Starling (talk | contribs) changed the status of r51338 [removed: new added: deferred]