r51026 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r51025‎ | r51026 | r51027 >
Date:15:33, 26 May 2009
Author:werdna
Status:deferred
Tags:
Comment:
Fix some more LiquidThreads annoyances:
* Bug where deleted threads would appear in the TOC. This was related to bad architecture -- the threads were being filtered out at the display end, rather than at the database end.
* Proper use of isAllowed() for checking rights.
* Special:DeleteThread was an unlisted special page for no apparent reason.
* PHP notices were spewed on Special:MoveThread
* Add a delete link to the main thread view.
* Code readability and internal documentation.
Modified paths:
  • /trunk/extensions/LiquidThreads/classes/LqtThread.php (modified) (history)
  • /trunk/extensions/LiquidThreads/classes/LqtView.php (modified) (history)
  • /trunk/extensions/LiquidThreads/pages/SpecialDeleteThread.php (modified) (history)
  • /trunk/extensions/LiquidThreads/pages/TalkpageView.php (modified) (history)

Diff [purge]

Index: trunk/extensions/LiquidThreads/classes/LqtView.php
@@ -48,6 +48,7 @@
4949
5050 function initializeQueries() {
5151
 52+ // Determine sort order
5253 if ( $this->methodApplies( 'talkpage_sort_order' ) ) {
5354 // Sort order is explicitly specified through UI
5455 global $wgRequest;
@@ -71,8 +72,12 @@
7273 $this->sort_order = $user_order;
7374 }
7475 }
 76+
 77+ // Create query group
7578 global $wgOut, $wgLqtThreadArchiveStartDays, $wgLqtThreadArchiveInactiveDays;
 79+ $dbr = wfGetDB( DB_SLAVE );
7680 $g = new QueryGroup();
 81+
7782 $startdate = Date::now()->nDaysAgo( $wgLqtThreadArchiveStartDays )->midnight();
7883 $recentstartdate = $startdate->nDaysAgo( $wgLqtThreadArchiveInactiveDays );
7984 $article_clause = Threads::articleClause( $this->article );
@@ -83,6 +88,8 @@
8489 } elseif ( $this->sort_order == LQT_OLDEST_THREADS ) {
8590 $sort_clause = 'ORDER BY thread.thread_created ASC';
8691 }
 92+
 93+ // Add standard queries
8794 $g->addQuery( 'fresh',
8895 array( $article_clause,
8996 'thread.thread_parent is null',
@@ -90,6 +97,11 @@
9198 ' OR (thread.thread_summary_page is NULL' .
9299 ' AND thread.thread_type=' . Threads::TYPE_NORMAL . '))' ),
93100 array( $sort_clause ) );
 101+
 102+ $g->extendQuery( 'fresh', 'fresh-undeleted',
 103+ array( 'thread_type != '. $dbr->addQuotes( Threads::TYPE_DELETED ) ) );
 104+
 105+
94106 $g->addQuery( 'archived',
95107 array( $article_clause,
96108 'thread.thread_parent is null',
@@ -97,12 +109,18 @@
98110 ' OR thread.thread_type=' . Threads::TYPE_NORMAL . ')',
99111 'thread.thread_modified < ' . $startdate->text() ),
100112 array( $sort_clause ) );
 113+
101114 $g->extendQuery( 'archived', 'recently-archived',
102115 array( '( thread.thread_modified >=' . $recentstartdate->text() .
103116 ' OR rev_timestamp >= ' . $recentstartdate->text() . ')',
104117 'summary_page.page_id = thread.thread_summary_page', 'summary_page.page_latest = rev_id' ),
105118 array(),
106119 array( 'page summary_page', 'revision' ) );
 120+
 121+ $g->addQuery( 'archived', 'archived-undeleted',
 122+ array( 'thread_type != '. $dbr->addQuotes( Threads::TYPE_DELETED ) ) );
 123+
 124+
107125 return $g;
108126 }
109127
@@ -454,6 +472,16 @@
455473 'href' => $this->permalinkUrlWithQuery( $thread, 'action=unwatch' ),
456474 'enabled' => true );
457475 }
 476+
 477+ if ( $this->user->isAllowed( 'delete' ) ) {
 478+ $delete_title = SpecialPage::getTitleFor( 'DeleteThread',
 479+ $thread->title()->getPrefixedText() );
 480+ $delete_href = $delete_title->getFullURL();
 481+
 482+ $commands[] = array( 'label' => wfMsg( 'delete' ),
 483+ 'href' => $delete_href,
 484+ 'enabled' => true );
 485+ }
