r51073 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r51072‎ | r51073 | r51074 >
Date:16:11, 27 May 2009
Author:werdna
Status:ok
Tags:
Comment:
Reimplement Special:MoveThread with the new HTMLForm interface, fixing several bugs in the process:
* Bug 18949 LiquidThreads allows threads to be moved to pages without LiquidThreads enabled.
* You could move a thread to the page it was already on, which would leave strange placeholders that don't make sense.
* Allowing wikitext in some new places where it makes sense.
Modified paths:
  • /trunk/extensions/LiquidThreads/Lqt.i18n.php (modified) (history)
  • /trunk/extensions/LiquidThreads/classes/LqtDispatch.php (modified) (history)
  • /trunk/extensions/LiquidThreads/classes/LqtThread.php (modified) (history)
  • /trunk/extensions/LiquidThreads/pages/SpecialMoveThread.php (modified) (history)
  • /trunk/phase3/includes/HTMLForm.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/HTMLForm.php
——@@ -146,6 +146,10 @@
147147 $this->mValidationErrorMessage = $msg;
148148 }
149149
 150+ function setIntro( $msg ) {
 151+ $this->mIntro = $msg;
 152+ }
 153+
150154 function displayForm( $submitResult ) {
151155 global $wgOut;
152156
——@@ -153,6 +157,10 @@
154158 $this->displayErrors( $submitResult );
155159 }
156160
 161+ if ( isset($this->mIntro) ) {
 162+ $wgOut->addHTML( $this->mIntro );
 163+ }
 164+
157165 $html = $this->getBody();
158166
159167 // Hidden fields
Index: trunk/extensions/LiquidThreads/classes/LqtDispatch.php
——@@ -83,18 +83,21 @@
8484 $view = new $viewname( $output, $article, $title, $user, $request );
8585 return $view->show();
8686 }
 87+
 88+ static function isLqtPage( $title ) {
 89+ global $wgLqtPages, $wgLqtTalkPages;
 90+ $isTalkPage = ($title->isTalkPage() && $wgLqtTalkPages) ||
 91+ in_array( $title->getPrefixedText(), $wgLqtPages );
 92+
 93+ return $isTalkPage;
 94+ }
