r109835 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r109834‎ | r109835 | r109836 >
Date:17:53, 23 January 2012
Author:gregchiasson
Status:resolved (Comments)
Tags:aft 
Comment:
AFT5 feedback page - bug fixes for fitler dropdown and a couple of bad translation messages, formatting the headers of all feedback buckets to match bucket 1 with the working links css classes etc, permissions fix to assume all oversighters are rollbackers, and fix one of the filtercount rollups not being incremented.
Modified paths:
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.i18n.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/SpecialArticleFeedbackv5.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5Utils.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.i18n.php
——@@ -80,14 +80,14 @@
8181 'articlefeedbackv5-form-header' => 'Feedback #$1, at $2',
8282 'articlefeedbackv5-form1-header-found' => '{{GENDER:$1|$1}} found what they were looking for:',
8383 'articlefeedbackv5-form1-header-not-found' => '{{GENDER:$1|$1}} did not find what they were looking for:',
84 - 'articlefeedbackv5-form2-header-praise' => '{{GENDER:$1|$1}} had a comment:',
85 - 'articlefeedbackv5-form2-header-problem' => '{{GENDER:$1|$1}} had a praise:',
 84+ 'articlefeedbackv5-form2-header-praise' => '{{GENDER:$1|$1}} had a praise:',
 85+ 'articlefeedbackv5-form2-header-problem' => '{{GENDER:$1|$1}} had a problem:',
8686 'articlefeedbackv5-form2-header-question' => '{{GENDER:$1|$1}} had a question:',
8787 'articlefeedbackv5-form2-header-suggestion' => '{{GENDER:$1|$1}} had a suggestion:',
88 - 'articlefeedbackv5-form3-header' => '$1 rated this page $2/5',
89 - 'articlefeedbackv5-form4-header' => 'User was presented with the CTA-only form.',
90 - 'articlefeedbackv5-form5-header' => '$1 rated this page:',
91 - 'articlefeedbackv5-form-not-shown' => 'User was not shown a feedback form.',
 88+ 'articlefeedbackv5-form3-header' => '{{GENDER:$1|$1}} rated this page $2/5',
 89+ 'articlefeedbackv5-form4-header' => '{{GENDER:$1|$1}} was presented with the CTA-only form.',
 90+ 'articlefeedbackv5-form5-header' => '{{GENDER:$1|$1}} rated this page:',
 91+ 'articlefeedbackv5-form-not-shown' => '{{GENDER:$1|$1}} was not shown a feedback form.',
9292 'articlefeedbackv5-form-invalid' => 'Invalid feedback form ID.',
9393 'articlefeedbackv5-delete-saved' => 'Feedback deleted',
9494 'articlefeedbackv5-abuse-saved' => 'Flagged as abuse',
Index: trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5.php
——@@ -276,6 +276,7 @@
277277
278278 # Update the overall number of records for this page.
279279 ApiArticleFeedbackv5Utils::incrementFilterCount( $pageId, 'all' );
 280+ ApiArticleFeedbackv5Utils::incrementFilterCount( $pageId, 'visible' );
280281 # If the feedbackrecord had a comment, update that filter count.
281282 if( $has_comment ) {
282283 ApiArticleFeedbackv5Utils::incrementFilterCount( $pageId, 'comment' );
Index: trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php
——@@ -387,9 +387,6 @@
388388 }
389389
390390 private function renderBucket1( $record ) {
391 - $name = htmlspecialchars( $record[0]->user_name );
392 - $link = $record[0]->af_user_id ? "User:$name" : "Special:Contributions/$name";
393 -
394391 if ( $record['found']->aa_response_boolean ) {
395392 $msg = 'articlefeedbackv5-form1-header-found';
396393 $class = 'positive';
——@@ -397,31 +394,24 @@
398395 $msg = 'articlefeedbackv5-form1-header-not-found';
399396 $class = 'negative';
400397 }
401 - $found = Html::openElement( 'h3' )
402 - . Html::element( 'span', array( 'class' => 'icon' ) )
403 - . Linker::link( Title::newFromText( $link ), $name )
404 - .Html::element( 'span', array(
405 - 'class' => $class,
406 - ), wfMessage( $msg, '')->escaped() )
407 - . Html::closeElement( 'h3' );
408398
 399+ $found = $this->feedbackHead( $msg, $class, $record[0] );
 400+
409401 return "$found
410402 <blockquote>" . htmlspecialchars( $record['comment']->aa_response_text )
411403 . '</blockquote>';
412404 }
413405
 406+