458486
459487 return $commands;
460488 }
@@ -658,6 +686,7 @@
659687 function showThread( $thread ) {
660688 global $wgLang; # TODO global.
661689
 690+ // Safeguard
662691 if ( $thread->type() == Threads::TYPE_DELETED
663692 && ! $this->request->getBool( 'lqt_show_deleted_threads' ) )
664693 return;
Index: trunk/extensions/LiquidThreads/classes/LqtThread.php
@@ -245,28 +245,43 @@
246246
247247 function __construct( $line, $children ) {
248248 /* SCHEMA changes must be reflected here. */
 249+
 250+ $dataLoads = array(
 251+ 'thread_id' => 'id',
 252+ 'thread_root' => 'rootId',
 253+ 'thread_article_namespace' => 'articleNamespace',
 254+ 'thread_article_title' => 'articleTitle',
 255+ 'thread_summary_page' => 'summaryId',
 256+ 'thread_ancestor' => 'ancestorId',
 257+ 'thread_parent' => 'parentId',
 258+ 'thread_modified' => 'modified',
 259+ 'thread_created' => 'created',
 260+ 'thread_revision' => 'revisionNumber',
 261+ 'thread_type' => 'type',
 262+ 'thread_change_type' => 'changeType',
 263+ 'thread_change_object' => 'changeObject',
 264+ 'thread_change_comment' => 'changeComment',
 265+ 'thread_change_user' => 'changeUser',
 266+ 'thread_change_user_text' => 'changeUserText',
 267+ 'thread_editedness' => 'editedness'
 268+ );
 269+
249270
250 - $this->id = $line->thread_id;
251 - $this->rootId = $line->thread_root;
252 - $this->articleNamespace = $line->thread_article_namespace;
253 - $this->articleTitle = $line->thread_article_title;
254 - $this->summaryId = $line->thread_summary_page;
255 - $this->ancestorId = $line->thread_ancestor;
256 - $this->parentId = $line->thread_parent;
257 - $this->modified = $line->thread_modified;
258 - $this->created = $line->thread_created;
259 - $this->revisionNumber = $line->thread_revision;
260 - $this->type = $line->thread_type;
261 - $this->changeType = $line->thread_change_type;
262 - $this->changeObject = $line->thread_change_object;
263 - $this->changeComment = $line->thread_change_comment;
264 - $this->changeUser = $line->thread_change_user;
265 - $this->changeUserText = $line->thread_change_user_text;
266 - $this->editedness = $line->thread_editedness;
 271+ foreach( $dataLoads as $db_field => $member_field ) {
 272+ if ( isset($line->$db_field) ) {
 273+ $this->$member_field = $line->$db_field;
 274+ }
 275+ }
 276+
 277+ if ( isset($line->page_namespace) && isset($line->page_title) ) {
 278+ $root_title = Title::makeTitle( $line->page_namespace, $line->page_title );
 279+ $this->root = new Post( $root_title );
 280+ $this->root->loadPageData( $line );
 281+ } else {
 282+ $root_title = Title::newFromID( $this->rootId );
 283+ $this->root = new Post( $root_title );
 284+ }
267285
268 - $root_title = Title::makeTitle( $line->page_namespace, $line->page_title );
269 - $this->root = new Post( $root_title );
270 - $this->root->loadPageData( $line );
271286 $this->rootRevision = $this->root->mLatest;
272287 }
273288
@@ -359,6 +374,7 @@
360375
361376 function article() {
362377 if ( $this->article ) return $this->article;
 378+
363379 $title = Title::newFromID( $this->articleId );
364380 if ( $title ) {
365381 $a = new Article( $title );
Index: trunk/extensions/LiquidThreads/pages/SpecialDeleteThread.php
@@ -2,7 +2,7 @@
33
44 if ( !defined( 'MEDIAWIKI' ) ) die;
55
6 -class SpecialDeleteThread extends UnlistedSpecialPage {
 6+class SpecialDeleteThread extends SpecialPage {
77 private $user, $output, $request, $title, $thread;
88
99 function __construct() {
@@ -58,7 +58,7 @@
5959 }
6060
6161 function checkUserRights() {
62 - if ( in_array( 'delete', $this->user->getRights() ) ) {
 62+ if ( $this->user->isAllowed( 'delete' ) ) {
6363 return true;
6464 } else {
6565 wfLoadExtensionMessages( 'LiquidThreads' );
Index: trunk/extensions/LiquidThreads/pages/TalkpageView.php
@@ -196,9 +196,12 @@
197197 $url = $this->talkpageUrl( $this->title, 'talkpage_new_thread' );
198198 $this->output->addHTML( "<strong><a class=\"lqt_start_discussion\" href=\"$url\">" . wfMsg( 'lqt_new_thread' ) . "</a></strong>" );
199199 }
 200+
 201+ $queryType =
 202+ $wgRequest->getBool( 'lqt_show_deleted_threads' )
 203+ ? 'fresh' : 'fresh-undeleted';
 204+ $threads = $this->queries->query( $queryType );
200205
201 - $threads = $this->queries->query( 'fresh' );
202 -
203206 $this->openDiv( 'lqt_toc_archive_wrapper' );
204207
205208 $this->openDiv( 'lqt_archive_teaser_empty' );

Status & tagging log