8795
8896 /**
8997 * If the page we recieve is a Liquid Threads page of any kind, process it
9098 * as needed and return True. If it's a normal, non-liquid page, return false.
9199 */
92100 static function tryPage( $output, $article, $title, $user, $request ) {
93 - global $wgLqtPages, $wgLqtTalkPages;
94 -
95 - $isTalkPage = ($title->isTalkPage() && $wgLqtTalkPages) ||
96 - in_array( $title->getPrefixedText(), $wgLqtPages );
97 -
98 - if ( $isTalkPage ) {
 101+ if ( LqtDispatch::isLqtPage( $title ) ) {
99102 return self::talkpageMain ( $output, $article, $title, $user, $request );
100103 } else if ( $title->getNamespace() == NS_LQT_THREAD ) {
101104 return self::threadPermalinkMain( $output, $article, $title, $user, $request );
——@@ -111,7 +114,7 @@
112115 Threads::topLevelClause() ) );
113116
114117 foreach ( $threads as $t ) {
115 - $t->moveToSubjectPage( $nt, false );
 118+ $t->moveToPage( $nt, false );
116119 }
117120
118121 return true;
Index: trunk/extensions/LiquidThreads/classes/LqtThread.php
——@@ -176,7 +176,7 @@
177177 $this->commitRevision( Threads::CHANGE_UNDELETED, $this, $reason );
178178 }
179179
180 - function moveToSubjectPage( $title, $reason, $leave_trace ) {
 180+ function moveToPage( $title, $reason, $leave_trace ) {
181181 $dbr =& wfGetDB( DB_MASTER );
182182
183183 $new_articleNamespace = $title->getNamespace();
Index: trunk/extensions/LiquidThreads/pages/SpecialMoveThread.php
——@@ -17,117 +17,136 @@
1818 wfLoadExtensionMessages( 'LiquidThreads' );
1919 return wfMsg( 'lqt_movethread' );
2020 }
21 -
22 - function handleGet() {
23 - wfLoadExtensionMessages( 'LiquidThreads' );
24 - $form_action = $this->getTitle()->getLocalURL() . '/' . $this->thread->title()->getPrefixedURL();
25 - $thread_name = $this->thread->title()->getPrefixedText();
26 - $article_name = $this->thread->article()->getTitle()->getPrefixedText();
27 - $edit_url = LqtView::permalinkUrl( $this->thread, 'edit', $this->thread );
28 - $wfMsg = 'wfMsg'; // functions can only be called within string expansion by variable name.
29 - // FIXME: awkward message usage and fixed parameter formatting. Would be nicer if all formatting
30 - // was done in the message itself, and the below code would be deweirded.
31 - $this->output->addHTML( <<<HTML
32 -<p>{$wfMsg('lqt_move_movingthread', "<b>$thread_name</b>", "<b>$article_name</b>")}</p>
33 -<p>{$wfMsg('lqt_move_torename', "<a href=\"$edit_url\">{$wfMsg('lqt_move_torename_edit')}</a>")}</p>
34 -<form id="lqt_move_thread_form" action="$form_action" method="POST">
35 -<table>
36 -<tr>
37 -<td><label for="lqt_move_thread_target_title">{$wfMsg('lqt_move_destinationtitle')}</label></td>
38 -<td><input id="lqt_move_thread_target_title" name="lqt_move_thread_target_title" tabindex="100" size="40" /></td>
39 -</tr><tr>
40 -<td><label for="lqt_move_thread_reason">{$wfMsg('movereason')}</label></td>
41 -<td><input id="lqt_move_thread_reason" name="lqt_move_thread_reason" tabindex="200" size="40" /></td>
42 -</tr><tr>
43 -<td>&nbsp;</td>
44 -<td><input type="submit" value="{$wfMsg('lqt_move_move')}" style="float:right;" tabindex="300" /></td>
45 -</tr>
46 -</table>
47 -</form>
48 -HTML
49 - );
50 -
 21+
 22+ function getFormFields() {
 23+ return
 24+ array(
 25+ 'dest-title' =>
 26+ array(
 27+ 'label-message' => 'lqt_move_destinationtitle',
 28+ 'type' => 'text',
 29+ 'validation-callback' => array( $this, 'validateTarget' ),
 30+ ),
 31+ 'reason' =>
 32+ array(
 33+ 'label-message' => 'movereason',
 34+ 'type' => 'text',
 35+ ),
 36+ );
5137 }
5238
53 - function checkUserRights() {
54 - if ( !$this->user->isAllowed( 'move' ) ) {
55 - $this->output->showPermissionsErrorPage( 'move' );
56 - return false;
57 - }
58 - if ( $this->user->isBlocked() ) {
59 - $this->output->blockedPage();
60 - return false;
61 - }
62 - if ( wfReadOnly() ) {
63 - $this->output->readOnlyPage();
64 - return false;
65 - }
66 - if ( $this->user->pingLimiter( 'move' ) ) {
67 - $this->output->rateLimited();
68 - return false;
69 - }
70 - /* Am I forgetting anything? */
71 - return true;
72 - }
73 -
74 - function redisplayForm( $problem_fields, $message ) {
75 - $this->output->addHTML( $message );
76 - $this->handleGet();
77 - }
78 -
79 - function handlePost() {
80 - if ( !$this->checkUserRights() ) return;
 39+ function execute( $par ) {
8140 wfLoadExtensionMessages( 'LiquidThreads' );
82 -
83 - $tmp = $this->request->getVal( 'lqt_move_thread_target_title' );
84 - if ( $tmp === "" ) {
85 - $this->redisplayForm( array( 'lqt_move_thread_target_title' ), wfMsg( 'lqt_move_nodestination' ) );
86 - return;
87 - }
88 - $newtitle = Title::newFromText( $tmp );
89 -
90 - $reason = $this->request->getVal( 'lqt_move_thread_reason', wfMsg( 'lqt_noreason' ) );
91 -
92 - // TODO no status code from this method.
93 - $this->thread->moveToSubjectPage( $newtitle, $reason, true );
94 -
95 - $this->showSuccessMessage( $newtitle );
96 - }
97 -
98 - function showSuccessMessage( $target_title ) {
99 - wfLoadExtensionMessages( 'LiquidThreads' );
100 - $this->output->addHTML( wfMsg( 'lqt_move_success',
101 - '<a href="' . $target_title->getFullURL() . '">' . $target_title->getPrefixedText() . '</a>' ) );
102 - }
103 -
104 - function execute( $par ) {
105 - global $wgOut, $wgRequest, $wgUser;
106 - $this->user = $wgUser;
107 - $this->output = $wgOut;
108 - $this->request = $wgRequest;
109 -
110 - $this->setHeaders();
111 -
 41+
 42+ global $wgOut;
 43+
 44+ // Page title
 45+ $wgOut->setPageTitle( wfMsg( 'lqt_movethread' ) );
 46+
 47+ // Handle parameter
 48+ $this->mTarget = $par;
11249 if ( $par === null || $par === "" ) {
11350 wfLoadExtensionMessages( 'LiquidThreads' );
11451 $this->output->addHTML( wfMsg( 'lqt_threadrequired' ) );
11552 return;
11653 }
117 - // TODO should implement Threads::withTitle(...).
11854 $thread = Threads::withRoot( new Article( Title::newFromURL( $par ) ) );
11955 if ( !$thread ) {
120 - wfLoadExtensionMessages( 'LiquidThreads' );
12156 $this->output->addHTML( wfMsg( 'lqt_nosuchthread' ) );
12257 return;
12358 }
 59+ $this->mThread = $thread;
 60+
 61+ // Generate introduction
 62+ $intro = '';
 63+
 64+ global $wgUser;
 65+ $sk = $wgUser->getSkin();
 66+ $page = $article_name = $thread->article()->getTitle()->getPrefixedText();
 67+
 68+ $edit_text = wfMsgExt( 'lqt_move_torename_edit', 'parseinline' );
 69+ $edit_link = $sk->link( $thread->title(), $edit_text, array(),
 70+ array( 'lqt_method' => 'edit', 'lqt_operand' => $thread->id() ) );
 71+
 72+ $intro .= wfMsgExt( 'lqt_move_movingthread', 'parse',
 73+ array('[['.$this->mTarget.']]', '[['.$page.']]') );
 74+ $intro .= wfMsgExt( 'lqt_move_torename', array( 'parse', 'replaceafter' ),
 75+ array( $edit_link ) );
 76+
 77+ $form = new HTMLForm( $this->getFormFields(), 'lqt-move' );
 78+
 79+ $form->setSubmitText( wfMsg('lqt_move_move') );
 80+ $form->setTitle( SpecialPage::getTitleFor( 'MoveThread', $par ) );
 81+ $form->setSubmitCallback( array( $this, 'trySubmit' ) );
 82+ $form->setIntro( $intro );
 83+
 84+ $form->show();
 85+ }
 86+
 87+ function checkUserRights( $oldTitle, $newTitle ) {
 88+ global $wgUser, $wgOut;
 89+
 90+ $oldErrors = $oldTitle->getUserPermissionsErrors( 'move', $wgUser );
 91+ $newErrors = $newTitle->getUserPermissionsErrors( 'move', $wgUser );
 92+
 93+ // Custom merge/unique function because we don't have the second parameter to
 94+ // array_unique on Wikimedia.
 95+ $mergedErrors = array();
 96+ foreach( array_merge( $oldErrors, $newErrors ) as $key => $value ) {
 97+ if ( !is_numeric($key) ) {
 98+ $mergedErrors[$key] = $value;
 99+ } elseif ( !in_array( $value, $mergedErrors ) ) {
 100+ $mergedErrors[] = $value;
 101+ }
 102+ }
 103+
 104+ if ( count($mergedErrors) > 0 ) {
 105+ return $wgOut->parse(
 106+ $wgOut->formatPermissionsErrorMessage( $mergedErrors, 'move' )
 107+ );
 108+ }
124109
125 - $this->thread = $thread;
 110+ return true;
 111+ }
 112+
 113+ function trySubmit( $data ) {
 114+ // Load data
 115+ $tmp = $data['dest-title'];
 116+ $newtitle = Title::newFromText( $tmp );
 117+ $reason = $data['reason'];
 118+
 119+ $rightsResult = $this->checkUserRights( $this->mThread->title(), $newtitle );
 120+
 121+ if ($rightsResult !== true)
 122+ return $rightsResult;
126123
127 - if ( $this->request->wasPosted() ) {
128 - $this->handlePost();
129 - } else {
130 - $this->handleGet();
 124+ // TODO no status code from this method.
 125+ $this->mThread->moveToPage( $newtitle, $reason, true );
 126+
 127+ global $wgOut, $wgUser;
 128+ $sk = $wgUser->getSkin();
 129+ $wgOut->addHTML( wfMsgExt( 'lqt_move_success', array( 'parse', 'replaceafter' ),
 130+ array( $sk->link( $newtitle ) ) ) );
 131+
 132+ return true;
 133+ }
 134+
 135+ function validateTarget( $target ) {
 136+ if (!$target) {
 137+ return wfMsgExt( 'lqt_move_nodestination', 'parseinline' );
131138 }
132 -
 139+
 140+ $title = Title::newFromText( $target );
 141+
 142+ if ( !$title || !LqtDispatch::isLqtPage( $title ) ) {
 143+ return wfMsgExt( 'lqt_move_thread_bad_destination', 'parseinline' );
 144+ }
 145+
 146+ if ( $title->equals( $this->mThread->article()->getTitle() ) ) {
 147+ return wfMsgExt( 'lqt_move_samedestination', 'parseinline' );
 148+ }
 149+
 150+ return true;
133151 }
 152+
134153 }
Index: trunk/extensions/LiquidThreads/Lqt.i18n.php
——@@ -79,6 +79,8 @@
8080 'lqt_move_destinationtitle' => 'Title of destination talk page:',
8181 'lqt_move_move' => 'Move',
8282 'lqt_move_nodestination' => 'You must specify a destination.',
 83+ 'lqt_move_thread_bad_destination' => 'The destination page is not a discussion page.',
 84+ 'lqt_move_samedestination' => 'The thread is already on this page!',
8385 'lqt_move_noreason' => 'No reason given.',
8486 'lqt_move_success' => 'The thread was moved to $1.',
8587 'lqt_delete_undeleting' => 'Undeleting \'\'\'$1\'\'\'.',

Status & tagging log