r26447 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r26446‎ | r26447 | r26448 >
Date:05:34, 6 October 2007
Author:david
Status:old
Tags:
Comment:
Fix New Messages so that only one instance of each top-level thread is shown; the specific threads that have been changed are highlighted. The fact that Threads::thread_children was a static variable was causing weird bugs. I have no idea why it was so in the first place; it doesn't make any sense. Hopefully there's no reason that I'm forgetting.
Modified paths:
  • /branches/liquidthreads/extensions/LqtBaseView.php (modified) (history)
  • /branches/liquidthreads/extensions/LqtModel.php (modified) (history)
  • /branches/liquidthreads/extensions/LqtPages.php (modified) (history)
  • /branches/liquidthreads/skins/monobook/main.css (modified) (history)

Diff [purge]

Index: branches/liquidthreads/skins/monobook/main.css
——@@ -209,13 +209,19 @@
210210 }
211211
212212 .lqt_post_changed_by_history {
213 -/* padding: .5em 1.5em .5em 1.5em;*/
214213 padding: 0.5em 1em;
215214 background-color: #eeeeee;
216215 border: solid 1pt #ffaa66;
217216 display: table; width: auto;
218217 }
219218
 219+.lqt_post_new_message {
 220+ padding: 0.5em 1em;
 221+ background-color: #eeeeee;
 222+ border: solid 1pt #ffaa66;
 223+ display: table; width: auto;
 224+}
 225+
