r105617 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r105616‎ | r105617 | r105618 >
Date:23:51, 8 December 2011
Author:gregchiasson
Status:ok (Comments)
Tags:
Comment:
AFTv5 feedback page: Fix the ratings fetch, form links and get titles properly.
Modified paths:
  • /trunk/extensions/ArticleFeedbackv5/SpecialArticleFeedbackv5.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php
@@ -51,33 +51,6 @@
5252 $result->addValue( 'data', 'feedback', $html );
5353 }
5454
55 - public function fetchOverallRating( $pageId ) {
56 - $rv = array();
57 - $dbr = wfGetDB( DB_SLAVE );
58 - $rows = $dbr->select(
59 - array( 'aft_article_feedback_ratings_rollup',
60 - 'aft_article_field' ),
61 - array( 'arr_total / arr_count AS rating',
62 - 'afi_name'
63 - ),
64 - array( 'arr_page_id' => $pageId,
65 - 'arr_rating_id = afi_id',
66 - "afi_name IN ('found', 'rating')"
67 - )
68 - );
69 -
70 - foreach( $rows as $row ) {
71 - if( $row->afi_name == 'found' ) {
72 - $rv['found'] = ( int ) ( 100 * $row->rating );
73 - } elseif( $row->afi_name == 'rating' ) {
74 - # Or should this be round/ceil/floor/float?
75 - $rv['rating'] = ( int ) $row->rating;
76 - }
77 - }
78 -
79 - return $rv;
80 - }
81 -
8255 public function fetchFeedbackCount( $pageId, $filter ) {
8356 $dbr = wfGetDB( DB_SLAVE );
8457 $where = $this->getFilterCriteria( $filter );
Index: trunk/extensions/ArticleFeedbackv5/SpecialArticleFeedbackv5.php
@@ -1,47 +1,44 @@
22 <?php
33 class SpecialArticleFeedbackv5 extends SpecialPage {
4 - private $api;
5 -
64 public function __construct() {
75 parent::__construct( 'ArticleFeedbackv5' );
86 }
97
10 - public function execute( $title ) {
 8+ public function execute( $param ) {
119 global $wgOut;
12 - $pageId = $this->pageIdFromTitle( $title );
1310
 11+ $title = Title::newFromText( $param );
 12+ if ( $title ) {
 13+ $pageId = $title->getArticleID();
 14+ } else {
 15+ $wgOut->addWikiMsg( 'articlefeedbackv5-invalid-page-id' );
 16+ return;
 17+ }
1418
15 -# private function getApi() {
16 -# $q = new ApiQuery(
17 -# 'ApiQuery', 'articlefeedbackv5-view-feedback' );
18 -# $api = new ApiViewFeedbackArticleFeedbackv5(
19 -# $q, 'articlefeedbackv5-view-feedback' );
20 -# return $api;
21 -# }
22 - $rating_params = new FauxRequest( array(
23 - 'action' => 'articlefeedbackv5-view-ratings',
24 - 'format' => 'json',
25 - 'pageId' => $pageId,
26 - ) );
27 - $api = new ApiMain( $rating_params, true );
28 -# $ratings = $api->fetchOverallRating( $pageId );
 19+ $ratings = $this->fetchOverallRating( $pageId );
 20+ $found = isset( $ratings['found'] ) ? $ratings['found'] : null;
 21+ $rating = isset( $ratings['rating'] ) ? $ratings['rating'] : null;
2922
30 -
31 - $found = isset( $ratings['found'] ) ? $ratings['found'] : null;
32 - $rating = isset( $ratings['rating'] ) ? $ratings['rating'] : null;
33 -
3423 $wgOut->setPagetitle( "Feedback for $title" );
3524
36 - if( !$pageId ) {
 25+ if( !$pageId ) {
3726 $wgOut->addWikiMsg( 'articlefeedbackv5-invalid-page-id' );
3827 } else {
39 - $wgOut->addWikiText(
40 - "[[Wikipedia:$title|"
41 - .wfMsg('articlefeedbackv5-go-to-article')."]]
42 - | [[Wikipedia:$title|"
43 - .wfMsg('articlefeedbackv5-discussion-page')."]]
44 - | [[Wikipedia:$title|"
45 - .wfMsg('articlefeedbackv5-whats-this')."]]"
 28+ $wgOut->addHTML(
 29+ Linker::link(
 30+ Title::newFromText( $param ),
 31+ wfMessage( 'articlefeedbackv5-go-to-article' )->escaped()
 32+ )
 33+ .' | '.
 34+ Linker::link(
 35+ Title::newFromText( $param ),
 36+ wfMessage( 'articlefeedbackv5-discussion-page' )->escaped()
 37+ )
 38+ .' | '.
 39+ Linker::link(
 40+ Title::newFromText( $param ),
 41+ wfMessage( 'articlefeedbackv5-whats-this' )->escaped()
 42+ )
4643 );
4744 }
4845
@@ -56,8 +53,6 @@
5754 $wgOut->addWikiMsg( 'articlefeedbackv5-special-title' );
5855
5956 $wgOut->addHTML(<<<EOH
60 -<!-- This is a terrible, terrible hack. I'm taking it out as soon as I stop
61 - being an idiot and sort this ResourceLoader thing out -->
6257 <script> var hackPageId = $pageId; </script>
6358 <script src="/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.js"></script>
6459 <!--
@@ -83,17 +78,39 @@
8479 );
8580 }
8681
 82+ private function fetchOverallRating( $pageId ) {
 83+ $rv = array();
 84+ $dbr = wfGetDB( DB_SLAVE );
 85+ $rows = $dbr->select(
 86+ array(
 87+ 'aft_article_feedback_ratings_rollup',
 88+ 'aft_article_field'
 89+ ),
 90+ array(
 91+ 'arr_total / arr_count AS rating',
 92+ 'afi_name'
 93+ ),
 94+ array(
 95+ 'arr_page_id' => $pageId,
 96+ 'arr_field_id = afi_id',
 97+ "afi_name IN ('found', 'rating')"
 98+ )
 99+ );
 100+
 101+ foreach( $rows as $row ) {
 102+ if( $row->afi_name == 'found' ) {
 103+ $rv['found'] = ( int ) ( 100 * $row->rating );
 104+ } elseif( $row->afi_name == 'rating' ) {
 105+ $rv['rating'] = ( int ) $row->rating;
 106+ }
 107+ }
 108+
 109+ return $rv;
 110+ }
 111+
 112+
87113 protected static function formatNumber( $number ) {
88114 global $wgLang;
89115 return $wgLang->formatNum( number_format( $number, 2 ) );
90116 }
91 -
92 - protected function pageIdFromTitle( $title ) {
93 - $dbr = wfGetDB( DB_SLAVE );
94 - return $dbr->selectField(
95 - 'page',
96 - 'page_id',
97 - array( 'page_title' => $title )
98 - );
99 - }
100117 }

Comments

#Comment by Catrope (talk | contribs)   19:48, 12 December 2011
+					Title::newFromText( $param ),

You already have this object in $title, you might as well reuse it rather than recreating it three times.

#Comment by Johnduhart (talk | contribs)   05:32, 2 January 2012

And why link to the same thing 3 times?

Status & tagging log