414407 private function renderBucket2( $record ) {
415 - $name = htmlspecialchars( $record[0]->user_name );
416 - $type = htmlspecialchars( $record['tag']->afo_name );
 408+ $type = htmlspecialchars( $record['tag']->afo_name );
 409+ $class = $type == 'problem' ? 'negative' : 'positive';
417410 // Document for grepping. Uses any of the messages:
418411 // * articlefeedbackv5-form2-header-praise
419412 // * articlefeedbackv5-form2-header-problem
420413 // * articlefeedbackv5-form2-header-question
421414 // * articlefeedbackv5-form2-header-suggestion
422 - return
423 - Html::openElement( 'h3' )
424 - . wfMessage( 'articlefeedbackv5-form2-header-' . $type, $name )->escaped()
425 - . Html::closeElement( 'h3' )
 415+ return $this->feedbackHead( "articlefeedbackv5-form2-header-$type", $class, $record[0], $type )
426416 . '<blockquote>' . htmlspecialchars( $record['comment']->aa_response_text )
427417 . '</blockquote>';
428418 }
——@@ -431,37 +421,48 @@
432422 private function renderBucket3( $record ) {
433423 $name = htmlspecialchars( $record[0]->user_name );
434424 $rating = htmlspecialchars( $record['rating']->aa_response_rating );
435 - return
436 - Html::openElement( 'h3' )
437 - . wfMessage( 'articlefeedbackv5-form3-header', $name, $rating )->escaped()
438 - . Html::closeElement( 'h3' )
 425+ $class = $record['rating']->aa_response_rating >= 3 ? 'positive' : 'negative';
 426+
 427+ return $this->feedbackHead( 'articlefeedbackv5-form3-header', $class, $record[0], $record['rating']->aa_response_rating )
439428 . '<blockquote>' . htmlspecialchars( $record['comment']->aa_response_text )
440429 . '</blockquote>';
441430 }
442431
443432 private function renderBucket4( $record ) {
444 - return wfMessage( 'articlefeedbackv5-form4-header' )->escaped();
 433+ return $this->feedbackHead(
 434+ 'articlefeedbackv5-form4-header',
 435+ 'positive',
 436+ $record[0]
 437+ );
445438 }
446439
447440 private function renderBucket5( $record ) {
448 - $name = htmlspecialchars( $record[0]->user_name );
449 - $rv =
450 - Html::openElement( 'h3' )
451 - . wfMessage( 'articlefeedbackv5-form5-header', $name )->escaped()
452 - . Html::closeElement( 'h3' );
453 - $rv .= '<ul>';
 441+ $name = htmlspecialchars( $record[0]->user_name );
 442+ $body = '<ul>';
 443+ $total = 0;
 444+ $count = 0;
454445 foreach ( $record as $key => $answer ) {
455446 if ( $answer->afi_data_type == 'rating' && $key != '0' ) {
456 - $rv .= "<li>"
 447+ $body .= "<li>"
457448 . htmlspecialchars( $answer->afi_name )
458449 . ': '
459450 . htmlspecialchars( $answer->aa_response_rating )
460451 . "</li>";
 452+ $total += $answer->aa_response_rating;
 453+ $count++;
461454 }
462455 }
463 - $rv .= "</ul>";
 456+ $body .= "</ul>";
 457+
 458+ $class = $total / $count >= 3 ? 'positive' : 'negative';
464459
465 - return $rv;
 460+ $head = $this->feedbackHead(
 461+ 'articlefeedbackv5-form5-header',
 462+ $class,
 463+ $record[0]
 464+ );
 465+
 466+ return $head.$body;
466467 }
467468
468469 private function renderBucket0( $record ) {
——@@ -470,13 +471,37 @@
471472 }
472473
473474 private function renderNoBucket( $record ) {
474 - return wfMessage( 'articlefeedbackv5-form-invalid' )->escaped();
 475+ return $this->feedbackHead(
 476+ 'articlefeedbackv5-form-invalid',
 477+ 'negative',
 478+ $record[0]
 479+ );
475480 }
476481
477482 private function renderBucket6( $record ) {
478 - return wfMessage( 'articlefeedbackv5-form-not-shown' )->escaped();
 483+ return $this->feedbackHead(
 484+ 'articlefeedbackv5-form-not-shown',
 485+ 'positive',
 486+ $record[0]
 487+ );
479488 }
480489
 490+ private function feedbackHead( $message, $class, $record, $extra = '' ) {
 491+ $gender = ''; #?
 492+ $name = htmlspecialchars( $record->user_name );
 493+ $link = $record->af_user_id ? "User:$name" : "Special:Contributions/$name";
 494+ $html = Html::openElement( 'h3' )
 495+ . Html::element( 'span', array( 'class' => 'icon' ) )
 496+ . Linker::link( Title::newFromText( $link ), $name )
 497+ . Html::element( 'span',
 498+ array( 'class' => $class ),
 499+ wfMessage( $message, $gender, $extra )->escaped()
 500+ )
 501+ . Html::closeElement( 'h3' );
 502+
 503+ return $html;
 504+ }
 505+