220226 .lqt_history_info {
221227 padding: .75em 1.5em .75em 1.5em;
222228 margin-bottom: 1em;
Index: branches/liquidthreads/extensions/LqtPages.php
——@@ -965,6 +965,8 @@
966966 class NewUserMessagesView extends LqtView {
967967
968968 protected $threads;
 969+ protected $tops;
 970+ protected $targets;
969971
970972 function addJSandCSS() {
971973 global $wgJsMimeType, $wgStylePath; // TODO globals.
——@@ -973,12 +975,14 @@
974976 }
975977
976978 function preShowThread($t) {
 979+// $t_ids = implode(',', array_map(create_function('$t', 'return $t->id();'), $this->targets[$t->id()]));
 980+ $t_ids = implode(',', $this->targets[$t->id()]);
977981 $this->output->addHTML(<<<HTML
978982 <table ><tr>
979983 <td style="padding-right: 1em; vertical-align: top; padding-top: 1em;" >
980984 <form method="POST">
981985 <input type="hidden" name="lqt_method" value="mark_as_read" />
982 - <input type="hidden" name="lqt_operand" value="{$t->id()}" />
 986+ <input type="hidden" name="lqt_operand" value="{$t_ids}" />
983987 <input type="submit" value="Read" name="lqt_read_button" title="Remove this thread from New Messages." />
984988 </form>
985989 </td>
——@@ -995,45 +999,89 @@
9961000 );
9971001 }
9981002
999 - function showUndo($t) {
 1003+ function showUndo($ids) {
 1004+ if( count($ids) == 1 ) {
 1005+ $t = Threads::withId($ids[0]);
 1006+ $msg = "Thread <b>{$t->subject()}</b> marked as read.";
 1007+ } else {
 1008+ $count = count($ids);
 1009+ $msg = "$count messages marked as read.";
 1010+ }
 1011+ $operand = implode(',', $ids);
10001012 $this->output->addHTML(<<<HTML
10011013 <form method="POST" class="lqt_undo_mark_as_read">
1002 - Thread <b>{$t->subject()}</b> marked as read.
 1014+ $msg
10031015 <input type="hidden" name="lqt_method" value="mark_as_unread" />
1004 - <input type="hidden" name="lqt_operand" value="{$t->id()}" />
 1016+ <input type="hidden" name="lqt_operand" value="{$operand}" />
10051017 <input type="submit" value="Undo" name="lqt_read_button" title="Bring back the thread you just dismissed." />
10061018 </form>
10071019 HTML
10081020 );
10091021 }
10101022
1011 - function show() {
 1023+ function postDivClass($thread) {
 1024+ $topid = $thread->topmostThread()->id();
 1025+ if( in_array($thread->id(), $this->targets[$topid]) )
 1026+ return 'lqt_post_new_message';
 1027+ else
 1028+ return 'lqt_post';
 1029+ }
 1030+
 1031+ function showOnce() {
10121032 $this->addJSandCSS();
10131033
1014 - // TODO, this will be invoked twice because show() is invoked twice. not fatal but hurts performance.
10151034 if( $this->request->wasPosted() && $this->methodApplies('mark_as_unread') ) {
1016 - $thread_id = $this->request->getInt( 'lqt_operand', null );
1017 - if( $thread_id !== null )
1018 - NewMessages::markThreadAsUnreadByUser($thread_id, $this->user);
1019 - $this->output->redirect( $this->title->getFullURL() );
 1035+ $ids = explode(',', $this->request->getVal('lqt_operand', ''));
 1036+ if( $ids !== false ) {
 1037+ foreach($ids as $id) {
 1038+ NewMessages::markThreadAsUnreadByUser(Threads::withId($id), $this->user);
 1039+ }
 1040+ $this->output->redirect( $this->title->getFullURL() );
 1041+ }
10201042 }
10211043
 1044+ else if( $this->request->wasPosted() && $this->methodApplies('mark_as_read') ) {
 1045+ $ids = explode(',', $this->request->getVal('lqt_operand', ''));
 1046+ if( $ids !== false ) {
 1047+ foreach($ids as $id) {
 1048+ NewMessages::markThreadAsReadByUser(Threads::withId($id), $this->user);
 1049+ }
 1050+ $query = 'lqt_method=undo_mark_as_read&lqt_operand=' . implode(',', $ids);
 1051+ $this->output->redirect( $this->title->getFullURL($query) );
 1052+ }
 1053+ }
 1054+
 1055+ else if( $this->methodApplies('undo_mark_as_read') ) {
 1056+ $ids = explode(',', $this->request->getVal('lqt_operand', ''));
 1057+ $this->showUndo($ids);
 1058+ }
 1059+ }
 1060+
 1061+ function show() {
10221062 if ( ! is_array( $this->threads ) ) {
10231063 throw new MWException('You must use NewUserMessagesView::setThreads() before calling NewUserMessagesView::show().');
10241064 }
10251065
1026 - foreach($this->threads as $t) {
 1066+ // Do everything by id, because we can't depend on reference identity; a simple Thread::withId
 1067+ // can change the cached value and screw up your references.
 1068+
 1069+ $this->targets = array();
 1070+ $this->tops = array();
 1071+ foreach( $this->threads as $t ) {
 1072+ $top = $t->topmostThread();
 1073+ if( !in_array($top->id(), $this->tops) )
 1074+ $this->tops[] = $top->id();
 1075+ if( !array_key_exists($top->id(), $this->targets) )
 1076+ $this->targets[$top->id()] = array();
 1077+ $this->targets[$top->id()][] = $t->id();
 1078+ }
 1079+
 1080+ foreach($this->tops as $t_id) {
 1081+ $t = Threads::withId($t_id);
10271082 // It turns out that with lqtviews composed of threads from various talkpages,
10281083 // each thread is going to have a different article... this is pretty ugly.
10291084 $this->article = $t->article();
10301085
1031 - if( $this->request->wasPosted() && $this->methodAppliesToThread('mark_as_read', $t) ) {
1032 - NewMessages::markThreadAsReadByUser($t, $this->user);
1033 - $this->showUndo($t);
1034 - continue;
1035 - }
1036 -
1037 - // Call for POST as well as GET so that edit, reply, etc. will work.
10381086 $this->preShowThread($t);
10391087 $this->showThread($t);
10401088 $this->postShowThread($t);
——@@ -1071,17 +1119,16 @@
10721120
10731121 $this->setHeaders();
10741122
1075 - $this->output->addHTML('<h2 class="lqt_newmessages_section">Messages sent to you:</h2>');
1076 -
10771123 $view = new NewUserMessagesView( $this->output, new Article($this->title),
10781124 $this->title, $this->user, $this->request );
10791125 $view->setHeaderLevel(3);
 1126+ $view->showOnce();
 1127+
 1128+ $this->output->addHTML('<h2 class="lqt_newmessages_section">Messages sent to you:</h2>');
10801129 $view->setThreads( NewMessages::newUserMessages($this->user) );
10811130 $view->show();
10821131
1083 - // and then the same for the other talkpage messagess.
10841132 $this->output->addHTML('<h2 class="lqt_newmessages_section">Messages on other talkpages:</h2>');
1085 -
10861133 $view->setThreads( NewMessages::watchedThreadsForUser($this->user) );
10871134 $view->show();
10881135 }
Index: branches/liquidthreads/extensions/LqtModel.php
——@@ -767,8 +767,6 @@
768768 static $cache_by_root = array();
769769 static $cache_by_id = array();
770770
771 - static $thread_children = array();
772 -
773771 static function newThread( $root, $article, $superthread = null, $type = self::TYPE_NORMAL ) {
774772
775773 // SCHEMA changes must be reflected here.
——@@ -865,6 +863,7 @@
866864
867865 $threads = array();
868866 $top_level_threads = array();
 867+ $thread_children = array();
869868
870869 while ( $line = $dbr->fetchObject($res) ) {
871870 $new_thread = new Thread($line, null);
——@@ -876,9 +875,9 @@
877876 // thread has a parent. extract second-to-last path element.
878877 preg_match( '/([^.]+)\.[^.]+$/', $line->thread_path, $path_matches );
879878 $parent_id = $path_matches[1];
880 - if( !array_key_exists( $parent_id, self::$thread_children ) )
881 - self::$thread_children[$parent_id] = array();
882 - self::$thread_children[$parent_id][] = $new_thread;
 879+ if( !array_key_exists( $parent_id, $thread_children ) )
 880+ $thread_children[$parent_id] = array();
 881+ $thread_children[$parent_id][] = $new_thread;
883882 }
884883 }
885884
——@@ -892,8 +891,8 @@
893892 */
894893
895894 foreach( $threads as $thread ) {
896 - if( array_key_exists( $thread->id(), self::$thread_children ) ) {
897 - $thread->initWithReplies( self::$thread_children[$thread->id()] );
 895+ if( array_key_exists( $thread->id(), $thread_children ) ) {
 896+ $thread->initWithReplies( $thread_children[$thread->id()] );
898897 } else {
899898 $thread->initWithReplies( array() );
900899 }
Index: branches/liquidthreads/extensions/LqtBaseView.php
——@@ -21,7 +21,25 @@
2222 $wgOut->addHTML(/*'<pre>' . htmlspecialchars($tmp,ENT_QUOTES) . '</pre>'*/ $tmp);
2323 }
2424
 25+function efThreadTable($ts) {
 26+ global $wgOut;
 27+ $wgOut->addHTML('<table>');
 28+ foreach($ts as $t)
 29+ efThreadTableHelper($t, 0);
 30+ $wgOut->addHTML('</table>');
 31+}
2532
 33+function efThreadTableHelper($t, $indent) {
 34+ global $wgOut;
 35+ $wgOut->addHTML('<tr>');
 36+ $wgOut->addHTML('<td>' . $indent);
 37+ $wgOut->addHTML('<td>' . $t->id());
 38+ $wgOut->addHTML('<td>' . $t->title()->getPrefixedText());
 39+ $wgOut->addHTML('</tr>');
 40+ foreach($t->subthreads() as $st)
 41+ efThreadTableHelper($st, $indent + 1);
 42+}
 43+
2644 require_once('LqtModel.php');
2745 require_once('Pager.php');
2846 require_once('PageHistory.php');

Status & tagging log