r95697 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r95696‎ | r95697 | r95698 >
Date:21:05, 29 August 2011
Author:jpostlethwaite
Status:resolved (Comments)
Tags:fundraising 
Comment:
Switched BannerAllocation form from post to get. See bug 30271.
Modified paths:
  • /trunk/extensions/CentralNotice/special/SpecialBannerAllocation.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CentralNotice/special/SpecialBannerAllocation.php
@@ -19,13 +19,19 @@
2020 * Handle different types of page requests
2121 */
2222 function execute( $sub ) {
23 - global $wgOut, $wgRequest, $wgExtensionAssetsPath, $wgNoticeProjects, $wgLanguageCode;
 23+ global $wgOut, $wgRequest, $wgExtensionAssetsPath, $wgNoticeProjects, $wgLanguageCode, $wgNoticeProject;
 24+
 25+ $locationSubmitted = false;
2426
25 - if ( $wgRequest->wasPosted() ) {
26 - $this->project = $wgRequest->getText( 'project', 'wikipedia' );
27 - $this->language = $wgRequest->getText( 'language', 'en' );
28 - $this->location = $wgRequest->getText( 'country', 'US' );
29 - }
 27+ $this->project = $wgRequest->getText( 'project', 'wikipedia', $wgNoticeProject );
 28+ $this->language = $wgRequest->getText( 'language', $wgLanguageCode );
 29+
 30+ // If the form has been submitted, the country code should be passed along.
 31+ $locationSubmitted = $wgRequest->getVal( 'country' );
 32+ $this->location = $locationSubmitted ? $locationSubmitted : $this->location;
 33+
 34+ // Convert submitted location to boolean value. If it true, showList() will be called.
 35+ $locationSubmitted = (boolean) $locationSubmitted;
3036
3137 // Begin output
3238 $this->setHeaders();
@@ -53,7 +59,7 @@
5460 // Begin Allocation selection fieldset
5561 $htmlOut .= Xml::openElement( 'fieldset', array( 'class' => 'prefsection' ) );
5662
57 - $htmlOut .= Xml::openElement( 'form', array( 'method' => 'post' ) );
 63+ $htmlOut .= Xml::openElement( 'form', array( 'method' => 'get' ) );
5864 $htmlOut .= Xml::element( 'h2', null, wfMsg( 'centralnotice-view-allocation' ) );
5965 $htmlOut .= Xml::tags( 'p', null, wfMsg( 'centralnotice-allocation-instructions' ) );
6066
@@ -116,7 +122,7 @@
117123 $wgOut->addHTML( $htmlOut );
118124
119125 // Handle form submissions
120 - if ( $wgRequest->wasPosted() ) {
 126+ if ( $locationSubmitted ) {
121127 $this->showList();
122128 }
123129

Follow-up revisions

RevisionCommit summaryAuthorDate
r95912Converted spaces to tabs, removed third parameter from $wgRequest->getText()....jpostlethwaite22:09, 31 August 2011
r98902MFT r95697, r95912, r95966, r95967, r95969awjrichards20:39, 4 October 2011

Comments

#Comment by 😂 (talk | contribs)   15:05, 31 August 2011
  • Has a weird mix of tabs and spaces, we use tabs for indentation.
  • If $locationSubmitted is just going to be a boolean and you don't need the actual value, you can use $wgRequest->getBool() and it'll be clearer :)
  • In $this->project = $wgRequest->getText( 'project', 'wikipedia', $wgNoticeProject );, I'm not sure what the 3rd parameter is for. getText() only takes 2 parameters.

If you haven't already, I'd recommend reading our coding conventions page.

#Comment by Jpostlethwaite (talk | contribs)   22:58, 6 September 2011

Converted spaces to tabs, removed third parameter from $wgRequest->getText(). See bug #30271. Fixed in r95912.

Status & tagging log