r76582 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r76581‎ | r76582 | r76583 >
Date:17:15, 12 November 2010
Author:ialex
Status:deferred
Tags:
Comment:
* Made the special page extend SpecialPage and use $wgSpecialPages instead of using deprecated SpecialPage::addPage()
* Added aliases file for special page
* Removed usage of deprecated method OutputPage::fatalError()
* WikiError -> Status
* Hardcoded method name -> __METHOD__
* Corrected TodoTemplate to output all HTML in the correct order
* $wgTitle -> $skin->getTitle() in SkinTemplateTabs hook
Modified paths:
  • /trunk/extensions/Todo/Todo.alias.php (added) (history)
  • /trunk/extensions/Todo/Todo.i18n.php (modified) (history)
  • /trunk/extensions/Todo/Todo.php (modified) (history)
  • /trunk/extensions/Todo/TodoForm.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Todo/TodoForm.php
——@@ -10,8 +10,8 @@
1111 $tododetail = wfMsg( 'todo-form-details' );
1212 $todoemail = wfMsg( 'todo-form-email' );
1313 $todosubmit = wfMsg( 'todo-form-submit' );
14 - $wgOut->addHTML( "
15 -<style type=\"text/css\">
 14+?>
 15+<style type="text/css">
1616 .mwTodoNewForm {
1717 border: solid 1px #ccc;
1818 background-color: #eee;
——@@ -22,38 +22,36 @@
2323 .mwTodoTitle {
2424 font-weight: bold;
2525 }
26 -</style>" );
27 -?>
 26+</style>
 27+
2828 <script type="text/javascript" src="<?php $this->text( 'script' ) ?>"></script>
2929
30 -<form action="<?php $this->text( 'action' ) ?>" method="post">
 30+<form action="<?php $this->text( 'action' ); ?>" method="post">
3131 <input type="hidden" name="wpNewItem" value="1" />
 32+ <div class="mwTodoNewForm">
3233 <p>
33 -<?php
34 -$wgOut->addHTML( "
35 - <div class=\"mwTodoNewForm\">
36 - <label for=\"wpSummary\">{$todosummary}</label>
 34+ <label for="wpSummary"><?php echo $todosummary; ?></label>
3735 <br />
38 - <input id=\"wpSummary\" name=\"wpSummary\" size=\"40\" />
 36+ <input id="wpSummary" name="wpSummary" size="40" />
3937 </p>
4038
4139 <p>
42 - <label for=\"wpComment\">{$tododetail}</label>
 40+ <label for="wpComment"><?php echo $tododetail ?></label>
4341 <br />
44 - <textarea id=\"wpComment\" name=\"wpComment\" cols=\"40\" rows=\"6\" wrap=\"virtual\"></textarea>
 42+ <textarea id="wpComment" name="wpComment" cols="40" rows="6" wrap="virtual"></textarea>
4543 </p>
4644
4745 <p>
48 - <label for=\"wpEmail\">{$todoemail}</label>
 46+ <label for="wpEmail"><?php echo $todoemail ?></label>
4947 <br />
50 - <input id=\"wpEmail\" name=\"wpEmail\" size=\"30\" />
 48+ <input id="wpEmail" name="wpEmail" size="30" />
5149 </p>
5250
5351 <p>
54 - <input type=\"submit\" value=\"{$todosubmit}\" />
 52+ <input type="submit" value="<?php echo $todosubmit; ?>" />
5553 </p>
5654 </div>
57 -</form>"
58 - );
 55+</form>
 56+<?php
5957 }
6058 }
Index: trunk/extensions/Todo/Todo.alias.php
——@@ -0,0 +1,15 @@
 2+<?php
 3+/**
 4+ * Aliases for special pages of Todo extension.
 5+ *
 6+ * @file
 7+ */
 8+
 9+$specialPageAliases = array();
 10+
 11+/**
 12+ * English
 13+ */
 14+$specialPageAliases['en'] = array(
 15+ 'Todo' => array( 'Todo' ),
 16+);
\ No newline at end of file
Property changes on: trunk/extensions/Todo/Todo.alias.php
___________________________________________________________________
Added: svn:eol-style
117 + native
Index: trunk/extensions/Todo/Todo.i18n.php
——@@ -35,6 +35,7 @@
3636 'todo-list-change' => 'Change',
3737 'todo-list-cancel' => 'Cancel',
3838 'todo-new-item' => 'New item',
 39+ 'todo-not-updated' => 'Could not update database record',
3940 'todo-issue-summary' => 'Issue summary:',
4041 'todo-form-details' => 'Details:',
4142 'todo-form-email' => 'To receive notification by e-mail when the item is closed, provide your address:',
Index: trunk/extensions/Todo/Todo.php
——@@ -25,11 +25,11 @@
2626 'descriptionmsg' => 'todo-desc',
2727 );
2828
29 -$wgExtensionFunctions[] = 'todoSetup';
3029 $wgHooks['SkinTemplateTabs'][] = 'todoAddTab';
3130
3231 $dir = dirname( __FILE__ ) . '/';
3332 $wgExtensionMessagesFiles['todoAddTab'] = $dir . 'Todo.i18n.php';
 33+$wgExtensionMessagesFiles['todoAddTabSp'] = $dir . 'Todo.alias.php';
