r91789 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r91788‎ | r91789 | r91790 >
Date:16:24, 9 July 2011
Author:devayon
Status:deferred (Comments)
Tags:
Comment:
fixed pagination, and added some stubs for form elements
Modified paths:
  • /trunk/extensions/SemanticMediaWiki/specials/AskSpecial/SMW_QueryUIHelper.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SemanticMediaWiki/specials/AskSpecial/SMW_QueryUIHelper.php
——@@ -8,15 +8,170 @@
99 * @author Yaron Koren
1010 * @author Sanyam Goyal
1111 * @author Jeroen De Dauw
 12+ * @author Devayon Das
1213 */
1314 abstract class SMWQueryUI extends SpecialPage {
1415 protected $m_ui_helper;
15 - protected abstract function execute( $p ) {
 16+
 17+ protected function makeResults($p){
1618 /*
17 - * Extract the parameters from the UI. Use either the constructor or
18 - * $this->m_ui_helper=new SMWUIHelper()
 19+ * TODO: extract parameters from $p and decide:
 20+ * (1) if form elements need to be displayed
 21+ * (2) if any results need to be displayed
 22+ * (3) which factory method of UIhelper should be called
 23+ * Most of the code here in this method will anyway be removed later
1924 */
 25+ global $wgOut, $wgRequest;
 26+ $htmloutput="";
 27+ $htmloutput.= $this->getForm();
 28+ $param=array();
 29+
 30+ $this->m_ui_helper = $helper = new SMWQueryUIHelper; //or some factory method
 31+ //here come some driver lines for testing; this is very temporary
 32+
 33+ // form parameters default values
 34+ $helper->setQueryString(
 35+ $wgRequest->getVal('q', '[[Located in:: Germany]]'));
 36+ $helper->setParams(array(
 37+ 'format' => $wgRequest->getVal('format', 'ol' ),
 38+ 'offset' => $wgRequest->getVal('offset', '0' ),
 39+ 'limit' => $wgRequest->getVal('limit', '20' )
 40+ ));
 41+ $helper->setPrintOuts(array('?Population'));
 42+ $helper->extractParameters($p);
 43+
 44+ $helper->execute();
 45+
 46+ if($this->usesNavigationBar()){
 47+ $htmloutput.= $this->getNavigationBar ($helper->getLimit(),$helper->getOffset(),$helper->hasFurtherResults()); //? can we preload offset and limit?
 48+ }
 49+
 50+ $htmloutput.= $helper->getHTMLResult();
 51+
 52+ if($this->usesNavigationBar()){
 53+ $htmloutput.= $this->getNavigationBar ($helper->getLimit(),$helper->getOffset(),$helper->hasFurtherResults()); //? can we preload offset and limit?
 54+ }
 55+ $wgOut->addHTML($htmloutput);
2056 }
 57+
 58+ /**
 59+ * Build the navigation bar for some given query result.
 60+ *
 61+ * UI may overload this for a different layout. The navigation bar
 62+ * can be hidden by overloading usesNavigationBar(). To change the url format,
 63+ * one may overload getUrlTail();
 64+ *
 65+ * @global int $smwgQMaxInlineLimit
 66+ * @param int $limit
 67+ * @param int $offset
 68+ * @param boolean $has_further_results
 69+ *
 70+ * @return string
 71+ */
 72+ public function getNavigationBar($limit, $offset, $has_further_results) {
 73+ global $smwgQMaxInlineLimit;
 74+ $urltail = $this->getUrlTail();
 75+ // Prepare navigation bar.
 76+ if ( $offset > 0 ) {
 77+ $navigation = Html::element(
 78+ 'a',
 79+ array(
 80+ 'href' => $this->getTitle()->getLocalURL(
 81+ 'offset=' . max( 0, $offset - $limit ) .
 82+ '&limit=' . $limit . $urltail
 83+ ),
 84+ 'rel' => 'nofollow'
 85+ ),
 86+ wfMsg( 'smw_result_prev' )
 87+ );
 88+
 89+ } else {
 90+ $navigation = wfMsg( 'smw_result_prev' );
 91+ }
 92+
 93+ $navigation .=
 94+ '&#160;&#160;&#160;&#160; <b>' .
 95+ wfMsg( 'smw_result_results' ) . ' ' . ( $offset + 1 ) .
 96+ '&#150; ' .
 97+ ( $offset + $this->m_ui_helper->getResultCount() ) .
 98+ '</b>&#160;&#160;&#160;&#160;';
 99+
 100+ if ( $has_further_results ) {
 101+ $navigation .= Html::element(
 102+ 'a',
 103+ array(
 104+ 'href' => $this->getTitle()->getLocalURL(
 105+ 'offset=' . ( $offset + $limit ) .
 106+ '&limit=' . $limit . $urltail
 107+ ),
 108+ 'rel' => 'nofollow'
 109+ ),
 110+ wfMsg( 'smw_result_next' )
 111+ );
 112+ } else {
 113+ $navigation .= wfMsg( 'smw_result_next' );
 114+ }
 115+
 116+ $first = true;
 117+
 118+ foreach ( array( 20, 50, 100, 250, 500 ) as $l ) {
 119+ if ( $l > $smwgQMaxInlineLimit ) break;
 120+
 121+ if ( $first ) {
 122+ $navigation .= '&#160;&#160;&#160;&#160;&#160;&#160;&#160;&#160;(';
 123+ $first = false;
 124+ } else {
 125+ $navigation .= ' | ';
 126+ }
 127+
 128+ if ( $limit != $l ) {
 129+ $navigation .= Html::element(
 130+ 'a',
 131+ array(
 132+ 'href' => $this->getTitle()->getLocalURL(
 133+ 'offset=' . $offset .
 134+ '&limit=' . $l . $urltail
 135+ ),
 136+ 'rel' => 'nofollow'
 137+ ),
 138+ $l
 139+ );
 140+ } else {
 141+ $navigation .= '<b>' . $l . '</b>';
 142+ }
 143+ }
 144+
 145+ $navigation .= ')';
 146+
 147+ return $navigation;
 148+ }
 149+
 150+ /**
 151+ * Creates the form elements and populates them with parameters.
 152+ * UI implementations need to overload this if a different layout and form
 153+ * elements are desired
 154+ *
 155+ * @return string Form elements in HTML
 156+ */
 157+ protected function getForm(){
 158+ /*
 159+ * Although the following methods will retuen form elements, which can
 160+ * then be placed in wOut as pleased, they will
 161+ * also write javascript (if relevant) directly to wgOut.
 162+ */
 163+
 164+ //$result="";
 165+ //$result.= getQueryFormBox($content);
 166+ //$result.= getPOFormBox($content, $enableAutoComplete);
 167+ //$result.= getParamBox($content); //avoid ajax, load form elements in the UI by default
 168+ $result="<br>Stub: The Form elements come here<br><br>";
 169+ return $result;
 170+ }
 171+
 172+ /**
 173+ * A method which generates the url parameters based on passed parameters.
 174+ * UI implementations need to overload this if they use different form parameters
 175+ */
21176 protected function getUrlTail() {
22177 $urltail = '&q=' . urlencode( $this->m_ui_helper->getQuerystring() );
23178 $tmp_parray = array();
——@@ -36,12 +191,10 @@
37192 if ( $printoutstring != '' ) $urltail .= '&po=' . urlencode( $printoutstring );
38193 if ( array_key_exists( 'sort', $params ) ) $urltail .= '&sort=' . $params['sort'];
39194 if ( array_key_exists( 'order', $params ) ) $urltail .= '&order=' . $params['order'];
 195+ return $urltail;
40196 }
41197 protected function makeHtmlResult() {
42 - global $wgOut;
43 - if ( is_a( $this->m_ui_helper, 'SMWQueryUIHelper' ) ) {
44 -
45 - }
 198+ //STUB
46199 }
47200 /**
48201 * Display a form section showing the options for a given format,
——@@ -175,91 +328,6 @@
176329 return true;
177330 }
178331
179 - /**
180 - * Build the navigation for some given query result, reuse url-tail parameters.
181 - *
182 - * @param SMWQueryResult $res
183 - * @param string $urltail
184 - *
185 - * @return string
186 - */
187 - protected function getNavigationBar( SMWQueryResult $res, $urltail, $offset, $limit ) {
188 - global $smwgQMaxInlineLimit;
189 -
190 - // Prepare navigation bar.
191 - if ( $offset > 0 ) {
192 - $navigation = Html::element(
193 - 'a',
194 - array(
195 - 'href' => SpecialPage::getSafeTitleFor( 'Ask' )->getLocalURL( array(
196 - 'offset' => max( 0, $offset - $limit ),
197 - 'limit' => $limit . $urltail
198 - ) ),
199 - 'rel' => 'nofollow'
200 - ),
201 - wfMsg( 'smw_result_prev' )
202 - );
203 -
204 - } else {
205 - $navigation = wfMsg( 'smw_result_prev' );
206 - }
207 -
208 - $navigation .=
209 - '&#160;&#160;&#160;&#160; <b>' .
210 - wfMsg( 'smw_result_results' ) . ' ' . ( $offset + 1 ) .
211 - '&#150; ' .
212 - ( $offset + $res->getCount() ) .
213 - '</b>&#160;&#160;&#160;&#160;';
214 -
215 - if ( $res->hasFurtherResults() ) {
216 - $navigation .= Html::element(
217 - 'a',
218 - array(
219 - 'href' => SpecialPage::getSafeTitleFor( 'Ask' )->getLocalURL( array(
220 - 'offset' => ( $offset + $limit ),
221 - 'limit' => $limit . $urltail
222 - ) ),
223 - 'rel' => 'nofollow'
224 - ),
225 - wfMsg( 'smw_result_next' )
226 - );
227 - } else {
228 - $navigation .= wfMsg( 'smw_result_next' );
229 - }
230 -
231 - $first = true;
232 -
233 - foreach ( array( 20, 50, 100, 250, 500 ) as $l ) {
234 - if ( $l > $smwgQMaxInlineLimit ) break;
235 -
236 - if ( $first ) {
237 - $navigation .= '&#160;&#160;&#160;&#160;&#160;&#160;&#160;&#160;(';
238 - $first = false;
239 - } else {
240 - $navigation .= ' | ';
241 - }
242 -
243 - if ( $limit != $l ) {
244 - $navigation .= Html::element(
245 - 'a',
246 - array(
247 - 'href' => SpecialPage::getSafeTitleFor( 'Ask' )->getLocalURL( array(
248 - 'offset' => $offset,
249 - 'limit' => $l . $urltail
250 - ) ),
251 - 'rel' => 'nofollow'
252 - ),
253 - $l
254 - );
255 - } else {
256 - $navigation .= '<b>' . $l . '</b>';
257 - }
258 - }
259 -
260 - $navigation .= ')';
261 -
262 - return $navigation;
263 - }
264332 }
265333
266334 /**
——@@ -270,7 +338,6 @@
271339 *
272340 * @author Devayon Das
273341 *
274 - * @property boolean $enable_validation If set to TRUE causes each of the parametes to be checked for errors.
275342 */
276343 class SMWQueryUIHelper {
277344
——@@ -279,17 +346,17 @@
280347 protected $m_params = array(); // Parameters controlling how the results should be displayed
281348 protected $m_printouts = array(); // Properties to be printed along with results
282349 protected static $m_UIPages = array(); // A list of Query UIs
283 - public $enable_validation;
284350 private $fatal_errors = false;
285351 private $context;
286352 private $errors = array();
 353+ private $queryresult = null;
 354+
287355 const SPECIAL_PAGE = 0;// parameters passed from special page
288356 const WIKI_LINK = 1;// parameters passed from 'further links' in the wiki.
289357
290358
291359 // constructor
292 - public function __construct( $enable_validation = true, $context = self::SPECIAL_PAGE ) {
293 - $this -> enable_validation = $enable_validation;
 360+ public function __construct($context = self::SPECIAL_PAGE ) {
294361 $this->context = $context;
295362 }
296363
——@@ -297,6 +364,36 @@
298365 return $this->fatal_errors;
299366 }
300367
 368+ public function getLimit(){
 369+ if(key_exists('limit', $this->m_params)){
 370+ return $this->m_params['limit'];
 371+ }
 372+ else {
 373+ return 0;
 374+ }
 375+ }
 376+
 377+ public function getOffset(){
 378+ if(key_exists('offset', $this->m_params)){
 379+ return $this->m_params['offset'];
 380+ }
 381+ else{
 382+ return 20;
 383+ }
 384+ }
 385+ public function hasFurtherResults(){
 386+ if(is_a($this->queryresult,'SMWQueryResult')){
 387+ return $this->queryresult->hasFurtherResults();
 388+ }
 389+ else{
 390+ return false;
 391+ }
 392+ }
 393+
 394+ public function getResultObject(){
 395+ return $this->getResultObject();
 396+ }
 397+
301398 /**
302399 *
303400 * Returns an array of errors, if any have occured.
——@@ -332,10 +429,10 @@
333430 * @param string $querystring The query
334431 * @return array array of errors, if any.
335432 */
336 - public function setQueryString( $querystring = "" ) {
 433+ public function setQueryString( $querystring = "", $enable_validation=true ) {
337434 $this -> m_querystring = $querystring;
338435 $errors = array();
339 - if ( $this->enable_validation ) {
 436+ if ( $enable_validation ) {
340437 if ( $querystring == '' ) {
341438 $errors[] = "No query has been specified"; // TODO i18n
342439 }
——@@ -361,9 +458,9 @@
362459 * @param array $printouts Array of additional properties to be shown in results
363460 * @return array array of errors, if any.
364461 */
365 - public function setPrintOuts( array $printouts = array() ) {
 462+ public function setPrintOuts( array $printouts = array(), $enable_validation=true ) {
366463 $errors = array();
367 - if ( $this -> enable_validation ) {
 464+ if ( $enable_validation ) {
368465 foreach ( $printouts as $key => $prop ) {
369466 if ( $prop[0] != '?' ) {
370467 $printouts[$key] = "?" . $printouts[$key];
——@@ -379,7 +476,7 @@
380477 return $errors;
381478 }
382479
383 - public function setParams( array $params = array() ) {
 480+ public function setParams( array $params = array(), $enable_validation=true ) {
384481 /*
385482 *Validate, and add missing params. *
386483 */
——@@ -397,7 +494,7 @@
398495 if ( !array_key_exists( 'offset', $params ) )
399496 $params['offset'] = 0;
400497
401 - if ( $this->enable_validation ) {
 498+ if ( $enable_validation ) {
402499 // validating the format
403500 if ( !array_key_exists( $params['format'], $smwgResultFormats ) ) {
404501 $errors[] = "The chosen format " + $params['format'] + " does not exist for this wiki"; // TODO i18n
——@@ -421,21 +518,21 @@
422519 }
423520
424521 $this -> m_params = $params;
425 - $this->errors = array_merge( $errors, $this->errors );
 522+ $this -> errors = array_merge( $errors, $this->errors );
426523 return $errors;
427524 }
428525
429 - public function makeHTMLResult() {
 526+ public function execute() {
430527 /*
431528 * Once $m_querystring, $m_params, $m_printouts are set, generates the
432529 * results / or link. The pagination links (or navigation bar) are expected
433530 * to be created by the UI designer. (or maybe we can put a method here to
434531 * make the nav-bar which also calls makeHTMLResult().
435532 */
436 - $result = '';
437533 $errors = array();
438534 $query = SMWQueryProcessor::createQuery( $this->m_querystring, $this->m_params, SMWQueryProcessor::SPECIAL_PAGE , $this->m_params['format'], $this->m_printouts );
439535 $res = smwfGetStore()->getQueryResult( $query );
 536+ $this->queryresult=$res;
440537 $errors = array_merge( $errors, $res->getErrors() );
441538 if ( !empty( $errors ) ) {
442539 $this->fatal_errors = true;
——@@ -469,7 +566,11 @@
470567 }
471568 }
472569 // END: Try to be smart for rss/ical if no description/title is given and we have a concept query
473 -
 570+ }
 571+
 572+ public function getHTMLResult(){
 573+ $result = '';
 574+ $res= $this->queryresult;
474575 $printer = SMWQueryProcessor::getResultPrinter( $this->m_params['format'], SMWQueryProcessor::SPECIAL_PAGE );
475576 $result_mime = $printer->getMimeType( $res );
476577
——@@ -526,6 +627,13 @@
527628 return $this->m_querystring;
528629 }
529630
 631+ public function getResultCount(){
 632+ if(is_a($this->queryresult, 'SMWQueryResult')){
 633+ return $this->queryresult->getCount();
 634+ }
 635+ else return 0;
 636+
 637+ }
530638 public function getParams() {
531639 return $this->m_params;
532640 }
——@@ -551,8 +659,10 @@
552660 * @return SMWQueryUIHelper
553661 */
554662 public static function makeFromInfoLink( $p, $enable_validation = true ) {
555 - $result = new SMWQueryUIHelper( $enable_validation, self::WIKI_LINK );
 663+ //TODO handle validation for infolink parameters
 664+ $result = new SMWQueryUIHelper(self::WIKI_LINK );
556665 $result->extractParameters( $p );
 666+ $result->execute();
557667 return $result;
558668 }
559669 /**
——@@ -567,11 +677,12 @@
568678 * @return SMWQueryUIHelper
569679 */
570680 public static function makeFromUI( $query, array $params, array $printouts, $enable_validation = true ) {
571 - $result = new SMWQueryUIHelper( $enable_validation, self::SPECIAL_PAGE );
572 - $result->setParams( $params );
573 - $result->setPrintOuts( $printouts );
574 - $result->setQueryString( $query );
 681+ $result = new SMWQueryUIHelper(self::SPECIAL_PAGE );
 682+ $result->setParams( $params, $enable_validation);
 683+ $result->setPrintOuts( $printouts, $enable_validation );
 684+ $result->setQueryString( $query, $enable_validation );
575685 $result->extractParameters( "" );
 686+ $result->execute();
576687 return $result;
577688 }
578689 /**

Follow-up revisions

RevisionCommit summaryAuthorDate
r91840followup to r91789devayon17:59, 10 July 2011

Comments

#Comment by Nikerabbit (talk | contribs)   17:11, 9 July 2011

Did you test this? I don't see function key_exists defined anywhere.

Btw, is_a is deprecated. instanceof operator can be used instead.

#Comment by Devayon (talk | contribs)   17:41, 9 July 2011

Hi, I see why this seems a mess. I was just incrementally coding this file. I guess the commit message wasn't really suitable. The code is definitely unstable. I'm just putting up daily commits so that Markus Krötzsch can keep an eye on this. Not to worry, this code does not currently load on anybody's installation, so it won't break anything.

Thanks for pointing out the is_a. :)

Is a 'follow up:rXXXX' message more suited to such commits?

#Comment by Nikerabbit (talk | contribs)   18:44, 9 July 2011

Daily commits are fine, but if you start making a lots of changes it is preferably to try to split them up as one issue - one commit. Since this is work-in-progress you don't have to worry so much. But you might get good tips from comments here regardless :)

Follow-up statement in the commit message makes sense if you are fixing some thing that was broken in that revision.

#Comment by Devayon (talk | contribs)   17:55, 10 July 2011

Hi, is_a() seems to have come out of deprecation :)[1].

#Comment by Devayon (talk | contribs)   18:01, 10 July 2011

Thanks for pointing out key_exists(). It turns out it's a deprecated form of array_key_exists :)

Status & tagging log