r110326 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r110325‎ | r110326 | r110327 >
Date:20:40, 30 January 2012
Author:gregchiasson
Status:resolved (Comments)
Tags:aft 
Comment:
AFT5 bug fixes:
- Finally fix that thing where the articlefeedbackv5-error-loading-feedback message was undocumented and not loaded on the Special page.
- Bold the active sort order.
- Change permalinks from #id=ID to /ID.
- Re-un-comment the Special page and API calls, per Yoni.
- Fix bug with user names going to contributions page instead of user page
- Switch header to put smiley face before name instead of after.
Modified paths:
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.hooks.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.i18n.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/SpecialArticleFeedbackv5.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.css (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.i18n.php
——@@ -335,6 +335,7 @@
336336 'articlefeedbackv5-form-delete' => '{{Identical|Delete}}',
337337 'articlefeedbackv5-form-oversight' => 'Request that an oversighter review this feedback',
338338 'articlefeedbackv5-form-unoversight' => 'Remove request for oversight',
 339+ 'articlefeedbackv5-error-loading-feedback' => 'Message displayed when there was an error loading feedback - result is a largely-blank page.',
339340 'articlefeedbackv5-form-header' => "* '''$1''' is the feedback ID.
340341 * '''$2''' is the date and time at which the feedback was given.",
341342 'articlefeedbackv5-form1-header-found' => 'Parameters
Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.css
——@@ -327,6 +327,7 @@
328328 .sort-active {
329329 width: 11px;
330330 height: 1em;
 331+ font-weight: bold;
331332 }
332333 .ascending {
333334 /* @embed */
Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.js
——@@ -85,12 +85,15 @@
8686
8787 // Process anything we found in the URL hash
8888 // Permalinks.
89 - var id = window.location.hash.match(/id=(\d+)/)
 89+ var id = window.location.href.match(/(.+)\/(\d+)$/)
9090 if( id ) {
9191 $.articleFeedbackv5special.listControls.filter = 'id';
92 - $.articleFeedbackv5special.listControls.filterValue = id[1];
 92+ $.articleFeedbackv5special.listControls.filterValue = id[2];
9393 }
9494
 95+ // Bold the default sort.
 96+ $( '#articleFeedbackv5-special-sort-age' ).addClass( 'sort-active' );
 97+
9598 // Grab the user's activity out of the cookie
9699 $.articleFeedbackv5special.activityCookieName += $.articleFeedbackv5special.page;
97100 $.articleFeedbackv5special.loadActivity();
Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.php
——@@ -274,14 +274,14 @@
275275 $wgHooks['ArticleSaveComplete'][] = 'ArticleFeedbackv5Hooks::trackEditSuccess';
276276
277277 // API Registration
278 -#$wgAPIListModules['articlefeedbackv5-view-ratings'] = 'ApiViewRatingsArticleFeedbackv5';
279 -#$wgAPIListModules['articlefeedbackv5-view-feedback'] = 'ApiViewFeedbackArticleFeedbackv5';
280 -#$wgAPIModules['articlefeedbackv5-flag-feedback'] = 'ApiFlagFeedbackArticleFeedbackv5';
 278+$wgAPIListModules['articlefeedbackv5-view-ratings'] = 'ApiViewRatingsArticleFeedbackv5';
 279+$wgAPIListModules['articlefeedbackv5-view-feedback'] = 'ApiViewFeedbackArticleFeedbackv5';
 280+$wgAPIModules['articlefeedbackv5-flag-feedback'] = 'ApiFlagFeedbackArticleFeedbackv5';
281281 $wgAPIModules['articlefeedbackv5'] = 'ApiArticleFeedbackv5';
282282
283283 // Special Page
284 -#$wgSpecialPages['ArticleFeedbackv5'] = 'SpecialArticleFeedbackv5';
285 -#$wgSpecialPageGroups['ArticleFeedbackv5'] = 'other';
 284+$wgSpecialPages['ArticleFeedbackv5'] = 'SpecialArticleFeedbackv5';
 285+$wgSpecialPageGroups['ArticleFeedbackv5'] = 'other';
286286
287287 $wgAvailableRights[] = 'aftv5-hide-feedback';
288288 $wgAvailableRights[] = 'aftv5-delete-feedback';
Index: trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php
——@@ -463,7 +463,8 @@
464464
465465 private function renderPermalinkTimestamp( $record ) {
466466 global $wgLang;
467 - $id = $record->af_id;
 467+ $id = $record->af_id;
 468+ $title = $record->page_title;
468469
469470 // Taken from the Moodbar extension.
470471 $now = wfTimestamp( TS_UNIX );
——@@ -484,11 +485,12 @@
485486 return Html::openElement( 'span', array(
486487 'class' => 'articleFeedbackv5-comment-details-date'
487488 ) )
488 - . Html::element( 'a', array(
489 - 'class' => 'articleFeedbackv5-permalink',
490 - 'id' => "articleFeedbackv5-permalink-$id",
491 - 'href' => "#id=$id"
492 - ), $date )
 489+ . Linker::link(
 490+ Title::newFromText(
 491+ "Special:ArticleFeedbackv5/$title/$id"
 492+ ),
 493+ $date
 494+ )
493495 . Html::closeElement( 'span' );
494496 }
495497
——@@ -624,14 +626,12 @@
625627
626628 private function feedbackHead( $message, $class, $record, $extra = '' ) {
627629 $name = htmlspecialchars( $record->user_name );
628 - $link = $record->af_user_id ? "User:$name" : "Special:Contributions/$name";
 630+ $link = !$record->af_user_ip ? "User:$name" : "Special:Contributions/$name";
629631 $gender = $name;
630632
631 - return Html::openElement( 'h3', array(
632 - 'class' => $class
633 - ) )
 633+ return Html::openElement( 'h3', array( 'class' => $class) )
 634+ . Html::element( 'span', array( 'class' => 'icon' ) )
634635 . Linker::link( Title::newFromText( $link ), $name )
635 - . Html::element( 'span', array( 'class' => 'icon' ) )
636636 . Html::element( 'span',
637637 array( 'class' => 'result' ),
638638 wfMessage( $message, $gender, $extra )->text()
Index: trunk/extensions/ArticleFeedbackv5/SpecialArticleFeedbackv5.php
——@@ -59,6 +59,11 @@
6060 public function execute( $param ) {
6161 global $wgArticleFeedbackv5DashboardCategory;
6262 $out = $this->getOutput();
 63+
 64+ if( preg_match('/^(.+)\/(\d+)$/', $param, $m ) ) {
 65+ $param = $m[1];
 66+ }
 67+
6368 $title = Title::newFromText( $param );
6469
6570 // Page does not exist.
Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.hooks.php
——@@ -174,7 +174,8 @@
175175 'articlefeedbackv5-form-undelete',
176176 'articlefeedbackv5-hidden',
177177 'articlefeedbackv5-comment-more',
178 - 'articlefeedbackv5-comment-less'
 178+ 'articlefeedbackv5-comment-less',
 179+ 'articlefeedbackv5-error-loading-feedback'
179180 ),
180181 'dependencies' => array(
181182 'mediawiki.util',

Follow-up revisions

RevisionCommit summaryAuthorDate
r110402AFT5 - fix issues raised on r110326 - whitespace instead of tabs, inefficient...gregchiasson16:34, 31 January 2012

Comments

#Comment by Santhosh.thottingal (talk | contribs)   06:31, 31 January 2012

Whitespaces are used for tabs in renderPermalinkTimestamp method

#Comment by Catrope (talk | contribs)   08:01, 31 January 2012 
+		var id = window.location.href.match(/(.+)\/(\d+)$/)

Simply using .match(/\/(\d+)$/) should work, right? That regex would execute much faster, because the regex engine wouldn't have to backtrack after having the first .+ match everything. 

+			Title::newFromText( 
+				"Special:ArticleFeedbackv5/$title/$id" 
+			),

You can use SpecialPage::getTitleFor( 'ArticleFeedbackv5', "$title/$id" ) instead.

Looks good to me, except that the whitespace is messed up as pointed out by Santhosh.

Status & tagging log