3434
3535 // Creates a group of users who can have todo lists
3636 $wgGroupPermissions['todo']['todo'] = true;
——@@ -41,11 +41,7 @@
4242 $wgAvailableRights[] = 'todo';
4343 $wgAvailableRights[] = 'todosubmit';
4444
45 -// FIXME: use $wgSpecialPages and delay message loading
46 -function todoSetup() {
47 - wfLoadExtensionMessages( 'todoAddTab' );
48 - SpecialPage::addPage( new SpecialPage( 'Todo' ) );
49 -}
 45+$wgSpecialPages['Todo'] = 'SpecialTodo';
5046
5147 // FIXME: use class file(s) to delay loading
5248 /**
——@@ -55,9 +51,8 @@
5652 * @return bool true to continue running hooks, false to abort operation
5753 */
5854 function todoAddTab( $skin, &$actions ) {
59 - global $wgTitle;
60 - if ( $wgTitle->getNamespace() == NS_USER || $wgTitle->getNamespace() == NS_USER_TALK ) {
61 - $title = SpecialPage::getTitleFor( 'Todo', $wgTitle->getText() );
 55+ if ( $skin->getTitle()->getNamespace() == NS_USER || $skin->getTitle()->getNamespace() == NS_USER_TALK ) {
 56+ $title = SpecialPage::getTitleFor( 'Todo', $skin->getTitle()->getText() );
6257 $actions['todo'] = array(
6358 'text' => wfMsg( 'todo-tab' ),
6459 'href' => $title->getLocalUrl() );
——@@ -65,66 +60,63 @@
6661 return true;
6762 }
6863
69 -/**
70 - * Entry-point function for Special:Todo
71 - * @param mixed $par Will contain username to view on
72 - */
73 -function wfSpecialTodo( $par = null ) {
74 - if ( is_null( $par ) || $par == '' ) {
75 - global $wgUser;
76 - $user = $wgUser;
77 - } else {
78 - $user = User::newFromName( $par );
 64+class SpecialTodo extends SpecialPage {
 65+ function __construct() {
 66+ parent::__construct( 'Todo' );
7967 }
80 - if ( is_null( $user ) || !$user->isAllowed( 'todo' ) ) {
81 - global $wgOut;
82 - $wgOut->fatalError( wfMsgHtml( 'todo-user-invalide' ) );
83 - } else {
84 - global $wgRequest;
85 - $todo = new TodoForm( $user );
86 - if ( $wgRequest->wasPosted() ) {
87 - $todo->submit( $wgRequest );
 68+
 69+ public function execute( $par ) {
 70+ global $wgOut, $wgUser, $wgRequest;
 71+ if ( is_null( $par ) || $par === '' ) {
 72+ $user = $wgUser;
8873 } else {
89 - $todo->show();
 74+ $user = User::newFromName( $par );
9075 }
91 - }
92 -}
9376
94 -class TodoForm {
95 - function __construct( $user ) {
 77+ $this->setHeaders();
 78+ $this->outputHeader();
 79+
 80+ if ( is_null( $user ) || !$user->isAllowed( 'todo' ) ) {
 81+ return $wgOut->addWikiMsg( 'todo-user-invalide' );
 82+ }
 83+
9684 $this->target = $user;
97 - $this->self = SpecialPage::getTitleFor( 'Todo', $user->getName() );
 85+ $this->self = $this->getTitle( $user->getName() );
 86+ if ( $wgRequest->wasPosted() ) {
 87+ $this->submit( $wgRequest );
 88+ } else {
 89+ $this->show();
 90+ }
9891 }
9992
10093 function submit( $request ) {
10194 if ( $request->getVal( 'wpNewItem' ) ) {
102 - $this->submitNew( $request );
 95+ $result = $this->submitNew( $request );
10396 } elseif ( $request->getVal( 'wpUpdateField' ) ) {
104 - $this->submitUpdate( $request );
 97+ $result = $this->submitUpdate( $request );
10598 }
10699 $this->showError( $result );
107100 $this->show();
108101 }
109102
110103 function submitNew( $request ) {
111 - $result = TodoItem::add(
 104+ return TodoItem::add(
112105 $this->target,
113106 $request->getText( 'wpSummary' ),
114107 $request->getText( 'wpComment' ),
115108 $request->getVal( 'wpEmail' ) );
116 - return $result;
117109 }
118110
119111 function submitUpdate( $request ) {
120112 $id = $request->getInt( 'wpItem' );
121113 $item = TodoItem::loadFromId( $id );
122114 if ( is_null( $item ) ) {
123 - return new WikiError( wfMsgHtml( 'todo-invalid-item' ) );
 115+ return Status::newFatal( 'todo-invalid-item' );
124116 }
125117
126118 global $wgUser;
127119 if ( $item->owner != $wgUser->getId() ) {
128 - return new WikiError( wfMsgHtml( 'todo-update-else-item' ) );
 120+ return Status::newFatal( 'todo-update-else-item' );
129121 }
130122
131123 switch( $request->getVal( 'wpUpdateField' ) ) {
——@@ -138,7 +130,7 @@
139131 return $item->setTitle( $request->getText( 'wpTitle' ) );
140132 break;
141133 default:
142 - return new WikiError( wfMsgHtml( 'todo-unrecognize-type' ) );
 134+ return Status::newFatal( 'todo-unrecognize-type' );
143135 }
144136 }
145137
——@@ -149,7 +141,7 @@
150142
151143 $wgOut->addWikiText( "== " . wfMsg( 'todo-new-item' ) . " ==\n" );
152144
153 - require_once ( 'TodoForm.php' );
 145+ require_once( 'TodoForm.php' );
154146 $form = new TodoTemplate();
155147 $form->set( 'action', $this->self->getLocalUrl( 'action=submit' ) );
156148 $form->set( 'script', "$wgScriptPath/extensions/Todo/todo.js" );
——@@ -164,10 +156,9 @@
165157
166158 function showError( $result ) {
167159 global $wgOut;
168 - if ( WikiError::isError( $result ) ) {
169 - $wgOut->addHTML( '<p class="error">' .
170 - htmlspecialcahrs( $result->getMessage() ) .
171 - "</p>\n" );
 160+ if ( !$result->isOK() ) {
 161+ $wgOut->addWikiText( "<p class=\"error\">\n" .
 162+ $result->getWikiText() . "\n</p>\n" );
172163 }
173164 }
174165
——@@ -186,7 +177,7 @@
187178 $result = $dbr->select( 'todolist', '*', array(
188179 'todo_owner' => $this->owner,
189180 'todo_status' => 'open' ),
190 - 'TodoList::TodoList',
 181+ __METHOD__,
191182 array( 'ORDER BY' => 'todo_owner,todo_status,todo_queue,todo_timestamp DESC' ) );
192183
193184 $this->items = array();
——@@ -274,7 +265,7 @@
275266 $row = $dbr->selectRow( 'todolist',
276267 '*',
277268 array( 'todo_id' => intval( $id ) ),
278 - 'TodoForm::loadFromId' );
 269+ __METHOD__ );
279270 if ( $row ) {
280271 return new TodoItem( $row );
281272 } else {
——@@ -289,7 +280,7 @@
290281 * @param string $email
291282 * @static
292283 */
293 - function add( $owner, $summary, $comment, $email ) {
 284+ static function add( $owner, $summary, $comment, $email ) {
294285 $dbw = wfGetDB( DB_MASTER );
295286 $dbw->insert( 'todolist',
296287 array(
——@@ -300,8 +291,8 @@
301292 'todo_title' => $summary,
302293 'todo_comment' => $comment,
303294 'todo_email' => $email ),
304 - 'TodoItem::add' );
305 - return true;
 295+ __METHOD__ );
 296+ return Status::newGood();
306297 }
307298
308299
——@@ -429,8 +420,8 @@
430421 }
431422
432423 /**
433 - * @param string $comment
434 - * @param bool $sendMail false to supppress sending of email to submitter
 424+ * @param $comment String
 425+ * @param $sendMail Boolean: false to supppress sending of email to submitter
435426 */
436427 function close( $comment, $sendMail ) {
437428 $this->status = 'closed';
——@@ -441,20 +432,19 @@
442433 }
443434
444435 /**
445 - * @param string $closeComment
446 - * @return mixed true on success, WikiError on failure
 436+ * @param $closeComment String
447437 */
448438 function sendConfirmationMail( $closeComment ) {
449439 global $wgContLang;
450440
451441 $owner = User::newFromId( $this->owner );
452442 if ( is_null( $owner ) ) {
453 - return new WikiError( wfMsgHtml( 'todo-invalid-owner' ) );
 443+ return;
454444 }
455445
456446 $sender = new MailAddress( $owner );
457447 $recipient = new MailAddress( $this->email );
458 - return UserMailer::send( $recipient, $sender,
 448+ UserMailer::send( $recipient, $sender,
459449 wfMsgForContent( 'todo-mail-subject', $owner->getName() ),
460450 wordwrap( wfMsgForContent( 'todo-mail-body',
461451 $owner->getName(),
——@@ -469,9 +459,14 @@
470460 */
471461 function updateRecord( $changes ) {
472462 $dbw = wfGetDB( DB_MASTER );
473 - return $dbw->update( 'todolist',
 463+ $ret = $dbw->update( 'todolist',
474464 $changes,
475465 array( 'todo_id' => $this->id ),
476 - 'TodoItem::updateRecord' );
 466+ __METHOD__ );
 467+ if ( $ret ) {
 468+ return Status::newGood();
 469+ } else {
 470+ return Status::newFatal( 'todo-not-updated' );
 471+ }
477472 }
478473 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r76631Follow-up r76582: Add alias file for translatewikiraymond17:00, 13 November 2010

Status & tagging log