481506 /**
482507 * Gets the allowed parameters
483508 *
Index: trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5Utils.php
——@@ -193,15 +193,28 @@
194194
195195 public function initializeAccess() {
196196 global $wgUser;
197 - return array(
 197+ $permissions = array(
198198 'blocked' => $wgUser->isBlocked(),
199199 'anon' => $wgUser->isAnon(),
200200 'registered' => !$wgUser->isAnon() && !$wgUser->isBlocked(),
201201 'autoconfirmed' => in_array('autoconfirmed', $wgUser->getEffectiveGroups()),
202202 'rollbackers' => in_array('rollbacker', $wgUser->getEffectiveGroups()),
203 - 'admins' => in_array('sysop', $wgUser->getEffectiveGroups()),
204 - 'oversight' => in_array('oversight', $wgUser->getEffectiveGroups())
 203+ 'admins' => false,
 204+ 'oversight' => false
205205 );
 206+
 207+ if( in_array('sysop', $wgUser->getEffectiveGroups() ) ) {
 208+ $permissions['admins'] = true;
 209+ $permissions['rollbackers'] = true;
 210+ }
 211+
 212+ if( in_array('oversight ', $wgUser->getEffectiveGroups() ) ) {
 213+ $permissions['oversight'] = true;
 214+ $permissions['rollbackers'] = true;
 215+ $permissions['admins'] = true;
 216+ }
 217+
 218+ return $permissions;
206219 }
207220 }
208221
Index: trunk/extensions/ArticleFeedbackv5/SpecialArticleFeedbackv5.php
——@@ -18,7 +18,6 @@
1919 private $access;
2020 private $filters = array(
2121 'visible',
22 - 'all',
2322 'comment'
2423 );
2524 private $sorts = array(
——@@ -35,10 +34,11 @@
3635 $this->access = ApiArticleFeedbackv5Utils::initializeAccess();
3736
3837 if( $this->access[ 'rollbackers' ] ) {
39 - $filter[] = 'invisible';
 38+ $this->filters[] = 'invisible';
 39+ $this->filters[] = 'all';
4040 }
4141 if( $this->access[ 'oversight' ] ) {
42 - $filter[] = 'deleted';
 42+ $this->filters[] = 'deleted';
4343 }
4444 }
4545

Comments

#Comment by Nikerabbit (talk | contribs)   06:59, 24 January 2012

Why did you drop passing the real username to the gender magic word?

#Comment by Catrope (talk | contribs)   06:21, 31 January 2012 
+		$gender = ''; #?

The parameter that's passed into GENDER: is simply the username (unescaped!).

OK otherwise.

#Comment by Catrope (talk | contribs)   10:24, 31 January 2012

This was fixed later, marking resolved.

Status & tagging log